Lecture Reminder #394

shp7724 · 2025-11-22T12:23:09Z

No description provided.

- APIClientError를 protocol에서 enum으로 변경하여 타입 안정성 강화 - Error extension으로 apiClientError 편의 프로퍼티 추가 - CancellationError 감지 로직 추가로 불필요한 에러 알림 방지 - ErrorAlertHandler에서 CancellationError 자동 무시 처리

- LectureEditDetailScene 애니메이션 추가 - LectureTimeConflictHandler 에러 처리 로직 간소화 - TimeSelectionSheetViewModel 중복 메서드 제거 - 테스트 타겟에 내부 의존성 자동 추가하도록 개선

Copilot

Pull request overview

This PR implements a lecture reminder feature that allows users to receive push notifications at customizable times (10 minutes before, on time, or 10 minutes after) for lectures in their primary timetable for the current or upcoming semester.

Key Changes:

Added lecture reminder domain models, repositories, and view models
Created UI for managing lecture reminders in settings and lecture detail screens
Refactored AppDelegate to separate concerns into focused extensions for SDK initialization and notification handling
Updated notification system to support FCM data messages and background notifications
Added semester status tracking to determine current/next semester

Reviewed changes

Copilot reviewed 48 out of 48 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`InfoPlist.swift`	Added UIBackgroundModes for remote notifications and disabled Firebase method swizzling
`AppDelegate.swift`	Refactored into cleaner structure with comprehensive documentation
`AppDelegate+ThirdPartySDKs.swift`	Extracted Firebase and Facebook SDK initialization logic
`AppDelegate+Notifications.swift`	Consolidated all notification handling with detailed documentation
`LectureReminderSettingsScene.swift`	New UI for managing all lecture reminders
`LectureReminderPicker.swift`	Segmented picker component for reminder options
`LectureReminderViewModel.swift`	Manages individual lecture reminder state and API updates
`LectureEditDetailViewModel.swift`	Integrated reminder picker into lecture detail screen
`APIClientError.swift`	Refactored error handling to use enum instead of protocol
OpenAPI and localization files	Added reminder API endpoints and localized strings

SNUTT/Modules/Shared/SharedUIComponents/Sources/ErrorAlertHandler/ErrorAlertHandler.swift

SNUTT/Modules/Feature/Timetable/Sources/UI/LectureReminder/LectureReminderSettingsScene.swift

SNUTT/Modules/Feature/Timetable/Sources/UI/LectureEditDetail/LectureEditDetailScene.swift

Copilot · 2025-11-22T12:24:16Z

SNUTT/Modules/Feature/Timetable/Sources/Presentation/LectureReminderSettingsViewModel.swift

+                LectureReminderViewModel(lectureReminder: reminder, timetableID: timetableID)
+            }
+            loadState = .loaded(reminderViewModels)
+        } catch let error where error.isCancellationError {


[nitpick] The cancellation error check is duplicated in multiple places. Consider extracting this into errorAlertHandler.withAlert to avoid repeating this pattern throughout the codebase.

SNUTT/Modules/Feature/Timetable/Sources/Presentation/LectureEditDetailViewModel.swift

SNUTT/Modules/Feature/APIClientInterface/Sources/Error/APIClientError.swift

Copilot

Pull request overview

Copilot reviewed 50 out of 50 changed files in this pull request and generated 4 comments.

SNUTT/Modules/Feature/Timetable/Sources/Presentation/LectureReminderSettingsViewModel.swift

SNUTT/Modules/Feature/APIClientInterface/Sources/Error/APIClientError.swift

SNUTT/Modules/Feature/Timetable/Sources/Presentation/LectureReminderViewModel.swift

SNUTT/Modules/Feature/Settings/Sources/Presentation/SettingsViewModel.swift

Copilot

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

SNUTT/Modules/Feature/APIClientInterface/Sources/Error/APIClientError.swift:1

[nitpick] The DTO type name optionPayload is unclear. Consider requesting the API team to rename it to a more descriptive name like ReminderOptionDTO or document why this unusual naming exists.

//

SNUTT/Modules/Feature/Timetable/Sources/UI/LectureEditDetail/LectureEditDetailScene.swift

SNUTT/Modules/Feature/Timetable/Sources/Presentation/LectureReminderViewModel.swift

Co-authored-by: Copilot <[email protected]>

Copilot

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated 3 comments.

Copilot · 2025-11-26T12:01:58Z

SNUTT/Tuist/ProjectDescriptionHelpers/InfoPlist.swift

            ],
+            "UIBackgroundModes": [
+                "remote-notification"
+            ],


The FirebaseAppDelegateProxyEnabled setting should be documented with a comment explaining why method swizzling is disabled. Based on the AppDelegate+Notifications.swift documentation, this is intentionally disabled to manually call appDidReceiveMessage for analytics tracking.

Suggested change

],

],

// Method swizzling is intentionally disabled to allow manual calls to `appDidReceiveMessage`

// for analytics tracking, as documented in AppDelegate+Notifications.swift.

Copilot · 2025-11-26T12:01:58Z

SNUTT/SNUTT/Sources/AppDelegate.swift

+    /// - **Debug**: `GoogleServiceDebugInfo.plist` (dev Firebase project)
+    /// - **Release**: `GoogleServiceReleaseInfo.plist` (prod Firebase project)
+    ///
+    /// Ensures dev/prod data isolation for analytics, crash reports, and push notifications.


The visibility modifier change from private to internal should be documented. Consider adding a comment explaining why this property needs internal access (likely for testing purposes based on the documentation style).

Suggested change

/// Ensures dev/prod data isolation for analytics, crash reports, and push notifications.

/// Ensures dev/prod data isolation for analytics, crash reports, and push notifications.

///

/// - Note: This property is `internal` (not `private`) to allow access from unit tests and other files within the module.

Copilot · 2025-11-26T12:01:58Z

SNUTT/Modules/Feature/Timetable/Sources/Presentation/LectureEditDetailViewModel.swift

+        async let bookmarkTask: Void = fetchBookmarkStatus()
+        async let vacancyTask: Void = fetchVacancyNotificationStatus()
+        async let reminderTask: Void = initializeReminderViewModelIfNeeded()
+        _ = try await (bookmarkTask, vacancyTask, reminderTask)


[nitpick] The tuple result is discarded with _ =. Since these are Void-returning functions, this pattern is correct, but consider using Swift's structured concurrency with async let without the discard for clarity: simply try await (bookmarkTask, vacancyTask, reminderTask).

Suggested change

_ = try await (bookmarkTask, vacancyTask, reminderTask)

try await (bookmarkTask, vacancyTask, reminderTask)

shp7724 added 5 commits November 22, 2025 21:22

⏰ Lecture Reminder 구현

d0989cf

♻️ 코드 정리 및 빌드 설정 개선

19eae65

- LectureEditDetailScene 애니메이션 추가 - LectureTimeConflictHandler 에러 처리 로직 간소화 - TimeSelectionSheetViewModel 중복 메서드 제거 - 테스트 타겟에 내부 의존성 자동 추가하도록 개선

🔔 Notification 코드 정리

38d362e

⏰ Help Text 추가

81bb34b

Copilot AI review requested due to automatic review settings November 22, 2025 12:23

shp7724 requested a review from a team as a code owner November 22, 2025 12:23

Copilot AI reviewed Nov 22, 2025

View reviewed changes

Copilot AI review requested due to automatic review settings November 22, 2025 14:48

shp7724 force-pushed the shp7724/reminder branch from 43bc533 to cb61904 Compare November 22, 2025 14:48

Copilot AI reviewed Nov 22, 2025

View reviewed changes

🩹 Add CI timeout

b6d44a7

shp7724 force-pushed the shp7724/reminder branch from cb61904 to b6d44a7 Compare November 26, 2025 09:27

🩹 Fix test failure

5f784bd

Copilot AI review requested due to automatic review settings November 26, 2025 11:39

Copilot AI reviewed Nov 26, 2025

View reviewed changes

SNUTT/Modules/Feature/Timetable/Sources/UI/LectureEditDetail/LectureEditDetailScene.swift Outdated Show resolved Hide resolved

SNUTT/Modules/Feature/Timetable/Sources/Presentation/LectureReminderViewModel.swift Outdated Show resolved Hide resolved

Apply suggestions from code review

4bcfd09

Co-authored-by: Copilot <[email protected]>

Copilot AI review requested due to automatic review settings November 26, 2025 12:00

Copilot AI reviewed Nov 26, 2025

View reviewed changes

🩹 Increase test timeout

9afb49f

-            ],
+            ],
+            // Method swizzling is intentionally disabled to allow manual calls to `appDidReceiveMessage`
+            // for analytics tracking, as documented in AppDelegate+Notifications.swift.

	_ = try await (bookmarkTask, vacancyTask, reminderTask)
	try await (bookmarkTask, vacancyTask, reminderTask)

Lecture Reminder #394

Are you sure you want to change the base?

Lecture Reminder #394

Conversation

shp7724 commented Nov 22, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants