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

IQSS/9185 contact email updates #9186

Merged

Conversation

qqmyers
Copy link
Member

@qqmyers qqmyers commented Nov 23, 2022

NOTE: per review, the settings have been replaced with Microprofile entries. The documentation has been updated but the PR notes below have not been updated!

What this PR does / why we need it: This PR includes several enhancements to the emails from the contact forms that have been implemented and deployed at QDR. It also include fixes to make the /admin/feedback endpoint work.

Specific changes:

  • new :CCSupportOnContactEmails setting which will include the support email address as a CC: entry when contact/feedback emails are sent to the contacts for a collection, dataset, or datafile. The setting also adds a CC line in the form so that users are aware that it will happen
  • new :SupportEmail setting that allows a separate email, distinct from the :SystemEmail to be used as the from address in emails from the contact form/ feedback api. (Useful when admins want :SystemEmail to be a no-reply address. and/or to direct contact form emails to a different set of people/different support system.)
  • Switch to sending one email to all contacts instead of sending an email to each. This is hopefully helpful in general (contacts can see who else is being informed) and simplifies adding a cc to support.
  • Use of the PersonOrOrg algorithm to decide if a name refers to a person, in which case a <lastname>,<firstname> order should be reversed in the greeting ("Hello Homer Simpson"), or an Org (where a name with commas should not be changed).
  • Addition of an algorithm to separate names with commas and include a final "and" when there are multiple contacts (e.g. "Hello Homer Simpson, Marge Simpson, and Bart Simpson")
  • Update of tests to match
  • Bug fixes/completion of the /admin/feedback endpoint so it sends emails.
  • Docs for the api call and new settings, release notes.

Which issue(s) this PR closes:

Closes #9185

Special notes for your reviewer:

Depends on #9089 for the PersonOrOrgUtil changes. I've copied that class here so it will build.
Any of this can be broken out/dropped if the whole set of changes is not desired.

Suggestions on how to test this:

  • Try the contact form and API call. Use multiple contacts for some tests to verify that one email is sent.
  • Try the CC setting and confirm there's a cc in the UI and the email.
  • Try the :SupportEmail setting and confirm that emails from the contact forms (and not other emails) are from that address.
  • Try Org names like "IBM, Inc." to make sure they don't get flipped to "Hello Inc. IBM" in the greeting line in the emails.

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

Using the CCSupportOnContactEmails setting adds a line to the contact form:
image

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

Additional documentation:

@qqmyers qqmyers added the GDCC: QDR of interest to QDR label Nov 23, 2022
@qqmyers qqmyers marked this pull request as ready for review November 23, 2022 18:15
@coveralls
Copy link

coveralls commented Nov 23, 2022

Coverage Status

Coverage: 20.146% (-0.04%) from 20.184% when pulling cd2f196 on QualitativeDataRepository:IQSS/9185-contact_email_updates into e4961fe on IQSS:develop.

@mreekie
Copy link

mreekie commented Jan 23, 2023

Grooming:

This issue was in the New or No Status columns on the Dataverse Global Backlog Board.
Those columns have been removed.
This issue has been removed from the board

This is NOT a reflection of the priority of these issues.
However, as we have worked on the grooming process, it became clear that these issues which are in columns that are not stewarded will likely not get prioritized or sized.

To get this item onto the Dataverse Global Backlog Board, please reach out to one of the Stewards on the board.
You can also reach out to @mreekie and I can connect you with the steward of the appropriate queue

@jggautier
Copy link
Contributor

jggautier commented Jan 23, 2023

new :SupportEmail setting that allows a separate email, distinct from the :SystemEmail to be used as the from address in emails from the contact form/ feedback api. (Useful when admins want :SystemEmail to be a no-reply address. and/or to direct contact form emails to a different set of people/different support system.)

  • I've been excited about this PR because of this line! Right now, the same email address is used when:
    Dataverse sends automated email notifications to users when certain things happen ("your account was created", "you were given a role", etc.), and
  • When people click the Support link at the top of the page to email the installation admins

So with this new :SupportEmail setting, an installation can use two different email addresses, one for the automated email notifications and another for when people email "support". That would help address an issue at the Harvard repository where I think one reason why email clients, like Gmail, keep marking email sent from our one email address (support[at]dataverse.harvard.edu) as spam is because the email clients notice that people aren't opening the automated email notifications that the installation sends, and this also means that emails that installation staff send to users using that same email address get marked as spam. (Of course there are ways to improve the helpfulness of those automated messages, so they aren't ignored as often, and that's tackled in existing notification settings and other GitHub issues.)

This affects repository staff's ability to contact users, especially using the repository's support ticketing system, which might be using the same email address. This is one of the many potential reasons why email clients might keep marking emails from our support email address as spam. I'm talking with repository staff about the other reasons and possible solutions.

But with the setting described in this PR - using two different emails (one email address for the automated notifications and another for when depositors and repository staff contact each other) - we might help increase the odds that depositors see the emails that repository staff send.

IQSS/9185-contact_email_updates
@qqmyers
Copy link
Member Author

qqmyers commented Feb 7, 2023

Relatively simple changes to how the contact form sends emails (see changes in the description). Sizing at 10 to allow review (including the code making the feedback admin API call functional and changing to send one email to all contacts) and testing of the two new settings.

@qqmyers qqmyers added the Size: 10 A percentage of a sprint. 7 hours. label Feb 7, 2023
Copy link
Contributor

@sekmiller sekmiller left a comment

Choose a reason for hiding this comment

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

Looks good. thanks for putting in the mp config. Just a couple more comments on some unused code.

@qqmyers qqmyers removed their assignment Apr 28, 2023
@qqmyers
Copy link
Member Author

qqmyers commented Apr 28, 2023

FWIW: It looks like the failing unit test is because https://ddialliance.org/Specification/DDI-Codebook/2.5/XMLSchema/codebook.xsd couldn't be read - as has been seen on other branches lately.

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.

I haven't been following this but @qqmyers thanks for switching to MPCONFIG!

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 hitting "Approve" to clear my "requested changes" status. But then I'll put this back where it was (In Review).

@sekmiller sekmiller removed their assignment Apr 28, 2023
@kcondon kcondon self-assigned this May 3, 2023
@kcondon
Copy link
Contributor

kcondon commented May 4, 2023

Issues found:

  1. Feedback to contacts API example shows demo as server_url but endpoint only supports local host
  2. Feedback to contacts API example has jsonid on the command line example in front of $SERVER_URL, no space. Should not be there as it is in the json.
  3. Calling the Feedback to contacts API endpoint results in null ptr in server.log if point of contact name of form kcjim11
    emailapi_err.txt
  4. CC config doesn't update form with addresses and doesn't send cc: mail.
  5. So far cannot confirm SupportEmail supersedes SystemEmail for notifications.
  6. :SupportEmail doc needs updating to remove the settings example and name. It is mpconfig enabled now so need jvm-option or env var (mpconfig supported methods).

@kcondon kcondon merged commit c07f3a8 into IQSS:develop May 5, 2023
@pdurbin pdurbin added this to the 5.14 milestone May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GDCC: QDR of interest to QDR Size: 3 A percentage of a sprint. 2.1 hours.
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Feature Request/Idea: Improve Contact Form Email
7 participants