-
Notifications
You must be signed in to change notification settings - Fork 1
LDP-2328: add optional moderation state to the function cloneNodeByTitle #20
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
Conversation
| * @return {Response} The response. | ||
| */ | ||
| visitNodeEditPage: async ([page, node_title], langcode = '') => { | ||
| visitNodeEditPage: async ([page, node_title, langcode = '']) => { |
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.
Testing @sindurigf 's change made me realize that I don't know Javascript (well I knew that part) and that it is fine to have an optional part of an array, inside an array parameter.
In other words, this change is possible and does not change anything - and that means the recent addition that I did of langcode outside of the array is unnecessary... and inconsistent.
So I 'fixed' that in this same PR.
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.
None of the current visitNodeEditPage() calls pass the optional langcode, so nothing breaks. (This was added for LDP-1787 which isn't merged yet.)
| * @aliases nc | ||
| */ | ||
| public function cloneNodeByTitle($node_type, $node_title, $new_node_title) { | ||
| public function cloneNodeByTitle($node_type, $node_title, $new_node_title, $moderation_state) { |
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 would add a default value for moderation state here as well, even though we have it in the helper. Not a biggie
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.
done
No description provided.