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

Contribution expired notification #304

Merged
merged 6 commits into from
Mar 11, 2019
Merged

Contribution expired notification #304

merged 6 commits into from
Mar 11, 2019

Conversation

wbobeirne
Copy link
Collaborator

Part of #198.

What This Does

Creates a new task that gets made on contribution creation. 24h after the contribution is made, if it's still pending, emails the user to indicate the contribution has expired. Doesn't change anything in the database, simply sends the notification. This of course does not work on anonymous contributions.

Maybe we should add a new contribution.status for expired contributions instead of it being kind of implicit, let me know what y'all think.

Steps to Test

  • Create a contribution, confirm a new row shows up in the task table
  • Change the execute_after date to be in the past and hit http://localhost:5000/api/v1/task, confirm you get an expiry email
  • Create an anonymous contribution, confirm no new task row is created

Screenshots

screen shot 2019-03-06 at 3 31 27 pm

@wbobeirne wbobeirne requested review from dternyak and AMStrix March 6, 2019 20:37
@wbobeirne
Copy link
Collaborator Author

No new packages were introduced in this PR, so the snyk errors should be ignored (for the sake of this PR, to be addressed separately.)


@staticmethod
def process_task(task):
from grant.proposal.models import ProposalContribution
Copy link
Collaborator

Choose a reason for hiding this comment

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

avoiding circular deps?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, unfortunately all of the job classes seem to need to handle it this way.

Copy link
Collaborator

@AMStrix AMStrix left a comment

Choose a reason for hiding this comment

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

Lovely! Works great.

I've no strong opinion on whether to add a contribution.status. It would be fairly painless to add if needed at some point.

I did notice, in the course of testing, that attempting to make two contributions in a row sends the user directly to "Success" modal content w/o payment info. I'll make an issue for that.

@dternyak dternyak merged commit ad632dd into develop Mar 11, 2019
@dternyak dternyak deleted the expired-notif branch March 11, 2019 17:07
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.

3 participants