[DS-3951] Send Request Copy email to multiple recipients, and some new recipient strategies #2115

mwoodiupui · 2018-07-12T15:57:22Z

Fixes #7298
https://jira.duraspace.org/browse/DS-3951

Recipient strategy classes now return a List rather than a single recipient.

The CombiningRequestItemStrategy composes a list of recipients from the outputs of other strategies, as configured by Spring.

Checklist

My PR is small in size (e.g. less than 1,000 lines of code, not including comments & integration tests). Exceptions may be made if previously agreed upon.
My PR passes Checkstyle validation based on the Code Style Guide.
My PR includes Javadoc for all new (or modified) public methods and classes. It also includes Javadoc for large or complex private methods.
My PR passes all tests and includes new/updated Unit or Integration Tests based on the Code Testing Guide.
If my PR includes new, third-party dependencies (in any pom.xml), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
If my PR modifies the REST API, I've linked to the REST Contract page (or open PR) related to this change.

…list collection administrators and to combine other strategies.

mwoodiupui · 2018-11-12T15:47:04Z

Rebased to cure recent merge conflict.

mwoodiupui · 2020-08-04T18:47:49Z

Close/open to get Travis to run.

mwoodiupui · 2021-09-09T17:31:09Z

Close/open to jog tests.

abollini

Thanks @mwoodiupui for this PR, I like the direction that you suggest. Inline some request of clarification/small changes

...pi/src/main/java/org/dspace/app/requestitem/CollectionAdministratorsRequestItemStrategy.java

dspace-api/src/main/java/org/dspace/app/requestitem/CombiningRequestItemStrategy.java

dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemAuthorExtractor.java

dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemAuthor.java

dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemMetadataStrategy.java

dspace-api/src/test/java/org/dspace/app/requestitem/CombiningRequestItemStrategyTest.java

abollini · 2022-07-23T17:29:39Z

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java

-        if (!authorizer.getEmail().equals(context.getCurrentUser().getEmail())) {
+
+        boolean authorized = false;
+        String requester = context.getCurrentUser().getEmail();
+        for (RequestItemAuthor authorizer : authorizers) {
+            authorized |= authorizer.getEmail().equals(requester);
+        }
+        if (!authorized) {


this is not related to this PR as the behavior is not changing but I want to raise a question that can be eventually moved to a followup ticket if other feel reasonable. Why are we checking authorization here? the user has a valid token, maybe the request email as been forwarded to a trusted colleague asking to proceed in this way we will prevent that. This also require the user to be loggedin and maybe the author is external or not longer affiliated with the repo (this would require also removing the auth annotation check on the method)

just discovered that this is the topic of #8319

dspace/config/spring/api/requestitem.xml

...pi/src/main/java/org/dspace/app/requestitem/CollectionAdministratorsRequestItemStrategy.java

tdonohue · 2022-08-19T16:14:11Z

@mwoodiupui : Just a quick note that this seems to have a merge conflict now.

tdonohue

👍 Thanks @mwoodiupui ! I tested this manually by enabling the CombiningRequestItemStrategy to use the other three options (Metadata, Helpdesk and Collection Admin). It works!

I have a few very minor comments inline which I think should be cleaned up prior to merger. But, overall, this looks great.

dspace/config/spring/api/requestitem.xml

tdonohue · 2022-09-07T17:11:06Z

@abollini : If you have a chance, could you re-review this PR? I believe all your prior feedback has been addressed & this is PR already has a +1 from me (and I tested it and found it to work well). Thanks!

abollini

thanks @mwoodiupui all my previous feedback have been addressed. Merging as it is already +1

mwoodiupui added improvement new feature work in progress PR is still being worked on & is not currently ready for review labels Jul 12, 2018

mwoodiupui added this to the 7.0 milestone Jul 12, 2018

tdonohue added authorization Related to user authorization / permissions backend: email-system Related to emails sent from DSpace labels Aug 21, 2018

[DS-3951] Change request-copy recipient to a List; add strategies to …

d6a8002

…list collection administrators and to combine other strategies.

mwoodiupui force-pushed the DS-3951 branch from feb456d to d6a8002 Compare November 12, 2018 15:46

mwoodiupui removed the work in progress PR is still being worked on & is not currently ready for review label Dec 5, 2018

mwoodiupui changed the base branch from master to main July 14, 2020 13:48

mwoodiupui closed this Aug 4, 2020

mwoodiupui reopened this Aug 4, 2020

github-actions bot added the merge conflict PR has a merge conflict that needs resolution label Oct 8, 2020

Merge remote-tracking branch 'upstream/main' into DS-3951

80975df

mwoodiupui removed the merge conflict PR has a merge conflict that needs resolution label Mar 13, 2021

[DS-3951] Update a newer test to cope with lists of recipients.

5c6cd3e

mwoodiupui removed this from the 7.0 milestone Mar 14, 2021

mwoodiupui added the needs triage New issue needs triage and/or scheduling label Mar 14, 2021

mwoodiupui closed this Sep 9, 2021

mwoodiupui reopened this Sep 9, 2021

github-actions bot added the merge conflict PR has a merge conflict that needs resolution label Oct 19, 2021

Merge branch 'main' into DS-3951

f04e263

mwoodiupui removed the merge conflict PR has a merge conflict that needs resolution label Mar 25, 2022

mwoodiupui and others added 3 commits March 25, 2022 17:00

[DS-3951] A new class needs to understand multiple recipients.

758b02f

[DS-3951] Fix another missed usage.

bd80b0e

[DS-3951] Add tests for new strategy classes.

2312724

mwoodiupui removed the needs triage New issue needs triage and/or scheduling label Mar 28, 2022

mwoodiupui added this to Needs Reviewers Assigned in DSpace 7.4 release via automation May 5, 2022

mwoodiupui self-assigned this Jun 22, 2022

tdonohue requested review from abollini and tdonohue July 14, 2022 14:36

tdonohue moved this from Needs Reviewers Assigned to Under Review in DSpace 7.4 release Jul 14, 2022

abollini requested changes Jul 23, 2022

View reviewed changes

[DS-3951] Address review comments.

27143d8

mwoodiupui requested a review from abollini August 12, 2022 18:22

[DS-3951] More review comments.

237ffe4

github-actions bot added the merge conflict PR has a merge conflict that needs resolution label Aug 17, 2022

mwoodiupui added 2 commits August 26, 2022 11:15

Merge branch 'main' into DS-3951

8f3289e

[DS-3951] Remove unused import to satisfy Checkstyle.

e4a4737

mwoodiupui removed the merge conflict PR has a merge conflict that needs resolution label Aug 26, 2022

mwoodiupui mentioned this pull request Aug 29, 2022

Reassign submitter email address for Request a Copy DSpace/dspace-angular#1785

Closed

tdonohue approved these changes Aug 29, 2022

View reviewed changes

dspace/config/spring/api/requestitem.xml Outdated Show resolved Hide resolved

dspace/config/spring/api/requestitem.xml Outdated Show resolved Hide resolved

[DS-3951] Improve configuration documentation.

07a4649

tdonohue moved this from Under Review to Reviewer Approved in DSpace 7.4 release Sep 1, 2022

abollini approved these changes Sep 15, 2022

View reviewed changes

abollini merged commit 3b23ff9 into DSpace:main Sep 15, 2022

DSpace 7.4 release automation moved this from Reviewer Approved to Done Sep 15, 2022

mwoodiupui mentioned this pull request Sep 16, 2022

Enable to answer a request a copy using "helpdesk" strategy #8319

Merged

6 tasks

tdonohue added this to the 7.4 milestone Oct 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DS-3951] Send Request Copy email to multiple recipients, and some new recipient strategies #2115

[DS-3951] Send Request Copy email to multiple recipients, and some new recipient strategies #2115

mwoodiupui commented Jul 12, 2018 •

edited

mwoodiupui commented Nov 12, 2018

mwoodiupui commented Aug 4, 2020

mwoodiupui commented Sep 9, 2021

abollini left a comment

abollini Jul 23, 2022

abollini Jul 23, 2022

tdonohue commented Aug 19, 2022

tdonohue left a comment

tdonohue commented Sep 7, 2022

abollini left a comment

[DS-3951] Send Request Copy email to multiple recipients, and some new recipient strategies #2115

[DS-3951] Send Request Copy email to multiple recipients, and some new recipient strategies #2115

Conversation

mwoodiupui commented Jul 12, 2018 • edited

Checklist

mwoodiupui commented Nov 12, 2018

mwoodiupui commented Aug 4, 2020

mwoodiupui commented Sep 9, 2021

abollini left a comment

Choose a reason for hiding this comment

abollini Jul 23, 2022

Choose a reason for hiding this comment

abollini Jul 23, 2022

Choose a reason for hiding this comment

tdonohue commented Aug 19, 2022

tdonohue left a comment

Choose a reason for hiding this comment

tdonohue commented Sep 7, 2022

abollini left a comment

Choose a reason for hiding this comment

mwoodiupui commented Jul 12, 2018 •

edited