diff --git a/README.md b/README.md index 741fbd4..ad7e483 100644 --- a/README.md +++ b/README.md @@ -203,7 +203,7 @@ Concrete steps, mirroring the real `first-ad-create` fixture in `apps/promotions ) ``` -3. **Attach a recipient.** The common case: pay the user who fired the event, a flat amount, no percentage math. Set `promotion_type` to route the payout to the right wallet — `first_ad_view` (or unset) pays into the user's cash-like reward wallet; `capture`/`first_ad_create` fund billboard/ad credit instead (`WALLET_ADVERTISING_TRANSIT`) — see `Recipient.get_wallet_category_uuid()`. A `wallet_uuid` set directly on the `Recipient` always wins over `promotion_type`. +3. **Attach a recipient.** The common case: pay the user who fired the event, a flat amount, no percentage math. Set `wallet_destination` to pick which wallet the payout lands in — `user_reward` (the default) pays into the user's cash-like reward wallet; `advertising_transit` funds billboard/ad credit instead (`WALLET_ADVERTISING_TRANSIT`) — see `Recipient.get_wallet_category_uuid()`. A `wallet_uuid` set directly on the `Recipient` always wins over `wallet_destination`. ```python Recipient.objects.create( @@ -211,7 +211,7 @@ Concrete steps, mirroring the real `first-ad-create` fixture in `apps/promotions label="first-ad-create", recipient_uuid_field="->event:user", base_amount_field="30000", # literal, or "event:data_key" - promotion_type=PromotionTypeChoices.FIRST_AD_CREATE, + wallet_destination=WalletDestinationChoices.ADVERTISING_TRANSIT, ) ``` @@ -315,9 +315,9 @@ Non-secret values only — pulled from `main/settings.py` and this checkout's ow | `ACCOUNTS_BASE_PUBLIC_URL` | Accounts service REST API (user/application profile reads). | | `WALLET_BASE_PUBLIC_URL` | Wallet service REST API (deposit submit/verify). | | `WALLET_RIAL_DEPOSIT` | User-side "real money" wallet type UUID. Not currently read by any code path in this service — kept for parity with the shared naming convention used across the other Gooyal repos that touch the same wallet-service UUIDs (`advertising`, `settlement`, `ipg`). | -| `WALLET_USER_BILLBOARD_VISIT_INCOME` | User-side reward-token wallet type UUID (formerly `WALLET_REWARD`). `payee_wallet` for a cash-like user reward payout (e.g. `first_ad_view`) — see `Recipient.get_wallet_category_uuid()`. | +| `WALLET_USER_BILLBOARD_VISIT_INCOME` | User-side reward-token wallet type UUID (formerly `WALLET_REWARD`). `payee_wallet` when `Recipient.wallet_destination` is `user_reward` (the default) — see `Recipient.get_wallet_category_uuid()`. | | `WALLET_PROMOTIONS_TRANSIT` | Company-side pool payouts are drawn from — the deposit's `payer_wallet` (formerly hardcoded to the same UUID as the payee side; see [`docs/wallet_refactor.md`](docs/wallet_refactor.md)). Needs a real UUID from the wallet-service team before this service can submit a deposit. | -| `WALLET_ADVERTISING_TRANSIT` | Same wallet-service UUID as advertising's own setting of the same name. `payee_wallet` for promotion types that fund billboard/ad credit rather than a user reward (`capture`, `first_ad_create`) — see `Recipient.get_wallet_category_uuid()`. | +| `WALLET_ADVERTISING_TRANSIT` | Same wallet-service UUID as advertising's own setting of the same name. `payee_wallet` when `Recipient.wallet_destination` is `advertising_transit` — billboard/ad credit rather than a user reward — see `Recipient.get_wallet_category_uuid()`. | | `NOTIFICATIONS_BASE_PUBLIC_URL` | Notifications service REST API. | | `OAUTH2_CLIENT_ID` / `_SECRET` / `_SCOPES` | This service's own client-credentials identity, used for every outbound call above via `login_as_client_credentials()` (token cached under the key `promotions_access_token`). | diff --git a/apps/promotions/handlers.py b/apps/promotions/handlers.py index bc2481f..9852867 100644 --- a/apps/promotions/handlers.py +++ b/apps/promotions/handlers.py @@ -15,9 +15,7 @@ class ProcessorTypeChoices(models.TextChoices): OTHERS = 'others', _('others') class PromotionTypeChoices(models.TextChoices): - FIRST_AD_VIEW = 'first_ad_view', _('first ad view') - CAPTURE = 'capture', _('capture') - FIRST_AD_CREATE = 'first_ad_create', _('first ad create') + pass class BasePromotionHandler: diff --git a/apps/promotions/migrations/0010_recipient_wallet_destination.py b/apps/promotions/migrations/0010_recipient_wallet_destination.py new file mode 100644 index 0000000..42c87e6 --- /dev/null +++ b/apps/promotions/migrations/0010_recipient_wallet_destination.py @@ -0,0 +1,19 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('promotions', '0009_recipient_access_type_alloweduser'), + ] + + operations = [ + migrations.AddField( + model_name='recipient', + name='wallet_destination', + field=models.CharField(choices=[('user_reward', 'user reward wallet'), + ('advertising_transit', 'advertising transit wallet')], + db_index=True, default='user_reward', max_length=32, + verbose_name='wallet destination'), + ), + ] diff --git a/apps/promotions/models.py b/apps/promotions/models.py index bfd1f03..98b984b 100644 --- a/apps/promotions/models.py +++ b/apps/promotions/models.py @@ -193,12 +193,20 @@ class RecipientTypeChoices(models.TextChoices): PUBLIC = 'public', _('public') +class WalletDestinationChoices(models.TextChoices): + USER_REWARD = 'user_reward', _('user reward wallet') + ADVERTISING_TRANSIT = 'advertising_transit', _('advertising transit wallet') + + class Recipient(BaseModel): label = models.CharField(max_length=255, db_index=True) plan = models.ForeignKey(Plan, on_delete=models.PROTECT, related_name='recipients', null=True, blank=True) wallet_uuid = models.UUIDField(null=True, blank=True) promotion_type = models.CharField(max_length=64, verbose_name=_('promotion type'), db_index=True, choices=PromotionTypeChoices.choices, null=True, blank=True) + wallet_destination = models.CharField(max_length=32, verbose_name=_('wallet destination'), db_index=True, + choices=WalletDestinationChoices.choices, + default=WalletDestinationChoices.USER_REWARD) access_type = models.CharField(max_length=64, verbose_name=_('access type'), db_index=True, choices=RecipientTypeChoices.choices, default=RecipientTypeChoices.PUBLIC) @@ -223,9 +231,7 @@ class Recipient(BaseModel): if self.wallet_uuid: return self.wallet_uuid - if self.promotion_type in (PromotionTypeChoices.CAPTURE, PromotionTypeChoices.FIRST_AD_CREATE): - # Billboard/ad credit, not a cash-like user reward — funds the advertising - # service's own transit wallet instead of the user's reward wallet. + if self.wallet_destination == WalletDestinationChoices.ADVERTISING_TRANSIT: return settings.WALLET_ADVERTISING_TRANSIT return settings.WALLET_USER_BILLBOARD_VISIT_INCOME diff --git a/apps/promotions/tests.py b/apps/promotions/tests.py index b681927..cbdd984 100644 --- a/apps/promotions/tests.py +++ b/apps/promotions/tests.py @@ -10,8 +10,7 @@ from rest_framework.test import APITestCase, override_settings, APIClient from apps.promotions.tasks import analyze_event_task from apps.users.models import User from apps.promotions.models import Plan, Promotion, EventSaver, ProcessorTypeChoices, Event, Recipient, \ - PaymentStateChoices -from apps.promotions.handlers import PromotionTypeChoices + PaymentStateChoices, WalletDestinationChoices AccessToken = get_access_token_model() Application = get_application_model() @@ -791,7 +790,7 @@ class ApplicationApiFlowsTests(APITestCase): defaults={ 'recipient_uuid_field': '->event:user', 'base_amount_field': str(promotion_amount), - 'promotion_type': PromotionTypeChoices.FIRST_AD_CREATE, + 'wallet_destination': WalletDestinationChoices.ADVERTISING_TRANSIT, }, ) @@ -861,22 +860,20 @@ class ApplicationApiFlowsTests(APITestCase): def test_get_wallet_category_uuid_routing(self): from django.conf import settings - first_ad_view_recipient = Recipient(promotion_type=PromotionTypeChoices.FIRST_AD_VIEW) - self.assertEqual(first_ad_view_recipient.get_wallet_category_uuid(), + default_recipient = Recipient() + self.assertEqual(default_recipient.get_wallet_category_uuid(), settings.WALLET_USER_BILLBOARD_VISIT_INCOME) - unset_type_recipient = Recipient() - self.assertEqual(unset_type_recipient.get_wallet_category_uuid(), + user_reward_recipient = Recipient(wallet_destination=WalletDestinationChoices.USER_REWARD) + self.assertEqual(user_reward_recipient.get_wallet_category_uuid(), settings.WALLET_USER_BILLBOARD_VISIT_INCOME) - capture_recipient = Recipient(promotion_type=PromotionTypeChoices.CAPTURE) - self.assertEqual(capture_recipient.get_wallet_category_uuid(), settings.WALLET_ADVERTISING_TRANSIT) - - first_ad_create_recipient = Recipient(promotion_type=PromotionTypeChoices.FIRST_AD_CREATE) - self.assertEqual(first_ad_create_recipient.get_wallet_category_uuid(), settings.WALLET_ADVERTISING_TRANSIT) + transit_recipient = Recipient(wallet_destination=WalletDestinationChoices.ADVERTISING_TRANSIT) + self.assertEqual(transit_recipient.get_wallet_category_uuid(), settings.WALLET_ADVERTISING_TRANSIT) explicit_wallet_uuid = uuid.uuid4() - override_recipient = Recipient(promotion_type=PromotionTypeChoices.CAPTURE, wallet_uuid=explicit_wallet_uuid) + override_recipient = Recipient(wallet_destination=WalletDestinationChoices.ADVERTISING_TRANSIT, + wallet_uuid=explicit_wallet_uuid) self.assertEqual(override_recipient.get_wallet_category_uuid(), explicit_wallet_uuid) def test_first_ad_create_event_status_not_processed_when_only_event_exists(self): diff --git a/docs/wallet_refactor.md b/docs/wallet_refactor.md index 22aca1d..8285ea8 100644 --- a/docs/wallet_refactor.md +++ b/docs/wallet_refactor.md @@ -199,65 +199,84 @@ scaffolding noted in the README's watch list) — unrelated, left as-is. --- -## 6. Follow-up: per-promotion-type wallet routing (2026-08-25) +## 6. Follow-up: explicit per-recipient wallet destination (2026-08-25) -Not every payout is a cash-like user reward. Some promotion types fund billboard/ad credit -instead — money that should land in the **advertising** service's own transit wallet, not -the user's personal reward wallet: +Not every payout is a cash-like user reward. Some fund billboard/ad credit instead — money +that should land in the **advertising** service's own transit wallet, not the user's +personal reward wallet. -| Promotion type | Nature | Destination | +An earlier version of this follow-up tried to infer the destination from +`PromotionTypeChoices` (`first_ad_view` / `capture` / `first_ad_create`), a promotion +*category* field that existed but had never been given real values. That was reverted: it +buried a wallet-routing decision inside a general-purpose categorization field, coupling +two things that should vary independently — a promotion's category and where its money +goes are not the same fact, and the next new promotion type would need someone to remember +to also classify it for wallet purposes. + +Instead, `Recipient` gets a field that says the routing decision directly: + +```python +class WalletDestinationChoices(models.TextChoices): + USER_REWARD = 'user_reward', _('user reward wallet') + ADVERTISING_TRANSIT = 'advertising_transit', _('advertising transit wallet') +``` + +| `wallet_destination` | Nature | Resolves to | |---|---|---| -| `first_ad_view` (or unset) | Cash-like user reward | `WALLET_USER_BILLBOARD_VISIT_INCOME` (user-side) | -| `capture` | Billboard/ad credit | `WALLET_ADVERTISING_TRANSIT` (advertising's own company pool) | -| `first_ad_create` | Billboard/ad credit | `WALLET_ADVERTISING_TRANSIT` (advertising's own company pool) | +| `user_reward` (default) | Cash-like user reward | `WALLET_USER_BILLBOARD_VISIT_INCOME` | +| `advertising_transit` | Billboard/ad credit | `WALLET_ADVERTISING_TRANSIT` (advertising's own company pool) | -Before this follow-up, none of this was implemented: `PromotionTypeChoices` (in -`apps/promotions/handlers.py`) was an **empty** `TextChoices` enum, so `Recipient.promotion_type` -could never hold a real value, and `get_wallet_category_uuid()` had no way to distinguish one -promotion from another — every payout without an explicit `Recipient.wallet_uuid` fell -through to the same single default. +`PromotionTypeChoices` is left as it was before any of this — an empty enum, unused. It's +not part of this decision. ### Changes -- `apps/promotions/handlers.py` — `PromotionTypeChoices` now has three real members: - `FIRST_AD_VIEW`, `CAPTURE`, `FIRST_AD_CREATE`. - `main/settings.py` — new `WALLET_ADVERTISING_TRANSIT` setting. Same wallet-service UUID as advertising's own setting of the same name — copy that repo's real value in, don't provision a second UUID for the same wallet. -- `apps/promotions/models.py` — `Recipient.get_wallet_category_uuid()` now branches: +- `apps/promotions/models.py`: + - New `WalletDestinationChoices` enum, next to the existing `RecipientTypeChoices`. + - New `Recipient.wallet_destination` field (`CharField`, `db_index=True`, default + `USER_REWARD`) — a real DB column, unlike the earlier `promotion_type` attempt which + only changed field-level `choices=` metadata. + - `Recipient.get_wallet_category_uuid()`: - ```python - def get_wallet_category_uuid(self): - if self.wallet_uuid: - return self.wallet_uuid + ```python + def get_wallet_category_uuid(self): + if self.wallet_uuid: + return self.wallet_uuid - if self.promotion_type in (PromotionTypeChoices.CAPTURE, PromotionTypeChoices.FIRST_AD_CREATE): - return settings.WALLET_ADVERTISING_TRANSIT + if self.wallet_destination == WalletDestinationChoices.ADVERTISING_TRANSIT: + return settings.WALLET_ADVERTISING_TRANSIT - return settings.WALLET_USER_BILLBOARD_VISIT_INCOME - ``` + return settings.WALLET_USER_BILLBOARD_VISIT_INCOME + ``` - Priority order: an explicit `Recipient.wallet_uuid` always wins (per-recipient override, - from the original refactor above); otherwise `promotion_type` picks the wallet; otherwise - the default cash-reward wallet. + Priority order: an explicit `Recipient.wallet_uuid` always wins (the per-recipient raw + override from the original refactor above); otherwise `wallet_destination` picks the + wallet type; `user_reward` is the default so existing rows behave exactly as before + this change until someone opts them into `advertising_transit`. +- `apps/promotions/migrations/0010_recipient_wallet_destination.py` — new migration adding + the column, `AddField` with `default='user_reward'` so existing rows backfill safely. - `apps/promotions/tests.py` — the `first-ad-create` fixture now sets - `promotion_type=PromotionTypeChoices.FIRST_AD_CREATE`; `test_first_ad_create_event_status_processed` - asserts the deposit call actually receives `WALLET_PROMOTIONS_TRANSIT` as `payer_wallet` - and `WALLET_ADVERTISING_TRANSIT` as `payee_wallet`; a new `test_get_wallet_category_uuid_routing` - unit-tests all four routing cases directly against `Recipient.get_wallet_category_uuid()`. + `wallet_destination=WalletDestinationChoices.ADVERTISING_TRANSIT`; + `test_first_ad_create_event_status_processed` asserts the deposit call actually receives + `WALLET_PROMOTIONS_TRANSIT` as `payer_wallet` and `WALLET_ADVERTISING_TRANSIT` as + `payee_wallet`; `test_get_wallet_category_uuid_routing` unit-tests all four routing cases + directly against `Recipient.get_wallet_category_uuid()`. - `README.md` — config reference table and the playbook's step 3 example updated to show - setting `promotion_type` on a new `Recipient`. + setting `wallet_destination` on a new `Recipient`. ### What's still manual -- No existing `Plan`/`Recipient` rows in a live database get `promotion_type` set - automatically — this is a schema/behavior change, not a data migration. Whoever owns the - `capture` and `first_ad_create` plans needs to set `promotion_type` on their `Recipient` - rows via admin (or a data migration) for this routing to take effect on existing data. +- Existing `Plan`/`Recipient` rows in a live database default to `wallet_destination='user_reward'` + on migrate — behavior for them doesn't change. Whoever owns the real `capture` / + `first-ad-create` plans needs to explicitly set `wallet_destination='advertising_transit'` + on their `Recipient` rows (via admin or a follow-up data migration) for those specific + payouts to actually route to the advertising transit wallet. - `WALLET_ADVERTISING_TRANSIT` is a second **required** env var on top of `WALLET_PROMOTIONS_TRANSIT` — same deploy-blocking caveat as §5: missing it fails `main/settings.py` import, not just a payout at runtime. -- `Recipient.promotion_type`'s field-level `choices=` metadata changed (empty → three - values). This has no DB-level effect (Django doesn't enforce `choices` with a `CHECK` - constraint), but running `manage.py makemigrations` will want to record a no-op migration - for it — harmless to generate, just for `makemigrations --check` cleanliness in CI. +- This migration hasn't been run against a real database in this environment (no local + `.env`/DB configured here) — run `manage.py migrate` and confirm `0010` applies cleanly + before deploying.