Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

notify users of card charges #3301

Merged
merged 16 commits into from
Apr 16, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions emails/charge_failed.spt
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
{{ _("Oh no! A problem supporting {0}!", top_tippee) }}

[---] text/html
{{ _("We tried to charge your credit card {0} today, to fund your ongoing support for {1}, but the charge failed with this message:",
format_currency(exchange.amount + exchange.fee, 'USD'),
('<b><a href="{0}">{1}</a></b>'|safe).format(
participant.profile_url+'giving/',
top_tippee if ntippees == 1 else ngettext('{0} and {n} other',
'{0} and {n} others',
ntippees - 1,
top_tippee))) }}

<pre>{{ exchange.note }}</pre>

<a href="{{ participant.profile_url+'routes/credit-card.html' }}"
style="{{ button_style }}">{{ _("Fix Credit Card") }}</a>

[---] text/plain
{{ _("We tried to charge your credit card {0} today, to fund your ongoing support for {1}, but the charge failed with this message:",
format_currency(exchange.amount + exchange.fee, 'USD'),
top_tippee if ntippees == 1 else ngettext('{0} and {n} other',
'{0} and {n} others',
ntippees - 1,
top_tippee)) }}

{{ exchange.note }}

{{ _("Follow this link to fix your credit card:") }} {{ participant.profile_url+'routes/credit-card.html' }}
27 changes: 27 additions & 0 deletions emails/charge_succeeded.spt
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{{ _("Thanks for supporting {0}!", top_tippee) }}

[---] text/html
{{ _("We charged your credit card {0} today, to fund your ongoing support for {1}. Thanks for using Gratipay!",
format_currency(exchange.amount + exchange.fee, 'USD'),
('<b><a href="{0}">{1}</a></b>'|safe).format(
participant.profile_url+'giving/',
top_tippee if ntippees == 1 else ngettext('{0} and {n} other',
'{0} and {n} others',
ntippees - 1,
top_tippee))) }}
<br>
<br>
<a href="{{ '{}receipts/{}.html'.format(participant.profile_url, exchange.id) }}"
style="{{ button_style }}">{{ _("View Receipt") }}</a>

[---] text/plain
{{ _("We charged your credit card {0} today, to fund your ongoing support for {1}. Thanks for using Gratipay!",
format_currency(exchange.amount + exchange.fee, 'USD'),
top_tippee if ntippees == 1 else ngettext('{0} and {n} other',
'{0} and {n} others',
ntippees - 1,
top_tippee)) }}

{{ _("Follow this link to view your receipt:") }} {{ '{}receipts/{}.html'.format(participant.profile_url, exchange.id) }}

{{ _("Follow this link if you want to view or modify your payments:") }} {{ participant.profile_url+'giving/' }}
2 changes: 1 addition & 1 deletion emails/verification.spt
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
('<b><a href="https://gratipay.com/{0}">{0}</a></b>'|safe).format(username)) }}
<br>
<br>
<a href="{{ link }}" style="color: #fff; text-decoration:none; display:inline-block; padding: 0 15px; background: #396; font: normal 14px/40px Arial, sans-serif; white-space: nowrap; border-radius: 3px">{{ _("Yes, proceed!") }}</a>
<a href="{{ link }}" style="{{ button_style }}">{{ _("Yes, proceed!") }}</a>

[---] text/plain
{{ _("We've received a request to connect {0} to the {1} account on Gratipay. Sound familiar?",
Expand Down
8 changes: 4 additions & 4 deletions gratipay/billing/exchanges.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ def _prep_hit(unrounded):
return cents, amount_str, upcharged, fee


def record_exchange(db, route, amount, fee, participant, status, error=''):
def record_exchange(db, route, amount, fee, participant, status, error=None):
"""Given a Bunch of Stuff, return an int (exchange_id).

Records in the exchanges table have these characteristics:
Expand All @@ -284,10 +284,10 @@ def record_exchange(db, route, amount, fee, participant, status, error=''):

exchange_id = cursor.one("""
INSERT INTO exchanges
(amount, fee, participant, status, route)
VALUES (%s, %s, %s, %s, %s)
(amount, fee, participant, status, route, note)
VALUES (%s, %s, %s, %s, %s, %s)
RETURNING id
""", (amount, fee, participant.username, status, route.id))
""", (amount, fee, participant.username, status, route.id, error))

if status == 'failed':
propagate_exchange(cursor, participant, route, error, 0)
Expand Down
51 changes: 51 additions & 0 deletions gratipay/billing/payday.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ def run(self):
self.mark_stage_done()

self.end()
self.notify_participants()

_end = aspen.utils.utcnow()
_delta = _end - _start
Expand Down Expand Up @@ -706,6 +707,56 @@ def end(self):
""", default=NoPayday).replace(tzinfo=aspen.utils.utc)


def notify_participants(self):
ts_start, ts_end = self.ts_start, self.ts_end
exchanges = self.db.all("""
SELECT e.id, amount, fee, note, status, p.*::participants AS participant
FROM exchanges e
JOIN participants p ON e.participant = p.username
WHERE "timestamp" >= %(ts_start)s
AND "timestamp" < %(ts_end)s
AND amount > 0
AND p.notify_charge > 0
""", locals())
for e in exchanges:
if e.status not in ('failed', 'succeeded'):
log('exchange %s has an unexpected status: %s' % (e.id, e.status))
continue
i = 1 if e.status == 'failed' else 2
p = e.participant
if p.notify_charge & i == 0:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I find this code difficult to read. You mentioned (IRC) wanting to refactor how we're doing notifications. Does this point us in that direction? Are you thinking we'll fold all notifications together into one column that we'll work with via bitmasks? Are you expecting that we'll use bitwise operators with ints? I'd find it easier to comprehend bitwise code if it used 0b00 (Python) and B'00' (Postgres). It'd be even easier if we used constants (e.g., NOTIFY_CHARGE_FAILURE).

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this point us in that direction?

Yes.

Are you thinking we'll fold all notifications together into one column that we'll work with via bitmasks?

No, but that may be a better idea than what I had in mind, I'll see if it works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Changaco Can we please not use ints for bitmasks? Too clever for me, sorry. :-(

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 on that. Either don't use them at all, or use abstraction so that we don't have to work with bits directly.

continue
username = p.username
ntippees, top_tippee = self.db.one("""
WITH tippees AS (
SELECT p.username, amount
FROM ( SELECT DISTINCT ON (tippee) tippee, amount
FROM tips
WHERE mtime < %(ts_start)s
AND tipper = %(username)s
ORDER BY tippee, mtime DESC
) t
JOIN participants p ON p.username = t.tippee
WHERE t.amount > 0
AND (p.goal IS NULL or p.goal >= 0)
AND p.is_suspicious IS NOT true
AND p.claimed_time < %(ts_start)s
)
SELECT ( SELECT count(*) FROM tippees ) AS ntippees
, ( SELECT username
FROM tippees
ORDER BY amount DESC
LIMIT 1
) AS top_tippee
""", locals())
p.queue_email(
'charge_'+e.status,
exchange=dict(id=e.id, amount=e.amount, fee=e.fee, note=e.note),
ntippees=ntippees,
top_tippee=top_tippee,
)


# Record-keeping.
# ===============

Expand Down
13 changes: 13 additions & 0 deletions gratipay/models/participant.py
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,13 @@ def remove_email(self, address):
(self.username, address))

def send_email(self, spt_name, **context):
context['participant'] = self
context['username'] = self.username
context['button_style'] = (
"color: #fff; text-decoration:none; display:inline-block; "
"padding: 0 15px; background: #396; white-space: nowrap; "
"font: normal 14px/40px Arial, sans-serif; border-radius: 3px"
)
context.setdefault('include_unsubscribe', True)
email = context.setdefault('email', self.email_address)
langs = i18n.parse_accept_lang(self.email_lang or 'en')
Expand Down Expand Up @@ -779,6 +785,13 @@ def get_cryptocoin_addresses(self):
# Random Junk
# ===========

@property
def profile_url(self):
scheme = gratipay.canonical_scheme
host = gratipay.canonical_host
username = self.username
return '{scheme}://{host}/{username}/'.format(**locals())

def get_teams(self):
"""Return a list of teams this user is a member of.
"""
Expand Down
16 changes: 9 additions & 7 deletions js/gratipay/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,22 +112,24 @@ Gratipay.settings.init = function() {
});

// Wire up notification preferences
// ==============================
// ================================

$('.email-notifications input').click(function(e) {
var field = $(e.target).data('field');
var bits = $(e.target).data('bits') || 1;
jQuery.ajax(
{ url: '../emails/notifications.json'
, type: 'POST'
, data: {toggle: field}
, data: {toggle: field, bits: bits}
, dataType: 'json'
, success: function(data) {
if (data.msg) {
Gratipay.notification(data.msg, 'success');
}
$(e.target).attr('checked', data[field]);
Gratipay.notification(data.msg, 'success');
$(e.target).attr('checked', data.new_value & bits)
}
, error: Gratipay.error
, error: [
Gratipay.error,
function(){ $(e.target).attr('checked', !$(e.target).attr('checked')) },
]
});
});

Expand Down
7 changes: 7 additions & 0 deletions sql/branch.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
BEGIN;
ALTER TABLE participants ADD COLUMN notify_charge int DEFAULT 3;
ALTER TABLE participants
ALTER COLUMN notify_on_opt_in DROP DEFAULT,
ALTER COLUMN notify_on_opt_in TYPE int USING notify_on_opt_in::int,
ALTER COLUMN notify_on_opt_in SET DEFAULT 1;
END;
26 changes: 23 additions & 3 deletions tests/py/test_billing_payday.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from gratipay.models.participant import Participant
from gratipay.testing import Foobar, Harness
from gratipay.testing.balanced import BalancedHarness
from gratipay.testing.emails import EmailHarness


class TestPayday(BalancedHarness):
Expand Down Expand Up @@ -245,13 +246,11 @@ def test_end_raises_NoPayday(self):

@mock.patch('gratipay.billing.payday.log')
@mock.patch('gratipay.billing.payday.Payday.payin')
@mock.patch('gratipay.billing.payday.Payday.end')
def test_payday(self, end, payin, log):
def test_payday(self, payin, log):
greeting = 'Greetings, program! It\'s PAYDAY!!!!'
Payday.start().run()
log.assert_any_call(greeting)
assert payin.call_count == 1
assert end.call_count == 1


class TestPayin(BalancedHarness):
Expand Down Expand Up @@ -529,3 +528,24 @@ def test_payout_ach_error(self, ach_credit):
Payday.start().payout()
payday = self.fetch_payday()
assert payday['nach_failing'] == 1


class TestNotifyParticipants(EmailHarness):

def test_it_notifies_participants(self):
kalel = self.make_participant('kalel', claimed_time='now', is_suspicious=False,
email_address='[email protected]', notify_charge=3)
lily = self.make_participant('lily', claimed_time='now', is_suspicious=False)
kalel.set_tip_to(lily, 10)

for status in ('failed', 'succeeded'):
payday = Payday.start()
self.make_exchange('balanced-cc', 10, 0, kalel, status)
payday.end()
payday.notify_participants()

emails = self.db.one('SELECT * FROM email_queue')
assert emails.spt_name == 'charge_'+status

Participant.dequeue_emails()
assert self.get_last_email()['to'][0]['email'] == '[email protected]'
2 changes: 1 addition & 1 deletion tests/py/test_email_notifs.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def test_take_over_sends_notifications_to_patrons(self):

def test_opt_in_notification_includes_unsubscribe(self):
carl_twitter = self.make_elsewhere('twitter', 1, 'carl')
roy = self.make_participant('roy', claimed_time='now', email_address='[email protected]', notify_on_opt_in=True)
roy = self.make_participant('roy', claimed_time='now', email_address='[email protected]', notify_on_opt_in=1)
roy.set_tip_to(carl_twitter.participant.username, '100')

AccountElsewhere.from_user_name('twitter', 'carl').opt_in('carl')
Expand Down
8 changes: 4 additions & 4 deletions www/%username/emails/notifications.json.spt
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ request.allow("POST")
participant = get_participant(state, restrict=True)

field = request.body.get("toggle")
if field not in ["notify_on_opt_in"]:
if field not in ["notify_charge", "notify_on_opt_in"]:
raise Response(400, "Invalid notification preference.")

new_value = website.db.one("""
UPDATE participants
SET {0}=not {0}
WHERE username=%s
SET {0} = {0} # %s
WHERE id = %s
RETURNING {0}
""".format(field), (participant.username,))
""".format(field), (request.body.get("bits", 1), participant.id))
assert new_value is not None

[---] application/json via json_dump
Expand Down
25 changes: 20 additions & 5 deletions www/%username/settings/index.html.spt
Original file line number Diff line number Diff line change
Expand Up @@ -202,15 +202,30 @@ emails = participant.get_emails()
</p>

<h2 id="notifications">{{ _("Notifications") }}</h2>
<p class="email-notifications">
<span> Send me notifications via email when:</span>
<div class="email-notifications">
<span>{{ _("Send me notifications via email:") }}</span>
<br />
<label>
<input type="checkbox" data-field="notify_on_opt_in"
{% if participant.notify_on_opt_in %}checked="true"{% endif %} />
{{ _("People I pledge to join Gratipay") }}
{{ 'checked' if participant.notify_on_opt_in }} />
{{ _("When people I pledge to join Gratipay") }}
</label>
</p>
<div>
{{ _("When my credit card is charged:") }}
<br />
<label>
<input type="checkbox" data-field="notify_charge" data-bits="1"
{{ 'checked' if participant.notify_charge.__and__(1) }} />
{{ _("if the charge fails") }}
</label>
<br />
<label>
<input type="checkbox" data-field="notify_charge" data-bits="2"
{{ 'checked' if participant.notify_charge.__and__(2) }} />
{{ _("if the charge succeeds") }}
</label>
</div>
</div>

<div class="close">
<h2>{{ _("Close") }}</h2>
Expand Down