From 6aa26430ffabf7a7596bbf9bebbf0572280093bb Mon Sep 17 00:00:00 2001 From: Changaco Date: Sat, 13 Dec 2014 18:47:58 +0100 Subject: [PATCH] move `is_locked` from `elsewhere` to `participants` --- branch.sql | 12 ++++++++++++ fake_payday.sql | 13 +++++++------ gratipay/models/__init__.py | 15 --------------- gratipay/models/account_elsewhere.py | 8 +------- gratipay/models/participant.py | 11 ++++++++--- gratipay/testing/__init__.py | 7 ++----- gratipay/utils/fake_data.py | 2 +- tests/py/test_billing_payday.py | 7 +++---- tests/py/test_participant.py | 2 +- www/on/%platform/%user_name/index.html.spt | 6 +++--- www/on/%platform/associate.spt | 2 +- 11 files changed, 39 insertions(+), 46 deletions(-) create mode 100644 branch.sql diff --git a/branch.sql b/branch.sql new file mode 100644 index 0000000000..e2d277f88d --- /dev/null +++ b/branch.sql @@ -0,0 +1,12 @@ +BEGIN; + + ALTER TABLE participants ADD COLUMN is_locked bool NOT NULL DEFAULT FALSE; + ALTER TABLE participants ADD CONSTRAINT claimed_not_locked CHECK (NOT (claimed_time IS NOT NULL AND is_locked)); + + UPDATE participants p + SET is_locked = true + FROM elsewhere e + WHERE e.participant = p.username + AND e.is_locked; + +END; diff --git a/fake_payday.sql b/fake_payday.sql index 14e89d276e..1c98dfec90 100644 --- a/fake_payday.sql +++ b/fake_payday.sql @@ -3,6 +3,7 @@ CREATE TEMPORARY TABLE temp_participants ON COMMIT DROP AS SELECT username , claimed_time + , is_locked , balance AS fake_balance , 0::numeric(35,2) AS giving , 0::numeric(35,2) AS pledging @@ -41,17 +42,17 @@ CREATE TEMPORARY TABLE temp_takes CREATE OR REPLACE FUNCTION fake_tip() RETURNS trigger AS $$ DECLARE tipper temp_participants; - tippee_elsewhere elsewhere; + tippee temp_participants; BEGIN tipper := ( SELECT p.*::temp_participants FROM temp_participants p WHERE username = NEW.tipper ); - tippee_elsewhere := ( - SELECT e.*::elsewhere - FROM elsewhere e - WHERE participant = NEW.tippee + tippee := ( + SELECT p.*::temp_participants + FROM temp_participants p + WHERE username = NEW.tippee LIMIT 1 ); IF (NEW.amount > tipper.fake_balance AND NOT tipper.credit_card_ok) THEN @@ -62,7 +63,7 @@ CREATE OR REPLACE FUNCTION fake_tip() RETURNS trigger AS $$ SET fake_balance = (fake_balance - NEW.amount) , giving = (giving + NEW.amount) WHERE username = NEW.tipper; - ELSIF (NOT tippee_elsewhere.is_locked) THEN + ELSIF (NOT tippee.is_locked) THEN UPDATE temp_participants SET fake_balance = (fake_balance - NEW.amount) , pledging = (pledging + NEW.amount) diff --git a/gratipay/models/__init__.py b/gratipay/models/__init__.py index b3ed6bb79d..0a6ae1bec2 100644 --- a/gratipay/models/__init__.py +++ b/gratipay/models/__init__.py @@ -39,7 +39,6 @@ def check_db(cursor): _check_orphans(cursor) _check_orphans_no_tips(cursor) _check_paydays_volumes(cursor) - _check_claimed_not_locked(cursor) def _check_tips(cursor): @@ -227,20 +226,6 @@ def _check_paydays_volumes(cursor): assert len(ach_fees_volume) == 0 -def _check_claimed_not_locked(cursor): - locked = cursor.all(""" - SELECT participant - FROM elsewhere - WHERE EXISTS ( - SELECT * - FROM participants - WHERE username=participant - AND claimed_time IS NOT NULL - ) AND is_locked - """) - assert len(locked) == 0 - - def add_event(c, type, payload): SQL = """ INSERT INTO events (type, payload) diff --git a/gratipay/models/account_elsewhere.py b/gratipay/models/account_elsewhere.py index 6e150bc288..5f34c42435 100644 --- a/gratipay/models/account_elsewhere.py +++ b/gratipay/models/account_elsewhere.py @@ -149,7 +149,7 @@ def opt_in(self, desired_username): """Given a desired username, return a User object. """ from gratipay.security.user import User - self.set_is_locked(False) + self.participant.set_is_locked(False) user = User.from_username(self.participant.username) assert not user.ANON, self.participant # sanity check if self.participant.is_claimed: @@ -175,12 +175,6 @@ def save_token(self, token): """, (token, self.id)) self.set_attributes(token=token) - def set_is_locked(self, is_locked): - self.db.run( 'UPDATE elsewhere SET is_locked=%s WHERE id=%s' - , (is_locked, self.id) - ) - self.set_attributes(is_locked=is_locked) - def get_account_elsewhere(website, request): path = request.line.uri.path diff --git a/gratipay/models/participant.py b/gratipay/models/participant.py index 0f3c4dc36a..33d8b399fc 100644 --- a/gratipay/models/participant.py +++ b/gratipay/models/participant.py @@ -621,6 +621,12 @@ def set_email_lang(self, accept_lang): # Random Junk # =========== + def set_is_locked(self, is_locked): + self.db.run( 'UPDATE participants SET is_locked=%s WHERE id=%s' + , (is_locked, self.id) + ) + self.set_attributes(is_locked=is_locked) + def get_teams(self): """Return a list of teams this user is a member of. """ @@ -785,7 +791,7 @@ def update_giving(self, cursor=None): # Update giving and pledging on participant giving, pledging = (cursor or self.db).one(""" WITH our_tips AS ( - SELECT amount, tippee, p2.claimed_time + SELECT amount, tippee, p2.claimed_time, p2.is_locked FROM current_tips JOIN participants p2 ON p2.username = tippee WHERE tipper = %(username)s @@ -802,9 +808,8 @@ def update_giving(self, cursor=None): , pledging = COALESCE(( SELECT sum(amount) FROM our_tips - JOIN elsewhere ON elsewhere.participant = tippee WHERE claimed_time IS NULL - AND elsewhere.is_locked = false + AND is_locked = false ), 0) WHERE p.username = %(username)s RETURNING giving, pledging diff --git a/gratipay/testing/__init__.py b/gratipay/testing/__init__.py index 327a5d3810..a90a6af02e 100644 --- a/gratipay/testing/__init__.py +++ b/gratipay/testing/__init__.py @@ -109,16 +109,13 @@ def clear_tables(self): self.db.run("ALTER SEQUENCE participants_id_seq RESTART WITH 1") - def make_elsewhere(self, platform, user_id, user_name, is_locked=False, **kw): + def make_elsewhere(self, platform, user_id, user_name, **kw): info = UserInfo( platform=platform , user_id=unicode(user_id) , user_name=user_name , **kw ) - account = AccountElsewhere.upsert(info) - if is_locked: - account.set_is_locked(True) - return account + return AccountElsewhere.upsert(info) def show_table(self, table): diff --git a/gratipay/utils/fake_data.py b/gratipay/utils/fake_data.py index eb5325073d..75491f38d4 100644 --- a/gratipay/utils/fake_data.py +++ b/gratipay/utils/fake_data.py @@ -78,6 +78,7 @@ def fake_participant(db, number="singular", is_admin=False): , last_bill_result='' # Needed to not be suspicious , claimed_time=faker.date_time_this_year() , number=number + , is_locked=False ) #Call participant constructor to perform other DB initialization return Participant.from_username(username) @@ -118,7 +119,6 @@ def fake_elsewhere(db, participant, platform): , platform=platform , user_id=fake_text_id() , user_name=participant.username - , is_locked=False , participant=participant.username , extra_info=None ) diff --git a/tests/py/test_billing_payday.py b/tests/py/test_billing_payday.py index cc28361308..d272456cdc 100644 --- a/tests/py/test_billing_payday.py +++ b/tests/py/test_billing_payday.py @@ -9,7 +9,6 @@ from gratipay.billing.payday import NoPayday, Payday from gratipay.exceptions import NegativeBalance from gratipay.models.participant import Participant -from gratipay.models.account_elsewhere import AccountElsewhere from gratipay.testing import Foobar, Harness from gratipay.testing.balanced import BalancedHarness @@ -106,8 +105,8 @@ def test_update_cached_amounts(self): bob = self.make_participant('bob', claimed_time='now', last_bill_result=None) carl = self.make_participant('carl', claimed_time='now', last_bill_result="Fail!") dana = self.make_participant('dana', claimed_time='now') - emma = self.make_elsewhere('github', 58946, 'emma').participant - roy = self.make_elsewhere('github', 58947, 'roy', is_locked=True).participant + emma = self.make_participant('emma') + roy = self.make_participant('roy', is_locked=True) alice.set_tip_to(dana, '3.00') alice.set_tip_to(bob, '6.00') alice.set_tip_to(emma, '1.00') @@ -125,7 +124,7 @@ def check(): bob = Participant.from_username('bob') carl = Participant.from_username('carl') dana = Participant.from_username('dana') - emma = AccountElsewhere.from_user_name('github','emma').participant + emma = Participant.from_username('emma') assert alice.giving == D('13.00') assert alice.pledging == D('1.00') assert alice.receiving == D('5.00') diff --git a/tests/py/test_participant.py b/tests/py/test_participant.py index 00782ca14c..a73f2dc070 100644 --- a/tests/py/test_participant.py +++ b/tests/py/test_participant.py @@ -617,7 +617,7 @@ def test_pledging_only_counts_latest_tip(self): def test_pledging_doesnt_count_locked_accounts(self): alice = self.make_participant('alice', claimed_time='now', last_bill_result='') - bob = self.make_elsewhere('github', 58946, 'bob', is_locked=True).participant + bob = self.make_participant('bob', is_locked=True) alice.set_tip_to(bob, '3.00') assert alice.pledging == Decimal('0.00') diff --git a/www/on/%platform/%user_name/index.html.spt b/www/on/%platform/%user_name/index.html.spt index e4e33fc774..56c10c7b28 100644 --- a/www/on/%platform/%user_name/index.html.spt +++ b/www/on/%platform/%user_name/index.html.spt @@ -16,8 +16,8 @@ if account.participant.is_claimed: request.redirect('/%s/' % account.participant.username) title = username = user_name = account.user_name -locked = account.is_locked participant = account.participant +locked = participant.is_locked is_team = account.is_team allow_pledges = not locked and (not is_team or platform.allows_team_connect) @@ -194,7 +194,7 @@ else: {% for member in members %} {% set on_gratipay = member.participant.is_claimed %} - {% set declines_gifts = not member.participant.accepts_tips or member.is_locked %} + {% set declines_gifts = not member.participant.accepts_tips or member.participant.is_locked %} {% if on_gratipay %} @@ -218,7 +218,7 @@ else: - {% if member.is_locked %} + {% if member.participant.is_locked %} declines gifts (opted out) {% else %} hasn't joined Gratipay yet diff --git a/www/on/%platform/associate.spt b/www/on/%platform/associate.spt index 98898a4a77..f746d377dd 100644 --- a/www/on/%platform/associate.spt +++ b/www/on/%platform/associate.spt @@ -68,7 +68,7 @@ elif action == 'connect': raise request.resource.respond(request, dispatch_result) elif action in {'lock', 'unlock'}: - account.set_is_locked(action == 'lock') + account.participant.set_is_locked(action == 'lock') else: raise Response(400)