From 597787a6c93c067dc66ef608d05121a569fcaf96 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 23:29:19 +0000 Subject: [PATCH] fix: address all PR review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - HTTPClient: replace isRefreshing bool with shared Task to safely coalesce concurrent 401 refresh attempts; surface JSON serialization error instead of silently dropping request body - AuthService.logout: always clear Keychain tokens via defer, even when refresh token is absent, preventing stale access token - NotificationsAPIService: remove updateAppBadge (UIKit call moved to @MainActor NotificationsViewModel); drop unused UIKit import - NotificationsViewModel: guard startPolling() against duplicate tasks; update badge directly on @MainActor instead of hopping to actor - VerifyEmailView: replace Timer (never invalidated) with async Task cancelled in .onDisappear - NotificationsView: use Text(date, style: .relative) — auto-updates without custom formatter; remove duplicate Date extension - SettingsView: handle logoutAll errors explicitly with alert instead of silently proceeding with local logout - MaydayLiveActivity/Info.plist: add NSExtensionPrincipalClass so the widget extension is discoverable by the system - Live Activity widget: replace frozen duration(from:) with Text(date, style: .timer); replace frozen relativeFormatted with Text(date, style: .relative); localize status badge to Russian Co-authored-by: robonen <26167508+robonen@users.noreply.github.com> --- Mayday/Services/AuthService.swift | 4 +- Mayday/Services/HTTPClient.swift | 54 ++++++++++++++----- Mayday/Services/NotificationsAPIService.swift | 5 -- .../ViewModels/NotificationsViewModel.swift | 9 ++-- Mayday/Views/Auth/VerifyEmailView.swift | 22 ++++---- .../Notifications/NotificationsView.swift | 14 +---- Mayday/Views/Settings/SettingsView.swift | 20 ++++++- MaydayLiveActivity/Info.plist | 2 + .../MaydayLiveActivityLiveActivity.swift | 39 ++++---------- 9 files changed, 91 insertions(+), 78 deletions(-) diff --git a/Mayday/Services/AuthService.swift b/Mayday/Services/AuthService.swift index d0e07aa..4827108 100644 --- a/Mayday/Services/AuthService.swift +++ b/Mayday/Services/AuthService.swift @@ -52,9 +52,11 @@ actor AuthService { } func logout() async throws { + // Always clear local tokens, regardless of whether the network call succeeds, + // to avoid leaving a stale access token in Keychain. + defer { keychain.clearTokens() } guard let refreshToken = keychain.loadRefreshToken() else { return } let _: EmptyResponse = try await client.request(.logout(refreshToken: refreshToken)) - keychain.clearTokens() } func getMe() async throws -> UserResponse { diff --git a/Mayday/Services/HTTPClient.swift b/Mayday/Services/HTTPClient.swift index 54b1bb2..d31d70f 100644 --- a/Mayday/Services/HTTPClient.swift +++ b/Mayday/Services/HTTPClient.swift @@ -122,7 +122,8 @@ actor HTTPClient { private let baseURL: String private let keychain = KeychainService.shared - private var isRefreshing = false + // Single in-flight refresh task; concurrent 401s await this rather than racing. + private var refreshTask: Task? private init() { #if DEBUG @@ -133,8 +134,7 @@ actor HTTPClient { } func request(_ endpoint: Endpoint) async throws -> T { - let response: T = try await performRequest(endpoint, retryOnUnauthorized: endpoint.requiresAuth) - return response + try await performRequest(endpoint, retryOnUnauthorized: endpoint.requiresAuth) } private func performRequest(_ endpoint: Endpoint, retryOnUnauthorized: Bool) async throws -> T { @@ -160,7 +160,11 @@ actor HTTPClient { urlRequest.url = components.url } } else { - urlRequest.httpBody = try? JSONSerialization.data(withJSONObject: body) + do { + urlRequest.httpBody = try JSONSerialization.data(withJSONObject: body) + } catch { + throw APIError.networkError(error) + } } } @@ -175,10 +179,12 @@ actor HTTPClient { throw APIError.networkError(URLError(.badServerResponse)) } - if httpResponse.statusCode == 401 && retryOnUnauthorized && !isRefreshing { - isRefreshing = true - defer { isRefreshing = false } - try await refreshTokens() + if httpResponse.statusCode == 401 && retryOnUnauthorized { + do { + try await ensureTokenRefreshed() + } catch { + throw APIError.unauthorized + } return try await performRequest(endpoint, retryOnUnauthorized: false) } @@ -210,15 +216,35 @@ actor HTTPClient { } } - private func refreshTokens() async throws { + /// Ensures tokens are refreshed exactly once even when multiple requests receive 401 + /// concurrently. All callers await the same Task; only one network request is made. + private func ensureTokenRefreshed() async throws { + if let existing = refreshTask { + try await existing.value + return + } + guard let refreshToken = keychain.loadRefreshToken() else { + keychain.clearTokens() throw APIError.unauthorized } - let response: TokenRefreshResponse = try await performRequest( - .refresh(refreshToken: refreshToken), - retryOnUnauthorized: false - ) - try keychain.saveTokens(response.tokens) + + let task = Task { + let response: TokenRefreshResponse = try await self.performRequest( + .refresh(refreshToken: refreshToken), + retryOnUnauthorized: false + ) + try self.keychain.saveTokens(response.tokens) + } + refreshTask = task + do { + try await task.value + refreshTask = nil + } catch { + refreshTask = nil + keychain.clearTokens() + throw error + } } } diff --git a/Mayday/Services/NotificationsAPIService.swift b/Mayday/Services/NotificationsAPIService.swift index 2061310..26d16b6 100644 --- a/Mayday/Services/NotificationsAPIService.swift +++ b/Mayday/Services/NotificationsAPIService.swift @@ -1,5 +1,4 @@ import Foundation -import UIKit actor NotificationsAPIService { static let shared = NotificationsAPIService() @@ -31,10 +30,6 @@ actor NotificationsAPIService { func changePassword(current: String, new: String) async throws -> UserResponse { try await client.request(.changePassword(current: current, new: new)) } - - func updateAppBadge(_ count: Int) async { - await UIApplication.shared.setApplicationIconBadgeNumber(count) - } } struct LogoutAllResponse: Decodable { diff --git a/Mayday/ViewModels/NotificationsViewModel.swift b/Mayday/ViewModels/NotificationsViewModel.swift index 58e0700..26db44d 100644 --- a/Mayday/ViewModels/NotificationsViewModel.swift +++ b/Mayday/ViewModels/NotificationsViewModel.swift @@ -1,5 +1,6 @@ import Foundation import SwiftUI +import UIKit @MainActor class NotificationsViewModel: ObservableObject { @@ -70,8 +71,8 @@ class NotificationsViewModel: ObservableObject { } func startPolling() { - // Polling always reloads page 1 to pick up new notifications. - // Users who have scrolled to older pages will have the list reset on each interval. + // Guard against starting a second polling loop if already running. + guard pollingTask == nil else { return } pollingTask = Task { while !Task.isCancelled { try? await Task.sleep(for: .seconds(30)) @@ -88,8 +89,6 @@ class NotificationsViewModel: ObservableObject { private func updateBadge() { let unreadCount = notifications.filter { !$0.isRead }.count - Task { - await service.updateAppBadge(unreadCount) - } + UIApplication.shared.applicationIconBadgeNumber = unreadCount } } diff --git a/Mayday/Views/Auth/VerifyEmailView.swift b/Mayday/Views/Auth/VerifyEmailView.swift index 06bd837..9056925 100644 --- a/Mayday/Views/Auth/VerifyEmailView.swift +++ b/Mayday/Views/Auth/VerifyEmailView.swift @@ -7,7 +7,7 @@ struct VerifyEmailView: View { @State private var codeDigits: [String] = Array(repeating: "", count: 6) @State private var resendCooldown = 0 @FocusState private var focusedIndex: Int? - @State private var resendTimer: Timer? + @State private var cooldownTask: Task? var body: some View { VStack(spacing: 32) { @@ -64,6 +64,7 @@ struct VerifyEmailView: View { .navigationTitle("Подтверждение") .navigationBarTitleDisplayMode(.inline) .onAppear { focusedIndex = 0 } + .onDisappear { cooldownTask?.cancel() } } private func handleDigitChange(index: Int, value: String) { @@ -99,21 +100,22 @@ struct VerifyEmailView: View { private func resendCode() async { do { try await AuthService.shared.resendCode(email: email) - resendCooldown = 60 - startCooldownTimer() + startCooldown() } catch { authViewModel.error = error.localizedDescription } } - private func startCooldownTimer() { - resendTimer?.invalidate() - resendTimer = Timer.scheduledTimer(withTimeInterval: 1, repeats: true) { _ in - if resendCooldown > 0 { - resendCooldown -= 1 - } else { - resendTimer?.invalidate() + private func startCooldown() { + cooldownTask?.cancel() + cooldownTask = Task { + for remaining in stride(from: 60, through: 1, by: -1) { + guard !Task.isCancelled else { return } + resendCooldown = remaining + try? await Task.sleep(for: .seconds(1)) } + guard !Task.isCancelled else { return } + resendCooldown = 0 } } } diff --git a/Mayday/Views/Notifications/NotificationsView.swift b/Mayday/Views/Notifications/NotificationsView.swift index 79b194e..929e3b7 100644 --- a/Mayday/Views/Notifications/NotificationsView.swift +++ b/Mayday/Views/Notifications/NotificationsView.swift @@ -104,7 +104,7 @@ struct NotificationRowView: View { Text(notification.subject) .font(.body) .fontWeight(notification.isRead ? .regular : .semibold) - Text(notification.createdAt.relativeFormatted) + Text(notification.createdAt, style: .relative) .font(.caption) .foregroundStyle(.secondary) } @@ -114,15 +114,3 @@ struct NotificationRowView: View { .padding(.vertical, 4) } } - -extension Date { - var relativeFormatted: String { - Date.relativeDateTimeFormatter.localizedString(for: self, relativeTo: Date()) - } - - private static let relativeDateTimeFormatter: RelativeDateTimeFormatter = { - let formatter = RelativeDateTimeFormatter() - formatter.locale = Locale(identifier: "ru_RU") - return formatter - }() -} diff --git a/Mayday/Views/Settings/SettingsView.swift b/Mayday/Views/Settings/SettingsView.swift index b1a1365..eec09cc 100644 --- a/Mayday/Views/Settings/SettingsView.swift +++ b/Mayday/Views/Settings/SettingsView.swift @@ -7,6 +7,7 @@ struct SettingsView: View { @State private var showChangePassword = false @State private var showSessions = false @State private var showLogoutAllConfirm = false + @State private var logoutAllError: String? var body: some View { NavigationStack { @@ -81,12 +82,27 @@ struct SettingsView: View { ) { Button("Выйти везде", role: .destructive) { Task { - _ = try? await NotificationsAPIService.shared.logoutAll() - await authViewModel.logout() + do { + _ = try await NotificationsAPIService.shared.logoutAll() + await authViewModel.logout() + } catch { + logoutAllError = error.localizedDescription + } } } Button("Отмена", role: .cancel) {} } + .alert( + "Ошибка", + isPresented: Binding( + get: { logoutAllError != nil }, + set: { if !$0 { logoutAllError = nil } } + ) + ) { + Button("OK") { logoutAllError = nil } + } message: { + Text(logoutAllError ?? "") + } .task { await viewModel.loadSessions() } diff --git a/MaydayLiveActivity/Info.plist b/MaydayLiveActivity/Info.plist index 806c31b..a2ac9bc 100644 --- a/MaydayLiveActivity/Info.plist +++ b/MaydayLiveActivity/Info.plist @@ -24,6 +24,8 @@ NSExtensionPointIdentifier com.apple.widgetkit-extension + NSExtensionPrincipalClass + $(PRODUCT_MODULE_NAME).MaydayLiveActivityBundle diff --git a/MaydayLiveActivity/MaydayLiveActivityLiveActivity.swift b/MaydayLiveActivity/MaydayLiveActivityLiveActivity.swift index fc614a2..df3808c 100644 --- a/MaydayLiveActivity/MaydayLiveActivityLiveActivity.swift +++ b/MaydayLiveActivity/MaydayLiveActivityLiveActivity.swift @@ -35,9 +35,13 @@ struct MaydayLiveActivityLiveActivity: Widget { Spacer() VStack(alignment: .trailing, spacing: 2) { statusBadge(context.state.status) - Text("Длит.: \(duration(from: context.state.startedAt))") - .font(.caption2) - .foregroundStyle(.secondary) + // Text(date, style: .timer) updates automatically without re-render. + HStack(spacing: 2) { + Text("Длит.:") + Text(context.state.startedAt, style: .timer) + } + .font(.caption2) + .foregroundStyle(.secondary) } } .padding(.horizontal) @@ -77,7 +81,8 @@ struct MaydayLiveActivityLiveActivity: Widget { .font(.caption) .foregroundStyle(severityColor(context.attributes.severity)) } - Text(context.state.startedAt.relativeFormatted) + // Text(date, style: .relative) updates automatically without re-render. + Text(context.state.startedAt, style: .relative) .font(.caption2) .foregroundStyle(.secondary) } @@ -92,8 +97,8 @@ struct MaydayLiveActivityLiveActivity: Widget { @ViewBuilder func statusBadge(_ status: AlertStatus) -> some View { let (text, color): (String, Color) = status == .active - ? ("active", .red) - : ("resolved", .green) + ? ("активен", .red) + : ("завершён", .green) Text(text) .font(.caption2.bold()) .padding(.horizontal, 6) @@ -110,26 +115,4 @@ struct MaydayLiveActivityLiveActivity: Widget { case .info: return .blue } } - - func duration(from startDate: Date) -> String { - let interval = Date().timeIntervalSince(startDate) - let minutes = Int(interval / 60) - let hours = minutes / 60 - if hours > 0 { - return "\(hours)ч \(minutes % 60)м" - } - return "\(minutes)м" - } -} - -extension Date { - var relativeFormatted: String { - Date.relativeDateTimeFormatter.localizedString(for: self, relativeTo: Date()) - } - - private static let relativeDateTimeFormatter: RelativeDateTimeFormatter = { - let formatter = RelativeDateTimeFormatter() - formatter.locale = Locale(identifier: "ru_RU") - return formatter - }() }