Skip to content

Actually use diffTracker values for display to the user. #3375

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.

Sign up for GitHub

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

Merged
rtibbles merged 1 commit into from
Apr 20, 2022
Merged

Actually use diffTracker values for display to the user. #3375

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 22 additions & 11 deletions contentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -690,20 +690,31 @@
return value !== nonUniqueValue;
},
getValueFromNodes(key) {
if (Object.prototype.hasOwnProperty.call(this.diffTracker, key)) {
return this.diffTracker[key];
}
let results = uniq(this.nodes.map(node => node[key] || null));
const results = uniq(
this.nodes.map(node => {
if (Object.prototype.hasOwnProperty.call(this.diffTracker[node.id] || {}, key)) {
Copy link
Copy Markdown
Contributor

@sairina sairina Apr 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, why use Object.prototype.hasOwnProperty.call()? Does this have to do the this.diffTracker[node.id] object having to use call from the prototype?

Copy link
Copy Markdown
Member Author

@rtibbles rtibbles Apr 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hasOwnProperty is the best method for determining if a Javascript object actually has a property in itself (and it's not inherited from a parent type) - so it's the best way of telling if we have actually assigned e.g. this.diffTracker[node.id] = {}.

In theory you could call it like this: this.diffTracker.hasOwnProperty(node.id) because it is just a method of any Object that inherits from or is instantiated from the Object constructor. The reason for using the Object.prototype.hasOwnProperty.call is to ensure that the method has not been maliciously overwritten on the object itself - we have a linting rule https://eslint.org/docs/rules/no-prototype-builtins that guards against this - by calling it directly from the Object prototype we guard against it having been overwritten from JSON data or somesuch. It is probably not a real concern in this instance, but simpler to obey the linting rule.

return this.diffTracker[node.id][key];
}
return node[key] || null;
})
);
return getValueFromResults(results);
},
getExtraFieldsValueFromNodes(key, defaultValue = null) {
if (
Object.prototype.hasOwnProperty.call(this.diffTracker, 'extra_fields') &&
Object.prototype.hasOwnProperty.call(this.diffTracker.extra_fields, key)
) {
return this.diffTracker.extra_fields[key];
}
let results = uniq(this.nodes.map(node => node.extra_fields[key] || defaultValue));
const results = uniq(
this.nodes.map(node => {
if (
Object.prototype.hasOwnProperty.call(
this.diffTracker[node.id] || {},
'extra_fields'
) &&
Object.prototype.hasOwnProperty.call(this.diffTracker[node.id].extra_fields, key)
) {
return this.diffTracker[node.id].extra_fields[key];
}
return node.extra_fields[key] || defaultValue;
})
);
return getValueFromResults(results);
},
getPlaceholder(field) {
Expand Down