notify users of card charges #3301

chadwhitacre · 2015-03-30T21:21:08Z

Closes #1124.

TODO

wire up variables for email
implement text/plain content
Settings UI
review format of received email and tweak as necessary

chadwhitacre · 2015-03-30T21:46:47Z

@chrisdev @citruspi asked for this ahead of PyCon in 10 days.

chadwhitacre · 2015-03-30T21:49:14Z

That's towards the bottom of gratipay/inside.gratipay.com#111 (comment).

Changaco · 2015-04-08T11:56:42Z

Ready for review.

Changaco · 2015-04-08T14:08:18Z

Actually, I forgot the settings UI.

Changaco · 2015-04-08T19:03:13Z

UI done, ready for review.

chadwhitacre · 2015-04-08T19:49:53Z

UI seems to work as advertised (clicking persists). Testing email ...

chadwhitacre · 2015-04-08T19:50:49Z

I think I'm seeing that we're defaulting to sending notifications for failures but not successes. I think we should default to sending notifications for both.

chadwhitacre · 2015-04-08T19:57:45Z

No tests for notify_participants?

chadwhitacre · 2015-04-08T22:03:36Z

gratipay/billing/payday.py

+                continue
+            i = 1 if e.status == 'failed' else 2
+            p = e.participant
+            if p.notify_charge & i == 0:


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).

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.

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

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

chadwhitacre · 2015-04-08T22:26:21Z

I get the following error from payday, on a db with two users, one tipping the other $10 (credit card attached and is_suspicious=False).

pid-47425 thread-140735152444160 (MainThread) Starting a new payday.
pid-47425 thread-140735152444160 (MainThread) Payday started at 2015-04-08 22:21:03.751296+00:00.
pid-47425 thread-140735152444160 (MainThread) Greetings, program! It's PAYDAY!!!!
pid-47425 thread-140735152444160 (MainThread) Prepared the DB.
pid-47425 thread-4368371712 (Thread-1) Holding 1061 cents ($10.00 + $0.61 fee = $10.61) on Balanced for whit537 ... succeeded.
pid-47425 thread-4368371712 (Thread-9) Captured 1000 cents ($9.41 [rounded up from $2.59] + $0.59 fee = $10.00) on Balanced for whit537
pid-47425 thread-140735152444160 (MainThread) Captured 1 card holds.
pid-47425 thread-140735152444160 (MainThread) Canceled 0 card holds.
pid-47425 thread-140735152444160 (MainThread) Updated the balances of 2 participants.
pid-47425 thread-140735152444160 (MainThread) Starting payout loop.
pid-47425 thread-140735152444160 (MainThread) Did payout for 0 participants.
pid-47425 thread-140735152444160 (MainThread) Checked the DB.
pid-47425 thread-140735152444160 (MainThread) Updated payday stats.
pid-47425 thread-140735152444160 (MainThread) Updated receiving amounts.
pid-47425 thread-140735152444160 (MainThread) Traceback (most recent call last):
pid-47425 thread-140735152444160 (MainThread)   File "/Users/whit537/personal/gratipay/gratipay.com/gratipay/cli.py", line 27, in payday
pid-47425 thread-140735152444160 (MainThread)     Payday.start().run()
pid-47425 thread-140735152444160 (MainThread)   File "/Users/whit537/personal/gratipay/gratipay.com/gratipay/billing/payday.py", line 144, in run
pid-47425 thread-140735152444160 (MainThread)     self.notify_participants()
pid-47425 thread-140735152444160 (MainThread)   File "/Users/whit537/personal/gratipay/gratipay.com/gratipay/billing/payday.py", line 751, in notify_participants
pid-47425 thread-140735152444160 (MainThread)     """, locals())
pid-47425 thread-140735152444160 (MainThread)   File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/postgres/__init__.py", line 463, in one
pid-47425 thread-140735152444160 (MainThread)     return cursor.one(sql, parameters, default)
pid-47425 thread-140735152444160 (MainThread)   File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/postgres/cursors.py", line 105, in one
pid-47425 thread-140735152444160 (MainThread)     out = self._some(sql, parameters, lo=0, hi=1)
pid-47425 thread-140735152444160 (MainThread)   File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/postgres/cursors.py", line 127, in _some
pid-47425 thread-140735152444160 (MainThread)     self.execute(sql, parameters)
pid-47425 thread-140735152444160 (MainThread)   File "/Users/whit537/personal/gratipay/gratipay.com/env/lib/python2.7/site-packages/psycopg2/extras.py", line 288, in execute
pid-47425 thread-140735152444160 (MainThread)     return super(NamedTupleCursor, self).execute(query, vars)
pid-47425 thread-140735152444160 (MainThread) ProgrammingError: subquery in FROM must have an alias
pid-47425 thread-140735152444160 (MainThread) LINE 4:                            FROM ( SELECT DISTINCT ON (tippee...
pid-47425 thread-140735152444160 (MainThread)                                         ^
pid-47425 thread-140735152444160 (MainThread) HINT:  For example, FROM (SELECT ...) [AS] foo.
pid-47425 thread-140735152444160 (MainThread)

chadwhitacre · 2015-04-08T22:50:35Z

Interesting. Travis shows a failure but it's a js failure, not the failure I'm seeing locally.

chadwhitacre · 2015-04-08T22:59:13Z

I force-pushed a cleaner test. It still fails for me in the way noted above. Let's see about Travis.

chadwhitacre · 2015-04-08T23:00:59Z

There we go, Travis is showing the failure now. Out of time for now.

Changaco · 2015-04-09T09:11:29Z

Ready for more review.

Changaco · 2015-04-09T09:14:13Z

I think I'm seeing that we're defaulting to sending notifications for failures but not successes. I think we should default to sending notifications for both.

You think the percentage of users who want to receive success notifications is high enough to turn them on by default?

kaguillera · 2015-04-09T20:43:44Z

+1 For default notifications of success since most site that you use your credit card for sends you a email when they charge you.

chadwhitacre · 2015-04-09T21:29:10Z

Also, my educated guess (haven't verified) is that we have a large number of users that tip a small amount so they get charged once initially and then a second time two months later, after they've forgotten about us. I think it's good practice to notify on success by default so that they're notified of that second charge and it doesn't become something they're confused about a year later.

Changaco · 2015-04-09T21:42:41Z

Default changed.

chadwhitacre · 2015-04-13T20:56:17Z

Tests pass for me now.

chadwhitacre · 2015-04-13T21:07:53Z

Email received:

chadwhitacre · 2015-04-13T21:09:50Z

Needs some tweaks ...

chadwhitacre · 2015-04-13T22:31:19Z

@Changaco We're trying to pull the failure message from exchanges.note, but that field is null and the error message is in exchange_routes.error. Is the error ending up in the wrong field or are we trying to pull it from the wrong location?

chadwhitacre · 2015-04-13T22:44:10Z

We should link to the receipt for successful charges.

chadwhitacre · 2015-04-13T23:49:44Z

@Changaco Last two things I'm seeing are:

fix error message in failure notification (notify users of card charges #3301 (comment))
don't use ints for bitmasks (notify users of card charges #3301 (comment))

notify users of card charges

chadwhitacre · 2015-04-16T18:45:42Z

We'll deal with bitmasks separately, I guess.

chadwhitacre added the 3 - Work in Progress label Mar 30, 2015

chadwhitacre removed the 3 - Work in Progress label Mar 31, 2015

chadwhitacre mentioned this pull request Apr 6, 2015

Radar 1 gratipay/inside.gratipay.com#176

Closed

chadwhitacre and others added 3 commits April 8, 2015 10:12

Rough in charge notifications

cf2be43

send email notifications at the end of payday

284baf1

fix and complete email content

b0c6a8e

Changaco force-pushed the charge-notifications branch from 6e7d018 to 4633085 Compare April 8, 2015 10:13

Changaco added 2 commits April 8, 2015 13:33

wire up email variables

fb651a7

add email simplate for failed charges

9e86733

Changaco force-pushed the charge-notifications branch from 4633085 to 9e86733 Compare April 8, 2015 11:33

Changaco added the Review label Apr 8, 2015

rohitpaulk removed the Review label Apr 8, 2015

add UI for the notify_charge setting

a64d326

Changaco added the Review label Apr 8, 2015

fix test

d73d422

chadwhitacre reviewed Apr 8, 2015
View reviewed changes

Here's a failing test for notify_participants

c2cee2d

chadwhitacre force-pushed the charge-notifications branch from 2b287ac to b4fe99e Compare April 8, 2015 22:57

Changaco added 2 commits April 9, 2015 11:04

modify test to avoid test fixtures and cover both email simplates

bd5f789

fix payday notifications

047b0c2

Changaco force-pushed the charge-notifications branch from b4fe99e to 047b0c2 Compare April 9, 2015 09:08

send notifications for successful charges by default

5973342

This was referenced Apr 10, 2015

migrate to Braintree #3287

Closed

Radar 2 gratipay/inside.gratipay.com#184

Closed

Show total amount charged, including processor fee

a5c9f8f

Tweak language of charge notifications

3e39acd

Link to receipt for successful charges

f37d238

Changaco added 2 commits April 15, 2015 10:27

fix text page of charge_failed.spt

cd7f418

fix record_exchange: store the error in note

c67335a

Changaco mentioned this pull request Apr 15, 2015

Credit Card Failures Re: Capture and Presentation #3171

Closed

7 tasks

chadwhitacre added a commit that referenced this pull request Apr 16, 2015

Merge pull request #3301 from gratipay/charge-notifications

8a6d93e

notify users of card charges

chadwhitacre merged commit 8a6d93e into master Apr 16, 2015

chadwhitacre deleted the charge-notifications branch April 16, 2015 18:45

chadwhitacre mentioned this pull request Apr 16, 2015

don't use ints for bitmasks #3344

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

notify users of card charges #3301

notify users of card charges #3301

chadwhitacre commented Mar 30, 2015

chadwhitacre commented Mar 30, 2015

chadwhitacre commented Mar 30, 2015

Changaco commented Apr 8, 2015

Changaco commented Apr 8, 2015

Changaco commented Apr 8, 2015

chadwhitacre commented Apr 8, 2015

chadwhitacre commented Apr 8, 2015

chadwhitacre commented Apr 8, 2015

chadwhitacre Apr 8, 2015

Changaco Apr 9, 2015

chadwhitacre Apr 13, 2015

rohitpaulk Apr 14, 2015

chadwhitacre commented Apr 8, 2015

chadwhitacre commented Apr 8, 2015

chadwhitacre commented Apr 8, 2015

chadwhitacre commented Apr 8, 2015

Changaco commented Apr 9, 2015

Changaco commented Apr 9, 2015

kaguillera commented Apr 9, 2015

chadwhitacre commented Apr 9, 2015

Changaco commented Apr 9, 2015

chadwhitacre commented Apr 13, 2015

chadwhitacre commented Apr 13, 2015

chadwhitacre commented Apr 13, 2015

chadwhitacre commented Apr 13, 2015

chadwhitacre commented Apr 13, 2015

chadwhitacre commented Apr 13, 2015

chadwhitacre commented Apr 16, 2015

notify users of card charges #3301

notify users of card charges #3301

Conversation

chadwhitacre commented Mar 30, 2015

TODO

chadwhitacre commented Mar 30, 2015

chadwhitacre commented Mar 30, 2015

Changaco commented Apr 8, 2015

Changaco commented Apr 8, 2015

Changaco commented Apr 8, 2015

chadwhitacre commented Apr 8, 2015

chadwhitacre commented Apr 8, 2015

chadwhitacre commented Apr 8, 2015

chadwhitacre Apr 8, 2015

Choose a reason for hiding this comment

Changaco Apr 9, 2015

Choose a reason for hiding this comment

chadwhitacre Apr 13, 2015

Choose a reason for hiding this comment

rohitpaulk Apr 14, 2015

Choose a reason for hiding this comment

chadwhitacre commented Apr 8, 2015

chadwhitacre commented Apr 8, 2015

chadwhitacre commented Apr 8, 2015

chadwhitacre commented Apr 8, 2015

Changaco commented Apr 9, 2015

Changaco commented Apr 9, 2015

kaguillera commented Apr 9, 2015

chadwhitacre commented Apr 9, 2015

Changaco commented Apr 9, 2015

chadwhitacre commented Apr 13, 2015

chadwhitacre commented Apr 13, 2015

chadwhitacre commented Apr 13, 2015

chadwhitacre commented Apr 13, 2015

chadwhitacre commented Apr 13, 2015

chadwhitacre commented Apr 13, 2015

chadwhitacre commented Apr 16, 2015