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

Speed up the Payment Instruments page (and fraud reviews) #2528

Merged
merged 2 commits into from
Jan 28, 2025
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
11 changes: 7 additions & 4 deletions liberapay/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,17 @@ def check_bits(bits):
BIRTHDAY = date(2015, 5, 22)

CARD_BRANDS = {
'amex': 'American Express',
'diners': 'Diners Club',
'american_express': 'American Express',
'cartes_bancaires': 'CB',
'diners_club': 'Diners Club',
'discover': 'Discover',
'eftpos_australia': 'eftpos',
'interac': 'Interac',
'jcb': 'JCB',
'mastercard': 'Mastercard',
'unionpay': 'UnionPay',
'union_pay': 'UnionPay',
'visa': 'Visa',
'unknown': '',
'other': '',
}


Expand Down
93 changes: 70 additions & 23 deletions liberapay/models/exchange_route.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,21 @@

@classmethod
def insert(cls, participant, network, address, status,
one_off=False, remote_user_id=None, country=None, currency=None):
one_off=False, remote_user_id=None, country=None, currency=None,
brand=None, last4=None, fingerprint=None, owner_name=None,
expiration_date=None):
p_id = participant.id
cls.db.hit_rate_limit('add_payment_instrument', str(p_id), TooManyAttempts)
r = cls.db.one("""
INSERT INTO exchange_routes AS r
(participant, network, address, status,
one_off, remote_user_id, country, currency)
one_off, remote_user_id, country, currency,
brand, last4, fingerprint, owner_name,
expiration_date)
VALUES (%(p_id)s, %(network)s, %(address)s, %(status)s,
%(one_off)s, %(remote_user_id)s, %(country)s, %(currency)s)
%(one_off)s, %(remote_user_id)s, %(country)s, %(currency)s,
%(brand)s, %(last4)s, %(fingerprint)s, %(owner_name)s,
%(expiration_date)s)
ON CONFLICT (participant, network, address) DO NOTHING
RETURNING r
""", locals()) or cls.db.one("""
Expand Down Expand Up @@ -114,8 +120,17 @@
def attach_stripe_payment_method(cls, participant, pm, one_off):
if pm.type == 'card':
network = 'stripe-card'
card = pm.card
brand = card.display_brand
currency = None
day = monthrange(card.exp_year, card.exp_month)[-1]
expiration_date = date(card.exp_year, card.exp_month, day)
del card, day
elif pm.type == 'sepa_debit':
network = 'stripe-sdd'
brand = None
currency = 'EUR'
expiration_date = None
else:
raise NotImplementedError(pm.type)
customer_id = cls.db.one("""
Expand All @@ -140,12 +155,13 @@
participant.id, pm.id
),
).id
pm_country = getattr(getattr(pm, pm.type), 'country', None)
pm_currency = getattr(getattr(pm, pm.type), 'currency', None)
pm_instrument = getattr(pm, pm.type)
route = cls.insert(
participant, network, pm.id, 'chargeable',
one_off=one_off, remote_user_id=customer_id,
country=pm_country, currency=pm_currency,
country=pm_instrument.country, currency=currency, brand=brand,
last4=pm_instrument.last4, fingerprint=pm_instrument.fingerprint,
expiration_date=expiration_date, owner_name=pm.billing_details.name,
)
route.stripe_payment_method = pm
if network == 'stripe-sdd':
Expand Down Expand Up @@ -175,7 +191,8 @@
usage='off_session',
idempotency_key='create_SI_for_route_%i' % route.id,
)
route.set_mandate(si.mandate)
mandate = stripe.Mandate.retrieve(si.mandate)
route.set_mandate(si.mandate, mandate.payment_method_details.sepa_debit.reference)
assert not si.next_action, si.next_action
return route

Expand Down Expand Up @@ -239,13 +256,14 @@
id=self.id, network=self.network, currency=currency,
))

def set_mandate(self, mandate_id):
def set_mandate(self, mandate_id, mandate_reference):
self.db.run("""
UPDATE exchange_routes
SET mandate = %s
, mandate_reference = %s
WHERE id = %s
""", (mandate_id, self.id))
self.set_attributes(mandate=mandate_id)
""", (mandate_id, mandate_reference, self.id))
self.set_attributes(mandate=mandate_id, mandate_reference=mandate_reference)

def update_status(self, new_status):
id = self.id
Expand All @@ -259,21 +277,25 @@
self.set_attributes(status=new_status)

def get_brand(self):
if self.network == 'stripe-card':
if self.brand:
brand = self.brand
elif self.network == 'stripe-card':
if self.address.startswith('pm_'):
brand = self.stripe_payment_method.card.brand
return CARD_BRANDS.get(brand, brand)
brand = self.stripe_payment_method.card.display_brand
else:
return self.stripe_source.card.brand
brand = self.stripe_source.card.brand

Check warning on line 286 in liberapay/models/exchange_route.py

View check run for this annotation

Codecov / codecov/patch

liberapay/models/exchange_route.py#L286

Added line #L286 was not covered by tests
elif self.network == 'stripe-sdd':
if self.address.startswith('pm_'):
return getattr(self.stripe_payment_method.sepa_debit, 'bank_name', '')
else:
return getattr(self.stripe_source.sepa_debit, 'bank_name', '')
else:
raise NotImplementedError(self.network)
return CARD_BRANDS.get(brand, brand)

def get_expiration_date(self):
if self.expiration_date:
return self.expiration_date
if self.network == 'stripe-card':
if self.address.startswith('pm_'):
card = self.stripe_payment_method.card
Expand Down Expand Up @@ -305,27 +327,52 @@
website.tell_sentry(NotImplementedError(self.network))
return

def get_partial_number(self):
def get_mandate_reference(self):
if self.mandate_reference:
return self.mandate_reference
if self.network == 'stripe-sdd':
if self.address.startswith('pm_'):
mandate = stripe.Mandate.retrieve(self.mandate)
return mandate.payment_method_details.sepa_debit.reference

Check warning on line 336 in liberapay/models/exchange_route.py

View check run for this annotation

Codecov / codecov/patch

liberapay/models/exchange_route.py#L333-L336

Added lines #L333 - L336 were not covered by tests
else:
return self.stripe_source.sepa_debit.mandate_reference

Check warning on line 338 in liberapay/models/exchange_route.py

View check run for this annotation

Codecov / codecov/patch

liberapay/models/exchange_route.py#L338

Added line #L338 was not covered by tests
else:
raise NotImplementedError(self.network)

Check warning on line 340 in liberapay/models/exchange_route.py

View check run for this annotation

Codecov / codecov/patch

liberapay/models/exchange_route.py#L340

Added line #L340 was not covered by tests

def get_last4(self):
if self.last4:
return self.last4
if self.network == 'stripe-card':
if self.address.startswith('pm_'):
return '⋯' + str(self.stripe_payment_method.card.last4)
return self.stripe_payment_method.card.last4
else:
return '⋯' + str(self.stripe_source.card.last4)
return self.stripe_source.card.last4

Check warning on line 349 in liberapay/models/exchange_route.py

View check run for this annotation

Codecov / codecov/patch

liberapay/models/exchange_route.py#L349

Added line #L349 was not covered by tests
elif self.network == 'stripe-sdd':
from ..payin.stripe import get_partial_iban
if self.address.startswith('pm_'):
return get_partial_iban(self.stripe_payment_method.sepa_debit)
return self.stripe_payment_method.sepa_debit.last4

Check warning on line 352 in liberapay/models/exchange_route.py

View check run for this annotation

Codecov / codecov/patch

liberapay/models/exchange_route.py#L352

Added line #L352 was not covered by tests
else:
return get_partial_iban(self.stripe_source.sepa_debit)
return self.stripe_source.sepa_debit.last4
else:
raise NotImplementedError(self.network)

Check warning on line 356 in liberapay/models/exchange_route.py

View check run for this annotation

Codecov / codecov/patch

liberapay/models/exchange_route.py#L356

Added line #L356 was not covered by tests

def get_partial_number(self):
if self.network == 'stripe-card':
return f'⋯{self.get_last4()}'
elif self.network == 'stripe-sdd':
return f'{self.country}⋯{self.get_last4()}'
else:
raise NotImplementedError(self.network)

def get_postal_address(self):
def get_owner_name(self):
if self.owner_name:
return self.owner_name
if self.network.startswith('stripe-'):
if self.address.startswith('pm_'):
return self.stripe_payment_method.billing_details.address
return self.stripe_payment_method.billing_details.name

Check warning on line 371 in liberapay/models/exchange_route.py

View check run for this annotation

Codecov / codecov/patch

liberapay/models/exchange_route.py#L371

Added line #L371 was not covered by tests
else:
return self.stripe_source.owner.address
return self.stripe_source.owner.name

Check warning on line 373 in liberapay/models/exchange_route.py

View check run for this annotation

Codecov / codecov/patch

liberapay/models/exchange_route.py#L373

Added line #L373 was not covered by tests
elif self.network == 'paypal':
return None # TODO
else:
raise NotImplementedError(self.network)

Expand Down
21 changes: 4 additions & 17 deletions liberapay/payin/stripe.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,6 @@ def repr_charge_error(charge):
return ''


def get_partial_iban(sepa_debit):
return '%s⋯%s' % (sepa_debit.country, sepa_debit.last4)


def charge(db, payin, payer, route, update_donor=True):
"""Initiate the Charge for the given payin.

Expand Down Expand Up @@ -362,15 +358,6 @@ def send_payin_notification(db, payin, payer, charge, route):
"""Send the legally required notification for SEPA Direct Debits.
"""
if route.network == 'stripe-sdd' and charge.status != 'failed':
if route.address.startswith('pm_'):
sepa_debit = stripe.PaymentMethod.retrieve(route.address).sepa_debit
mandate = stripe.Mandate.retrieve(route.mandate)
mandate_url = mandate.payment_method_details.sepa_debit.url
mandate_reference = mandate.payment_method_details.sepa_debit.reference
else:
sepa_debit = stripe.Source.retrieve(route.address).sepa_debit
mandate_url = sepa_debit.mandate_url
mandate_reference = sepa_debit.mandate_reference
tippees = db.all("""
SELECT DISTINCT tippee_p.id AS tippee_id, tippee_p.username AS tippee_username
FROM payin_transfers pt
Expand All @@ -383,10 +370,10 @@ def send_payin_notification(db, payin, payer, charge, route):
email_unverified_address=True,
payin_id=payin.id, # unused but required for uniqueness
payin_amount=payin.amount,
bank_name=getattr(sepa_debit, 'bank_name', None),
partial_bank_account_number=get_partial_iban(sepa_debit),
mandate_url=mandate_url,
mandate_id=mandate_reference,
bank_name=route.get_brand(),
partial_bank_account_number=route.get_partial_number(),
mandate_url=route.get_mandate_url(),
mandate_id=route.get_mandate_reference(),
mandate_creation_date=route.ctime.date(),
creditor_identifier=website.app_conf.sepa_creditor_identifier,
average_settlement_seconds=PAYIN_SETTLEMENT_DELAYS['stripe-sdd'].total_seconds(),
Expand Down
28 changes: 23 additions & 5 deletions liberapay/testing/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -425,18 +425,36 @@ def insert_email(self, address, participant_id, verified=True):
RETURNING *
""", locals())

def upsert_route(self, participant, network,
status='chargeable', one_off=False, address='x', remote_user_id='x'):
def upsert_route(
self, participant, network, address='x',
status='chargeable', one_off=False, remote_user_id='x',
country=None, brand=None, last4=None, fingerprint=None, owner_name=None,
expiration_date=None,
):
r = self.db.one("""
INSERT INTO exchange_routes AS r
(participant, network, address, status, one_off, remote_user_id)
VALUES (%s, %s, %s, %s, %s, %s)
(participant, network, address,
status, one_off, remote_user_id, country, brand, last4,
fingerprint, owner_name, expiration_date)
VALUES (%s, %s, %s,
%s, %s, %s, %s, %s, %s,
%s, %s, %s)
ON CONFLICT (participant, network, address) DO UPDATE
SET status = excluded.status
, one_off = excluded.one_off
, remote_user_id = excluded.remote_user_id
, country = excluded.country
, brand = excluded.brand
, last4 = excluded.last4
, fingerprint = excluded.fingerprint
, owner_name = excluded.owner_name
, expiration_date = excluded.expiration_date
RETURNING r
""", (participant.id, network, address, status, one_off, remote_user_id))
""", (
participant.id, network, address,
status, one_off, remote_user_id, country, brand, last4,
fingerprint, owner_name, expiration_date,
))
r.__dict__['participant'] = participant
return r

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 @@
ALTER TABLE exchange_routes
ADD COLUMN brand text,
ADD COLUMN last4 text,
ADD COLUMN fingerprint text,
ADD COLUMN owner_name text,
ADD COLUMN expiration_date date,
ADD COLUMN mandate_reference text;
Loading