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

Use support email in the system email message "closing" text #10504

Merged
merged 8 commits into from
Sep 18, 2024

Conversation

PaulBoon
Copy link
Contributor

@PaulBoon PaulBoon commented Apr 18, 2024

What this PR does / why we need it:
Uses the support email address (when configured) in the system email 'closing' text, which refers to contacting support.

Which issue(s) this PR closes:

Closes #10287

Special notes for your reviewer:

Suggestions on how to test this:
Force an email sent to you, for instance by requesting to change the password.
When the support email has been configured and is different from the system email it should show that at the end of the email message ('from' is still the system email).
When the support email is not configured the system email should be used instead.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

Is there a release notes update needed for this change?:
Warn sysadmins that if they have set that support-email, that will be used in those emails and if they don't want that they should either change or remove it.

Additional documentation:

@PaulBoon PaulBoon changed the title Use support email in the system email message 'closing' text 10287: Use support email in the system email message 'closing' text Apr 18, 2024
@coveralls
Copy link

Coverage Status

coverage: 20.72% (+0.001%) from 20.719%
when pulling fdb5932 on PaulBoon:UseSupportEmailInClosingText
into e6b2661 on IQSS:develop.

@PaulBoon PaulBoon marked this pull request as ready for review April 18, 2024 13:13
Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

This seems fine but can you please add docs and a release note?

@PaulBoon
Copy link
Contributor Author

PaulBoon commented May 2, 2024

@pdurbin would be great if this is moved to QA

@pdurbin
Copy link
Member

pdurbin commented May 2, 2024

@PaulBoon well, it's not up to me. 😄

@cmbz and @scolapasta how to do you feel about prioritizing this?

Do note that for Harvard Dataverse (according to recent support tickets), the email closing already ends with a support address...

"You may contact us for support at support@dataverse.harvard.edu.

Thank you,
Harvard Dataverse Support"

... so I suppose this means that we don't have dataverse.mail.support-email configured. Basically, there should be no change for Harvard Dataverse (not that I've tested this PR). But it will help other installations, like DANS.

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

Just a suggestion to change straight quotes to double quotes. (I should write this up in a style guide.)

doc/sphinx-guides/source/installation/config.rst Outdated Show resolved Hide resolved
PaulBoon and others added 2 commits May 8, 2024 15:29
Co-authored-by: Philip Durbin <philipdurbin@gmail.com>
…xt.md

Co-authored-by: Philip Durbin <philipdurbin@gmail.com>
@Louis-wr
Copy link
Contributor

Hi,
Is there any planed progress on this? @PaulBoon @pdurbin

@pdurbin pdurbin added Type: Feature a feature request Feature: Notifications Size: 3 A percentage of a sprint. 2.1 hours. labels Jul 10, 2024
@pdurbin
Copy link
Member

pdurbin commented Jul 10, 2024

@Louis-wr it's a nice, small change. I'm ok with it being moved forward. I threw a small size on it (3).

@qqmyers this is under "external commitments (Jim)" right now. I'm not sure why.

@cmbz @scolapasta how do you feel about prioritizing this for an upcoming sprint?

@cmbz
Copy link

cmbz commented Jul 10, 2024

Okay by me, just noting though that there are a large number of items currently in Sprint Ready, so it might take a while if @scolapasta agrees.

@qqmyers qqmyers added this to the 6.4 milestone Jul 10, 2024
@qqmyers qqmyers added the Consider For Next Release A simple change (eg bug fix) that would be good to prioritize since it has been seen in the wild label Jul 10, 2024
@qqmyers qqmyers removed this from the 6.4 milestone Jul 10, 2024
@cmbz cmbz added the FY25 Sprint 6 FY25 Sprint 6 label Sep 11, 2024
@stevenwinship stevenwinship self-assigned this Sep 12, 2024
@stevenwinship stevenwinship removed their assignment Sep 12, 2024
@pdurbin
Copy link
Member

pdurbin commented Sep 17, 2024

@PaulBoon I don't have permission to push to your fork. Can you please merge the latest from the "develop" branch into this pull request? It should resolve the error I'm seeing:

dev_dataverse> /opt/payara/scripts/init_2_configure.sh: line 55: POSTBOOT_COMMANDS: unbound variable

Thanks!

@coveralls
Copy link

Coverage Status

Changes unknown
when pulling 167799f on PaulBoon:UseSupportEmailInClosingText
into ** on IQSS:develop**.

@PaulBoon
Copy link
Contributor Author

@pdurbin Just had some trouble pulling and pushing, but it should be ok now.

@pdurbin
Copy link
Member

pdurbin commented Sep 18, 2024

@PaulBoon thanks, this seems to work fine. I'm seeing the email I configured (in blue, below)...

Screenshot 2024-09-18 at 11 59 31 AM

... when I put it in my docker compose file:

% git diff docker-compose-dev.yml
diff --git a/docker-compose-dev.yml b/docker-compose-dev.yml
index 402a95c0e1..407b509044 100644
--- a/docker-compose-dev.yml
+++ b/docker-compose-dev.yml
@@ -24,6 +24,7 @@ services:
       DATAVERSE_AUTH_OIDC_CLIENT_SECRET: 94XHrfNRwXsjqTqApRrwWmhDLDHpIYV8
       DATAVERSE_AUTH_OIDC_AUTH_SERVER_URL: http://keycloak.mydomain.com:8090/realms/test
       DATAVERSE_SPI_EXPORTERS_DIRECTORY: "/dv/exporters"
+      DATAVERSE_MAIL_SUPPORT_EMAIL: foo@bar.com
       # These two oai settings are here to get HarvestingServerIT to pass
       dataverse_oai_server_maxidentifiers: "2"
       dataverse_oai_server_maxrecords: "2"

Merging.

@pdurbin pdurbin merged commit 1fffec4 into IQSS:develop Sep 18, 2024
12 checks passed
@pdurbin pdurbin removed their assignment Sep 18, 2024
@pdurbin pdurbin added this to the 6.4 milestone Sep 18, 2024
@pdurbin pdurbin changed the title 10287: Use support email in the system email message 'closing' text Use support email in the system email message "closing" text Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Consider For Next Release A simple change (eg bug fix) that would be good to prioritize since it has been seen in the wild Feature: Notifications FY25 Sprint 6 FY25 Sprint 6 Size: 3 A percentage of a sprint. 2.1 hours. Type: Feature a feature request
Projects
Status: Done 🧹
Development

Successfully merging this pull request may close these issues.

Use support email in the 'closing' text of the system email message
7 participants