From e3b53cd4a973e230078e874c69b461cc8dce146e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Kucharczyk?= Date: Tue, 12 May 2026 13:57:59 +0200 Subject: [PATCH 1/3] Add needs_price_update field to Purchase model Replace fragile price change detection in Purchase.save() with a lazy dirty flag approach. A pre_save/post_save signal pair detects price/currency changes without extra DB queries, and convert_prices() uses the flag to determine which purchases need conversion. - Add needs_price_update BooleanField with db_index - Add pre_save signal to store old price/currency values - Add post_save signal to set needs_price_update=True when price/currency changes - Simplify Purchase.save() to remove DB reload + comparison logic - Remove price_or_currency_differ_from() method - Update convert_prices() to filter on needs_price_update flag - Extract _get_exchange_rate() and _save_converted_price() helpers - Add tests for the new behavior --- .../migrations/0016_add_needs_price_update.py | 22 ++++ games/models.py | 20 +--- games/signals.py | 23 ++++ games/tasks.py | 100 +++++++++------- games/tests.py | 110 +++++++++++++++++- 5 files changed, 209 insertions(+), 66 deletions(-) create mode 100644 games/migrations/0016_add_needs_price_update.py diff --git a/games/migrations/0016_add_needs_price_update.py b/games/migrations/0016_add_needs_price_update.py new file mode 100644 index 0000000..83f06fa --- /dev/null +++ b/games/migrations/0016_add_needs_price_update.py @@ -0,0 +1,22 @@ +# Generated by Django 6.0.1 on 2026-05-12 11:57 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('games', '0015_alter_purchase_date_purchased_and_more'), + ] + + operations = [ + migrations.AddField( + model_name='purchase', + name='needs_price_update', + field=models.BooleanField(db_index=True, default=True), + ), + migrations.RunSQL( + "UPDATE games_purchase SET needs_price_update = FALSE WHERE converted_price IS NOT NULL AND converted_currency != ''", + reverse_sql="UPDATE games_purchase SET needs_price_update = TRUE WHERE converted_price IS NOT NULL AND converted_currency != ''", + ), + ] diff --git a/games/models.py b/games/models.py index 777a247..2899672 100644 --- a/games/models.py +++ b/games/models.py @@ -179,6 +179,7 @@ class Purchase(models.Model): price_currency = models.CharField(max_length=3, default="USD") converted_price = models.FloatField(null=True) converted_currency = models.CharField(max_length=3, blank=True, default="") + needs_price_update = models.BooleanField(default=True, db_index=True) price_per_game = GeneratedField( expression=Coalesce(F("converted_price"), F("price"), 0) / F("num_purchases"), output_field=models.FloatField(), @@ -240,12 +241,6 @@ class Purchase(models.Model): def is_game(self): return self.type == self.GAME - def price_or_currency_differ_from(self, purchase_to_compare): - return ( - self.price != purchase_to_compare.price - or self.price_currency != purchase_to_compare.price_currency - ) - def refund(self): self.date_refunded = timezone.now() self.save() @@ -255,19 +250,6 @@ class Purchase(models.Model): raise ValidationError( f"{self.get_type_display()} must have a related purchase." ) - if self.pk is not None: - # Retrieve the existing instance from the database - existing_purchase = Purchase.objects.get(pk=self.pk) - # If price has changed, reset converted fields - if existing_purchase.price_or_currency_differ_from(self): - from games.tasks import currency_to - - exchange_rate = get_or_create_rate( - self.price_currency, currency_to, self.date_purchased.year - ) - if exchange_rate: - self.converted_price = floatformat(self.price * exchange_rate, 0) - self.converted_currency = currency_to super().save(*args, **kwargs) diff --git a/games/signals.py b/games/signals.py index e2f805f..5f17bc6 100644 --- a/games/signals.py +++ b/games/signals.py @@ -17,6 +17,29 @@ from games.models import Game, GameStatusChange, Purchase, Session logger = logging.getLogger("games") +@receiver(pre_save, sender=Purchase) +def store_purchase_price_snapshot(sender, instance, **kwargs): + """Store old price values before save so we can detect changes.""" + if instance.pk is not None: + try: + old_instance = sender.objects.get(pk=instance.pk) + instance._old_price = old_instance.price + instance._old_currency = old_instance.price_currency + except sender.DoesNotExist: + pass + + +@receiver(post_save, sender=Purchase) +def mark_needs_price_update(sender, instance, created, **kwargs): + """Mark purchase for price update if price or currency changed.""" + if not created and hasattr(instance, "_old_price"): + if ( + instance.price != instance._old_price + or instance.price_currency != instance._old_currency + ): + sender.objects.filter(pk=instance.pk).update(needs_price_update=True) + + @receiver(m2m_changed, sender=Purchase.games.through) def update_num_purchases(sender, instance, action, reverse, **kwargs): if not reverse and action.startswith("post_"): diff --git a/games/tasks.py b/games/tasks.py index b21fb35..f59ecb4 100644 --- a/games/tasks.py +++ b/games/tasks.py @@ -1,6 +1,7 @@ import logging import requests +from django.db import models from django.template.defaultfilters import floatformat logger = logging.getLogger("games") @@ -12,68 +13,77 @@ currency_to = "CZK" currency_to = currency_to.upper() -def save_converted_info(purchase, converted_price, converted_currency): +def _get_exchange_rate(currency_from, currency_to, year): logger.info( - f"Setting converted price of {purchase} to {converted_price} {converted_currency} (originally {purchase.price} {purchase.price_currency})" + f"[convert_prices]: Looking for exchange rate in database: {currency_from}->{currency_to}" + ) + exchange_rate = ExchangeRate.objects.filter( + currency_from=currency_from, currency_to=currency_to, year=year + ).first() + if not exchange_rate: + logger.info( + f"[convert_prices]: Getting exchange rate from {currency_from} to {currency_to} for {year}..." + ) + try: + response = requests.get( + f"https://cdn.jsdelivr.net/npm/@fawazahmed0/currency-api@{year}-01-01/v1/currencies/{currency_from.lower()}.json" + ) + response.raise_for_status() + data = response.json() + currency_from_data = data.get(currency_from.lower()) + rate = currency_from_data.get(currency_to.lower()) + if rate: + logger.info(f"[convert_prices]: Got {rate}, saving...") + exchange_rate = ExchangeRate.objects.create( + currency_from=currency_from, + currency_to=currency_to, + year=year, + rate=floatformat(rate, 2), + ) + exchange_rate = exchange_rate.rate + else: + logger.info("[convert_prices]: Could not get an exchange rate.") + except requests.RequestException as e: + logger.info( + f"[convert_prices]: Failed to fetch exchange rate for {currency_from}->{currency_to} in {year}: {e}" + ) + elif exchange_rate: + exchange_rate = exchange_rate.rate + return exchange_rate + + +def _save_converted_price(purchase, converted_price, needs_update): + logger.info( + f"Setting converted price of {purchase} to {converted_price} {currency_to} (originally {purchase.price} {purchase.price_currency})" ) purchase.converted_price = converted_price - purchase.converted_currency = converted_currency - purchase.save() + purchase.converted_currency = currency_to + if needs_update: + purchase.needs_price_update = False + purchase.save(update_fields=["converted_price", "converted_currency", "needs_price_update"]) def convert_prices(): purchases = Purchase.objects.filter( - converted_price__isnull=True, converted_currency="" - ) + models.Q(needs_price_update=True) | models.Q(converted_price__isnull=True) + ).distinct() if purchases.count() == 0: logger.info("[convert_prices]: No prices to convert.") + return for purchase in purchases: + needs_update = purchase.needs_price_update if purchase.price_currency.upper() == currency_to or purchase.price == 0: - save_converted_info(purchase, purchase.price, currency_to) + _save_converted_price(purchase, purchase.price, needs_update) continue year = purchase.date_purchased.year currency_from = purchase.price_currency.upper() - - exchange_rate = ExchangeRate.objects.filter( - currency_from=currency_from, currency_to=currency_to, year=year - ).first() - logger.info( - f"[convert_prices]: Looking for exchange rate in database: {currency_from}->{currency_to}" - ) - if not exchange_rate: - logger.info( - f"[convert_prices]: Getting exchange rate from {currency_from} to {currency_to} for {year}..." - ) - try: - # this API endpoint only accepts lowercase currency string - response = requests.get( - f"https://cdn.jsdelivr.net/npm/@fawazahmed0/currency-api@{year}-01-01/v1/currencies/{currency_from.lower()}.json" - ) - response.raise_for_status() - data = response.json() - currency_from_data = data.get(currency_from.lower()) - rate = currency_from_data.get(currency_to.lower()) - - if rate: - logger.info(f"[convert_prices]: Got {rate}, saving...") - exchange_rate = ExchangeRate.objects.create( - currency_from=currency_from, - currency_to=currency_to, - year=year, - rate=floatformat(rate, 2), - ) - else: - logger.info("[convert_prices]: Could not get an exchange rate.") - except requests.RequestException as e: - logger.info( - f"[convert_prices]: Failed to fetch exchange rate for {currency_from}->{currency_to} in {year}: {e}" - ) + exchange_rate = _get_exchange_rate(currency_from, currency_to, year) if exchange_rate: - save_converted_info( + _save_converted_price( purchase, - floatformat(purchase.price * exchange_rate.rate, 0), - currency_to, + floatformat(purchase.price * exchange_rate, 0), + needs_update, ) diff --git a/games/tests.py b/games/tests.py index 7ce503c..fea225b 100644 --- a/games/tests.py +++ b/games/tests.py @@ -1,3 +1,109 @@ -from django.test import TestCase +from datetime import date -# Create your tests here. +from django.test import TestCase, override_settings + +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) + + with override_settings( + CACHES={ + "default": { + "BACKEND": "django.core.cache.backends.locmem.LocMemCache" + } + } + ): + 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) -- 2.52.0 From 4ba3ed555f384038679664d9c7f08de53b54a56c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Kucharczyk?= Date: Tue, 12 May 2026 14:51:59 +0200 Subject: [PATCH 2/3] Add info on statuses --- STATUSES.md | 157 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 157 insertions(+) create mode 100644 STATUSES.md diff --git a/STATUSES.md b/STATUSES.md new file mode 100644 index 0000000..3223759 --- /dev/null +++ b/STATUSES.md @@ -0,0 +1,157 @@ +# Game & Purchase Status Definitions + +## Game Statuses + +Games have a `status` field with the following values: + +| Status | Code | Description | +|--------|------|-------------| +| **Unplayed** | `u` | Game was purchased but never played | +| **Played** | `p` | Game was played but not yet finished | +| **Finished** | `f` | Game has been completed | +| **Retired** | `r` | Game was intentionally retired (e.g., no longer accessible, collector's item) | +| **Abandoned** | `a` | Game was played but the user gave up on it | + +**Setting game status:** +- Users explicitly set game status via the UI (finish/drop purchase buttons, status change form) +- Status changes are tracked in `GameStatusChange` model +- Refunding a purchase always marks its games as abandoned + +--- + +## Purchase-Level Status Concepts + +These concepts determine whether a purchase appears in the "unfinished" or "dropped" lists in stats views. + +### Finished + +A purchase is considered **finished** when: + +``` +Game.status == "f" OR Purchase.games.* has a PlayEvent with an ended date +``` + +Either signal indicates the game is complete: +- **Explicit**: User marked the game as finished (`Game.status = "f"`) +- **Implicit**: A PlayEvent exists with `ended` date set (data-driven) + +This uses **OR** logic during a transition period. Later, these signals should be kept in sync so only one source of truth is needed. + +### Dropped + +A purchase is considered **dropped** when: + +``` +Game.status == "a" OR Purchase.date_refunded IS NOT NULL +``` + +Either signal indicates the user no longer has an active interest in the game: +- **Explicit**: User marked the game as abandoned (`Game.status = "a"`) +- **Implicit**: User refunded the purchase (which automatically sets games to abandoned) + +Note: Refunding a purchase always marks its games as abandoned. There is no option to refund without abandoning. + +--- + +## Unfinished vs. Dropped + +The stats views categorize purchases into **unfinished** and **dropped** lists. + +### Unfinished + +A purchase is **unfinished** when: +1. It was purchased in the relevant time period (this year for yearly stats, all time for all-time stats) +2. It was NOT refunded (only counts toward unfinished/backlog) +3. It is NOT finished (per the finished definition above) +4. It is NOT dropped (per the dropped definition above) +5. It is NOT infinite (subscription, etc.) +6. It IS a game or DLC (not season passes or battle passes) + +**Unfinished = Active backlog** — games the user may still play. + +### Dropped + +A purchase is **dropped** when: +1. It was purchased in the relevant time period +2. It is NOT finished (per the finished definition above) +3. It matches at least one dropped signal (per the dropped definition above) +4. It is NOT infinite +5. It IS a game or DLC + +**Dropped = Terminal state** — games the user has given up on or refunded. + +### Summary Table + +| Category | Includes Refunded? | Key Condition | +|----------|-------------------|---------------| +| **Unfinished** | No | NOT finished, NOT dropped | +| **Dropped** | Yes | Finished OR Abandoned/Retired | +| **Refunded** | Yes | `date_refunded IS NOT NULL` | +| **Infinite** | Yes | `infinite = True` | + +--- + +## Query Patterns + +### Checking if a game is finished + +```python +game.finished() # Returns True if status="f" or has PlayEvent with ended date +``` + +### Checking if a game is abandoned + +```python +game.abandoned() # Returns True if status="a" +``` + +### Getting finished purchases + +```python +Purchase.objects.finished() # All purchases where games are finished +``` + +### Getting dropped purchases + +```python +Purchase.objects.dropped() # All purchases that are abandoned or refunded +``` + +--- + +## Transition State + +The system uses **OR logic** for both finished and dropped to catch any mismatch between explicit user actions and data signals: + +- **Finished**: `status="f" OR PlayEvent.ended` +- **Dropped**: `status="a" OR date_refunded` + +This bridges the gap between the old model (where `date_finished` and `date_dropped` were on the Purchase model) and the new model (where `Game.status` and `PlayEvent` are the sources of truth). + +**Future:** These signals should be kept in sync. For example: +- Setting `Game.status = "f"` should create a PlayEvent with `ended` date +- When the sync is reliable, the OR can be simplified to a single check + +Note: Refunding a purchase always automatically sets its games' status to Abandoned. This is not optional — there is no way to refund without abandoning. + +--- + +## Edge Cases + +### Unplayed games +- Unplayed games (`status="u"`) are considered **unfinished**, not dropped +- They appear in the unfinished/backlog list since they are still games the user may play +- Unplayed games that are refunded DO count as **dropped** (refund signal overrides) + +### Multiple games per purchase +- A purchase can have multiple games via `Purchase.games` (many-to-many) +- A purchase is finished if ANY of its games is finished +- A purchase is dropped if ANY of its games is abandoned OR the purchase itself is refunded + +### PlayEvents without ended date +- A PlayEvent with `started` but no `ended` does NOT count as finished +- This represents a game that was started but not completed + +### Retired games +- Retired games (`status="r"`) are considered **dropped** +- Retirement is for games the user intentionally removed from their collection (collector's items, no longer accessible, etc.) -- 2.52.0 From 5003b739d33f6475843fd6e15f9cde7e319cd661 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Kucharczyk?= Date: Tue, 12 May 2026 14:56:59 +0200 Subject: [PATCH 3/3] PR review --- games/tasks.py | 22 +++++++++++----------- games/tests.py | 11 ++--------- 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/games/tasks.py b/games/tasks.py index f59ecb4..7fa94d1 100644 --- a/games/tasks.py +++ b/games/tasks.py @@ -14,14 +14,14 @@ currency_to = currency_to.upper() def _get_exchange_rate(currency_from, currency_to, year): - logger.info( + logger.debug( f"[convert_prices]: Looking for exchange rate in database: {currency_from}->{currency_to}" ) - exchange_rate = ExchangeRate.objects.filter( + rate = ExchangeRate.objects.filter( currency_from=currency_from, currency_to=currency_to, year=year ).first() - if not exchange_rate: - logger.info( + if not rate: + logger.debug( f"[convert_prices]: Getting exchange rate from {currency_from} to {currency_to} for {year}..." ) try: @@ -40,16 +40,16 @@ def _get_exchange_rate(currency_from, currency_to, year): year=year, rate=floatformat(rate, 2), ) - exchange_rate = exchange_rate.rate + rate = exchange_rate.rate else: logger.info("[convert_prices]: Could not get an exchange rate.") except requests.RequestException as e: logger.info( f"[convert_prices]: Failed to fetch exchange rate for {currency_from}->{currency_to} in {year}: {e}" ) - elif exchange_rate: - exchange_rate = exchange_rate.rate - return exchange_rate + elif rate: + rate = rate.rate + return rate def _save_converted_price(purchase, converted_price, needs_update): @@ -78,11 +78,11 @@ def convert_prices(): continue year = purchase.date_purchased.year currency_from = purchase.price_currency.upper() - exchange_rate = _get_exchange_rate(currency_from, currency_to, year) - if exchange_rate: + rate = _get_exchange_rate(currency_from, currency_to, year) + if rate: _save_converted_price( purchase, - floatformat(purchase.price * exchange_rate, 0), + floatformat(purchase.price * rate, 0), needs_update, ) diff --git a/games/tests.py b/games/tests.py index fea225b..c8a01b0 100644 --- a/games/tests.py +++ b/games/tests.py @@ -1,6 +1,6 @@ from datetime import date -from django.test import TestCase, override_settings +from django.test import TestCase from games.models import Game, Platform, Purchase from games.tasks import convert_prices @@ -29,14 +29,7 @@ class PurchaseNeedsPriceUpdateTest(TestCase): purchase.games.add(self.game) self.assertTrue(purchase.needs_price_update) - with override_settings( - CACHES={ - "default": { - "BACKEND": "django.core.cache.backends.locmem.LocMemCache" - } - } - ): - convert_prices() + convert_prices() purchase.refresh_from_db() self.assertFalse(purchase.needs_price_update) -- 2.52.0