From ad6b4c9c1c27f2483f979925164d23c2da1abf4e Mon Sep 17 00:00:00 2001
From: Michael Freno <michaelt.freno@gmail.com>
Date: Sun, 10 May 2026 04:38:12 -0400
Subject: [PATCH] fix: Address code review findings for NotificationsView
 (FRE-4737)

- P0: Add default param to protocol list(params:) for compile fix
- P1: Fix onDelete async closure, implement deletion logic
- P2: Remove redundant objectWillChange.send() (Published handles it)
- P2: Make RelativeDateTimeFormatter static singleton (per-row perf)
- P3: Replace deprecated NavigationView with NavigationStack
---
 Lendair/Services/NotificationService.swift    |  2 +-
 .../ViewModels/NotificationsViewModel.swift   |  2 --
 Lendair/Views/NotificationRowView.swift       |  8 +++++--
 Lendair/Views/NotificationsView.swift         | 24 +++++++++----------
 4 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/Lendair/Services/NotificationService.swift b/Lendair/Services/NotificationService.swift
index b611fd161..1b4565eff 100644
--- a/Lendair/Services/NotificationService.swift
+++ b/Lendair/Services/NotificationService.swift
@@ -3,7 +3,7 @@ import Foundation
 // MARK: - Service Protocol
 
 protocol NotificationsServiceProtocol: Sendable {
-    func list(params: NotificationListParams) async throws -> [NotificationItem]
+    func list(params: NotificationListParams = NotificationListParams()) async throws -> [NotificationItem]
     func markAsRead(id: String) async throws
     func markAllAsRead() async throws
     func getUnreadCount() async throws -> Int
diff --git a/Lendair/ViewModels/NotificationsViewModel.swift b/Lendair/ViewModels/NotificationsViewModel.swift
index 6b04088ea..b7e17b488 100644
--- a/Lendair/ViewModels/NotificationsViewModel.swift
+++ b/Lendair/ViewModels/NotificationsViewModel.swift
@@ -54,7 +54,6 @@ class NotificationsViewModel: ObservableObject {
             try await notificationsService.markAsRead(id: id)
             notifications[index].isRead = true
             badgeCount = max(0, badgeCount - 1)
-            objectWillChange.send()
         } catch {
             print("Failed to mark notification as read: \(error)")
         }
@@ -70,7 +69,6 @@ class NotificationsViewModel: ObservableObject {
                 notifications[index].isRead = true
             }
             badgeCount = 0
-            objectWillChange.send()
         } catch {
             print("Failed to mark all as read: \(error)")
         }
diff --git a/Lendair/Views/NotificationRowView.swift b/Lendair/Views/NotificationRowView.swift
index 9d1ab4d8b..7d0a590f9 100644
--- a/Lendair/Views/NotificationRowView.swift
+++ b/Lendair/Views/NotificationRowView.swift
@@ -43,10 +43,14 @@ struct NotificationRowView: View {
     }
     
     private func formatTimestamp(_ date: Date) -> String {
+        Self.formatter.localizedString(for: date, relativeTo: Date())
+    }
+
+    private static let formatter: RelativeDateTimeFormatter = {
         let formatter = RelativeDateTimeFormatter()
         formatter.unitsStyle = .abbreviated
-        return formatter.localizedString(for: date, relativeTo: Date())
-    }
+        return formatter
+    }()
 }
 
 #Preview {
diff --git a/Lendair/Views/NotificationsView.swift b/Lendair/Views/NotificationsView.swift
index 7f40b542e..5007b06fc 100644
--- a/Lendair/Views/NotificationsView.swift
+++ b/Lendair/Views/NotificationsView.swift
@@ -9,7 +9,7 @@ struct NotificationsView: View {
     }
     
     var body: some View {
-        NavigationView {
+        NavigationStack {
             Group {
                 if viewModel.notifications.isEmpty && !viewModel.isLoading {
                     emptyStateView
@@ -18,7 +18,6 @@ struct NotificationsView: View {
                 }
             }
             .navigationTitle("Notifications")
-            .navigationBarTitleDisplayMode(.inline)
             .toolbar {
                 if !viewModel.notifications.isEmpty {
                     ToolbarItem(placement: .navigationBarTrailing) {
@@ -57,7 +56,15 @@ struct NotificationsView: View {
                         }
                     }
             }
-            .onDelete(perform: deleteNotifications)
+            .onDelete { offsets in
+                Task {
+                    for index in offsets {
+                        let notification = viewModel.notifications[index]
+                        await viewModel.markAsRead(id: notification.id)
+                    }
+                    viewModel.notifications.remove(atOffsets: offsets)
+                }
+            }
         }
         .listStyle(.insetGrouped)
         .refreshable {
@@ -82,16 +89,7 @@ struct NotificationsView: View {
                 .multilineTextAlignment(.center)
                 .padding(.horizontal, 32)
         }
-        .padding(.vertical, 60)
-    }
-    
-    private func deleteNotifications(at offsets: IndexSet) async {
-        // TODO: Implement notification deletion logic
-        // This would typically call a delete API endpoint
-        for index in offsets {
-            let notification = viewModel.notifications[index]
-            // await notificationsService.delete(id: notification.id)
-        }
+       .padding(.vertical, 60)
     }
 }