Returning to author now requires a reason that is sent by email to the author #10137

luddaniel · 2023-11-22T17:57:09Z

What this PR does / why we need it:

It displays a required textarea inside the "return to author" popup that is used to comment the reason of the return. This comment is sent by email to the author.

Which issue(s) this PR closes:

Closes #3702

Special notes for your reviewer:

This PR does not cover the management of displaying reasons of return in notification page.
Notification does not share the same architecture as WorkflowComment on a DatasetVersion, so it's not easy to join on comment to a notification in case there is multiple return to the author on the same Dataset version.

Bundle.properties new values must be arbitrated, as the closing of #3702

Suggestions on how to test this:

All the XHTML part with required field and remoteCommand seems really "delicate", an attention must be paid to the other commands in the dataset (like editing/saving a dataset).
Also, validation message seems to be removed a bit too fast when tags are updated, it might need a little improvement.

Does this PR introduce a user interface change?

return_to_author.mp4

Is there a release notes update needed for this change?:

Definitely yes.

…is sent by email to the author

src/main/webapp/dataverseuser.xhtml

src/main/webapp/dataset.xhtml

src/main/java/propertyFiles/Bundle.properties

coveralls · 2023-11-22T18:05:02Z

coverage: 20.273% (-0.002%) from 20.275%
when pulling 0028b90 on Recherche-Data-Gouv:3702-return-to-author
into de45c13 on IQSS:develop.

pdurbin

Wow, I'm impressed by how little code is needed. Great job, @luddaniel!

Also, I love the video!

Now I think it's a matter of seeing if we want this or not. And if so, where it falls in our priorities.

src/main/java/propertyFiles/Bundle.properties

src/main/webapp/dataset.xhtml

pdurbin

One more question.

src/main/java/edu/harvard/iq/dataverse/DatasetPage.java

pdurbin

Adding feedback from @sbarbosadataverse (from Slack).

src/main/webapp/dataset.xhtml

src/main/java/propertyFiles/Bundle.properties

Co-authored-by: Philip Durbin <philipdurbin@gmail.com>

src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java

…acts of collection in return to author email

…thorCommand

luddaniel · 2023-11-29T17:30:40Z

Here is the demo of the last version :

demo_return_to_author_v3.mp4

DS-INRAE · 2023-11-29T14:38:12Z

src/main/java/propertyFiles/Bundle.properties

@@ -781,7 +781,8 @@ notification.email.createDataverse=Your new dataverse named {0} (view at {1} ) w
 # Bundle file editors, please note that "notification.email.createDataset" is used in a unit test
 notification.email.createDataset=Your new dataset named {0} (view at {1} ) was created in {2} (view at {3} ). To learn more about what you can do with a dataset, check out the Dataset Management - User Guide at {4}/{5}/user/dataset-management.html .
 notification.email.wasSubmittedForReview={0} (view at {1} ) was submitted for review to be published in {2} (view at {3} ). Don''t forget to publish it or send it back to the contributor, {4} ({5})\!
-notification.email.wasReturnedByReviewer={0} (view at {1} ) was returned by the curator of {2} (view at {3} ).
+notification.email.wasReturnedByReviewer={0} (view at {1} ) was returned by the curator of {2} (view at {3} ).{4}


Would be a good opportunity to be more precise :)

Suggested change

notification.email.wasReturnedByReviewer={0} (view at {1} ) was returned by the curator of {2} (view at {3} ).{4}

notification.email.wasReturnedByReviewer=Your dataset named {0} (view at {1} ) was returned by the curator of {2} (view at {3} ).{4}

Also in wasSubmittedForReview, see other suggestion

DS-INRAE · 2023-11-29T14:38:39Z

src/main/java/propertyFiles/Bundle.properties

@@ -781,7 +781,8 @@ notification.email.createDataverse=Your new dataverse named {0} (view at {1} ) w
 # Bundle file editors, please note that "notification.email.createDataset" is used in a unit test
 notification.email.createDataset=Your new dataset named {0} (view at {1} ) was created in {2} (view at {3} ). To learn more about what you can do with a dataset, check out the Dataset Management - User Guide at {4}/{5}/user/dataset-management.html .
 notification.email.wasSubmittedForReview={0} (view at {1} ) was submitted for review to be published in {2} (view at {3} ). Don''t forget to publish it or send it back to the contributor, {4} ({5})\!


Suggested change

notification.email.wasSubmittedForReview={0} (view at {1} ) was submitted for review to be published in {2} (view at {3} ). Don''t forget to publish it or send it back to the contributor, {4} ({5})\!

notification.email.wasSubmittedForReview=Your dataset named {0} (view at {1} ) was submitted for review to be published in {2} (view at {3} ). Don''t forget to publish it or send it back to the contributor, {4} ({5})\!

see other comment

src/main/java/propertyFiles/Bundle.properties

src/main/webapp/dataset.xhtml

luddaniel · 2023-12-06T16:52:24Z

Hello @pdurbin, it seems like everybody's desire has been listened, how can we move forward on this PR ?

pdurbin · 2023-12-08T17:22:57Z

@luddaniel yes, thanks for being so responsive! Next we check with @cmbz and @scolapasta to see where this fits into priorities, etc.

cmbz · 2023-12-11T17:49:02Z

2023/12/11

@luddaniel @pdurbin As per conversation with @scolapasta we're adding it to the 6.2 proposals.
Forgot to add: this needs sizing; please suggest a size.

cmbz · 2023-12-11T17:53:52Z

2023/12/11: Updated labels to match original issue: #3702

pdurbin · 2023-12-11T18:55:12Z

@cmbz thanks!

My only reservation is that it's a new required field. Will some curators be annoyed by it? And does the API allow you to get around this requirement? We should probably have consistency between the UI and API, either required or not. And maybe it should be configurable (globally, for now) if it's required or not. @luddaniel do you have thoughts on this?

As for size, if we accept the code as-is with no changes, it's probably a 3. If we need to meet and discuss what the behavior should be, it's probably higher. Maybe we can continue discussing here.

src/main/java/edu/harvard/iq/dataverse/api/Datasets.java

DS-INRAE · 2023-12-22T10:01:43Z

Mentionning the SPA frontend issue for this PR to appear there :

Feature request : Return to Author - feedback note in the UI dataverse-frontend#201

cmbz · 2024-01-08T19:08:01Z

2024/01/08: Moved to sprint ready as per prioritization meeting

pdurbin · 2024-02-22T17:55:03Z

@luddaniel over at #3702 (comment) @philippconzett is asking if the feature can be turned off. What do you think? Should we make it configurable? Make it possible to turn it off?

luddaniel · 2024-02-26T14:37:28Z

@luddaniel over at #3702 (comment) @philippconzett is asking if the feature can be turned off. What do you think? Should we make it configurable? Make it possible to turn it off?

I think we can do that with some work; Can we agree and the spec before developments/tests ?
A new setting must be added, named :ReturnToAuthorReasonRequired, by default true (if absent) and can be disabled using this curl command curl -X PUT -d false http://localhost:8080/api/admin/settings/:ReturnToAuthorReasonRequired.
This setting set to false will act the same on GUI popin (not displayed therefore not required) and API endpoint (not required).
OR; @philippconzett can you handle this by explain to curators to type something like "A curation report with feedback and suggestions/instructions will follow in another email ?".
I mean this required reason can be addressed with whatever meaningful informations as long as a minimum of 1 character...
The release note can be decorated by Do not consider this requirement as a barrier as it can be supplemented with a creative and meaningful comment such as "The author would like to modify his dataset", "Files are missing", "Nothing to report", "A curation report with comments and suggestions/instructions will follow in another email".

As time is precious, I prefer option 2.

Waiting for your response @pdurbin @philippconzett
Regards

philippconzett · 2024-02-26T15:41:04Z

Thanks, @luddaniel! Option 2 is fine for us. Please make sure to include the instruction in the release notes and the guide update.

sekmiller

Looks like all of the previous update requests have been implemented. @luddaniel can you please make sure that this branch is up to date with the latest from develop? thanks!

philippconzett · 2024-06-23T12:59:30Z

I'm having second thoughts (sorry!) about this feature being mandatory for all Dataverse installations starting with v6.2, for a couple of reasons:

Our repository already has a well-proven curation workflow which would been unnecessary disturbed by this feature. Curators do not want to need adding an additional message. Also, some curators might start using this feature to give initial feedback, and then further feedback by email through our curation report. This will be confusing for both depositors and curators.
Testing v6.2, I've discovered that there issues with system notifications not being sent to some email types, meaning that we might not even know whether the depositor ever gets the message sent with this feature.

Thus, I wonder how we could mitigate the trouble this feature might cause for repositories with established curation workflows. Here are some suggestions:
Solution a. Make the feature configurable and turn if off by default. See discussion above about making the feature configurable.
Solution b. Make it possible to add a pre-filled, non-editable text to the form.

Suggestion a is preferable. Can someone estimate the resources necessary to implement solution b.

I know I should have realized and discussed this earlier, but I think in the future, we should have some guidelines for how ideas of features which might have unfortunate consequences for other installations should be handled before PRs are created and merged. For example, in the Invenio RDM community, new feature ideas are first discussed in GitHub Discussions and only after enough votes and a consensus, they are moved forward.

Thanks!

IQSS#3702 - Returning to author now requires a commented reason that …

d527fc8

…is sent by email to the author

luddaniel commented Nov 22, 2023

View reviewed changes

src/main/webapp/dataverseuser.xhtml Outdated Show resolved Hide resolved

luddaniel commented Nov 22, 2023

View reviewed changes

src/main/webapp/dataset.xhtml Show resolved Hide resolved

luddaniel commented Nov 22, 2023

View reviewed changes

src/main/java/propertyFiles/Bundle.properties Outdated Show resolved Hide resolved

pdurbin reviewed Nov 28, 2023

View reviewed changes

src/main/java/propertyFiles/Bundle.properties Outdated Show resolved Hide resolved

src/main/webapp/dataset.xhtml Show resolved Hide resolved

pdurbin mentioned this pull request Nov 28, 2023

Return to Author: When returning datasets that were submitted for review, I want to add a feedback note. #3702

Closed

pdurbin reviewed Nov 28, 2023

View reviewed changes

src/main/java/edu/harvard/iq/dataverse/DatasetPage.java Show resolved Hide resolved

pdurbin reviewed Nov 29, 2023

View reviewed changes

src/main/webapp/dataset.xhtml Outdated Show resolved Hide resolved

src/main/java/propertyFiles/Bundle.properties Outdated Show resolved Hide resolved

Update src/main/java/propertyFiles/Bundle.properties

caf5f67

Co-authored-by: Philip Durbin <philipdurbin@gmail.com>

pdurbin reviewed Nov 29, 2023

View reviewed changes

src/main/java/edu/harvard/iq/dataverse/MailServiceBean.java Outdated Show resolved Hide resolved

luddaniel added 3 commits November 29, 2023 17:16

IQSS#3702 - Increased maxlength of message to 2000 + Added email cont…

36a259f

…acts of collection in return to author email

IQSS#3702 - Added test on null or empty comment for ReturnDatasetToAu…

136d42b

…thorCommand

Merge branch 'develop' into 3702-return-to-author

5e5377f

IQSS#3702 fixed Unit Tests errors

1112ae7

DS-INRAE reviewed Nov 29, 2023

View reviewed changes

cmbz added this to the 6.2 milestone Dec 11, 2023

pdurbin added the Size: 3 A percentage of a sprint. 2.1 hours. label Dec 11, 2023

luddaniel commented Dec 13, 2023

View reviewed changes

src/main/java/edu/harvard/iq/dataverse/api/Datasets.java Show resolved Hide resolved

IQSS#3702 - Code commentary to be removed

7d5e940

pdurbin changed the title ~~#3702 - Returning to author now requires a commented reason that is sent by email to the author~~ Returning to author now requires a reason that is sent by email to the author Dec 15, 2023

DS-INRAE mentioned this pull request Dec 22, 2023

Feature request : Return to Author - feedback note in the UI IQSS/dataverse-frontend#201

Open

IQSS#3702 - Adding release note

ba8c58d

IQSS#3702 - Adding last Bundle.properties corrections

3c5f989

luddaniel added 2 commits February 28, 2024 14:57

IQSS#3702 - Updated release note and guide

2efa607

Merge branch 'develop' into 3702-return-to-author

0028b90

sekmiller self-assigned this Feb 29, 2024

sekmiller approved these changes Feb 29, 2024

View reviewed changes

sekmiller removed their assignment Feb 29, 2024

stevenwinship self-assigned this Mar 6, 2024

stevenwinship merged commit 4716c7a into IQSS:develop Mar 6, 2024
12 checks passed

stevenwinship removed their assignment Mar 6, 2024

cmbz mentioned this pull request Apr 1, 2024

Epic: GREI - Dataverse Rearchitecture IQSS/dataverse-pm#123

Open

qqmyers mentioned this pull request Jun 25, 2024

IQSS/10137-2 Add flag to remove Return To Author Reason #10655

Merged

luddaniel deleted the 3702-return-to-author branch September 19, 2024 14:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Returning to author now requires a reason that is sent by email to the author #10137

Returning to author now requires a reason that is sent by email to the author #10137

luddaniel commented Nov 22, 2023 •

edited

Loading

coveralls commented Nov 22, 2023 •

edited

Loading

pdurbin left a comment

pdurbin left a comment

pdurbin left a comment

luddaniel commented Nov 29, 2023

DS-INRAE Nov 29, 2023

DS-INRAE Nov 29, 2023

luddaniel commented Dec 6, 2023

pdurbin commented Dec 8, 2023

cmbz commented Dec 11, 2023 •

edited

Loading

cmbz commented Dec 11, 2023

pdurbin commented Dec 11, 2023

DS-INRAE commented Dec 22, 2023

cmbz commented Jan 8, 2024

pdurbin commented Feb 22, 2024

luddaniel commented Feb 26, 2024

philippconzett commented Feb 26, 2024

sekmiller left a comment

philippconzett commented Jun 23, 2024

	notification.email.wasReturnedByReviewer={0} (view at {1} ) was returned by the curator of {2} (view at {3} ).{4}
	notification.email.wasReturnedByReviewer=Your dataset named {0} (view at {1} ) was returned by the curator of {2} (view at {3} ).{4}

	notification.email.wasSubmittedForReview={0} (view at {1} ) was submitted for review to be published in {2} (view at {3} ). Don''t forget to publish it or send it back to the contributor, {4} ({5})\!
	notification.email.wasSubmittedForReview=Your dataset named {0} (view at {1} ) was submitted for review to be published in {2} (view at {3} ). Don''t forget to publish it or send it back to the contributor, {4} ({5})\!

Returning to author now requires a reason that is sent by email to the author #10137

Returning to author now requires a reason that is sent by email to the author #10137

Conversation

luddaniel commented Nov 22, 2023 • edited Loading

coveralls commented Nov 22, 2023 • edited Loading

pdurbin left a comment

Choose a reason for hiding this comment

pdurbin left a comment

Choose a reason for hiding this comment

pdurbin left a comment

Choose a reason for hiding this comment

luddaniel commented Nov 29, 2023

DS-INRAE Nov 29, 2023

Choose a reason for hiding this comment

DS-INRAE Nov 29, 2023

Choose a reason for hiding this comment

luddaniel commented Dec 6, 2023

pdurbin commented Dec 8, 2023

cmbz commented Dec 11, 2023 • edited Loading

cmbz commented Dec 11, 2023

pdurbin commented Dec 11, 2023

DS-INRAE commented Dec 22, 2023

cmbz commented Jan 8, 2024

pdurbin commented Feb 22, 2024

luddaniel commented Feb 26, 2024

philippconzett commented Feb 26, 2024

sekmiller left a comment

Choose a reason for hiding this comment

philippconzett commented Jun 23, 2024

luddaniel commented Nov 22, 2023 •

edited

Loading

coveralls commented Nov 22, 2023 •

edited

Loading

cmbz commented Dec 11, 2023 •

edited

Loading