# Phase D — Polish

*Estimated: 2 days · Depends on: Phase A + C merged (Phase B nice-to-have but not blocking)*

---

## Goal

Harden the notification system for production: per-user preferences enforcement, quiet hours, rate limiting, notification grouping, badge accuracy, and data retention. These are quality-of-life improvements that prevent notification fatigue and keep the system clean at scale.

---

## Scope

1. Per-user notification preferences enforcement audit (verify all paths use dispatcher)
2. Quiet hours — server-side suppression window
3. Rate limiting — per-type hourly cap
4. Notification grouping — iOS thread-id, Android tag
5. Badge sync correctness across foreground/background/kill states
6. Retention policy — auto-delete old read notifications

---

## Affected Components

### Backend — New/Modified Files

| File | Change |
|---|---|
| `services/main/app/services/notification_dispatcher.py` | Add quiet hours gate, rate limiter |
| `services/main/app/repositories/notification_repository.py` | Rate limit counter queries, retention cleanup |
| `services/main/app/models/notifications.py` | Quiet hours fields on preferences model |
| `supabase/migrations/20260512000000_notification_polish.sql` | Schema changes for quiet hours, rate limit counters, retention |
| `services/main/app/services/notification_retention.py` (new) | Cron job for retention cleanup |

### Mobile — Modified Files

| File | Change |
|---|---|
| `src/features/notification/presentation/screens/NotificationSettingsScreen.tsx` | Quiet hours time picker UI |
| `src/features/notification/presentation/hooks/useNotificationSettingsViewModel.ts` | Quiet hours state management |
| `src/features/notification/data/dto/notificationPreferencesDto.ts` | Add quiet hours fields |
| `src/features/notification/data/mappers/notificationPreferencesMapper.ts` | Map quiet hours fields |
| `src/features/notification/business/models/NotificationPreferences.ts` | Add quiet hours model fields |
| `src/core/providers/NotificationHandler.tsx` (from Phase A) | Badge sync improvements |

---

## Concrete Tasks (commit-sized)

### Task 1: Quiet Hours — Backend (~0.5 day)

Add quiet hours to the notification preferences schema:

```sql
-- Migration: add quiet hours columns to notification_preferences
ALTER TABLE notification_preferences
  ADD COLUMN quiet_hours_enabled BOOLEAN NOT NULL DEFAULT false,
  ADD COLUMN quiet_hours_start TIME NOT NULL DEFAULT '22:00',
  ADD COLUMN quiet_hours_end TIME NOT NULL DEFAULT '08:00',
  ADD COLUMN timezone TEXT NOT NULL DEFAULT 'America/New_York';
```

Update `NotificationDispatcher.dispatch()`:

```python
async def _is_in_quiet_hours(self, user_id: str) -> bool:
    prefs = await notification_repository.get_preferences(user_id)
    if not prefs.get("quiet_hours_enabled", False):
        return False
    tz = ZoneInfo(prefs.get("timezone", "America/New_York"))
    now_local = datetime.now(tz).time()
    start = time.fromisoformat(prefs["quiet_hours_start"])
    end = time.fromisoformat(prefs["quiet_hours_end"])
    # Handle overnight ranges (e.g. 22:00 → 08:00)
    if start <= end:
        return start <= now_local <= end
    return now_local >= start or now_local <= end
```

During quiet hours:
- **In-app notification:** Still created (stored in `notifications` table, appears in feed)
- **Push notification:** Suppressed (not sent to APNS/FCM)

This ensures users don't miss notifications in their feed but aren't woken by pushes at night.

**Tests:**
- Verify quiet hours gate blocks push but allows in-app storage
- Verify overnight range (22:00–08:00) works across midnight
- Verify timezone conversion is correct

### Task 2: Quiet Hours — Mobile UI (~0.5 day)

Add to NotificationSettingsScreen below the individual toggles:

```
── Quiet Hours ──────────────────────
[Toggle] Do Not Disturb
  Start: [22:00]  End: [08:00]
  Timezone: America/New_York
```

- Time pickers: use `@react-native-community/datetimepicker` (already in deps check needed) or a custom wheel picker
- Timezone: auto-detect from device via `expo-localization` (`Localization.getCalendars()[0].timeZone`), allow manual override
- Sends `PUT /api/notifications/preferences` with `quiet_hours_enabled`, `quiet_hours_start`, `quiet_hours_end`, `timezone`

### Task 3: Rate Limiting (~0.5 day)

**Goal:** Prevent notification spam — max 5 pushes per hour per notification type per user.

**Implementation:** Lightweight counter in the `notifications` table itself (no new table):

```python
async def _check_rate_limit(self, user_id: str, notification_type: str) -> bool:
    """Returns True if under rate limit."""
    one_hour_ago = datetime.now(UTC) - timedelta(hours=1)
    client = supabase.get_service_client()
    result = (
        await client.table("notifications")
        .select("id", count="exact")
        .eq("user_id", user_id)
        .eq("type", notification_type)
        .gte("created_at", one_hour_ago.isoformat())
        .execute()
    )
    count = result.count or 0
    return count < 5  # max 5 per type per hour
```

When rate limit is hit:
- **In-app notification:** Still created (user can see it in feed later)
- **Push notification:** Suppressed
- Log: `rate_limit_hit` with user_ref and type for monitoring

**Configurable limits:**

| Type | Max per hour | Rationale |
|---|---|---|
| `price_alert` | 5 | Multiple alerts can fire during volatile sessions |
| `earnings_alert` | 2 | At most 1 per stock per day, unlikely to hit |
| `volume_alert` | 5 | Same as price |
| `portfolio_summary` | 1 | Cron sends once/day |
| `learning_reminder` | 2 | At most 2 per day |
| `competition_update` | 3 | Events are infrequent |
| `product_update` | 2 | Admin-controlled |

Store limits as a config dict, not hardcoded per-type.

**Tests:**
- Create 5 notifications of same type within 1 hour → 6th is suppressed (no push)
- Different types have independent counters
- Counter resets after 1 hour

### Task 4: Notification Grouping (~0.5 day)

**iOS — Thread ID:**

Add `thread-id` to APNS payload so related notifications are grouped in Notification Center:

```python
# In APNSProvider.send():
body = {
    "aps": {
        "alert": {"title": payload.title, "body": payload.body},
        "sound": "default",
        "thread-id": payload.data.get("type", "general"),  # Group by type
    },
}
```

Grouping key options:
- By type: all price alerts grouped, all portfolio summaries grouped
- By ticker: all alerts for AAPL grouped (use `f"alert:{ticker}"` as thread-id)
- Recommended: by ticker when available, by type otherwise

**Android — Tag:**

FCM's `tag` field replaces previous notifications with the same tag. Use for notifications that supersede each other (e.g., portfolio summary replaces the previous day's summary):

```python
# In FCMProvider.send():
message = {
    "message": {
        "token": token,
        "notification": {"title": payload.title, "body": payload.body},
        "android": {
            "notification": {
                "tag": payload.data.get("group_tag"),  # Optional
            }
        },
    },
}
```

**Decision matrix:**

| Type | iOS thread-id | Android tag | Behavior |
|---|---|---|---|
| `price_alert` | `alert:{ticker}` | — (don't collapse) | Group by ticker, show each |
| `earnings_alert` | `alert:{ticker}` | — | Group by ticker |
| `volume_alert` | `alert:{ticker}` | — | Group by ticker |
| `portfolio_summary` | `portfolio` | `portfolio_summary` | Group + replace previous |
| `learning_reminder` | `learning` | `learning_reminder` | Group + replace previous |
| `product_update` | `product` | — | Group, show each |

**Tests:**
- Verify APNS payload includes `thread-id`
- Verify FCM payload includes `android.notification.tag` when `group_tag` is set
- Verify Android tag causes replacement (manual E2E test)

### Task 5: Badge Sync Correctness (~0.25 day)

Ensure badge count is correct across all app states:

| State | Badge Update Mechanism |
|---|---|
| App foregrounded | Realtime subscription (Phase B) or query invalidation |
| App backgrounded | `content-available` silent push → background fetch updates badge |
| App killed | Badge set in push payload via `badge` field in APNS / `notification.notification_count` in FCM |

**APNS badge in push payload:**

```python
# Fetch current unread count before sending push
unread = await notification_repository.get_unread_count(user_id)
body = {
    "aps": {
        "alert": {"title": payload.title, "body": payload.body},
        "badge": unread + 1,  # +1 for the notification being sent now
        "sound": "default",
    },
}
```

**Mobile: Clear badge on app open:**

```typescript
// In AppProviders or root layout
useEffect(() => {
  const subscription = AppState.addEventListener('change', (state) => {
    if (state === 'active') {
      // Fetch actual unread count and sync badge
      queryClient.invalidateQueries({ queryKey: ['unreadNotificationCount'] });
    }
  });
  return () => subscription.remove();
}, []);
```

### Task 6: Retention Policy (~0.25 day)

**Goal:** Auto-delete read notifications older than 90 days to keep the table lean.

**Backend cron job:** `run_notification_retention_cleanup()`

```python
async def run_notification_retention_cleanup():
    """Runs daily at 03:00 UTC. Deletes read notifications older than 90 days."""
    while True:
        try:
            now = datetime.now(UTC)
            next_run = now.replace(hour=3, minute=0, second=0, microsecond=0)
            if next_run <= now:
                next_run += timedelta(days=1)
            await asyncio.sleep((next_run - now).total_seconds())

            cutoff = (datetime.now(UTC) - timedelta(days=90)).isoformat()
            client = supabase.get_service_client()
            result = (
                await client.table("notifications")
                .delete()
                .eq("read", True)
                .lt("created_at", cutoff)
                .execute()
            )
            deleted = len(result.data) if result.data else 0
            logger.info("retention_cleanup", extra={
                "service": "notifications",
                "deleted": deleted,
                "cutoff": cutoff,
            })
        except asyncio.CancelledError:
            raise
        except Exception:
            logger.exception("retention_cleanup_failed", extra={"service": "notifications"})
```

**Policy details:**
- Only deletes **read** notifications — unread notifications are never auto-deleted regardless of age
- 90-day window is configurable via settings
- Runs daily at 03:00 UTC (low-traffic window)
- Logged for monitoring

**Migration:** Add index to optimize cleanup query:

```sql
CREATE INDEX idx_notifications_retention
  ON notifications (read, created_at)
  WHERE read = true;
```

---

## Test Plan

1. **Quiet hours:** Set quiet hours 22:00–08:00, trigger notification at 23:00 local → push suppressed, notification in feed
2. **Rate limit:** Fire 6 price alerts in 1 hour → only 5 pushes sent, all 6 in feed
3. **Grouping (iOS):** Send 3 AAPL alerts → grouped under one thread in Notification Center
4. **Grouping (Android):** Send 2 portfolio summaries → second replaces first
5. **Badge count:** Kill app → receive 3 pushes → open app → badge shows 3 → mark all read → badge shows 0
6. **Retention:** Insert read notification dated 91 days ago → run cleanup → deleted. Insert unread notification dated 91 days ago → still present.
7. **Quiet hours UI:** Set quiet hours on mobile → verify `PUT /api/notifications/preferences` sends correct payload → verify backend respects the setting

---

## Open Questions / Dependencies

1. **Time picker component:** Does the app already have a time picker or do we need to add `@react-native-community/datetimepicker`? Check deps. If not present, consider a simpler text-input approach (dropdown with 30-min increments) to avoid a new dependency.

2. **Badge count on push payload:** Computing `unread_count + 1` before each push adds a DB query per notification. For bulk sends (portfolio summary to all users), this could be expensive. Alternative: set badge to a large number (e.g., 99) and let the app correct it on open. Less accurate but much cheaper.

3. **Retention policy — unread expiry:** Should unread notifications expire eventually? Current policy keeps them forever. Product decision: "never auto-delete unread" is safest but the table could grow. A 1-year hard limit on all notifications (read or unread) may be warranted at scale.

4. **Rate limit scope:** Per-type per-hour limits are proposed. Should there also be a global limit across all types (e.g., max 20 pushes/day total)? The original NAN-57 spec mentioned "max 20/day per user" — confirm if this is still desired.

5. **Depends on Phase A** for the dispatcher integration point. Phase C must also be merged (or at least the dispatcher pattern) for quiet hours and rate limiting to have effect across all notification types.