Fix more code smells

tests/test_components.py

@@ -36,9 +36,7 @@ class ComponentCacheTest(unittest.TestCase):
         self.assertGreaterEqual(info.hits, 1)  # served from cache
 
     def test_cache_is_bounded(self):
-        self.assertEqual(
-            components._render_element.cache_parameters()["maxsize"], 4096
-        )
+        self.assertEqual(components._render_element.cache_parameters()["maxsize"], 4096)
 
     def test_safe_and_unsafe_children_do_not_collide(self):
         """A SafeText "<b>" and a plain "<b>" are equal as strings but must
@@ -207,7 +205,9 @@ class ComponentReturnTypeTest(unittest.TestCase):
     def test_a_url_name_reversed(self):
         from unittest.mock import patch
 
-        with patch("common.components.reverse", return_value="/resolved/url"):
+        with patch(
+            "common.components.primitives.reverse", return_value="/resolved/url"
+        ):
             result = components.A([], "link", url_name="some_name")
             self.assertIn('href="/resolved/url"', result)
 
@@ -666,7 +666,7 @@ class ResolveNameWithIconTest(unittest.TestCase):
         override_game.name = "Override"
         override_game.platform = self.mock_platform
         override_game.pk = 99
-        with patch("common.components.reverse", return_value="/game/99"):
+        with patch("common.components.domain.reverse", return_value="/game/99"):
             name, platform, emulated, create_link, link = (
                 components._resolve_name_with_icon(
                     "", override_game, self.mock_session, True
@@ -676,7 +676,7 @@ class ResolveNameWithIconTest(unittest.TestCase):
         self.assertIsNot(name, "Override")
 
     def test_game_only_provides_platform(self):
-        with patch("common.components.reverse", return_value="/game/1"):
+        with patch("common.components.domain.reverse", return_value="/game/1"):
             name, platform, emulated, create_link, link = (
                 components._resolve_name_with_icon("", self.mock_game, None, True)
             )
@@ -713,7 +713,7 @@ class ResolveNameWithIconTest(unittest.TestCase):
         self.assertEqual(link, "")
 
     def test_linkify_true_creates_link(self):
-        with patch("common.components.reverse", return_value="/game/42"):
+        with patch("common.components.domain.reverse", return_value="/game/42"):
             name, platform, emulated, create_link, link = (
                 components._resolve_name_with_icon("", self.mock_game, None, True)
             )

tests/test_filter_bars.py

@@ -0,0 +1,109 @@
+"""Characterization tests locking the rendered output of the three filter bars.
+
+The FilterBar family (FilterBar / SessionFilterBar / PurchaseFilterBar) is the
+target of an upcoming dedup + module split. These tests pin the structural
+contract — form/input ids, the hidden ``filter`` field, preset wiring, the
+filter_json round-trip, and no double-escaping — so that refactor stays
+behaviour-preserving. The renderers were previously untested.
+"""
+
+import json
+
+from django.test import TestCase
+
+from common.components import (
+    FilterBar,
+    PurchaseFilterBar,
+    SelectableFilter,
+    SessionFilterBar,
+)
+from games.models import Device, Game, Platform
+
+_ESCAPED_TAG_MARKERS = ["<div", "<span", "<button", "<input", "<a"]
+
+
+class FilterBarRenderingTest(TestCase):
+    def setUp(self):
+        self.platform = Platform.objects.create(name="PC", icon="pc")
+        self.device = Device.objects.create(name="Desktop")
+        self.game = Game.objects.create(name="Test Game", platform=self.platform)
+
+    def assertNoEscapedTags(self, html):
+        for marker in _ESCAPED_TAG_MARKERS:
+            self.assertNotIn(marker, html, f"double-escaped markup ({marker!r})")
+
+    def _assert_shell(self, html, list_url, save_url):
+        """Markers every filter bar must keep through the refactor."""
+        self.assertIn('id="filter-bar-form"', html)
+        self.assertIn('id="filter-json-input"', html)
+        self.assertIn('name="filter"', html)
+        self.assertIn(list_url, html)  # preset list URL wired in
+        self.assertIn(save_url, html)  # preset save URL wired in
+        self.assertNoEscapedTags(html)
+
+    def test_game_filter_bar(self):
+        html = str(
+            FilterBar(
+                filter_json="",
+                preset_list_url="/presets/games/list",
+                preset_save_url="/presets/games/save",
+            )
+        )
+        self._assert_shell(html, "/presets/games/list", "/presets/games/save")
+
+    def test_session_filter_bar(self):
+        html = str(
+            SessionFilterBar(
+                filter_json="",
+                preset_list_url="/presets/sessions/list",
+                preset_save_url="/presets/sessions/save",
+            )
+        )
+        self._assert_shell(html, "/presets/sessions/list", "/presets/sessions/save")
+
+    def test_purchase_filter_bar(self):
+        html = str(
+            PurchaseFilterBar(
+                filter_json="",
+                preset_list_url="/presets/purchases/list",
+                preset_save_url="/presets/purchases/save",
+            )
+        )
+        self._assert_shell(html, "/presets/purchases/list", "/presets/purchases/save")
+
+    def test_game_filter_bar_roundtrips_selected_status(self):
+        """A status in filter_json renders as a selected tag in the widget."""
+        filter_json = json.dumps({"status": {"value": ["f"], "modifier": ""}})
+        html = str(
+            FilterBar(
+                filter_json=filter_json, preset_list_url="/l", preset_save_url="/s"
+            )
+        )
+        self.assertIn("sf-tag", html)
+        self.assertIn('data-value="f"', html)  # selected status reflected in widget
+        self.assertIn("Finished", html)  # ...with its label
+        self.assertNoEscapedTags(html)
+        # The hidden #filter-json-input must be escaped exactly once, so the DOM
+        # value is valid JSON the apply/preset JS can re-parse. Regression guard
+        # for the double-escape bug the dedup fixed.
+        self.assertIn(""status"", html)
+        self.assertNotIn(""", html)
+
+
+class SelectableFilterTest(TestCase):
+    """The shared widget the deduped FilterBar will be built on."""
+
+    OPTIONS = [("f", "Finished"), ("a", "Abandoned"), ("u", "Unplayed")]
+
+    def test_plain_widget_has_no_tags(self):
+        html = str(SelectableFilter("status", self.OPTIONS))
+        self.assertNotIn("sf-tag", html)
+
+    def test_include_and_exclude_tags(self):
+        html = str(
+            SelectableFilter("status", self.OPTIONS, selected=["f"], excluded=["a"])
+        )
+        self.assertIn('data-type="include"', html)
+        self.assertIn('data-type="exclude"', html)
+        self.assertIn("Finished", html)
+        self.assertIn("Abandoned", html)

tests/test_filters.py

@@ -10,11 +10,10 @@ from common.criteria import (
     ChoiceCriterion,
     IntCriterion,
     Modifier,
-    MultiCriterion,
     StringCriterion,
 )
-from common.components import FilterBar, SelectableFilter
-from games.filters import GameFilter, parse_game_filter
+from common.components import FilterBar
+from games.filters import GameFilter
 
 
 class TestStringCriterion:
@@ -30,7 +29,9 @@ class TestStringCriterion:
 class TestIntCriterion:
     def test_between(self):
         c = IntCriterion(value=2020, value2=2024, modifier=Modifier.BETWEEN)
-        assert c.to_q("year_released") == Q(year_released__gte=2020, year_released__lte=2024)
+        assert c.to_q("year_released") == Q(
+            year_released__gte=2020, year_released__lte=2024
+        )
 
 
 class TestBoolCriterion:
@@ -67,7 +68,9 @@ class TestChoiceCriterion:
         assert q == Q(status__in=["f"]) & ~Q(status__in=["a"])
 
     def test_include_two_and_exclude_one(self):
-        c = ChoiceCriterion(value=["f", "p"], excludes=["a"], modifier=Modifier.INCLUDES)
+        c = ChoiceCriterion(
+            value=["f", "p"], excludes=["a"], modifier=Modifier.INCLUDES
+        )
         q = c.to_q("status")
         assert q == Q(status__in=["f", "p"]) & ~Q(status__in=["a"])
 
@@ -105,6 +108,7 @@ class TestChoiceCriterionAgainstDB:
     def _seed_games(self):
         """Create test games with different statuses."""
         from games.models import Game, Platform
+
         platform, _ = Platform.objects.get_or_create(name="Test", icon="test")
         statuses = ["u", "p", "f", "r", "a"]
         for i, s in enumerate(statuses):
@@ -115,11 +119,15 @@ class TestChoiceCriterionAgainstDB:
 
     def _count(self, c: ChoiceCriterion) -> int:
         from games.models import Game
+
         return Game.objects.filter(c.to_q("status")).count()
 
     def _statuses(self, c: ChoiceCriterion) -> set[str]:
         from games.models import Game
-        return set(Game.objects.filter(c.to_q("status")).values_list("status", flat=True))
+
+        return set(
+            Game.objects.filter(c.to_q("status")).values_list("status", flat=True)
+        )
 
     @pytest.mark.django_db
     def test_include_finished_includes_only_finished(self):
@@ -138,7 +146,9 @@ class TestChoiceCriterionAgainstDB:
     def test_include_and_exclude(self):
         """Include Finished but exclude Abandoned."""
         self._seed_games()
-        c = ChoiceCriterion(value=["f", "a"], excludes=["a"], modifier=Modifier.INCLUDES)
+        c = ChoiceCriterion(
+            value=["f", "a"], excludes=["a"], modifier=Modifier.INCLUDES
+        )
         # Include f and a, but exclude a → only f
         assert self._statuses(c) == {"f"}
 
@@ -198,7 +208,10 @@ class TestGameFilterFromJson:
         assert gf.platform.value == ["1", "3"]
 
     def test_round_trip(self):
-        data = {"status": {"value": ["f"], "modifier": "INCLUDES"}, "mastered": {"value": True, "modifier": "EQUALS"}}
+        data = {
+            "status": {"value": ["f"], "modifier": "INCLUDES"},
+            "mastered": {"value": True, "modifier": "EQUALS"},
+        }
         gf = GameFilter.from_json(data)
         json_out = gf.to_json()
         gf2 = GameFilter.from_json(json_out)
@@ -236,7 +249,9 @@ class TestFilterBarRendering:
         html = str(
             FilterBar(
                 platform_options=[],
-                filter_json=json.dumps({"mastered": {"value": True, "modifier": "EQUALS"}}),
+                filter_json=json.dumps(
+                    {"mastered": {"value": True, "modifier": "EQUALS"}}
+                ),
             )
         )
         assert 'checked="true"' in html
@@ -245,7 +260,9 @@ class TestFilterBarRendering:
         html = str(
             FilterBar(
                 platform_options=[],
-                filter_json=json.dumps({"status": {"value": ["f"], "modifier": "INCLUDES"}}),
+                filter_json=json.dumps(
+                    {"status": {"value": ["f"], "modifier": "INCLUDES"}}
+                ),
             )
         )
         assert 'data-value="f"' in html

tests/test_middleware_integration.py

@@ -18,9 +18,7 @@ class MiddlewareIntegrationTest(TestCase):
 
     @staticmethod
     def _create_user():
-        return User.objects.create_user(
-            username="testuser", password="testpass123"
-        )
+        return User.objects.create_user(username="testuser", password="testpass123")
 
     def setUp(self):
         self.client = Client()
@@ -97,10 +95,10 @@ class MiddlewareIntegrationTest(TestCase):
         self.assertEqual(data["show-toast"]["message"], "Purchase refunded")
         # Verify the row HTML contains the updated row id
         body = response.content.decode()
-        self.assertIn(f'purchase-row-{purchase.id}', body)
+        self.assertIn(f"purchase-row-{purchase.id}", body)
         # Verify OoO modal close element
-        self.assertIn('hx-swap-oob', body)
-        self.assertIn('refund-confirmation-modal', body)
+        self.assertIn("hx-swap-oob", body)
+        self.assertIn("refund-confirmation-modal", body)
         # Verify the purchase is actually refunded
         purchase.refresh_from_db()
         self.assertIsNotNone(purchase.date_refunded)

tests/test_price_update.py

@@ -0,0 +1,102 @@
+from datetime import date
+
+from django.test import TestCase
+
+from games.models import Game, Platform, Purchase
+from games.tasks import convert_prices
+
+
+class PurchaseNeedsPriceUpdateTest(TestCase):
+    def setUp(self):
+        self.platform = Platform.objects.create(name="PC", icon="pc", group="PC")
+        self.game = Game.objects.create(name="Test Game", platform=self.platform)
+
+    def test_new_purchase_has_needs_price_update_true(self):
+        purchase = Purchase.objects.create(
+            price=50.0,
+            price_currency="USD",
+            date_purchased=date(2025, 1, 1),
+        )
+        purchase.games.add(self.game)
+        self.assertTrue(purchase.needs_price_update)
+
+    def test_convert_prices_sets_flag_to_false(self):
+        purchase = Purchase.objects.create(
+            price=50.0,
+            price_currency="USD",
+            date_purchased=date(2025, 1, 1),
+        )
+        purchase.games.add(self.game)
+        self.assertTrue(purchase.needs_price_update)
+
+        convert_prices()
+
+        purchase.refresh_from_db()
+        self.assertFalse(purchase.needs_price_update)
+
+    def test_price_change_sets_needs_price_update(self):
+        purchase = Purchase.objects.create(
+            price=50.0,
+            price_currency="USD",
+            date_purchased=date(2025, 1, 1),
+        )
+        purchase.games.add(self.game)
+        purchase.converted_price = 1000
+        purchase.converted_currency = "CZK"
+        purchase.needs_price_update = False
+        purchase.save()
+
+        purchase.price = 60.0
+        purchase.save()
+        purchase.refresh_from_db()
+        self.assertTrue(purchase.needs_price_update)
+
+    def test_currency_change_sets_needs_price_update(self):
+        purchase = Purchase.objects.create(
+            price=50.0,
+            price_currency="USD",
+            date_purchased=date(2025, 1, 1),
+        )
+        purchase.games.add(self.game)
+        purchase.converted_price = 1000
+        purchase.converted_currency = "CZK"
+        purchase.needs_price_update = False
+        purchase.save()
+
+        purchase.price_currency = "EUR"
+        purchase.save()
+        purchase.refresh_from_db()
+        self.assertTrue(purchase.needs_price_update)
+
+    def test_name_change_does_not_set_needs_price_update(self):
+        purchase = Purchase.objects.create(
+            price=50.0,
+            price_currency="USD",
+            date_purchased=date(2025, 1, 1),
+        )
+        purchase.games.add(self.game)
+        purchase.converted_price = 1000
+        purchase.converted_currency = "CZK"
+        purchase.needs_price_update = False
+        purchase.save()
+
+        purchase.name = "New Name"
+        purchase.save()
+        purchase.refresh_from_db()
+        self.assertFalse(purchase.needs_price_update)
+
+    def test_convert_prices_skips_already_converted(self):
+        purchase = Purchase.objects.create(
+            price=50.0,
+            price_currency="USD",
+            date_purchased=date(2025, 1, 1),
+        )
+        purchase.games.add(self.game)
+        purchase.converted_price = 1000
+        purchase.converted_currency = "CZK"
+        purchase.needs_price_update = False
+        purchase.save()
+
+        convert_prices()
+        purchase.refresh_from_db()
+        self.assertFalse(purchase.needs_price_update)

tests/test_session_formatting.py

@@ -16,9 +16,7 @@ class FormatDurationTest(TestCase):
     def test_duration_format(self):
         g = Game(name="The Test Game")
         g.save()
-        p = Purchase(
-            date_purchased=datetime(2022, 9, 26, 14, 58, tzinfo=ZONEINFO)
-        )
+        p = Purchase(date_purchased=datetime(2022, 9, 26, 14, 58, tzinfo=ZONEINFO))
         p.save()
         p.games.add(g)
         p.save()

tests/test_stats.py

@@ -47,14 +47,20 @@ class ComputeStatsTest(TestCase):
 
         # Game A in 2023: 1h + 1.5h on the same day = 2.5h
         Session.objects.create(
-            game=self.game_a, timestamp_start=dt(2023, 6, 10, 10), timestamp_end=dt(2023, 6, 10, 11)
+            game=self.game_a,
+            timestamp_start=dt(2023, 6, 10, 10),
+            timestamp_end=dt(2023, 6, 10, 11),
         )
         Session.objects.create(
-            game=self.game_a, timestamp_start=dt(2023, 6, 10, 14), timestamp_end=dt(2023, 6, 10, 15, 30)
+            game=self.game_a,
+            timestamp_start=dt(2023, 6, 10, 14),
+            timestamp_end=dt(2023, 6, 10, 15, 30),
         )
         # Game B in 2023: 1h tracked + 2h manual (no end) = 3h total
         Session.objects.create(
-            game=self.game_b, timestamp_start=dt(2023, 7, 1, 20), timestamp_end=dt(2023, 7, 1, 21)
+            game=self.game_b,
+            timestamp_start=dt(2023, 7, 1, 20),
+            timestamp_end=dt(2023, 7, 1, 21),
         )
         Session.objects.create(
             game=self.game_b,
@@ -63,7 +69,9 @@ class ComputeStatsTest(TestCase):
         )
         # Game A in 2022 (only counts toward all-time): 2h
         Session.objects.create(
-            game=self.game_a, timestamp_start=dt(2022, 5, 1, 10), timestamp_end=dt(2022, 5, 1, 12)
+            game=self.game_a,
+            timestamp_start=dt(2022, 5, 1, 10),
+            timestamp_end=dt(2022, 5, 1, 12),
         )
 
     # ── shared metrics (characterization) ──

tests/test_streak.py

@@ -5,7 +5,6 @@ from common.time import daterange, streak_bruteforce
 
 
 class StreakTest(unittest.TestCase):
-
     def test_daterange_exclusive(self):
         d = daterange(date(2024, 8, 1), date(2024, 8, 3))
         self.assertEqual(
@@ -24,13 +23,15 @@ class StreakTest(unittest.TestCase):
         self.assertEqual(streak_bruteforce([date(2024, 8, 1)])["days"], 1)
 
     def test_2day_streak(self):
-        self.assertEqual(streak_bruteforce([date(2024, 8, 1), date(2024, 8, 2)])["days"], 2)
+        self.assertEqual(
+            streak_bruteforce([date(2024, 8, 1), date(2024, 8, 2)])["days"], 2
+        )
 
     def test_31day_streak(self):
         self.assertEqual(
-            streak_bruteforce(daterange(date(2024, 8, 1), date(2024, 8, 31), end_inclusive=True))[
-                "days"
-            ],
+            streak_bruteforce(
+                daterange(date(2024, 8, 1), date(2024, 8, 31), end_inclusive=True)
+            )["days"],
             31,
         )