Merge pull request 'REFACTOR(notifications): rename code/click_object_id to action_code/click_object_id_value' (#2) from feature/notification-actions into master
Reviewed-on: #2
This commit is contained in:
commit
c919eef00f
7 changed files with 100 additions and 67 deletions
|
|
@ -14,9 +14,9 @@ admin.site.register(PushUser)
|
|||
|
||||
@admin.register(NotificationLink)
|
||||
class NotificationLinkAdmin(admin.ModelAdmin):
|
||||
list_display = ['code', 'url_template', 'is_active', 'description']
|
||||
list_display = ['action_code', 'url_template', 'is_active', 'description']
|
||||
list_filter = ['is_active']
|
||||
search_fields = ['code', 'description']
|
||||
search_fields = ['action_code', 'description']
|
||||
|
||||
|
||||
from django import forms
|
||||
|
|
|
|||
|
|
@ -0,0 +1,28 @@
|
|||
# Generated by Django 5.2.6 on 2026-08-23 08:38
|
||||
|
||||
from django.db import migrations
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('push_notifications', '0005_notificationlink_pushmessage_click_object_id_and_more'),
|
||||
]
|
||||
|
||||
operations = [
|
||||
migrations.RenameField(
|
||||
model_name='notificationlink',
|
||||
old_name='code',
|
||||
new_name='action_code',
|
||||
),
|
||||
migrations.RenameField(
|
||||
model_name='pushmessage',
|
||||
old_name='code',
|
||||
new_name='action_code',
|
||||
),
|
||||
migrations.RenameField(
|
||||
model_name='pushmessage',
|
||||
old_name='click_object_id',
|
||||
new_name='click_object_id_value',
|
||||
),
|
||||
]
|
||||
|
|
@ -46,12 +46,12 @@ class PushUser(BaseModel):
|
|||
|
||||
class NotificationLink(BaseModel):
|
||||
"""
|
||||
Admin-managed code -> URL mapping for notification click actions. Producer
|
||||
services send a `code` (documented per notification type) instead of a raw
|
||||
URL, so where a tap should navigate is a config change here, not a code
|
||||
deploy across every producer service.
|
||||
Admin-managed action_code -> URL mapping for notification click actions.
|
||||
Producer services send an `action_code` (documented per notification
|
||||
type) instead of a raw URL, so where a tap should navigate is a config
|
||||
change here, not a code deploy across every producer service.
|
||||
"""
|
||||
code = models.SlugField(max_length=100, unique=True, db_index=True)
|
||||
action_code = models.SlugField(max_length=100, unique=True, db_index=True)
|
||||
description = models.CharField(max_length=255, blank=True)
|
||||
url_template = models.CharField(
|
||||
max_length=1000,
|
||||
|
|
@ -60,7 +60,7 @@ class NotificationLink(BaseModel):
|
|||
is_active = models.BooleanField(default=True)
|
||||
|
||||
def __str__(self):
|
||||
return self.code
|
||||
return self.action_code
|
||||
|
||||
def resolve(self, object_id=None):
|
||||
if object_id and '{object_id}' in self.url_template:
|
||||
|
|
@ -89,28 +89,30 @@ class PushMessage(BaseModel):
|
|||
priority = models.IntegerField(default=5)
|
||||
extras = models.JSONField(default=dict)
|
||||
click_url = models.URLField(max_length=1000, null=True, blank=True)
|
||||
# Alternative to a raw click_url: a documented per-notification-type code,
|
||||
# resolved against NotificationLink at send time (see get_gotify_extras).
|
||||
# click_object_id is substituted into that code's {object_id} placeholder.
|
||||
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)
|
||||
# Alternative to a raw click_url: a documented per-notification-type
|
||||
# action_code, resolved against NotificationLink at send time (see
|
||||
# get_gotify_extras). click_object_id_value is substituted into that
|
||||
# action_code's {object_id} placeholder.
|
||||
action_code = models.SlugField(max_length=100, null=True, blank=True, db_index=True)
|
||||
click_object_id_value = models.CharField(max_length=255, 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 resolve_click_url(self):
|
||||
"""An explicit click_url always wins; otherwise resolve via `code`
|
||||
against NotificationLink. Returns None (not an error) if `code` isn't
|
||||
set, or isn't configured/active yet — not every notification is
|
||||
clickable, and a not-yet-configured code shouldn't block delivery."""
|
||||
"""An explicit click_url always wins; otherwise resolve via
|
||||
`action_code` against NotificationLink. Returns None (not an error)
|
||||
if `action_code` isn't set, or isn't configured/active yet — not
|
||||
every notification is clickable, and a not-yet-configured
|
||||
action_code shouldn't block delivery."""
|
||||
if self.click_url:
|
||||
return self.click_url
|
||||
if not self.code:
|
||||
if not self.action_code:
|
||||
return None
|
||||
link = NotificationLink.objects.filter(code=self.code, is_active=True).first()
|
||||
link = NotificationLink.objects.filter(action_code=self.action_code, is_active=True).first()
|
||||
if not link:
|
||||
return None
|
||||
return link.resolve(self.click_object_id)
|
||||
return link.resolve(self.click_object_id_value)
|
||||
|
||||
def get_gotify_extras(self):
|
||||
"""Merge the resolved click url into extras using Gotify's click-action
|
||||
|
|
@ -161,8 +163,8 @@ class BulkPushMessage(BaseModel):
|
|||
priority=data[user_uuid].get('priority', 5),
|
||||
extras=data[user_uuid].get("extras", {}),
|
||||
click_url=data[user_uuid].get("click_url") or None,
|
||||
code=data[user_uuid].get("code") or None,
|
||||
click_object_id=data[user_uuid].get("click_object_id") or None)
|
||||
action_code=data[user_uuid].get("action_code") or None,
|
||||
click_object_id_value=data[user_uuid].get("click_object_id_value") or None)
|
||||
push_message.send_push()
|
||||
|
||||
self.success_count += 1
|
||||
|
|
@ -193,8 +195,8 @@ class BulkPushMessage(BaseModel):
|
|||
else:
|
||||
extras = {}
|
||||
click_url = str(sh.cell(rowx=row, colx=5).value).strip() if sh.ncols > 5 else ""
|
||||
code = str(sh.cell(rowx=row, colx=6).value).strip() if sh.ncols > 6 else ""
|
||||
click_object_id = str(sh.cell(rowx=row, colx=7).value).strip() if sh.ncols > 7 else ""
|
||||
action_code = str(sh.cell(rowx=row, colx=6).value).strip() if sh.ncols > 6 else ""
|
||||
click_object_id_value = str(sh.cell(rowx=row, colx=7).value).strip() if sh.ncols > 7 else ""
|
||||
|
||||
push_dict[user_uuid] = {
|
||||
"title" : title,
|
||||
|
|
@ -202,7 +204,7 @@ class BulkPushMessage(BaseModel):
|
|||
"priority" : priority,
|
||||
"extras" : extras,
|
||||
"click_url": click_url or None,
|
||||
"code": code or None,
|
||||
"click_object_id": click_object_id or None,
|
||||
"action_code": action_code or None,
|
||||
"click_object_id_value": click_object_id_value or None,
|
||||
}
|
||||
return push_dict
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@ class PushMessageSerializer(serializers.ModelSerializer):
|
|||
"priority",
|
||||
"extras",
|
||||
"click_url",
|
||||
"code",
|
||||
"click_object_id",
|
||||
"action_code",
|
||||
"click_object_id_value",
|
||||
)
|
||||
read_only_fields = ("push_user","application")
|
||||
|
|
|
|||
|
|
@ -52,33 +52,33 @@ error.
|
|||
## URLs are admin-configured, not hard-coded — nothing for you to build here
|
||||
|
||||
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.
|
||||
notification type has an **action_code**, and the notifications service
|
||||
resolves that action_code against an admin-managed table (`NotificationLink`:
|
||||
`action_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 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.
|
||||
action_code below with your actual URL format, and every notification of
|
||||
that type immediately starts carrying it. If an action_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.
|
||||
|
||||
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.
|
||||
than what's currently configured for any action_code, ask whoever owns that
|
||||
panel to update it.
|
||||
|
||||
## What's clickable today, by code
|
||||
## What's clickable today, by action_code
|
||||
|
||||
All of these are live in the `advertising` service:
|
||||
|
||||
### Billboards & content
|
||||
|
||||
| Code | Notification text (fa) |
|
||||
| Action code | Notification text (fa) |
|
||||
|---|---|
|
||||
| `billboard.viewed` | شخصی شروع به مشاهده بیلبورد شما کرد. |
|
||||
| `billboard.created` | بیلبود شما با موفقیت ساخته شد. |
|
||||
|
|
@ -93,7 +93,7 @@ All of these are live in the `advertising` service:
|
|||
| `billboard.credit_low` | اعتبار پین رو به اتمامه! |
|
||||
| `billboard.credit_exhausted` | اعتبار پین شما به پایان رسید! |
|
||||
|
||||
Every code above sends the ad's `uuid` as `click_object_id`, so a
|
||||
Every action_code above sends the ad's `uuid` as `click_object_id_value`, 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
|
||||
|
|
@ -105,7 +105,7 @@ change, just a new admin row.
|
|||
|
||||
### Escrow deals
|
||||
|
||||
| Code | Trigger |
|
||||
| Action code | Trigger |
|
||||
|---|---|
|
||||
| `escrow.request_deal` | Buyer paid, seller needs to review |
|
||||
| `escrow.cancel_deal` | Buyer canceled before seller responded |
|
||||
|
|
@ -123,7 +123,7 @@ change, just a new admin row.
|
|||
| `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
|
||||
All fifteen send the escrow deal's `uuid` as `click_object_id_value`. 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.
|
||||
|
|
|
|||
|
|
@ -54,27 +54,29 @@ An admin action (`BulkPushMessageAdmin`, `apps/push_notifications/admin.py`) tri
|
|||
|
||||
| Col | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 |
|
||||
|---|---|---|---|---|---|---|---|---|
|
||||
| Field | `user_uuid` | `title` | `message` | `priority` | `extras` (JSON string) | `click_url` | `code` | `click_object_id` |
|
||||
| Field | `user_uuid` | `title` | `message` | `priority` | `extras` (JSON string) | `click_url` | `action_code` | `click_object_id_value` |
|
||||
|
||||
`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 action mechanism
|
||||
|
||||
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.
|
||||
`code`/`click_object_id` and the `NotificationLink` model, then renamed in
|
||||
`0006` to `action_code`/`click_object_id_value`. 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
|
||||
2. **`action_code` + `click_object_id_value`** — the preferred path.
|
||||
`action_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_value` 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
|
||||
(`action_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.
|
||||
|
|
@ -85,18 +87,18 @@ Two ways to set a destination on a `PushMessage`:
|
|||
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)
|
||||
action_code = models.SlugField(max_length=100, null=True, blank=True, db_index=True)
|
||||
click_object_id_value = 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:
|
||||
if not self.action_code:
|
||||
return None
|
||||
link = NotificationLink.objects.filter(code=self.code, is_active=True).first()
|
||||
link = NotificationLink.objects.filter(action_code=self.action_code, is_active=True).first()
|
||||
if not link:
|
||||
return None
|
||||
return link.resolve(self.click_object_id)
|
||||
return link.resolve(self.click_object_id_value)
|
||||
|
||||
def get_gotify_extras(self):
|
||||
extras = dict(self.extras or {})
|
||||
|
|
@ -109,10 +111,10 @@ def get_gotify_extras(self):
|
|||
```
|
||||
|
||||
Design decisions worth knowing if you touch this:
|
||||
- **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.
|
||||
- **An unresolvable action_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 action_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 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.
|
||||
- **`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 `action_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 action_code catalog they need to fill in.
|
||||
|
||||
## Email flow
|
||||
|
||||
|
|
@ -140,7 +142,8 @@ This service is an OAuth2 **resource server** (`django-oauth-toolkit`), not an O
|
|||
## Notable migrations
|
||||
|
||||
- `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.
|
||||
- `0005_notificationlink_pushmessage_click_object_id_and_more` — adds `NotificationLink`, `PushMessage.code`, `PushMessage.click_object_id`.
|
||||
- `0006_rename_code_notificationlink_action_code_and_more` — renames `NotificationLink.code`/`PushMessage.code` to `action_code`, and `PushMessage.click_object_id` to `click_object_id_value`. See "The click action mechanism" above.
|
||||
|
||||
## Known technical debt (implementation-level)
|
||||
|
||||
|
|
|
|||
|
|
@ -2,8 +2,8 @@
|
|||
|
||||
> **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)
|
||||
> send an `action_code` + `click_object_id_value`, 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.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue