From 98202683d64523aefe85ff7c5f58bce7cfb0c735 Mon Sep 17 00:00:00 2001 From: "russell@unturf.com" Date: Mon, 9 Feb 2026 11:54:51 -0500 Subject: [PATCH] Fix subscribe page always highlighting None instead of user's actual frequency The view returned empty context on GET, so the template hardcoded checked on the None radio. Now the view looks up the logged-in user's existing subscription and passes current_frequency to the template, which sets checked on the correct radio button dynamically. --- make_post_sell/templates/subscribe.j2 | 9 +- make_post_sell/tests/test_functional.py | 107 ++++++++++++++++++++++ make_post_sell/tests/test_integration.py | 77 ++++++++++++++++ make_post_sell/tests/test_models.py | 111 +++++++++++++++++++++++ make_post_sell/views/subscribe.py | 13 ++- 5 files changed, 312 insertions(+), 5 deletions(-) diff --git a/make_post_sell/templates/subscribe.j2 b/make_post_sell/templates/subscribe.j2 index a074396..d212a94 100644 --- a/make_post_sell/templates/subscribe.j2 +++ b/make_post_sell/templates/subscribe.j2 @@ -112,16 +112,17 @@

- + {% set freq = current_frequency %} + - + - + - + diff --git a/make_post_sell/tests/test_functional.py b/make_post_sell/tests/test_functional.py index 81bd64f..82b3642 100644 --- a/make_post_sell/tests/test_functional.py +++ b/make_post_sell/tests/test_functional.py @@ -2675,6 +2675,113 @@ class AuthenticatedFunctionalTests(FunctionalTests): res = res.follow() self.assertIn(b"Invalid unsubscribe link", res.body) + def test_subscribe_shows_current_frequency_daily(self): + """After subscribing with daily, the page highlights Daily radio.""" + from ..models.shop_subscription import get_subscription_for_email_and_shop + + shop = self._create_shop_helper( + shop_params={ + **self.shop1_params, + "name": "sub-freq-daily-shop", + } + ) + + # Subscribe with daily frequency + self.testapp.post( + "/subscribe", + {"frequency": "0", "submit": "Subscribe"}, + status=302, + ) + + # GET the page again — Daily should be checked + res = self.testapp.get("/subscribe", status=200) + self.assertIn(b'id="freq-daily" checked', res.body) + self.assertNotIn(b'id="freq-none" checked', res.body) + + def test_subscribe_shows_current_frequency_weekly(self): + """After subscribing with weekly, the page highlights Weekly radio.""" + shop = self._create_shop_helper( + shop_params={ + **self.shop1_params, + "name": "sub-freq-weekly-shop", + } + ) + + # Subscribe with weekly frequency + self.testapp.post( + "/subscribe", + {"frequency": "1", "submit": "Subscribe"}, + status=302, + ) + + # GET the page — Weekly should be checked + res = self.testapp.get("/subscribe", status=200) + self.assertIn(b'id="freq-weekly" checked', res.body) + self.assertNotIn(b'id="freq-none" checked', res.body) + + def test_subscribe_shows_current_frequency_immediate(self): + """After subscribing with immediate, the page highlights Immediate radio.""" + shop = self._create_shop_helper( + shop_params={ + **self.shop1_params, + "name": "sub-freq-imm-shop", + } + ) + + # Subscribe with immediate frequency + self.testapp.post( + "/subscribe", + {"frequency": "2", "submit": "Subscribe"}, + status=302, + ) + + # GET the page — Immediate should be checked + res = self.testapp.get("/subscribe", status=200) + self.assertIn(b'id="freq-immediate" checked', res.body) + self.assertNotIn(b'id="freq-none" checked', res.body) + + def test_subscribe_shows_none_after_unsubscribe(self): + """After unsubscribing, the page highlights None radio.""" + from ..models.shop_subscription import get_subscription_for_email_and_shop + + shop = self._create_shop_helper( + shop_params={ + **self.shop1_params, + "name": "sub-freq-unsub-shop", + } + ) + + # Subscribe first + self.testapp.post( + "/subscribe", + {"frequency": "0", "submit": "Subscribe"}, + status=302, + ) + + # Unsubscribe (frequency=-1) + self.testapp.post( + "/subscribe", + {"frequency": "-1", "submit": "Subscribe"}, + status=302, + ) + + # GET the page — None should be checked + res = self.testapp.get("/subscribe", status=200) + self.assertIn(b'id="freq-none" checked', res.body) + self.assertNotIn(b'id="freq-daily" checked', res.body) + + def test_subscribe_no_subscription_defaults_to_none(self): + """With no subscription yet, None radio is checked by default.""" + shop = self._create_shop_helper( + shop_params={ + **self.shop1_params, + "name": "sub-freq-default-shop", + } + ) + + res = self.testapp.get("/subscribe", status=200) + self.assertIn(b'id="freq-none" checked', res.body) + def test_watch_json_requires_watch_mode(self): """Test that /watch/{id}/json returns 404 when watch mode is disabled.""" shop = self._create_shop_helper( diff --git a/make_post_sell/tests/test_integration.py b/make_post_sell/tests/test_integration.py index e7da062..8a6df5c 100644 --- a/make_post_sell/tests/test_integration.py +++ b/make_post_sell/tests/test_integration.py @@ -3157,3 +3157,80 @@ class TestLazyCartCreationIntegration(DatabaseIntegrationTests): self.assertEqual(cart2.uuid_str, stored_uuid) self.assertEqual(str(cart2.id), str(cart1.id)) + + +class TestSubscribeFrequencyLookup(DatabaseIntegrationTests): + """Integration test: subscribe view returns current_frequency from real DB.""" + + def _make_shop(self): + shop = Shop( + name="freq-test-shop", + phone_number="555-555-5555", + billing_address="123 Test St", + description="A test shop", + ) + shop.subscriptions_enabled = True + shop.domain_name = "freqtest.com" + self.dbsession.add(shop) + self.dbsession.flush() + return shop + + def test_active_subscription_frequency_returned(self): + """View returns actual frequency from DB for active subscription.""" + from ..models.shop_subscription import ( + ShopSubscription, + FREQUENCY_WEEKLY, + get_subscription_for_email_and_shop, + ) + + shop = self._make_shop() + user = get_or_create_user_by_email(self.dbsession, "freq@example.com") + self.dbsession.add(user) + self.dbsession.flush() + + sub = ShopSubscription("freq@example.com", shop.id, user_id=user.id) + sub.frequency = FREQUENCY_WEEKLY + self.dbsession.add(sub) + self.dbsession.flush() + + # Verify the lookup returns the correct subscription + found = get_subscription_for_email_and_shop( + self.dbsession, "freq@example.com", shop.id + ) + self.assertIsNotNone(found) + self.assertFalse(found.disabled) + self.assertEqual(found.frequency, FREQUENCY_WEEKLY) + + def test_disabled_subscription_detected(self): + """Disabled subscription is found so view can map to -1.""" + from ..models.shop_subscription import ( + ShopSubscription, + get_subscription_for_email_and_shop, + ) + + shop = self._make_shop() + user = get_or_create_user_by_email(self.dbsession, "dis@example.com") + self.dbsession.add(user) + self.dbsession.flush() + + sub = ShopSubscription("dis@example.com", shop.id, user_id=user.id) + sub.disabled = True + self.dbsession.add(sub) + self.dbsession.flush() + + found = get_subscription_for_email_and_shop( + self.dbsession, "dis@example.com", shop.id + ) + self.assertIsNotNone(found) + self.assertTrue(found.disabled) + + def test_no_subscription_returns_none(self): + """No subscription in DB returns None from lookup.""" + from ..models.shop_subscription import get_subscription_for_email_and_shop + + shop = self._make_shop() + + found = get_subscription_for_email_and_shop( + self.dbsession, "nobody@example.com", shop.id + ) + self.assertIsNone(found) diff --git a/make_post_sell/tests/test_models.py b/make_post_sell/tests/test_models.py index 0d2f038..f828c0f 100644 --- a/make_post_sell/tests/test_models.py +++ b/make_post_sell/tests/test_models.py @@ -2571,6 +2571,117 @@ class TestShopSubscription(unittest.TestCase): self.assertEqual(sub.human_frequency, "Daily") +class TestSubscribeViewCurrentFrequency(unittest.TestCase): + """Test that subscribe view returns correct current_frequency for the template.""" + + def _make_request(self, user=None, shop=None): + """Build a mock request for the subscribe view.""" + request = mock.MagicMock() + request.method = "GET" + request.shop = shop + request.user = user + request.session = mock.MagicMock() + request.session.flash = mock.Mock() + return request + + def _make_shop(self): + shop = mock.MagicMock() + shop.subscriptions_enabled = True + shop.id = "shop-id-123" + shop.name = "Test Shop" + return shop + + def _make_user(self, email="test@example.com"): + user = mock.MagicMock() + user.authenticated = True + user.email = email + return user + + @mock.patch("make_post_sell.views.subscribe.get_subscription_for_email_and_shop") + def test_returns_active_frequency_daily(self, mock_get_sub): + """Logged-in user with active daily subscription sees freq=0.""" + from ..views.subscribe import subscribe + + sub = mock.MagicMock() + sub.disabled = False + sub.frequency = 0 # FREQUENCY_DAILY + mock_get_sub.return_value = sub + + request = self._make_request( + user=self._make_user(), shop=self._make_shop() + ) + result = subscribe(request) + self.assertEqual(result["current_frequency"], 0) + + @mock.patch("make_post_sell.views.subscribe.get_subscription_for_email_and_shop") + def test_returns_active_frequency_weekly(self, mock_get_sub): + """Logged-in user with active weekly subscription sees freq=1.""" + from ..views.subscribe import subscribe + + sub = mock.MagicMock() + sub.disabled = False + sub.frequency = 1 # FREQUENCY_WEEKLY + mock_get_sub.return_value = sub + + request = self._make_request( + user=self._make_user(), shop=self._make_shop() + ) + result = subscribe(request) + self.assertEqual(result["current_frequency"], 1) + + @mock.patch("make_post_sell.views.subscribe.get_subscription_for_email_and_shop") + def test_returns_active_frequency_immediate(self, mock_get_sub): + """Logged-in user with active immediate subscription sees freq=2.""" + from ..views.subscribe import subscribe + + sub = mock.MagicMock() + sub.disabled = False + sub.frequency = 2 # FREQUENCY_IMMEDIATE + mock_get_sub.return_value = sub + + request = self._make_request( + user=self._make_user(), shop=self._make_shop() + ) + result = subscribe(request) + self.assertEqual(result["current_frequency"], 2) + + @mock.patch("make_post_sell.views.subscribe.get_subscription_for_email_and_shop") + def test_disabled_subscription_returns_negative_one(self, mock_get_sub): + """Logged-in user with disabled subscription sees freq=-1 (None radio).""" + from ..views.subscribe import subscribe + + sub = mock.MagicMock() + sub.disabled = True + mock_get_sub.return_value = sub + + request = self._make_request( + user=self._make_user(), shop=self._make_shop() + ) + result = subscribe(request) + self.assertEqual(result["current_frequency"], -1) + + @mock.patch("make_post_sell.views.subscribe.get_subscription_for_email_and_shop") + def test_no_subscription_returns_none(self, mock_get_sub): + """Logged-in user with no subscription sees current_frequency=None.""" + from ..views.subscribe import subscribe + + mock_get_sub.return_value = None + + request = self._make_request( + user=self._make_user(), shop=self._make_shop() + ) + result = subscribe(request) + self.assertIsNone(result["current_frequency"]) + + def test_anonymous_user_returns_none(self): + """Anonymous user (not logged in) sees current_frequency=None.""" + from ..views.subscribe import subscribe + + request = self._make_request(user=None, shop=self._make_shop()) + result = subscribe(request) + self.assertIsNone(result["current_frequency"]) + + class TestMentionParsing(unittest.TestCase): """Test @mention parsing from comment text.""" diff --git a/make_post_sell/views/subscribe.py b/make_post_sell/views/subscribe.py index c680dda..55929a4 100644 --- a/make_post_sell/views/subscribe.py +++ b/make_post_sell/views/subscribe.py @@ -40,6 +40,17 @@ def subscribe(request): not_authenticated = not (request.user and request.user.authenticated) + # Look up existing subscription for logged-in users + current_frequency = None + if request.user and request.user.authenticated: + existing_sub = get_subscription_for_email_and_shop( + request.dbsession, request.user.email, shop.id + ) + if existing_sub and not existing_sub.disabled: + current_frequency = existing_sub.frequency + elif existing_sub and existing_sub.disabled: + current_frequency = -1 + if request.method == "GET" and not_authenticated: request.session.flash( ("Enter your email and verify it to get notifications.", "info") @@ -180,7 +191,7 @@ def subscribe(request): return HTTPFound("/subscribe") - return {} + return {"current_frequency": current_frequency} @view_config(route_name="subscribe_verify", renderer="subscribe.j2")