From 8f83a5e84a84bd41d2ef5dad403aa78342da494d Mon Sep 17 00:00:00 2001 From: Ali Asadi Date: Sat, 22 Aug 2026 09:36:06 +0330 Subject: [PATCH] FEAT(notifications): add optional click_url for notification tap actions Not every notification is clickable, so click_url is optional end to end (null/blank on the model, required=False on the serializer). When set, it's merged into Gotify's own client::notification.click.url extras convention without disturbing any other extras a producer already sends, so official Gotify clients can open it on tap. Wired through both message-creation paths: the single-push REST API and the bulk Excel upload. Also fixes a pre-existing NameError in the bulk path (json.loads(extras) referenced an undefined name instead of extras_str). Co-Authored-By: Claude Sonnet 5 --- .../migrations/0004_pushmessage_click_url.py | 18 ++ apps/push_notifications/models.py | 23 +- apps/push_notifications/serializers.py | 3 +- apps/push_notifications/tasks.py | 2 +- docs/notification-click-actions.md | 236 ++++++++++++++++++ 5 files changed, 278 insertions(+), 4 deletions(-) create mode 100644 apps/push_notifications/migrations/0004_pushmessage_click_url.py create mode 100644 docs/notification-click-actions.md diff --git a/apps/push_notifications/migrations/0004_pushmessage_click_url.py b/apps/push_notifications/migrations/0004_pushmessage_click_url.py new file mode 100644 index 0000000..3845293 --- /dev/null +++ b/apps/push_notifications/migrations/0004_pushmessage_click_url.py @@ -0,0 +1,18 @@ +# Generated by Django 5.2.6 on 2026-08-22 05:29 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('push_notifications', '0003_bulkpushmessage_pushmessage_bulk'), + ] + + operations = [ + migrations.AddField( + model_name='pushmessage', + name='click_url', + field=models.URLField(blank=True, max_length=1000, null=True), + ), + ] diff --git a/apps/push_notifications/models.py b/apps/push_notifications/models.py index 42fc57e..b176b43 100644 --- a/apps/push_notifications/models.py +++ b/apps/push_notifications/models.py @@ -45,6 +45,11 @@ class PushUser(BaseModel): class PushMessage(BaseModel): + # Gotify's reserved extras namespace for client-side actions. Official + # clients (Android/iOS/web) open this URL when the notification is tapped: + # https://gotify.net/docs/pushmsg#extras + GOTIFY_CLICK_EXTRA_KEY = "client::notification" + class StateChoices(models.IntegerChoices): INIT = 0, _('init') START = 1, _('Start') @@ -59,10 +64,21 @@ class PushMessage(BaseModel): message = models.TextField() priority = models.IntegerField(default=5) extras = models.JSONField(default=dict) + click_url = models.URLField(max_length=1000, null=True, blank=True) stats = models.IntegerField(default=StateChoices.INIT, choices=StateChoices.choices) bulk = models.ForeignKey("BulkPushMessage", null=True, blank=True, on_delete=models.PROTECT) + def get_gotify_extras(self): + """Merge click_url into extras using Gotify's click-action convention, + without disturbing any other extras keys the caller already set.""" + extras = dict(self.extras or {}) + if self.click_url: + notification_extra = dict(extras.get(self.GOTIFY_CLICK_EXTRA_KEY) or {}) + notification_extra["click"] = {"url": self.click_url} + extras[self.GOTIFY_CLICK_EXTRA_KEY] = notification_extra + return extras + def send_push(self): from apps.push_notifications.tasks import send_push_notification send_push_notification.delay(str(self.uuid)) @@ -98,7 +114,8 @@ class BulkPushMessage(BaseModel): title=data[user_uuid].get('title'), message=data[user_uuid].get('message'), priority=data[user_uuid].get('priority', 5), - extras=data[user_uuid].get("extras", {})) + extras=data[user_uuid].get("extras", {}), + click_url=data[user_uuid].get("click_url") or None) push_message.send_push() self.success_count += 1 @@ -125,14 +142,16 @@ class BulkPushMessage(BaseModel): priority = int(float(str(sh.cell(rowx=row, colx=3).value).strip())) extras_str = sh.cell(rowx=row, colx=4).value.strip() if extras_str: - extras = json.loads(extras) + extras = json.loads(extras_str) else: extras = {} + click_url = str(sh.cell(rowx=row, colx=5).value).strip() if sh.ncols > 5 else "" push_dict[user_uuid] = { "title" : title, "message" : message, "priority" : priority, "extras" : extras, + "click_url": click_url or None, } return push_dict diff --git a/apps/push_notifications/serializers.py b/apps/push_notifications/serializers.py index f266f55..5601759 100644 --- a/apps/push_notifications/serializers.py +++ b/apps/push_notifications/serializers.py @@ -21,6 +21,7 @@ class PushMessageSerializer(serializers.ModelSerializer): "title", "message", "priority", - "extras" + "extras", + "click_url" ) read_only_fields = ("push_user","application") diff --git a/apps/push_notifications/tasks.py b/apps/push_notifications/tasks.py index 05b13e3..9577403 100644 --- a/apps/push_notifications/tasks.py +++ b/apps/push_notifications/tasks.py @@ -10,7 +10,7 @@ def send_push_notification(push_message_uuid): title=push_message.title, message=push_message.message, priority=push_message.priority, - extras=push_message.extras) + extras=push_message.get_gotify_extras()) push_message.state = push_message.StateChoices.DONE push_message.save() diff --git a/docs/notification-click-actions.md b/docs/notification-click-actions.md new file mode 100644 index 0000000..86fc0aa --- /dev/null +++ b/docs/notification-click-actions.md @@ -0,0 +1,236 @@ +# Notification click actions + +How a tapped push notification carries a destination, why that job splits across +several repos, and what still needs building outside this service. + +## Your question, answered: frontend or backend? + +Both, split cleanly: + +- **Choosing the destination is a backend job.** A producer service (chat, + promotions, advertising, ...) decides which screen a tap should open and + hands that URL to this notifications service. That's what's implemented + below. +- **Acting on the tap is unavoidably client-side.** Only the process that + owns the tap event and the navigation stack — the Gooyal mobile/web app — + can respond to it. No backend service can intercept a notification tap; + Gotify's job ends the moment the message is delivered. + +Not every notification is clickable, and `click_url` is a **non-required** +field end to end — see [Optionality](#optionality-not-every-notification-is-clickable). + +## 1. How push flows across Gooyal today + +Three other services vendor a generated client for this one +(`gooyal_notifications_client`) and call into it over OAuth2 +client-credentials. Gotify then holds the message until the user's device +fetches it. + +``` +chat / promotions / advertising + │ POST /push/application//application/ + ▼ +notifications service (this repo) + │ create_message, extras: client::notification.click.url + ▼ +Gotify 2.6.3 + │ push delivery + ▼ +Gooyal client app (repo not found in this workspace) + │ tap → must read extras and navigate + ▼ +? destination screen +``` + +### What each producer sends today + +| Service | Call site | Extras sent | Status | +|---|---|---|---| +| **chat** | `apps/chat/events/publishers/push.py:40` | Bespoke keys: `{"conversation_uuid", "post_id"}` — not Gotify's own convention | live, fires on every chat message | +| **promotions** | `apps/promotions/models.py:544` | `extras={}` | wired, unused | +| **advertising** | `apps/users/models.py:91` | — | call commented out entirely | + +None of the three passed a click URL before this change. Chat's bespoke +`conversation_uuid`/`post_id` scheme implies some client already parses +custom keys per feature — every new use case would otherwise need its own +key and its own client-side handler. Standardizing on Gotify's own +`client::notification.click.url` convention means the client only needs one +tap handler instead of one per feature. + +## 2. What changed in this repo (`notifications`) + +Branch: `feature/notification-actions`. + +Added a `click_url` field to `PushMessage` and one merge method that folds +it into Gotify's reserved `client::notification` extras namespace — the key +official Gotify clients already read on tap +([gotify.net/docs/pushmsg#extras](https://gotify.net/docs/pushmsg#extras)). +Both message-creation paths funnel through this one method, so there is +exactly one place that builds the payload Gotify receives. + +`apps/push_notifications/models.py`: + +```python +# reserved extras namespace — official clients open this url on tap +GOTIFY_CLICK_EXTRA_KEY = "client::notification" + +click_url = models.URLField(max_length=1000, null=True, blank=True) + +def get_gotify_extras(self): + extras = dict(self.extras or {}) + if self.click_url: + notification_extra = dict(extras.get(self.GOTIFY_CLICK_EXTRA_KEY) or {}) + notification_extra["click"] = {"url": self.click_url} + extras[self.GOTIFY_CLICK_EXTRA_KEY] = notification_extra + return extras +``` + +Any existing keys a producer already sends — chat's `conversation_uuid`, +for instance — pass through untouched. `click_url` is additive, never a +replacement. + +### Optionality: not every notification is clickable + +`click_url` is `null=True, blank=True` on the model, which makes it +`required=False` / `allow_null=True` / `allow_blank=True` on the DRF +serializer automatically — confirmed directly: + +```python +>>> from apps.push_notifications.serializers import PushMessageSerializer +>>> PushMessageSerializer().get_fields()['click_url'].required +False +>>> PushMessageSerializer().get_fields()['click_url'].allow_null +True +``` + +When it's left unset, `get_gotify_extras()` returns `self.extras` completely +untouched — no `client::notification` key is added, so non-clickable +notifications are delivered exactly as before. + +### Both message-creation paths were wired through it + +| Path | Entry point | Change | +|---|---|---| +| **Single push** — used by chat / promotions / advertising | `POST /push/application//application/` | Added `click_url` to `PushMessageSerializer` — optional, alongside `title`/`message`/`extras` | +| **Bulk push** — admin-triggered Excel import | `BulkPushMessage.extract_data()` | Reads an optional 6th spreadsheet column as `click_url`; threaded through `push_to_all()` | + +Bulk spreadsheet columns: + +| Col | 0 | 1 | 2 | 3 | 4 | 5 (new) | +|---|---|---|---|---|---|---| +| Field | user_uuid | title | message | priority | extras (JSON) | click_url | + +While extending `extract_data()` for the new column, fixed a pre-existing +bug on the same line: the extras cell was parsed with `json.loads(extras)` +— a name that was never assigned — instead of `extras_str`. Any bulk row +with a populated extras cell would have raised `NameError` before reaching +Gotify at all. + +### Request/response example + +Request: + +```json +{ + "title": "Order shipped", + "message": "Tap to view your order", + "priority": 5, + "extras": {}, + "click_url": "https://gooyal.ir/orders/123" +} +``` + +Echoed back by a live local Gotify 2.6.3 container (sent alongside a +simulated chat-style `conversation_uuid` extra, to prove coexistence): + +```json +{ + "title": "Order shipped", + "message": "Tap to view your order", + "extras": { + "conversation_uuid": "abc", + "client::notification": { + "click": { "url": "https://gooyal.ir/orders/123" } + } + } +} +``` + +### Migration + +`apps/push_notifications/migrations/0004_pushmessage_click_url.py` — adds +`click_url` to `pushmessage`. Applied cleanly against the local Postgres +container. + +### Verified + +| Check | Result | +|---|---| +| Migration applied to live local Postgres | ✅ applied clean | +| `get_gotify_extras()`: click_url only | ✅ merges correctly | +| `get_gotify_extras()`: click_url + pre-existing custom extras | ✅ both keys coexist | +| `get_gotify_extras()`: no click_url set | ✅ extras left untouched | +| Live round trip against local Gotify 2.6.3 container | ✅ accepted & echoed back correctly | +| Bulk path: mocked spreadsheet rows with/without a click_url cell | ✅ parses correctly, incl. the extras_str fix | +| Serializer optionality (`required=False`, `allow_null=True`, `allow_blank=True`) | ✅ confirmed | + +## 3. Producer services updated to pass `click_url` through + +Each of the three services that call into this notifications service vendors +its own thin wrapper around the generated client. Added an optional +`click_url=None` parameter to each wrapper so producers can opt in without +being forced to — no existing call site was changed, so this is fully +backward compatible. + +Workflow used in every repo: `git checkout master` → `git pull origin +master` → `git checkout -b feature/notification-client-update` → edit. + +| Service | Branch | File changed | Function | Tests | +|---|---|---|---|---| +| **chat** | `feature/notification-client-update` | `utils/clients/notifications_client.py` | `push_user()` / `_PushBody` | 47/47 passed (`pytest`) | +| **promotions** | `feature/notification-client-update` | `utils/clients/notifications_client.py` | `notifications_push_user()` | no existing test suite for this file | +| **advertising** | `feature/notification-client-update` | `utils/clients/notifications_client.py` | `notifications_push_user()` | `apps.campaigns`/`apps.stores` suites: same failures with and without the change (pre-existing, tied to a real staging OAuth call returning `invalid_scope` — unrelated to this edit) | + +None of these commits were made — changes are sitting on each repo's +`feature/notification-client-update` branch, uncommitted, pending your +review. + +**Not done, and out of scope for this pass:** wiring an actual `click_url` +value into chat's message-push call site, promotions' or advertising's push +calls. Which notifications should be clickable, and where they should +navigate, is a product decision for each feature — this pass only makes the +plumbing available. + +## 4. Chat service: notification implementation review + +- **Location**: `apps/chat/events/publishers/push.py`, `PushPublisher.publish()`. + Pushes every recipient in a conversation (except the sender) on + `MessageSentEvent`, with `extras={"conversation_uuid": ..., "post_id": ...}`. + Per-recipient failures are caught and logged, not raised — one broken + recipient doesn't block the rest. +- **Is it in master?** Yes. Commit `0d262c6` ("Send push notifications on new + chat messages") is on `master`, and local `master` matches + `origin/master` exactly (`9f8b723f...`). Nothing related is stuck on an + unmerged feature branch — `feature/notification` is a stale, unrelated + branch (conversation-details work), not the source of this feature. +- **Does it work?** `python -m pytest apps/chat/tests/test_push_publisher.py` + → **4/4 passed**. Full suite (`pytest`) → **47/47 passed**. + `python manage.py check` → no issues. +- **One local-only gap found**: `NOTIFICATIONS_BASE_PUBLIC_URL` is not set in + the local dev `.env` (it defaults to `None` in `main/settings.py:277`), + even though `.env.example`/`env.sample` both document it + (`https://notifications-staging.gooyal.ir`). Pushes would fail locally + until that's added — this looks like an omission in the local `.env`, not + a code issue. Left as-is since it's your local working file. + +## 5. What's left, outside this repo + +1. **Producer call sites don't pass `click_url` yet.** The plumbing exists + (§3); deciding which notifications should be clickable and what URL each + should carry is a per-feature product decision. +2. **The receiving client needs a tap handler.** Checked every sibling + Gooyal repo in this workspace (`reservation-front`, `winofy-backend`, + `crm_backend`, etc.) — none of them handle Gotify push display or taps, + so the actual mobile/web app isn't in this workspace. Can't confirm + whether it already has a handler for chat's `conversation_uuid`/`post_id` + keys that could be extended, or has none at all.