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

Pause donations option for Goal #1999

Closed
wants to merge 3 commits into from
Closed

Pause donations option for Goal #1999

wants to merge 3 commits into from

Conversation

evcheng1
Copy link
Contributor

@evcheng1 evcheng1 commented Apr 19, 2021

In username > edit > goal, there is an option to pause donations on individual accounts. When the pause option is selected, all patrons are notified of the pause (except if the previous goal was "I'm here as a patron, and politely decline to receive gifts. "). When the goal is updated to no longer be paused, all patrons are notified again (except if the new goal is "I'm here as a patron, and politely decline to receive gifts. ").

Closes #1745.

Copy link
Member

@Changaco Changaco left a comment

Choose a reason for hiding this comment

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

The user's profile and /donate page also need to be modified to expose the fact that donations are paused.

www/%username/giving/index.html.spt Show resolved Hide resolved
@@ -290,6 +290,10 @@ next_payday = compute_next_payday_date()
<p class="text-warning">{{ glyphicon('warning-sign') }} {{ _(
"Inactive because the account of the recipient is closed."
) }}</p>
% elif tippee.goal == -2
<p class="text-warning">{{ glyphicon('warning-sign') }} {{ _(
Copy link
Member

Choose a reason for hiding this comment

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

The pause icon should probably be used instead of warning-sign, and perhaps the text style should be text-info instead of text-warning.

www/%username/edit/goal.spt Outdated Show resolved Hide resolved
recipient=self.username,
donations_paused=donations_paused,
donations_url=tipper.url('giving/')
)
Copy link
Member

Choose a reason for hiding this comment

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

You can't send a potentially large number of emails while processing a user request. Firstly because the request will time out, secondly because it can allow an attacker to flood users with duplicate emails, and thirdly because the user should be able to reverse their decision before the emails are sent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can this be accomplished by using Participant's notify() method? Also when I switched to using notify(), the request seemed to get stuck. Do you know why this might be happening?

also for the last point

thirdly because the user should be able to reverse their decision before the emails are sent

If a decision is reversed, what should occur? Do the emails that were queued to send get dequeued?

Copy link
Member

Choose a reason for hiding this comment

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

The notify method should be used, but in a separate function which would be either a payday method or a cron job, like send_donation_reminder_notifications or send_account_disabled_notifications.

Copy link
Member

Choose a reason for hiding this comment

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

Also when I switched to using notify(), the request seemed to get stuck. Do you know why this might be happening?

update_goal runs in a database transaction (which is confusingly called a "cursor" for legacy reasons), but notify doesn't, so if you tried to call notify from inside the with block you could have created a "deadlock" (two database workers trying to modify the same row, each one waiting for the other to complete its transaction before continuing).

If a decision is reversed, what should occur? Do the emails that were queued to send get dequeued?

No. Once the email is queued for sending we don't cancel it. What we do instead is delay the creation of the notification which results in the email being queued for sending. Donors don't need to be notified immediately that donations have been paused, this kind of information can be delayed for an hour or even a week.

emails/donations_paused.spt Outdated Show resolved Hide resolved
emails/donations_paused.spt Outdated Show resolved Hide resolved
emails/donations_paused.spt Outdated Show resolved Hide resolved
emails/donations_paused.spt Outdated Show resolved Hide resolved
@Changaco
Copy link
Member

  • We're going to have to explain to both donors and recipients what pausing donations actually does.
  • It would be good to have tests to confirm that pausing donations actually does what it's supposed to do.

@evcheng1 evcheng1 closed this by deleting the head repository Nov 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Creators can't pause their patrons' donations to them
2 participants