Adding/removing relationships in the submission form does not update the view automatically

[alexandrevryghem]

References

• Fixes Adding relationships through submission doesn't update view automatically #1671
• Requires Adding/removing relationships in the submission form does not update the view automatically DSpace#8848

Description

This PR fixes 2 problems The first problem is that the relationships list on submission forms is not updated when adding or removing new items. The second issue is that by default relationship fields can't retrieve their current scope in SubmissionSectionFormComponent.inCurrentSubmissionScope().

Instructions for Reviewers

Changes for the submission form not updating the view when adding/removing relationships:

• SubmissionSectionFormComponent: Use sectionData instead of sectionMetadata to retrieve the field value (because sectionMetadata did not include the relationships)
• Hide the repeatable field button for relationship fields instead of disabling it

Change for the default relationship field not retrieving it's scope:
In SubmissionSectionFormComponent#inCurrentSubmissionScope the scope was retrieved by checking if the FormField had a selectableMetadata, but relation fields don't have this field, they use the selectableRelationship field instead. The fix for this bug was just to add a custom if condition here in order to retrieve the correct field from formConfig.

Guidance for how to test this PR:
Testing the submission form not updating the view when adding/removing relationships:

• Create a new item with dspace.entity.type Person
• Create a collection with dspace.entity.type Publication
• Add this field to the submission form of the Publication collection:

  <relation-field>
      <relationship-type>isAuthorOfPublication</relationship-type>
      <search-configuration>person</search-configuration>
      <repeatable>true</repeatable>
      <label>Author</label>
      <hint>Enter the author’s name (Family name, Given names).</hint>
      <required></required>
  </relation-field>

• Run dspace initialize-entities -f dspace/config/entities/relationship-types.xml
• Create a new item in that collection and check that you can see the author after adding it using the relation field without having to refresh the page
• Delete the author and check that it disappears again without needing to refresh the page

Testing the default relationship field not retrieving it's scope:

• This can't currently be tested yet through the UI because of another bug but you can test it using the debugger. When SubmissionSectionFormComponent.inCurrentSubmissionScope() is called for a relationship field, check that the scope that is retrieved is correct.

Checklist

• 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.
• My PR passes ESLint validation using yarn lint
• My PR doesn't introduce circular dependencies (verified via yarn check-circ-deps)
• My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
• My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
• If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
• If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
• If my PR fixes an issue ticket, I've linked them together.

[atarix83]

thanks @alexandrevryghem

The bug seems to be resolved, i've only a small change request

[atarix83]

@alexandrevryghem

I think that instead to turning to use the section.data object it would be better to include into the sectionMetadata object the metadata needed for the relationchip. You can do it by changing the implementation of the computeSectionConfiguredMetadata in order to include not only the selectableMetadata but also the selectableRelationship

This is because the section.data object could contain also metadata that are related to a different section form of the current submission which should not be considered by this method

[atarix83]

@alexandrevryghem

could you commit the changes you mentioned here to a different branch? so i can take a look.
The point is that with this change now we are changing the current behaviour because, as already said, this method should consider only metatada that belong only to the current submission section

[ybnd]

@atarix83 we've updated the "metadata to be checked" to use sectionData instead of this.sectionData.data like before, but we can't really reinstate the this.sectionMetadata check

• if we try to add relation.* MDFs like you suggested, Relationships get duplicated in the form -- haven't been able to track down what causes this
• even if we fix this, the this.sectionMetadata check prevents removed Relationships from triggering a form update
  • looks like fields removed from sectionData aren't flagged as differences
  • without the check, the form update is triggered by an unexpected dspace.entity.type change in another section of the form. If we reinstate the check, removing Relationships stops working

So all in all: we've attempted to somewhat alleviate your concerns, but likely won't be able to fully solve all of the above bugs in time for 7.6

[atarix83]

@ybnd

just to be sure did you changed the computeSectionConfiguredMetadata in ordert to include the selectableRelationship into the sectionMetadata object, when you tried what i suggested?

[ybnd]

@atarix83 yes

[tdonohue]

@alexandrevryghem : I've been able to verify this works well in one scenario. But, it doesn't work as well in others.

The scenario where it works well:

• Ensure a Publication Entity submission form exists which links to Person entities via an Author field. For example: https://github.com/DSpace/DSpace/blob/main/dspace/config/submission-forms.xml#L247-L267
• From MyDSpace, create a new Publication Entity
• In Submission form, select multiple Person Entities in the Author field.
• You can see them selected immediately in the form
• You can reorder them
• You can delete one (or more). This all works well!

The scenario where it doesn't seem to work:

• Ensure a Person Entity submission form exists which links to a Publication entity via a "Publication" field. This setup is described in the initial ticket: #1671
• From MyDSpace, create a new Person Entity
• In Submission form, click the "Search" next to the "Publication" field. You should only be able to select a single value here by default, as it's not "repeatable" by default: https://github.com/DSpace/DSpace/blob/main/dspace/config/submission-forms.xml#L391-L399
• Click on a publication listed in the search -> It immediately appears in the form (Good!)
• Click on a different publication listed in the search -> A second publication now appears in the form?? (Not good)
• Close the popup window & you'll see something like this
  

A few things to notice:

• Obviously, somehow we have two publications. Only one should be shown.
• Clicking the Delete next to either publication removes both.
• If, before clicking Delete, I click reload in my browser, the page refreshes and only the second publication still appears. This implies that there's still a UI caching issue here... the first one is being replaced, but sometimes inaccurately still appears on the submission form.

Overall, the behavior in this PR is better than before. But, there still seems to be a small bug in Person entity form when selecting a single Publication.

[ybnd]

@tdonohue we also noticed this duplicate Relationship bug when re-testing yesterday. It doesn't seem to occur consistently -- sometimes the Relationships are replaced as expected -- but especially if you "toggle" them quickly from the lookup it's pretty easy to get more than one to show up...

I've got a hunch that this won't be easy to address though; it's likely a side effect of the WorkspaceItem/Relationship workarounds we intend to refactor out in #858.

[tdonohue]

@ybnd and @alexandrevryghem : In that case, we can accept this PR "as-is" once @atarix83 's feedback is addressed above. We can always create a follow-up ticket for the duplicate relationship problem on the "Person" entity submission form.

[alexandrevryghem]

@tdonohue: we did make the requested change here and it works for DSpace 7.2 but on DSpace 7.4 this change breaks this fix partially. Adding relations will still automatically update the view, but removing them requires a page reload. This is the reason why we didn't push that commit yet to this PR

[tdonohue]

@alexandrevryghem : Your comment back should go to @atarix83 as he is the one who requested the change... @atarix83 , see the response above.

I'm not able to help debug this issue right now as there are too many other 7.6 PRs needing attention. Hopefully you and @atarix83 can work out a compromise, or agree on creating a followup ticket to fix this issue. I think @atarix83 's request sounds reasonable, but I don't know why it's not working either.

[tdonohue]

@atarix83 : I've flagged both of us to re-review this PR. As @ybnd notes above, this PR is likely as good as we'll get for 7.6.. and it does fix the main issues. That said, we can always create follow-up tickets for ourselves as needed.

I'll also give this a review early next week.

[tdonohue]

👍 Thanks @alexandrevryghem and @ybnd . Overall, this provides better behavior, but as noted in my last review, it's not a complete fix. That said, it's better than the behavior on main, so I think we should include this in 7.6... and ensure any known bugs are logged separately.

I also want to note that I don't think this solves #1101 (as claimed in the description). That bug is still reproducible if I follow the steps listed in #1101. So, I'd recommend we leave this ticket open.

However, it does solve #1671 (at least the basic bug where clicking delete wasn't working...it works now). That said, as noted, sometimes multiple relationships temporarily appear for a single relationship field (if you attempt to add two in a row as I previously described). But, this doesn't happen at all times.

Overall, I think we should add this to 7.6 as the behavior is better. @atarix83 : I'd appreciate it if you could give this another look as your feedback has been partially addressed.

[atarix83]

@alexandrevryghem @tdonohue

I agree to merge this pr as is, even if it works only for misbehaviour.

we should keep track of it in a new issue

[tdonohue]

Merging. Per today's meeting, we decided this is "good enough" for 7.6. More improvements are definitely necessary, and I've logged the newly discovered bug in #2314

The discussions in this PR make me feel that we need to find time to fix #858 for 8.0... as we seem to have a lot of odd behaviors in the submission forms which point to necessary refactoring. So, I'll bump that priority to "high"