Skip to content
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

Fix cache issue when depositing a submission #2596

Merged
merged 1 commit into from
Nov 9, 2023

Conversation

atarix83
Copy link
Contributor

@atarix83 atarix83 commented Nov 2, 2023

References

Description

This is a quick fix for the problems mentioned before. It's not resolving the general problem with reRequestOnStale mentioned here

Instructions for Reviewers

Check the mentioned bugs are fixed

Include guidance for how to test or review your PR. This may include: steps to reproduce a bug, screenshots or description of a new feature, or reasons behind specific changes.

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 atarix83 requested review from tdonohue and artlowel November 2, 2023 16:04
@tdonohue tdonohue added component: submission high priority 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 Nov 2, 2023
@tdonohue
Copy link
Member

tdonohue commented Nov 2, 2023

@atarix83 : I tested this PR today, and I can still reproduce both bugs with this fix in place. So, I'm not sure what this is doing?

In my setup, I'm running this PR in the UI in production mode (yarn build:prod, yarn serve:ssr). I then tried to reproduce the bugs as described. For example, for #1924, I followed the exact steps in its description:

  1. Started a new "Publication" submission (collection selected doesn't matter)
  2. Do not fill out anything on the form. Immediately click "Deposit"
  3. Wait for validation error to go away. Immediately click "Deposit" again.
  4. Entire page hangs. If I watch the backend logs (dspace.log) I can see the UI page is requesting GET /server/api/submission/workspaceitems/[:id] over and over again (in an infinite loop)

I've rebuilt this PR twice in case something was cached. I'm just not able to figure out how to test this. Is it working properly for you when running in Production mode?

@atarix83
Copy link
Contributor Author

atarix83 commented Nov 6, 2023

@tdonohue

i've tested it with SSR because i didn't previously and it works fine. Both bugs are fixed for what i can reproduce.
@artlowel could you help us to verify if this PR is working for you ?

@tdonohue
Copy link
Member

tdonohue commented Nov 8, 2023

@atarix83 ; I re-tested quickly today and I can no longer reproduce the odd behavior I was seeing before. This appears to work, but I want to do some further testing tomorrow before giving it a +1.

Apologies, I have no idea what I must have done wrong before. Either I failed to pull down the PR code properly, or some sort of caching was getting in the way. But basic tests on Chrome today show this appears to be working. I'll triple check things tomorrow just to be sure :)

Copy link
Member

@artlowel artlowel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @atarix83!

That works

Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks @atarix83 ! Retested today and verified that both #2577 and #1924 are fixed by this PR. (My prior tests were somehow flawed... I suspect I didn't build the PR's branch and was accidentally running main.)

@tdonohue tdonohue added this to the 8.0 milestone Nov 9, 2023
@tdonohue tdonohue merged commit f116bdf into DSpace:main Nov 9, 2023
10 checks passed
@dspace-bot
Copy link
Contributor

Successfully created backport PR for dspace-7_x:

@tdonohue tdonohue removed the port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release label Nov 9, 2023
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 component: submission high priority
Projects
No open projects
Status: ✅ Done
4 participants