Skip to content

Comments

Notification settings persistence#36

Merged
fortune710 merged 2 commits intodevfrom
cursor/DEF-89-notification-settings-persistence-02a2
Jan 27, 2026
Merged

Notification settings persistence#36
fortune710 merged 2 commits intodevfrom
cursor/DEF-89-notification-settings-persistence-02a2

Conversation

@fortune710
Copy link
Owner

@fortune710 fortune710 commented Jan 27, 2026

Creates an initial notification settings record in the database based on OS permission outcome and enforces permission checks when re-enabling push notifications to fulfill DEF-89.

The issue required saving default notification preferences (all true for granted, all false for denied) upon the first permission request. It also mandated that the "Push Notifications" toggle re-check or request OS permission before allowing the user to turn it back on, ensuring consistency between app settings and OS permissions. A refinement was made to prevent overwriting existing user preferences if the settings row already existed when the push notification toggle was re-enabled.


Linear Issue: DEF-89

Open in Cursor Open in Web

Summary by CodeRabbit

  • Bug Fixes

    • Fixed push notification permission handling to properly request and track device-level permissions before enabling notifications.
  • Improvements

    • Notification settings are now automatically initialized based on device permission status.
    • Added iOS and Android permission verification for push notifications.
    • Push notifications now aligned with user profile context.

✏️ Tip: You can customize this high-level summary in your review settings.

cursoragent and others added 2 commits January 27, 2026 05:01
Co-authored-by: alebiosuf0802 <alebiosuf0802@students.bowiestate.edu>
Co-authored-by: alebiosuf0802 <alebiosuf0802@students.bowiestate.edu>
@cursor
Copy link

cursor bot commented Jan 27, 2026

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

@bolt-new-by-stackblitz
Copy link

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@vercel
Copy link

vercel bot commented Jan 27, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
keepsafe Ready Ready Preview, Comment Jan 27, 2026 5:14am

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 27, 2026

Caution

Review failed

The pull request is closed.

Walkthrough

This PR integrates OS-level push notification permission handling into the existing notification system. When enabling push notifications, the system now checks device support, requests OS permissions, and initializes notification settings based on permission outcomes. The usePushNotifications hook accepts a userId parameter to coordinate permission-based initialization across hooks and services.

Changes

Cohort / File(s) Summary
Hook Parameter Updates
frontend/app/capture/_layout.tsx
Passes profile?.id to usePushNotifications, aligning hook invocation with user context.
Push Notification Hook Enhancement
frontend/hooks/use-push-notifications.ts
Adds optional userId parameter; introduces permissionStatus state tracking; adds effect to initialize notification settings from permission when userId is available; fixes listener cleanup logic to correctly target responseListener.current?.remove().
Notification Settings Hook Integration
frontend/hooks/use-notification-settings.ts
Adds device/OS permission handling when enabling PUSH_NOTIFICATIONS: verifies device support, checks/requests OS permissions, calls new service initialization method, and persists permission outcomes before updating toggle states.
Push Notification Service Expansion
frontend/services/push-notification-service.ts
Adds ALL_NOTIFICATIONS_ON and ALL_NOTIFICATIONS_OFF constant maps; introduces saveNotificationsSettings() alias; adds initializeNotificationSettingsFromPermission(userId, permissionGranted) to create initial settings based on device-level permission status with existence checking and error handling.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant CaptureLayout
    participant usePushNotifications
    participant Device/OS
    participant PushNotificationService
    participant SettingsStorage

    User->>CaptureLayout: Load app
    CaptureLayout->>usePushNotifications: Call with profile?.id
    usePushNotifications->>Device/OS: Request notification permission
    Device/OS-->>usePushNotifications: Permission status (granted/denied)
    usePushNotifications->>usePushNotifications: Update permissionStatus state
    
    alt userId available and permission checked
        usePushNotifications->>PushNotificationService: initializeNotificationSettingsFromPermission(userId, isGranted)
        PushNotificationService->>SettingsStorage: Check existing settings
        
        alt Settings don't exist
            alt Permission granted
                PushNotificationService->>SettingsStorage: Save ALL_NOTIFICATIONS_ON
            else Permission denied
                PushNotificationService->>SettingsStorage: Save ALL_NOTIFICATIONS_OFF
            end
            PushNotificationService-->>usePushNotifications: true (first-time init)
        else Settings exist
            PushNotificationService-->>usePushNotifications: false (already initialized)
        end
    end
    
    usePushNotifications-->>CaptureLayout: Push notifications configured
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 A hop towards permissions so fine,
Device checks and OS calls align,
Settings bloom when granted they be,
Push notifications wild and free! 🔔✨

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@fortune710 fortune710 marked this pull request as ready for review January 27, 2026 05:17
@fortune710 fortune710 merged commit 2aac3d9 into dev Jan 27, 2026
2 of 3 checks passed
@fortune710 fortune710 deleted the cursor/DEF-89-notification-settings-persistence-02a2 branch February 5, 2026 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants