WITHDRAW / REINSTATE requests for an item

[alisaismailati]

Please note that this development has been funded by the University of California - California Digital Library.

References

RestContract PR: DSpace/RestContract#250
Back-end PR: DSpace/DSpace#9267

Description

This (PR) contains an implementation that enables all logged users to create WITHDRAWN and REINSTATE requests for an item using the quality assurance event mechanism.
Once a WITHDRAWN or REINSTATE request is submitted, it can also be also canceled.

Instructions for Reviewers and guidance

The user is provided with the option to withdraw an item from the details page of the item. Any authenticated user has the permission to withdraw an item.

• First step is to start the withdrawal

• Following that, the user can articulate the reason for the withdrawal and subsequently confirm it.

• An automatic notification will appear in a notification box, indicating that a withdrawal has been requested for this particular item.

• Clicking "Check" will navigate us to the "Quality Assurance" page, where we can view a list of topics/requests. In case there is only one topic available, it will automatically redirect to the item list ("accept/ignore/reject" page).

• The action button displays the count of elements associated with a specific topic. Clicking on it will lead to a different view of the list of items, tailored based on the user's role :

Administrator:

An administrator can reject the withdrawal

can ignore it, the request can be reconsidered later

or can accept, this item will not be discoverable and will not be archived,it can only be accessed through link

Other users:
As an example, a submitter has the authority to only reject the withdrawal request.

• If the withdrawal request is approved, the user will be notified and will be given the opportunity to reinstate the request.
  An administrator user will observe an eye icon, allowing them to proceed with a request for reinstatement:

A submitter will have a different view, a “Reinstate” button is available next to the “Take me to the home page” button and they will no longer see the item's details:

• To reinstate the request, providing a reason and confirming it would be necessary once again.

• In this case, will have the opportunity to revisit the list of topics:
  

• Additionally, there will be the option to accept this reinstatement request, thereby re-opening the possibility for withdrawal.

• Admin users have the possibility to access all the request, for all the items from "My Dspace" option:

On check button click the user will be redirected to "Quality Assurance" page

NOTE: e2e tests are failing due to the missing endpoint added on the REST PR (DSpace/DSpace#9267).

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

• 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]

@tdonohue @artlowel

@alisaismailati and @atarix83 : I retested this today, and while my prior feedback has been mostly addressed, I'm still running into some issues with this PR.

1. First, with this PR in place, the Publication Claim feature no longer works.  When I login as an Admin and go to "Notifications -> Publication Claim", I receive a 404 error.  (If I reinstall `main`, this error does not occur)

2. When visiting the MyDSpace page when a withdraw/reinstate request is submitted, I see a 404 in my browser's DevTools on this request: `GET http://localhost:4000/assets/images/qa-DSpaceUsers-logo.png`  Can we fix this?  We've had users of DSpace 7 complain on mailing lists about unnecessary 404 errors.  We should either create this image or change the code to just use the DSpace logo.  (Currently the code appears to expect that a 404 may be thrown, and I feel that's bad design. We should always use an image that we know exists.)

3. On the MyDSpace page, when using this feature alongside publication claim, we now see notifications of different types in different areas:
   ![multiple-notifications](https://private-user-images.githubusercontent.com/483997/307130493-74b3009b-f37d-4fd1-af6e-c56dde17e8f3.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MDg3MDAyNTEsIm5iZiI6MTcwODY5OTk1MSwicGF0aCI6Ii80ODM5OTcvMzA3MTMwNDkzLTc0YjMwMDliLWYzN2QtNGZkMS1hZjZlLWM1NmRkZTE3ZThmMy5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjQwMjIzJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI0MDIyM1QxNDUyMzFaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT03ZTk4MTlhMzQ2ZTk3NWJlYjkwNzkxNjU3ZjA1MjI2ZDkyNTAxYmZiNjFkMjY4YjAyZjA2OTExNzFkZjk2NTA4JlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCZhY3Rvcl9pZD0wJmtleV9pZD0wJnJlcG9faWQ9MCJ9.hKqHMRdQIrk3M2zffZyOilPF9L1-l9zt2HPYhhNJgIQ)
   
   * This is not a fault of this PR alone, but it's extremely confusing to see different types of notifications that surround the "Drag & Drop" box.  Ideally, all these various notifications would have the same general look and also be styled similarly.
   * If it's easier we can log this as a known bug to cleanup after these PRs are merged. But, I do feel this display is a bug.

All those previous points should be addressed now

4. If you login as a basic user (no special rights) and submit a Withdraw request on your MyDSpace page you'll see a generic notice that says "There are 1 pending review to check".  That notice is very confusing, as it implies that the user needs to check **their own** withdraw request.   I think we need to fix this as it's a usability issue with this feature.  Two possible fixes I can think of:
   
   * Perhaps the user who submits a request **should not see that request** on their MyDSpace page.  They can only delete it if they revisit the item in question.
   * Alternatively, we rename the generic text to say something like "You have submitted a request that is still under review".

5. That same generic message (in 4 above) also appears on the Item page for the Item you requested be withdrawn.  On that page, I think it'd be better to say something like `There are [number] pending withdraw request(s) for this item`.   That text is similar to the existing `There are [number] pending reinstatement(s) for this item.`

The framework we are using is the same used for the coar notify PR and the notifications system works in a generic way for all the sources (DSpace, openAIRE or COAR). In the mydpsace or item page context we don't have so accurate information needed to have more specific notification message unless we don't replicate all the rest requests done in the proper notification page. I honestly discourage this in order to not make the page heavier.

[artlowel]

The issue is still there, you can still change this to toBeFalse() and get the test to pass

[atarix83]

refactored

[tdonohue]

@atarix83 and @alisaismailati : A large number of e2e tests are now failing with the latest changes. (Though as I typed that I see another change was pushed, so hopefully that will help fix things up)

Regarding @atarix83 's comment above

Would it be possible on the Item page ONLY to simply change the notice that appears when you request a withdraw to say: There are [number] pending withdraw requests for this item.?

Currently, in this PR, when a reinstatement is requested, you will see a notice that says There are [number] pending reinstatement(s) for this item.. So, I'd simply like to see the same message regarding withdraw requests to make that display more consistent.

[atarix83]

@tdonohue

Would it be possible on the Item page ONLY to simply change the notice that appears when you request a withdraw to say: There are [number] pending withdraw requests for this item.?

Currently, in this PR, when a reinstatement is requested, you will see a notice that says There are [number] pending reinstatement(s) for this item.. So, I'd simply like to see the same message regarding withdraw requests to make that display more consistent.

We also reverted the reinstate message because it was based on a changed that is not compatible with the genric use of the framework. The component used is this one src/app/item-page/simple/qa-event-notification/qa-event-notification.component.html and the previous implementation where the message was different was based on a wrong assumption (basically it's checking if the item was archived or withdrawn) because it didn't consider that in the future we could add different type of correction request for the dspaceUser source and for what i've already said previously we don't have too specific information about the type of the correction for adding a specific message

[github-actions]

Hi @alisaismailati,
Conflicts have been detected against the base branch.
Please resolve these conflicts as soon as you can. Thanks!

[tdonohue]

@alisaismailati and @atarix83 : This is looking better to me now. It works based on my latest tests. I'm nearly a +1 except for some very minor things.

1. First, I have some suggestions on better text for the "There are [num] pending review to check" message. I understand it needs to be generic, so I tried to suggest a different wording (see inline below)
2. There's a minor accessibility bug (see inline below)
3. The e2e tests are failing in this PR and they are not failing on main. I haven't had a chance to dig into why, but it appears to be specific to this PR. We need to obviously get them working before we can merge this. My mistake, I realized they are supposed to fail as they depend on the backend PR first being merged. I have verified locally via yarn e2e that these all seem to pass with the backend PR installed

Thanks, overall I'm +1 the code. If you disagree with my suggested rewording (for 1 above), then I'd still merge this. But, I worry we may receive negative feedback from users once they test this.

[alisaismailati]

Hello @tdonohue ,
The modifications you recently requested have been implemented.

• the accessibility error is solved
• the text messages are updated according to your suggestion
  here is the result:
  
  

[tdonohue]

@alisaismailati or @atarix83 : While this has been updated, somehow a Code Conflict has occurred. Could you rebase this on main so that it can be mergeable again? (I'm not sure why there's a code conflict, but the PR is blocked from merging until it can be resolved)

[alisaismailati]

@tdonohue ,
the branch is now aligned with main.

[tdonohue]

👍 Thanks @alisaismailati and @atarix83 ! This looks good to me now & works well. I've also verified locally that e2e tests all pass with the backend installed.

[tdonohue]

Merged as both frontend & backend are ready. However, this feature requires documentation. So, adding the needs documentation flag until it can be added to the DSpace 8 docs.

[GraziaQuercia]

Hi @tdonohue, we worked on documentation. Let us know if any change is needed. Thanks in advance for your help!
Here's the link: https://wiki.lyrasis.org/display/DSDOC8x/Request+Withdrawn+and+Reinstate+of+an+item