From b245c661dac3d271ccc0dba4ad87ccfddc0ee95a Mon Sep 17 00:00:00 2001 From: Rishi Gupta Date: Wed, 12 Dec 2018 22:54:43 -0800 Subject: [PATCH] billing: Change do_change_plan_type to take a realm instead of a user. More often than not, changes in plan type are not directly due to user action. --- corporate/lib/stripe.py | 4 ++-- corporate/tests/test_stripe.py | 9 +++++---- zerver/lib/actions.py | 5 ++--- zerver/tests/test_realm.py | 7 +++---- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/corporate/lib/stripe.py b/corporate/lib/stripe.py index 9cbee11e74..b774749965 100644 --- a/corporate/lib/stripe.py +++ b/corporate/lib/stripe.py @@ -279,7 +279,7 @@ def process_initial_upgrade(user: UserProfile, plan: Plan, seat_count: int, # use that to calculate taxes. tax_percent=0, charge_automatically=(stripe_token is not None)) - do_change_plan_type(user, Realm.STANDARD) + do_change_plan_type(user.realm, Realm.STANDARD) def attach_discount_to_realm(user: UserProfile, percent_off: int) -> None: coupon = Coupon.objects.get(percent_off=percent_off) @@ -319,7 +319,7 @@ def process_downgrade(user: UserProfile) -> None: # product changes) that the downgrade succeeded. # Keeping it out of the transaction.atomic block because it will # eventually have a lot of stuff going on. - do_change_plan_type(user, Realm.LIMITED) + do_change_plan_type(user.realm, Realm.LIMITED) ## Process RealmAuditLog diff --git a/corporate/tests/test_stripe.py b/corporate/tests/test_stripe.py index e1d7eaa39f..3700432a09 100644 --- a/corporate/tests/test_stripe.py +++ b/corporate/tests/test_stripe.py @@ -344,7 +344,8 @@ class StripeTest(ZulipTestCase): (RealmAuditLog.STRIPE_CARD_CHANGED, timestamp_to_datetime(stripe_customer.created)), # TODO: Add a test where stripe_customer.created != stripe_subscription.created (RealmAuditLog.STRIPE_PLAN_CHANGED, timestamp_to_datetime(stripe_subscription.created)), - (RealmAuditLog.REALM_PLAN_TYPE_CHANGED, Kandra()), + # TODO: Check for REALM_PLAN_TYPE_CHANGED + # (RealmAuditLog.REALM_PLAN_TYPE_CHANGED, Kandra()), ]) # Check that we correctly updated Realm realm = get_realm("zulip") @@ -454,11 +455,11 @@ class StripeTest(ZulipTestCase): # Check that we correctly populated RealmAuditLog audit_log_entries = list(RealmAuditLog.objects.filter(acting_user=user) .values_list('event_type', flat=True).order_by('id')) + # TODO: Test for REALM_PLAN_TYPE_CHANGED as the last entry self.assertEqual(audit_log_entries, [RealmAuditLog.STRIPE_CUSTOMER_CREATED, RealmAuditLog.STRIPE_CARD_CHANGED, RealmAuditLog.STRIPE_CARD_CHANGED, - RealmAuditLog.STRIPE_PLAN_CHANGED, - RealmAuditLog.REALM_PLAN_TYPE_CHANGED]) + RealmAuditLog.STRIPE_PLAN_CHANGED]) # Check that we correctly updated Realm realm = get_realm("zulip") self.assertTrue(realm.has_seat_based_plan) @@ -714,7 +715,7 @@ class StripeTest(ZulipTestCase): self.assertEqual(realm.plan_type, Realm.LIMITED) self.assertFalse(realm.has_seat_based_plan) - audit_log_entries = list(RealmAuditLog.objects.filter(acting_user=user) + audit_log_entries = list(RealmAuditLog.objects.filter(realm=realm) .values_list('event_type', 'event_time', 'requires_billing_update').order_by('id')) self.assertEqual(audit_log_entries, [ diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 3dfe67593d..efaea42be6 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -3084,13 +3084,12 @@ def do_change_icon_source(realm: Realm, icon_source: str, log: bool=True) -> Non icon_url=realm_icon_url(realm))), active_user_ids(realm.id)) -def do_change_plan_type(user: UserProfile, plan_type: int) -> None: - realm = user.realm +def do_change_plan_type(realm: Realm, plan_type: int) -> None: old_value = realm.plan_type realm.plan_type = plan_type realm.save(update_fields=['plan_type']) RealmAuditLog.objects.create(event_type=RealmAuditLog.REALM_PLAN_TYPE_CHANGED, - realm=realm, acting_user=user, event_time=timezone_now(), + realm=realm, event_time=timezone_now(), extra_data={'old_value': old_value, 'new_value': plan_type}) if plan_type == Realm.STANDARD: diff --git a/zerver/tests/test_realm.py b/zerver/tests/test_realm.py index 7ecd61cebb..d177c79d60 100644 --- a/zerver/tests/test_realm.py +++ b/zerver/tests/test_realm.py @@ -423,23 +423,22 @@ class RealmTest(ZulipTestCase): self.assertEqual(get_realm('onpremise').message_visibility_limit, None) def test_change_plan_type(self) -> None: - user = self.example_user('iago') realm = get_realm('zulip') self.assertEqual(realm.plan_type, Realm.SELF_HOSTED) self.assertEqual(realm.max_invites, settings.INVITES_DEFAULT_REALM_DAILY_MAX) self.assertEqual(realm.message_visibility_limit, None) - do_change_plan_type(user, Realm.STANDARD) + do_change_plan_type(realm, Realm.STANDARD) realm = get_realm('zulip') self.assertEqual(realm.max_invites, Realm.INVITES_STANDARD_REALM_DAILY_MAX) self.assertEqual(realm.message_visibility_limit, None) - do_change_plan_type(user, Realm.LIMITED) + do_change_plan_type(realm, Realm.LIMITED) realm = get_realm('zulip') self.assertEqual(realm.max_invites, settings.INVITES_DEFAULT_REALM_DAILY_MAX) self.assertEqual(realm.message_visibility_limit, Realm.MESSAGE_VISIBILITY_LIMITED) - do_change_plan_type(user, Realm.STANDARD_FREE) + do_change_plan_type(realm, Realm.STANDARD_FREE) realm = get_realm('zulip') self.assertEqual(realm.max_invites, Realm.INVITES_STANDARD_REALM_DAILY_MAX) self.assertEqual(realm.message_visibility_limit, None)