dspace-angular
dspace-angular copied to clipboard
#3217: fix/rethink valueChange subscription for onebox values
References
- Fixes #3217
Description
This is a PR that demonstrates where the problem is occurring with controlled onebox value re-ordering -- there is a subscription to valueChanges from the model which updates currentValue of the onebox, but it uses this.model.id as a persistent index in the group array, which is not hte case -- after a reorder, this id will not match the real index.
My fix here might not be suitable, and I ask some experienced submission angular devs for their input - was the intention here to listen for changes to the workspace item while the submission form is open, beteween saves?
If we need to leave this subscription in (and I accept there might be good reasons), we need to find a way to track a current index for the onebox, or regenerating all the ids after a reorder.
In the meantime, those users suffering from #3217 or #2004 issues, related to reordering of onebox values in submission (where the fields are configured for vocab/auth), might like to test out this small fix and see if it fixes the problem.
Instructions for Reviewers
Without this PR, observe what happens when onebox values are reordered - I suggest author and subject, using default configuration - as per #3217
With this PR applied, try the same thing, and also ensure that submission saves / reloads / deposits perform as expected with no new side-effects.
For the advanced tester, make a change to one of the values of the workspace item in question using just the REST API, and see what happens to the active submission form with/without the PR applied
Notes for Reviewers
I am keen to hear from Angular submission devs to make sure I understand the intention of this subscription, and if it is necessary, discuss the best way to track the index so we can put the subscription back safely.
List of changes in this PR:
- Remove subscription to model valueChanges from onebox component
Checklist
This checklist provides a reminder of what we are going to look for when reviewing your PR. You do not need to complete this checklist prior creating your PR (draft PRs are always welcome). However, reviewers may request that you complete any actions in this list if you have not done so. If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!
- [x] My PR is created against the
mainbranch of code (unless it is a backport or is fixing an issue specific to an older branch). - [x] My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
- [x] My PR passes ESLint validation using
yarn lint - [x] My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
- [x] My PR includes details on how to test it. I've provided clear instructions to reviewers on how to successfully test this fix or feature.
- [x] If my PR fixes an issue ticket, I've linked them together.