diff --git a/docs/frontend-notification-click-actions.md b/docs/frontend-notification-click-actions.md index 40dc0f2..14b1135 100644 --- a/docs/frontend-notification-click-actions.md +++ b/docs/frontend-notification-click-actions.md @@ -49,69 +49,86 @@ You may also see other keys under `extras` alongside (or instead of) below). Ignore keys you don't recognize; don't treat an unfamiliar key as an error. -## ⚠️ The URL format below is a placeholder — needs your input +## URLs are admin-configured, not hard-coded — nothing for you to build here -Every `click_url` currently being sent is built from a setting called -`FRONTEND_BASE_URL`, defaulting to `https://app.gooyal.ir`, with a path like -`/ads/` or `/escrow/` appended. **Nobody has confirmed this -matches your actual app's routing** — whether you use a custom URI scheme -(`gooyal://ads/`), a universal/app link (`https://...`), or something -else entirely (a route name + params instead of a URL at all). +The actual destination string is no longer computed in backend code. Each +notification type has a **code**, and the notifications service resolves +that code against an admin-managed table (`NotificationLink`: `code`, +`url_template`, `is_active`) at send time. `url_template` supports a +`{object_id}` placeholder, e.g. `https://app.gooyal.ir/ads/{object_id}` or +`gooyal://ads/{object_id}` — whichever scheme your app actually uses. -This was built centrally on purpose so it's a one-line change once you tell -us: every `click_url` in the system is built by one helper function per -producer service (`utils/deep_links.py` in `advertising`), not scattered -across dozens of call sites. **Please confirm your scheme and we'll update -it** — nothing about the client-side contract above changes either way, -only the string inside `click.url`. +This means: **your routing scheme is a config decision, not a code change**. +Whoever manages the notifications service's admin panel enters one row per +code below with your actual URL format, and every notification of that type +immediately starts carrying it. If a code has no row yet (or its row is +marked inactive), that notification simply has no `click_url` — same as any +other non-clickable notification, no error. -## What's clickable today +Practically, this doesn't change anything about what you build — you still +just read `extras["client::notification"]["click"]["url"]` and navigate if +it's present. It changes who's responsible for the URL *string itself*: not +a backend deploy, just an admin panel entry. If you want a different value +than what's currently configured for any code, ask whoever owns that panel +to update it. -All of the following are live in the `advertising` service and carry a real -`click_url` pointing at the ad (`/ads/`) or escrow deal (`/escrow/`): +## What's clickable today, by code -### Billboards & content (→ `/ads/`) +All of these are live in the `advertising` service: -| Event | Notification text (fa) | +### Billboards & content + +| Code | Notification text (fa) | |---|---| -| Someone viewed your billboard | شخصی شروع به مشاهده بیلبورد شما کرد. | -| Billboard created | بیلبود شما با موفقیت ساخته شد. | -| Billboard approved | بیلبورد شما تایید شد. | -| Billboard rejected | بیلبود شما رد شد. | -| Someone commented on your billboard | یک نفر برای بیلبورد شما نظر گذاشت! | -| Someone replied to your comment | یک نفر به نظر شما پاسخ داد! | -| Someone bought your content | درآمد جدید دارید! یک نفر محتوای شما را خرید. | -| Someone supported your content | یک نفر از محتوای شما حمایت کرد! | -| Pin about to expire (time) | زمان پین رو به اتمامه! | -| Pin expired (time) | پین شما منقضی شد! | -| Pin credit about to run out (budget) | اعتبار پین رو به اتمامه! | -| Pin credit exhausted (budget) | اعتبار پین شما به پایان رسید! | +| `billboard.viewed` | شخصی شروع به مشاهده بیلبورد شما کرد. | +| `billboard.created` | بیلبود شما با موفقیت ساخته شد. | +| `billboard.approved` | بیلبورد شما تایید شد. | +| `billboard.rejected` | بیلبود شما رد شد. | +| `billboard.commented` | یک نفر برای بیلبورد شما نظر گذاشت! | +| `billboard.replied` | یک نفر به نظر شما پاسخ داد! | +| `billboard.content_bought` | درآمد جدید دارید! یک نفر محتوای شما را خرید. | +| `billboard.content_supported` | یک نفر از محتوای شما حمایت کرد! | +| `billboard.pin_expiring` | زمان پین رو به اتمامه! | +| `billboard.pin_expired` | پین شما منقضی شد! | +| `billboard.credit_low` | اعتبار پین رو به اتمامه! | +| `billboard.credit_exhausted` | اعتبار پین شما به پایان رسید! | -Note the last four are **two independent signals** — a billboard can lapse -because its display *time* ran out, or because its *budget* (balance vs. -reward-per-view) ran out. Both currently point at the same ad detail page; -if your UI wants to show a different call-to-action (extend time vs. top up -credit) based on which one fired, that distinction is in the `extras` payload -via which notification title/text arrived, not in the URL itself — ask if -you need a structured signal here instead of parsing title text. +Every code above sends the ad's `uuid` as `click_object_id`, so a +`url_template` of `https://app.gooyal.ir/ads/{object_id}` (or your app's +real equivalent) resolves correctly for all twelve. They're still separate +codes rather than one shared one, on purpose: `billboard.pin_expiring` and +`billboard.credit_low` are genuinely different situations (time running out +vs. budget running out) even though they'd point at the same screen today — +giving each its own code means that can diverge later (e.g. deep-linking +straight to an "extend time" vs. "top up credit" action) without any code +change, just a new admin row. -### Escrow deals (→ `/escrow/`) +### Escrow deals -The full P2P deal lifecycle — buyer pays, seller approves/rejects, delivery, -confirm, dispute, payout/refund. All 11 possible state transitions notify -whichever party (buyer or seller) needs to act or be informed next, each -with a `click_url` to that deal's detail page. If you're building an escrow -detail screen, treat every push in this domain as "go look at this deal" — -the screen itself should reflect current state, not the specific notification -that triggered the tap. +| Code | Trigger | +|---|---| +| `escrow.request_deal` | Buyer paid, seller needs to review | +| `escrow.cancel_deal` | Buyer canceled before seller responded | +| `escrow.reject_deal` | Seller declined the deal | +| `escrow.approve_deal` | Seller confirmed the deal | +| `escrow.cancel_deal_by_seller` | Seller canceled an active deal | +| `escrow.request_cancel` | Buyer requested cancellation | +| `escrow.approve_cancel` | Seller accepted the cancellation | +| `escrow.reject_cancel` | Seller declined the cancellation | +| `escrow.confirm_by_seller` | Seller marked it delivered | +| `escrow.confirm_by_buyer` | Buyer confirmed receipt — deal completed | +| `escrow.request_judge` | A dispute was opened | +| `escrow.cancel_judge` | Buyer withdrew their dispute | +| `escrow.approve_judge` | Dispute resolved for the buyer | +| `escrow.reject_judge` | Dispute resolved for the seller | +| `escrow.timeout` | Deal expired without a response | + +All fifteen send the escrow deal's `uuid` as `click_object_id`. Same +one-code-per-situation reasoning as billboards — they all currently resolve +to the same escrow-detail destination, but that's an admin config choice, +not a hard-coded one, so it's free to diverge later. ## Not yet migrated to this mechanism -- **chat**: sends `extras = {"conversation_uuid": ..., "post_id": ...}` directly, without a `click_url`. If your client already has custom handling for these two keys (built before this mechanism existed), it keeps working — this doc doesn't change that. If you'd rather chat also send a `click_url` once your routing scheme is confirmed, that's a small change on our side; let us know. +- **chat**: sends `extras = {"conversation_uuid": ..., "post_id": ...}` directly, without a code or `click_url`. If your client already has custom handling for these two keys (built before this mechanism existed), it keeps working — this doc doesn't change that. Let us know if you'd rather chat send a code too. - **promotions**: integrated with the notifications service but not currently sending any notification with real content (`extras={}` today) — nothing to build against yet. - -## Questions for you before we finalize - -1. What's your app's actual deep-link scheme — custom URI, universal link, or something else? -2. Do you want the "time expiring" vs. "credit expiring" pin notifications distinguished by something other than title text? -3. Do you want chat migrated onto `click_url` too, or is your existing `conversation_uuid`/`post_id` handling staying as-is? diff --git a/docs/implementation.md b/docs/implementation.md index 7c44ae7..55b5189 100644 --- a/docs/implementation.md +++ b/docs/implementation.md @@ -52,15 +52,32 @@ An admin action (`BulkPushMessageAdmin`, `apps/push_notifications/admin.py`) tri `extract_data()` reads a legacy `.xls` file (via `xlrd` — only `.xls`, not `.xlsx`, since `xlrd` dropped xlsx support at 2.0) with columns: -| Col | 0 | 1 | 2 | 3 | 4 | 5 | -|---|---|---|---|---|---|---| -| Field | `user_uuid` | `title` | `message` | `priority` | `extras` (JSON string) | `click_url` | +| Col | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | +|---|---|---|---|---|---|---|---|---| +| Field | `user_uuid` | `title` | `message` | `priority` | `extras` (JSON string) | `click_url` | `code` | `click_object_id` | `push_to_all()` looks up each row's `PushUser` by `user_id`; missing users are counted as failures rather than raising, so one bad row doesn't abort the batch. Each successful row creates a `PushMessage` and calls `send_push()` — it goes through the exact same Celery task and Gotify call as a single push, so there's no separate bulk-specific delivery code path to keep in sync. -## The click_url mechanism +## The click action mechanism -Added in migration `0004_pushmessage_click_url`. The design constraint: **most notifications aren't clickable**, so this had to be fully optional at every layer, and had to work identically for both the single and bulk paths without a second implementation. +Added in migration `0004_pushmessage_click_url`, extended in `0005` with +`code`/`click_object_id` and the `NotificationLink` model. The design +constraint throughout: **most notifications aren't clickable**, so this had +to be fully optional at every layer, and had to work identically for both +the single and bulk paths without a second implementation. + +Two ways to set a destination on a `PushMessage`: +1. **`click_url`** — a raw URL the caller already knows. Always wins if set. +2. **`code` + `click_object_id`** — the preferred path. `code` is a documented, + per-notification-type string (e.g. `billboard.approved`, `escrow.timeout` — + see [notification-click-actions.md](notification-click-actions.md) for the + full catalog); `click_object_id` is the id of the thing the notification is + about (an ad's or escrow deal's uuid, typically). Resolved at send time + against `NotificationLink`, an admin-managed table + (`code`, `url_template`, `is_active`) — `url_template` supports a + `{object_id}` placeholder. This is what moved the actual destination + strings out of Python and into the Django admin, so a routing-scheme + change is a config edit, not a deploy across every producer service. `apps/push_notifications/models.py`: @@ -68,23 +85,34 @@ Added in migration `0004_pushmessage_click_url`. The design constraint: **most n GOTIFY_CLICK_EXTRA_KEY = "client::notification" # Gotify's own reserved namespace click_url = models.URLField(max_length=1000, null=True, blank=True) +code = models.SlugField(max_length=100, null=True, blank=True, db_index=True) +click_object_id = models.CharField(max_length=255, null=True, blank=True) + +def resolve_click_url(self): + if self.click_url: + return self.click_url + if not self.code: + return None + link = NotificationLink.objects.filter(code=self.code, is_active=True).first() + if not link: + return None + return link.resolve(self.click_object_id) def get_gotify_extras(self): extras = dict(self.extras or {}) - if self.click_url: + click_url = self.resolve_click_url() + if click_url: notification_extra = dict(extras.get(self.GOTIFY_CLICK_EXTRA_KEY) or {}) - notification_extra["click"] = {"url": self.click_url} + notification_extra["click"] = {"url": click_url} extras[self.GOTIFY_CLICK_EXTRA_KEY] = notification_extra return extras ``` Design decisions worth knowing if you touch this: -- **`click_url` is a real column, not just an `extras` key** — so it's queryable/auditable independently, and callers don't need to know Gotify's raw extras convention to use it. +- **An unresolvable code is not an error** — no matching `NotificationLink`, or one marked `is_active=False`, just means no click action, same as a message with nothing set at all. A producer can start sending a new code before anyone's configured it in the admin; nothing breaks, the notification just isn't clickable yet. - **The merge is additive**: any other `extras` keys a producer already sends (e.g. chat's `conversation_uuid`/`post_id`) pass through untouched — `get_gotify_extras()` only ever adds the `client::notification` key, never removes others. -- **`get_gotify_extras()` is the single place this merge happens** — `tasks.py` calls it instead of reading `push_message.extras` directly, so both the single-push and bulk-push paths (which both eventually call the same task) get it for free. -- **Optionality is enforced at three layers**, not just the DB: `null=True, blank=True` on the field → DRF `ModelSerializer` derives `required=False, allow_null=True, allow_blank=True` automatically → and behaviorally, an unset `click_url` leaves `extras` completely untouched (verified: `get_gotify_extras()` on a message with no `click_url` returns the original `extras` dict unchanged). - -The actual URL values are built by producer services from their own `utils/deep_links.py`-style helpers (currently only `advertising` has one) against a placeholder base — see [frontend-notification-click-actions.md](frontend-notification-click-actions.md) for why that's still a placeholder and what needs to happen before it's final. +- **`get_gotify_extras()` is the single place resolution happens** — `tasks.py` calls it instead of reading `push_message.extras`/`click_url` directly, so the single-push path, the bulk-push path, and both `click_url` and `code` all go through one function. +- **The admin table is deliberately not seeded with rows by any migration.** Populating it is an ops/product decision (which is the entire point of moving it out of code) — see [frontend-notification-click-actions.md](frontend-notification-click-actions.md) for the current code catalog they need to fill in. ## Email flow @@ -111,7 +139,8 @@ This service is an OAuth2 **resource server** (`django-oauth-toolkit`), not an O ## Notable migrations -- `0004_pushmessage_click_url` — adds `click_url`. See "The click_url mechanism" above. +- `0004_pushmessage_click_url` — adds `click_url`. +- `0005_notificationlink_pushmessage_click_object_id_and_more` — adds `NotificationLink`, `PushMessage.code`, `PushMessage.click_object_id`. See "The click action mechanism" above. ## Known technical debt (implementation-level) diff --git a/docs/notification-click-actions.md b/docs/notification-click-actions.md index 86fc0aa..90dc1c8 100644 --- a/docs/notification-click-actions.md +++ b/docs/notification-click-actions.md @@ -1,5 +1,13 @@ # Notification click actions +> **Update**: this doc captures the initial implementation pass. Destinations +> are no longer built from a hard-coded `FRONTEND_BASE_URL` — producers now +> send a `code` + `click_object_id`, resolved against the admin-managed +> `NotificationLink` table. See [implementation.md](implementation.md#the-click-action-mechanism) +> for the current mechanism and [frontend-notification-click-actions.md](frontend-notification-click-actions.md) +> for the current code catalog. The parts of this doc about `click_url` itself, +> optionality, and the escrow/billboard notification catalog are still accurate. + How a tapped push notification carries a destination, why that job splits across several repos, and what still needs building outside this service.