Skip to content

Submission item/bitstream access conditions fixes#1698

Merged
tdonohue merged 4 commits intoDSpace:mainfrom
4Science:CST-6174-access-conditions-fix
Jun 23, 2022
Merged

Submission item/bitstream access conditions fixes#1698
tdonohue merged 4 commits intoDSpace:mainfrom
4Science:CST-6174-access-conditions-fix

Conversation

@atarix83
Copy link
Copy Markdown
Contributor

References

Instructions for Reviewers

follow instructions in the issues and check they are fixed

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.

@tdonohue tdonohue self-requested a review June 22, 2022 14:00
@tdonohue tdonohue added this to the 7.3 milestone Jun 22, 2022
@tdonohue tdonohue added the 1 APPROVAL pull request only requires a single approval to merge label Jun 22, 2022
Copy link
Copy Markdown
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.

@atarix83 : I tested this today & it partially works.

I see that #1696 is fixed. So, that's good.

However, #1697 is NOT fixed as far as I can see. It now throws an exception (in the DevTools) and results in a 500 error on the backend.

Here's what I tried:

  1. First, I used the default embargo setting for testing, which includes <property name="startDateLimit" value="+36MONTHS"/>
  2. I started a new submission, uploading a file.
  3. I selected to embargo the file, and set the start date at 5 years from now. (this should be invalid as the max is 3 years)
  4. After clicking Save, nothing happens in the UI. The popup refuses to close or save the value. The "Save" button disappears in the popup, and there is no way to close it.
  5. In my Chrome DevTools, I see that a PATCH was sent to the backend and the backend threw a 500 error. Here's the stacktrace:
org.springframework.data.rest.webmvc.json.patch.PatchException: Error processing the patch request
at org.dspace.app.rest.submit.SubmissionService.evaluatePatchToInprogressSubmission(SubmissionService.java:455)
at
org.dspace.app.rest.repository.WorkspaceItemRestRepository.patch(WorkspaceItemRestRepository.java:212)
at org.dspace.app.rest.repository.WorkspaceItemRestRepository.patch(WorkspaceItemRestRepository.java:75)
at org.dspace.app.rest.repository.WorkspaceItemRestRepository$$FastClassBySpringCGLIB$$987ae
...
Caused by: java.lang.IllegalStateException: The start date of access condition embargo should be earlier than +36MONTHS
from now.

So, it appears that the backend knows that the date is invalid & sent back a 500 exception. But, the frontend either needs to know how to process that 500 exception or avoid sending the PATCH request if it is known to be invalid.

@atarix83
Copy link
Copy Markdown
Contributor Author

@tdonohue

i found out it happened when the access condition was already selected. I that case the max start date wasn't set properly.
Now it should be fixed, you could not be able to select a date bigger than the limit now.
Anyway I agree the rest server should handle the invalid date case properly, so I've opened a new issue that could be address in 7.4 DSpace/DSpace#8371

@atarix83 atarix83 requested a review from tdonohue June 23, 2022 10:36
Copy link
Copy Markdown
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 this today and the bug I saw before is fixed. I can no longer select a date that is past the limit from the UI.

This PR seems like much improved behavior based on current main. So, I'm going to merge immediately to get it into 7.3. If we find any later bugs we can address them at a later time.

@tdonohue tdonohue merged commit f4c6284 into DSpace:main Jun 23, 2022
@abollini abollini deleted the CST-6174-access-conditions-fix branch June 26, 2022 13:33
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 high priority

Projects

None yet

2 participants