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

Update nihms pmc deposits using email integration #80

Merged
merged 27 commits into from
Jan 24, 2024

Conversation

rpoet-jh
Copy link
Contributor

@rpoet-jh rpoet-jh commented Jan 4, 2024

This PR adds an email integration to read Bulk submission emails from nihms.

The PR contains the following:

  • Add Deposit.statusMessage attribute in pass-data-client model
  • Set Deposit.depositStatusRef to nihms-package:<nihms_package_name> when the submission deposit is sent to nihms SFTP server.
  • Read the nihms emails every 10 minutes and use the contents of the emails to update the submission's deposit status. A success email will result in the deposit status being set to ACCEPTED and the deposit.depositStatusRef being set to nihms-id:<manuscript_id from success email>. A failure email will result in the deposit status being set to REJECTED and the deposit.statusMessage set to failure message in the email. The Deposit is updated in PASS.

I have a couple manual tests I want to perform:

  • Run the email integration locally and attach to actual inbox of [email protected] to read email and verify it comes in as expected
    • This has a pending action to determine if this is possible with the JHU inbox. I will test with a gmail account to move this along.
  • Run integration test in stage with actual nihms deposits

I will be doing these tests over the next couple days, but the code should be stable enough to review now.

@rpoet-jh rpoet-jh requested a review from markpatton January 4, 2024 18:55
@rpoet-jh rpoet-jh self-assigned this Jan 4, 2024
Copy link
Contributor

@markpatton markpatton 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. The spring mail support is nice.

This looks like it doesn't handle all the messages in https://docs.google.com/document/d/1JAf22f_mOrAG1-JZlEXcbTz7FbywYUmJy-16uxi3OFQ/edit?usp=sharing.

I also had a notion of flagging a deposit for manual review by us. But perhaps that is not needed if we are monitoring rejected submissions.

Also, when we don't recognize a message, there should probably be a warning logged.

@rpoet-jh
Copy link
Contributor Author

rpoet-jh commented Jan 5, 2024

Looks good. The spring mail support is nice.
Thanks for reviewing @markpatton!

This looks like it doesn't handle all the messages in https://docs.google.com/document/d/1JAf22f_mOrAG1-JZlEXcbTz7FbywYUmJy-16uxi3OFQ/edit?usp=sharing.

I'm pretty sure the regex patterns in NihmsReceiveMailService cover all the messages for which we can do something with the associated Submission. Check this sheet I created identifying the common patterns across email messages: https://docs.google.com/spreadsheets/d/11T744QjkFKFOZhEPYH9PPB8N9Lxac-dDPrst5bhcpAA/edit#gid=0

I also had a notion of flagging a deposit for manual review by us. But perhaps that is not needed if we are monitoring rejected submissions.

I assumed that Rejected implied it needed to be reviewed. As far as notifications and who gets notified, I also assumed this would be done in a follow up ticket.

Also, when we don't recognize a message, there should probably be a warning logged.

Well, I can add this, but it could become one of those noisy things because there are known messages we will receive that we can't do anything with (i.e. those emails without Package ID).

Copy link
Contributor

@markpatton markpatton left a comment

Choose a reason for hiding this comment

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

@rpoet-jh

Makes sense. And we can adjust later if we get relevant feedback from NIHMS.

@rpoet-jh
Copy link
Contributor Author

rpoet-jh commented Jan 5, 2024

Makes sense. And we can adjust later if we get relevant feedback from NIHMS.

I am going to log messages in the email that do not match any of the regex. I will log them as ERRORs so we get notified of them via AWS. We can tune this going forward if it becomes to spammy.

@rpoet-jh
Copy link
Contributor Author

rpoet-jh commented Jan 5, 2024

I am going to log messages in the email that do not match any of the regex. I will log them as ERRORs so we get notified of them via AWS.

@markpatton this is done.

@rpoet-jh rpoet-jh force-pushed the russ-856-email-intgr branch from 3b65699 to 251112a Compare January 16, 2024 14:32
@rpoet-jh rpoet-jh force-pushed the russ-856-email-intgr branch from cb7e9fe to 13bf448 Compare January 23, 2024 15:54
@rpoet-jh rpoet-jh merged commit a1eca57 into main Jan 24, 2024
2 checks passed
@rpoet-jh rpoet-jh deleted the russ-856-email-intgr branch August 1, 2024 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Email Integration to process Nihms deposit emails
2 participants