Skip to content

Auto-save in new Item Submission form breaks the form#4

Closed
alemarte wants to merge 14 commits intomainfrom
CSTPER-260
Closed

Auto-save in new Item Submission form breaks the form#4
alemarte wants to merge 14 commits intomainfrom
CSTPER-260

Conversation

@alemarte
Copy link
Copy Markdown

@alemarte alemarte commented Nov 30, 2020

References

Major issue related to this PR is #835
Other involved issues are:
#948
#864
#884
#882
#931

Before describing the approach adopted in this PR, it is useful to summarise the problems that it directly or indirectly tries to address.
Having investigated them for a long time, it has been possible to trace the root causes of these problems.

At this moment all the above problems except (5), (6) and (3) (which is a side effect) are mitigated by the redraw which is extremely frequent. The more you reduce the frequency of redraw, the more these problems arise.

But it is clear that redraw in turn causes many side effects and ideally should be avoided as much as possible.

Current workaround to all this mess

  • Notifications to the user have been reduced to only manual saves (by clicking on the submission's footer buttons)
  • The Save button is only displayed as enable if there are unsaved changes.
  • The hasMetadataEnrichment check responsible to avoid form redraw has been reintroduced by inserting the metadata submission in the store to limit the redraw. This with the aim of reducing (3).
  • Unfortunately, as said, this has highlighted a number of weaknesses linked to (2). Solving these kind of problems is out of the scope of this issue, and it would be a very expensive and not very scalable job because it involves writing code for each model, and not very maintainable because in addition to having a lot of code to mantain it forces the native behavior of the library that should be able to be upgraded easily.
  • We did not consider convenient to proceed with solving (3) by saving the values inserted in the first line, unless before a deep analysis and discussion with the community. The problem is the same, as well as making the store heavier it requires the implementation of specific patches for each model, an approach that is difficult to maintain and not scalable.
  • Since frequent or infrequent redraw is a scenario to deal with in any case, the information of touched fields has been added for the correct display of errors. But this is all made in vain by (1). Solving this problem without intervening directly on the repeatable field by rethinking it, becomes extremely difficult, unless you rewrite from scratch the error support already provided by angular (which is one of the most valuable aspects of the framework).

Moreover:
We tried to save the focused state of the section in order to postpone the form redraw which causes the emptying of populated fields and the loss of the focus field (6).
The result is that sometimes the redraw is not evoked at all as the user goes from one field to another without ever leaving the form. So we decided to revert (but still in commit history).

It doesn't solve in any way (1) whose fix, as already stated, is expensive to implement and unmaintainable.

Ideal solution: remove the template row from the repeatable field control
Removing the template row from the repeatable fields immediately fix (1), (2), (3).
The added submission's metadata into the store reduces the redraw so that the user experience is fluid most of the time. The redraw remains in case of interaction with lookups.
Moreover
Implementing the lookup so that it directly patches the form could be more than welcome.
Leave the redraw as the only strategy in case the server actually enriches the form with additional data.
In that case a notification will be displayed to the user and it would probably be acceptable to lose the information about the touched fields and have a solution more clean and mantainable.

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 TSLint validation using yarn run lint
  • My PR doesn't introduce circular dependencies
  • 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, third-party dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.

Disable autosave when timer is equal to 0
Notifications are disabled for submission section savings.
Notifications are enable only for manual submission savings.
Store additions:
1. Form AdditionalData: contains the list of the touched metadata
2. Submission metadata: contains the list of the metadata ids assignable for each section

We keep also track whether a section ha focused fields or not.
Section formId added to the section state.
Error filtering during the parsing of the submission response.
Section metadata dispatched to the store and retrieved in subscription.
Added test case for hasMetadataEnrichment.
Section Metadata field tested.
Form Touched filter tested.
Submission form Save button disabled when no pending operations are present
@alemarte alemarte closed this Dec 3, 2020
corrad82-4s pushed a commit to corrad82-4s/dspace-angular that referenced this pull request Jan 13, 2021
4science-it pushed a commit that referenced this pull request Feb 4, 2021
New Finnish keys January 2021
4science-it pushed a commit that referenced this pull request Apr 9, 2025
atarix83 pushed a commit that referenced this pull request Feb 20, 2026
[DQ-38] adapt info text

Approved-by: Giuseppe Digilio
atarix83 pushed a commit that referenced this pull request Feb 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant