corporate: Fix plan precedence issues with expired plans.

RemoteRealm customer takes precedence over RemoteServer
in general. But if an inactive plan is associated with
RemoteRealm and an active plan with RemoteServer, the
ACTIVE plan takes precendence.

Co-authored-by: Prakhar Pratyush <prakhar@zulip.com>

corporate/lib/stripe.py

@@ -4899,17 +4899,24 @@ def get_push_status_for_remote_request(
     remote_server: RemoteZulipServer, remote_realm: Optional[RemoteRealm]
 ) -> PushNotificationsEnabledStatus:
     # First, get the operative Customer object for this
-    # installation. If there's a `RemoteRealm` customer, that
-    # takes precedence.
+    # installation.
     customer = None
+    current_plan = None
 
     if remote_realm is not None:
         billing_session: BillingSession = RemoteRealmBillingSession(remote_realm)
         customer = billing_session.get_customer()
+        if customer is not None:
+            current_plan = get_current_plan_by_customer(customer)
 
-    if customer is None:
+    # If there's a `RemoteRealm` customer with an active plan, that
+    # takes precedence, but look for a current plan on the server if
+    # there is a customer with only inactive/expired plans on the Realm.
+    if customer is None or current_plan is None:
         billing_session = RemoteServerBillingSession(remote_server)
         customer = billing_session.get_customer()
+        if customer is not None:
+            current_plan = get_current_plan_by_customer(customer)
 
     if billing_session.is_sponsored():
         return PushNotificationsEnabledStatus(
@@ -4918,11 +4925,6 @@ def get_push_status_for_remote_request(
             message="Community plan",
         )
 
-    if customer is not None:
-        current_plan = get_current_plan_by_customer(customer)
-    else:
-        current_plan = None
-
     user_count: Optional[int] = None
     if current_plan is None:
         try:

zerver/tests/test_push_notifications.py

@@ -2266,7 +2266,7 @@ class AnalyticsBouncerTest(BouncerTestCase):
                 "corporate.lib.stripe.get_current_plan_by_customer", return_value=None
             ) as m:
                 with mock.patch(
-                    "corporate.lib.stripe.RemoteRealmBillingSession.current_count_for_billed_licenses",
+                    "corporate.lib.stripe.RemoteServerBillingSession.current_count_for_billed_licenses",
                     return_value=11,
                 ):
                     send_server_data_to_push_bouncer(consider_usage_statistics=False)
@@ -2283,13 +2283,10 @@ class AnalyticsBouncerTest(BouncerTestCase):
         with mock.patch(
             "zilencer.views.RemoteRealmBillingSession.get_customer", return_value=dummy_customer
         ):
-            with mock.patch(
-                "corporate.lib.stripe.get_current_plan_by_customer", return_value=None
-            ) as m:
+            with mock.patch("corporate.lib.stripe.get_current_plan_by_customer", return_value=None):
                 with mock.patch(
-                    "corporate.lib.stripe.RemoteRealmBillingSession.current_count_for_billed_licenses",
-                    return_value=11,
-                ):
+                    "corporate.lib.stripe.RemoteRealmBillingSession.current_count_for_billed_licenses"
+                ) as m:
                     send_server_data_to_push_bouncer(consider_usage_statistics=False)
                     m.assert_not_called()
                     realms = Realm.objects.all()
@@ -2412,6 +2409,41 @@ class AnalyticsBouncerTest(BouncerTestCase):
                         info_log.output[0],
                     )
 
+        # Remote realm is on an inactive plan. Remote server on active plan.
+        # ACTIVE plan takes precedence.
+        dummy_remote_realm_customer = mock.MagicMock()
+        dummy_remote_server_customer = mock.MagicMock()
+        dummy_remote_server_customer_plan = mock.MagicMock()
+        dummy_remote_server_customer_plan.status = CustomerPlan.ACTIVE
+
+        def get_current_plan_by_customer(customer: mock.MagicMock) -> Optional[mock.MagicMock]:
+            assert customer in [dummy_remote_realm_customer, dummy_remote_server_customer]
+            if customer == dummy_remote_server_customer:
+                return dummy_remote_server_customer_plan
+            return None
+
+        with mock.patch(
+            "corporate.lib.stripe.RemoteRealmBillingSession.get_customer",
+            return_value=dummy_remote_realm_customer,
+        ), mock.patch(
+            "corporate.lib.stripe.RemoteServerBillingSession.get_customer",
+            return_value=dummy_remote_server_customer,
+        ), mock.patch(
+            "zilencer.views.RemoteServerBillingSession.sync_license_ledger_if_needed"
+        ), mock.patch(
+            "corporate.lib.stripe.get_current_plan_by_customer",
+            side_effect=get_current_plan_by_customer,
+        ) as m:
+            send_server_data_to_push_bouncer(consider_usage_statistics=False)
+            m.assert_called()
+            realms = Realm.objects.all()
+            for realm in realms:
+                self.assertEqual(realm.push_notifications_enabled, True)
+                self.assertEqual(
+                    realm.push_notifications_enabled_end_timestamp,
+                    None,
+                )
+
         with mock.patch("zerver.lib.remote_server.send_to_push_bouncer") as m, self.assertLogs(
             "zulip.analytics", level="WARNING"
         ) as exception_log: