FIX(promotions): route payouts by explicit Recipient.wallet_destination, not promotion type
The first attempt at this inferred the payout destination from PromotionTypeChoices (first_ad_view/capture/first_ad_create), a general promotion-category field that had never held real values. That coupled two independent facts together — a promotion's category and where its money goes aren't the same thing, and every new promotion type would need someone to remember to also classify it for wallet purposes. Revert PromotionTypeChoices to empty (left as pre-existing dead scaffolding, untouched) and add Recipient.wallet_destination instead — a real field that states the routing decision directly. Recipient.get_wallet_category_uuid() now branches on it: user_reward (default) resolves to WALLET_USER_BILLBOARD_VISIT_INCOME, advertising_transit resolves to WALLET_ADVERTISING_TRANSIT. An explicit Recipient.wallet_uuid still wins over either. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
45a64f0013
commit
0fe4a3ed79
6 changed files with 102 additions and 63 deletions
|
|
@ -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`). |
|
||||
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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'),
|
||||
),
|
||||
]
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue