-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UI: Ember deprecation - addObject, removeObject #25952
Changes from all commits
e923955
95bd2c2
b0d2358
f991430
c3a57a9
de9946a
a1ca39e
a68d556
1438dca
5093292
de5984d
24a8030
1a6659c
0999af1
16e6ce9
4fadb8b
17c8712
d7a2b83
889a074
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,8 @@ import { tracked } from '@glimmer/tracking'; | |
import { action } from '@ember/object'; | ||
import { service } from '@ember/service'; | ||
import { task } from 'ember-concurrency'; | ||
import handleHasManySelection from 'core/utils/search-select-has-many'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👏 👏 👏 |
||
import { addManyToArray, addToArray } from 'vault/helpers/add-to-array'; | ||
import { removeFromArray } from 'vault/helpers/remove-from-array'; | ||
|
||
/** | ||
* @module MfaLoginEnforcementForm | ||
|
@@ -64,7 +65,7 @@ export default class MfaLoginEnforcementForm extends Component { | |
for (const { label, key } of this.targetTypes) { | ||
const targetArray = await this.args.model[key]; | ||
const targets = targetArray.map((value) => ({ label, key, value })); | ||
this.targets.addObjects(targets); | ||
this.targets = addManyToArray(this.targets, targets); | ||
} | ||
} | ||
async resetTargetState() { | ||
|
@@ -102,7 +103,6 @@ export default class MfaLoginEnforcementForm extends Component { | |
|
||
updateModelForKey(key) { | ||
const newValue = this.targets.filter((t) => t.key === key).map((t) => t.value); | ||
// Set the model value directly instead of using Array methods (like .addObject) | ||
this.args.model[key] = newValue; | ||
} | ||
|
||
|
@@ -126,8 +126,18 @@ export default class MfaLoginEnforcementForm extends Component { | |
|
||
@action | ||
async onMethodChange(selectedIds) { | ||
// first make sure the async relationship is loaded | ||
const methods = await this.args.model.mfa_methods; | ||
handleHasManySelection(selectedIds, methods, this.store, 'mfa-method'); | ||
// then remove items that are no longer selected | ||
const updatedList = methods.filter((model) => { | ||
return selectedIds.includes(model.id); | ||
}); | ||
// then add selected items that don't exist in the list already | ||
const modelIds = updatedList.map((model) => model.id); | ||
const toAdd = selectedIds | ||
.filter((id) => !modelIds.includes(id)) | ||
.map((id) => this.store.peekRecord('mfa-method', id)); | ||
this.args.model.mfa_methods = addManyToArray(updatedList, toAdd); | ||
} | ||
|
||
@action | ||
|
@@ -150,15 +160,15 @@ export default class MfaLoginEnforcementForm extends Component { | |
addTarget() { | ||
const { label, key } = this.selectedTarget; | ||
const value = this.selectedTargetValue; | ||
this.targets.addObject({ label, value, key }); | ||
this.targets = addToArray(this.targets, { label, value, key }); | ||
// recalculate value for appropriate model property | ||
this.updateModelForKey(key); | ||
this.selectedTargetValue = null; | ||
this.resetTargetState(); | ||
} | ||
@action | ||
removeTarget(target) { | ||
this.targets.removeObject(target); | ||
this.targets = removeFromArray(this.targets, target); | ||
// recalculate value for appropriate model property | ||
this.updateModelForKey(target.key); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -195,6 +195,7 @@ export default class SecretCreateOrUpdate extends Component { | |
if (isBlank(item.name)) { | ||
return; | ||
} | ||
// secretData is a KVObject/ArrayProxy so removeObject is fine here | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thank you for the comment! |
||
data.removeObject(item); | ||
this.checkRows(); | ||
this.handleChange(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if these helpers should live in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a good idea for a follow-on! I think it will be easier to track the changes outside of this PR |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,13 @@ function dedupe(items) { | |
return items.filter((v, i) => items.indexOf(v) === i); | ||
} | ||
|
||
export function addToArray([array, string]) { | ||
export function addManyToArray(array, otherArray) { | ||
assert(`Both values must be an array`, Array.isArray(array) && Array.isArray(otherArray)); | ||
const newArray = [...array].concat(otherArray); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. out of curiosity - is there a reason you used There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If there was, I don't remember it 😂 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TIL: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh neat! 💡 |
||
return dedupe(newArray); | ||
} | ||
|
||
export function addToArray(array, string) { | ||
if (!Array.isArray(array)) { | ||
assert(`Value provided is not an array`, false); | ||
} | ||
|
@@ -19,4 +25,9 @@ export function addToArray([array, string]) { | |
return dedupe(newArray); | ||
} | ||
|
||
export default buildHelper(addToArray); | ||
export default buildHelper(function ([array, string]) { | ||
if (Array.isArray(string)) { | ||
return addManyToArray(array, string); | ||
} | ||
return addToArray(array, string); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I simplified the interface for the exported methods so that we don't need to wrap the params in an array