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.
This commit is contained in:
parent
433f4e25ce
commit
98202683d6
5 changed files with 312 additions and 5 deletions
|
|
@ -112,16 +112,17 @@
|
|||
<label><b>Digest Frequency</b></label>
|
||||
<br /><br />
|
||||
|
||||
<input type="radio" name="frequency" value="-1" id="freq-none" checked />
|
||||
{% set freq = current_frequency %}
|
||||
<input type="radio" name="frequency" value="-1" id="freq-none" {{ 'checked' if freq is not none and freq == -1 else ('checked' if freq is none else '') }} />
|
||||
<label for="freq-none" class="freq-option freq-none">None</label>
|
||||
|
||||
<input type="radio" name="frequency" value="2" id="freq-immediate" />
|
||||
<input type="radio" name="frequency" value="2" id="freq-immediate" {{ 'checked' if freq == 2 }} />
|
||||
<label for="freq-immediate" class="freq-option">Immediate</label>
|
||||
|
||||
<input type="radio" name="frequency" value="0" id="freq-daily" />
|
||||
<input type="radio" name="frequency" value="0" id="freq-daily" {{ 'checked' if freq == 0 }} />
|
||||
<label for="freq-daily" class="freq-option">Daily</label>
|
||||
|
||||
<input type="radio" name="frequency" value="1" id="freq-weekly" />
|
||||
<input type="radio" name="frequency" value="1" id="freq-weekly" {{ 'checked' if freq == 1 }} />
|
||||
<label for="freq-weekly" class="freq-option">Weekly</label>
|
||||
|
||||
<input type="submit" name="submit" class="subscribe-btn" value="Save Preferences" />
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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."""
|
||||
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue