Skip to content

Dedupe patch operations by op and path#2778

Merged
alanorth merged 1 commit intoDSpace:mainfrom
TAMULib:dedupe-patch-operations
Feb 1, 2024
Merged

Dedupe patch operations by op and path#2778
alanorth merged 1 commit intoDSpace:mainfrom
TAMULib:dedupe-patch-operations

Conversation

@tamu-sad-iii
Copy link
Copy Markdown
Contributor

@tamu-sad-iii tamu-sad-iii commented Jan 26, 2024

References

Description

It was discovered that when many modifications of the submission form are done within a section, the PATCH request to update the section processes unnecessary patches on the back end. i.e. Change name, then title, then date multiple times, then name and title again. Only the last changes per input are necessary for the patch and the preceding by op and path are unnecessary to process on the backend.

Instructions for Reviewers

Determine the use case for providing the backend with all the UI form manipulations. If there is no valid use case, there is excessive processing done for each patch operation included that will nullify a previous patch.

List of changes in this PR:

  • for each new patch operation dispatched, dedupe any existing patch operations that where stored previously

To test concern:
Open inspector tools in the browser and watch the network traffic. During submission form change inputs repeatedly as if you either have a typo or inputted value in incorrect field or selected month and day of the date picker or changed your mind on an input. Then click save or click away from the submission section. There will be a PATCH request in the network traffic. Open the request body of the PATCH and there will be patch operations that are not the last changes and will not affect the JSON entity after PATCH.

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.

@tamu-sad-iii tamu-sad-iii force-pushed the dedupe-patch-operations branch from 3737eec to f40639d Compare January 26, 2024 22:01
@tdonohue tdonohue added bug 1 APPROVAL pull request only requires a single approval to merge port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release labels Jan 29, 2024
@alanorth
Copy link
Copy Markdown
Contributor

@WWelling I don't fully understand the issue this solves. Is it just an optimization or is there a bug? I am seeing values getting cleared when the autosave is triggered when moving between submission steps (#2781) though I don't know if it's related.

@tamu-sad-iii
Copy link
Copy Markdown
Contributor Author

tamu-sad-iii commented Jan 30, 2024

@alanorth it is technically not a bug. It more like an optimization but rather just preventing inefficient spurious processing on the backend and additional request bytes. It is not directly related to #2781 and I doubt it will address that issue.

@alanorth
Copy link
Copy Markdown
Contributor

@WWelling thanks. This is +1 by testing from me.

I managed to reproduce this (with network tab of developer tools open):

  1. Enter a title like "Title"
  2. Click outside the text field, then enter it again and change the title to "Title2"
  3. Click outside the text field, then enter it again and change the title to "Title3"
  4. Click outside the text field, then enter it again and change the title to "Title4"
  5. Move to another submission step to trigger the autosave

Inspect the request body of the PATCH and you see all four titles as separate replace operations:

[
   {
      "op":"replace",
      "path":"/sections/traditionalpageone/dc.title/0",
      "value":{
         "value":"Title",
         "language":null,
         "authority":null,
         "display":"Title",
         "confidence":-1,
         "place":0,
         "otherInformation":null
      }
   },
   {
      "op":"replace",
      "path":"/sections/traditionalpageone/dc.title/0",
      "value":{
         "value":"Title2",
         "language":null,
         "authority":null,
         "display":"Title2",
         "confidence":-1,
         "place":0,
         "otherInformation":null
      }
   },
   {
      "op":"replace",
      "path":"/sections/traditionalpageone/dc.title/0",
      "value":{
         "value":"Title3",
         "language":null,
         "authority":null,
         "display":"Title3",
         "confidence":-1,
         "place":0,
         "otherInformation":null
      }
   },
   {
      "op":"replace",
      "path":"/sections/traditionalpageone/dc.title/0",
      "value":{
         "value":"Title4",
         "language":null,
         "authority":null,
         "display":"Title4",
         "confidence":-1,
         "place":0,
         "otherInformation":null
      }
   }
]

With this PR the PATCH after the test sequence is:

[
   {
      "op":"replace",
      "path":"/sections/traditionalpageone/dc.title/0",
      "value":{
         "value":"Title4",
         "language":null,
         "authority":null,
         "display":"Title4",
         "confidence":-1,
         "place":0,
         "otherInformation":null
      }
   }
]

@tamu-sad-iii
Copy link
Copy Markdown
Contributor Author

@alanorth thanks for the testing.

@alanorth alanorth added this to the 8.0 milestone Feb 1, 2024
@dspace-bot
Copy link
Copy Markdown
Contributor

Successfully created backport PR for dspace-7_x:

@alanorth alanorth removed the port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release label Feb 1, 2024
@tamu-sad-iii tamu-sad-iii deleted the dedupe-patch-operations branch April 2, 2024 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1 APPROVAL pull request only requires a single approval to merge bug

Projects

No open projects
Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

4 participants