Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import Foundation

enum AppPath: String {
enum AppPath: String, Codable {
case home
case login
case shop
Expand Down
14 changes: 14 additions & 0 deletions Common/Core/NotificationHistoryError.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//
// NotificationHistoryError.swift
// koin
//
// Created by 홍기정 on 7/7/26.
//

import Foundation

struct NotificationHistoryError: Error {
let message: String

static let calendarDidFail = Self.init(message: "캘린더 오류 발생")
}
Comment on lines +10 to +14

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== Files ==\n'
git ls-files | rg 'Common/Core/NotificationHistoryError\.swift|NotificationViewModel|loadNotifications|NotificationHistoryError'

printf '\n== NotificationHistoryError.swift ==\n'
sed -n '1,120p' Common/Core/NotificationHistoryError.swift

printf '\n== Candidates using NotificationHistoryError or localizedDescription ==\n'
rg -n "NotificationHistoryError|localizedDescription" Common -g '!**/build/**' -g '!**/dist/**'

printf '\n== NotificationViewModel candidate file(s) ==\n'
fd -a 'NotificationViewModel' Common || true

Repository: BCSDLab/KOIN_iOS

Length of output: 1046


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,120p' Common/Notification/NotificationViewModel.swift

Repository: BCSDLab/KOIN_iOS

Length of output: 245


🌐 Web query:

Swift Error localizedDescription non LocalizedError generic system message documentation

💡 Result:

In Swift, the localizedDescription property on the Error protocol is not a customization point [1][2]. Because it is defined as an extension on the Error protocol within the Foundation framework rather than a requirement of the protocol itself, it cannot be overridden directly [3][4]. When you access localizedDescription on an error, the default implementation performs a bridge to NSError [3][4]. If your error type does not conform to LocalizedError, this default implementation provides a generic system message (typically indicating that the error domain and code do not have a localized description) [2][3]. To provide a custom, localized message that localizedDescription will return, your error type must conform to the LocalizedError protocol [2][5]. 1. Conforming to LocalizedError: When you conform your type to LocalizedError, you implement the errorDescription property [6][7]. 2. How it works: The Foundation framework's bridging logic checks if an error conforms to LocalizedError [6][8]. If it does, and if errorDescription returns a string, that string is used to populate the NSLocalizedDescriptionKey in the underlying NSError's user info dictionary, which localizedDescription then returns [6][8][7]. 3. Default Behavior: If your type does not conform to LocalizedError, or if the errorDescription property returns nil, localizedDescription falls back to the generic system behavior provided by NSError [2]. Therefore, attempting to override localizedDescription directly in your error type will not work as intended for dynamic dispatch; you must instead implement the errorDescription requirement defined by the LocalizedError protocol [3][5].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "extension\s+Error|extension\s+LocalizedError|errorDescription|localizedDescription" Common -g '!**/build/**' -g '!**/dist/**'

Repository: BCSDLab/KOIN_iOS

Length of output: 154


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== NotificationHistoryError usage ==\n'
rg -n "NotificationHistoryError|localizedDescription" . -g '!**/build/**' -g '!**/dist/**'

printf '\n== NotificationHistoryService.swift ==\n'
sed -n '110,160p' Common/NotificationHistory/NotificationHistoryService.swift

printf '\n== Notification view model files ==\n'
sed -n '1,140p' Koin/Presentation/NewHome/Notification/NotificationViewModel.swift
printf '\n---\n'
sed -n '1,140p' Koin/Presentation/CallVan/CallVanNotification/CallVanNotificationViewModel.swift

Repository: BCSDLab/KOIN_iOS

Length of output: 12862


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== NotificationHistoryError usage ==\n'
rg -n "NotificationHistoryError|localizedDescription" . -g '!**/build/**' -g '!**/dist/**'

printf '\n== NotificationHistoryService.swift ==\n'
sed -n '110,160p' Common/NotificationHistory/NotificationHistoryService.swift

printf '\n== Notification view model files ==\n'
sed -n '1,160p' Koin/Presentation/NewHome/Notification/NotificationViewModel.swift
printf '\n---\n'
sed -n '1,160p' Koin/Presentation/CallVan/CallVanNotification/CallVanNotificationViewModel.swift

Repository: BCSDLab/KOIN_iOS

Length of output: 12862


Adopt LocalizedError so the toast shows message.

NotificationViewModel.loadNotifications() surfaces this error with error.localizedDescription, but NotificationHistoryError only conforms to Error, so the toast falls back to a generic system message instead of "캘린더 오류 발생".

🩹 Proposed fix
-struct NotificationHistoryError: Error {
+struct NotificationHistoryError: LocalizedError {
     let message: String
     
     static let calendarDidFail = Self.init(message: "캘린더 오류 발생")
+
+    var errorDescription: String? { message }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
struct NotificationHistoryError: Error {
let message: String
static let calendarDidFail = Self.init(message: "캘린더 오류 발생")
}
struct NotificationHistoryError: LocalizedError {
let message: String
static let calendarDidFail = Self.init(message: "캘린더 오류 발생")
var errorDescription: String? { message }
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Common/Core/NotificationHistoryError.swift` around lines 10 - 14,
NotificationHistoryError currently only conforms to Error, so
error.localizedDescription in NotificationViewModel.loadNotifications() won’t
surface the custom message and the toast falls back to a generic system string.
Update NotificationHistoryError to conform to LocalizedError and make its error
description return the existing message property so the UI shows the intended
text. Keep the change centered on NotificationHistoryError and verify the
calendarDidFail case still exposes "캘린더 오류 발생" through localizedDescription.

145 changes: 145 additions & 0 deletions Common/NotificationHistory/NotificationHistoryService.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
//
// NotificationContainer.swift
// koin
//
// Created by 홍기정 on 7/6/26.
//

import SwiftData
import Foundation

protocol NotificationHistoryService {
func insert(record: NotificationRecord) async throws
func fetchAll() async throws -> [NotificationRecord]
func markAsRead(messageId: String) async throws
func markAllAsRead() async throws
func delete(messageId: String) async throws
func deleteAll() async throws
}

final class DefaultNotificationHistoryService: NotificationHistoryService {

// MARK: - Properties
private let container: ModelContainer?

// MARK: - Initializer
init() {
container = try? ModelContainer(
for: NotificationRecord.self,
configurations: .init(groupContainer: .identifier("group.com.bcsdlab.koin"))
)
}

// MARK: - Create
func insert(record: NotificationRecord) async throws {
guard let container else {
throw SwiftDataError.loadIssueModelContainer
}

switch record.category {
case .shop, .dining, .keyword, .chat, .callvan, .callvanChat:
try? await MainActor.run { [weak container] in
container?.mainContext.insert(record)
try container?.mainContext.save()
}
default:
break
}
}
Comment on lines +34 to +48

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

insert swallows persistence errors and silently drops unsupported categories.

try? around MainActor.run discards any error thrown by container?.mainContext.save(), so the async throws signature can never actually surface a real save failure to callers. Combined with the default: break branch, any category outside the whitelist also silently no-ops. Callers (e.g. NotificationService.didReceive persisting incoming pushes) have no way to detect a dropped/failed notification.

🩹 Proposed fix
         switch record.category {
         case .shop, .dining, .keyword, .chat, .callvan, .callvanChat:
-            try? await MainActor.run { [weak container] in
-                container?.mainContext.insert(record)
-                try container?.mainContext.save()
-            }
+            try await MainActor.run {
+                container.mainContext.insert(record)
+                try container.mainContext.save()
+            }
         default:
             break
         }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Common/NotificationHistory/NotificationHistoryService.swift` around lines 34
- 48, The NotificationHistoryService.insert(record:) implementation is
swallowing persistence failures because `try?` wraps the `MainActor.run` save
path, and unsupported categories are silently ignored in the `default` branch.
Update `insert(record:)` so save errors from `container.mainContext.save()` are
allowed to propagate to callers, and replace the silent no-op for unsupported
categories with an explicit failure path or validation using the existing
`record.category` switch. Keep the fix localized to `insert(record:)` and the
`NotificationRecord` category handling so `NotificationService.didReceive` can
detect dropped or failed inserts.


// MARK: - Read
func fetchAll() async throws -> [NotificationRecord] {
guard let container else {
throw SwiftDataError.loadIssueModelContainer
}

try await deleteExpiredNotifications()

return try await MainActor.run {
var descriptor = FetchDescriptor<NotificationRecord>(
sortBy: [SortDescriptor(\.createdAt, order: .reverse)]
)
descriptor.fetchLimit = .max

return try container.mainContext.fetch(descriptor)
}
}

// MARK: - Update
func markAsRead(messageId: String) async throws {
guard let container else {
throw SwiftDataError.loadIssueModelContainer
}

try await MainActor.run {
var descriptor = FetchDescriptor<NotificationRecord>(
predicate: #Predicate { notification in
notification.messageId == messageId
}
)
descriptor.fetchLimit = 1
if let notification = try container.mainContext.fetch(descriptor).first {
notification.isRead = true
}
try container.mainContext.save()
}
}

func markAllAsRead() async throws {
guard let container else {
throw SwiftDataError.loadIssueModelContainer
}

try await MainActor.run {
var descriptor = FetchDescriptor<NotificationRecord>()
descriptor.fetchLimit = .max
try container.mainContext.enumerate(descriptor) { notification in
notification.isRead = true
}
try container.mainContext.save()
}
}

// MARK: - Delete
func delete(messageId: String) async throws {
guard let container else {
throw SwiftDataError.loadIssueModelContainer
}

try await MainActor.run {
try container.mainContext.delete(model: NotificationRecord.self, where: #Predicate { notification in
notification.messageId == messageId
})
try container.mainContext.save()
}
}

func deleteAll() async throws {
guard let container else {
throw SwiftDataError.loadIssueModelContainer
}

try await MainActor.run {
try container.mainContext.delete(model: NotificationRecord.self)
}
}
Comment on lines +117 to +125

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win

deleteAll() never persists the deletion — missing save().

Per SwiftData's documented behavior, batch deletion via delete(model:)/delete(model:where:) is only applied to the persistent store on the next save(). This method never calls save(), unlike every other mutating method in this file (insert, markAsRead, markAllAsRead, delete(messageId:)), so deleteAll() will silently no-op unless a save happens to occur elsewhere afterward.

🩹 Proposed fix
         try await MainActor.run {
             try container.mainContext.delete(model: NotificationRecord.self)
+            try container.mainContext.save()
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func deleteAll() async throws {
guard let container else {
throw SwiftDataError.loadIssueModelContainer
}
try await MainActor.run {
try container.mainContext.delete(model: NotificationRecord.self)
}
}
func deleteAll() async throws {
guard let container else {
throw SwiftDataError.loadIssueModelContainer
}
try await MainActor.run {
try container.mainContext.delete(model: NotificationRecord.self)
try container.mainContext.save()
}
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Common/NotificationHistory/NotificationHistoryService.swift` around lines 117
- 125, The deleteAll() method in NotificationHistoryService only performs
container.mainContext.delete(model:) and never persists the change, so add a
save on the same context after the deletion inside the MainActor.run block,
following the pattern used by insert, markAsRead, markAllAsRead, and
delete(messageId:). Ensure the save is awaited and any thrown error is
propagated so the batch deletion is actually committed to the persistent store.

}

extension DefaultNotificationHistoryService {
private func deleteExpiredNotifications() async throws {
guard let container else {
throw SwiftDataError.loadIssueModelContainer
}

guard let expirationDate = Calendar.current.date(byAdding: .day, value: -7, to: Date()) else {
throw NotificationHistoryError.calendarDidFail
}

try await MainActor.run {
try container.mainContext.delete(model: NotificationRecord.self, where: #Predicate { notification in
notification.createdAt < expirationDate
})
try container.mainContext.save()
}
}
}
Comment on lines +128 to +145

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Retention window is 7 days, but PR intent states 14 days.

The commit summary describes "deleting notification records older than 14 days when reading them," but this cleanup uses value: -7. If 14 days is the intended retention policy, this needs correcting.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Common/NotificationHistory/NotificationHistoryService.swift` around lines 128
- 145, The cleanup window in
DefaultNotificationHistoryService.deleteExpiredNotifications() is using a 7-day
cutoff, which conflicts with the intended 14-day retention policy. Update the
date calculation in deleteExpiredNotifications() so the expirationDate is based
on 14 days ago, and keep the existing delete/save flow in the MainActor.run
block unchanged.

35 changes: 35 additions & 0 deletions Common/NotificationHistory/NotificationRecord.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
//
// NotificationRecord.swift
// koin
//
// Created by 홍기정 on 7/6/26.
//

import SwiftData
import Foundation

@Model
final class NotificationRecord: Sendable {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== File list ==\n'
git ls-files 'Common/NotificationHistory/*' 'Common/*Notification*' '*/NotificationService.swift' '*/NotificationHistoryService.swift'

printf '\n== Outline: NotificationRecord.swift ==\n'
ast-grep outline Common/NotificationHistory/NotificationRecord.swift --view expanded || true

printf '\n== NotificationRecord.swift ==\n'
cat -n Common/NotificationHistory/NotificationRecord.swift

printf '\n== NotificationService.swift matches ==\n'
rg -n "NotificationRecord|Task\s*\{|MainActor\.run|insert\(" Common/NotificationHistory NotificationService.swift Common -g '*.swift' || true

printf '\n== NotificationHistoryService.swift matches ==\n'
rg -n "NotificationRecord|Task\s*\{|MainActor\.run|insert\(" Common/NotificationHistory NotificationHistoryService.swift Common -g '*.swift' || true

Repository: BCSDLab/KOIN_iOS

Length of output: 11592


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== Relevant Swift files ==\n'
fd -a 'Notification(Service|HistoryService|Record)\.swift$' . || true

printf '\n== Search for Sendable / `@Model` / unique ==\n'
rg -n "`@Model`|Sendable|`@Attribute`\\(\\.unique\\)|messageId|PersistentIdentifier" Common -g '*.swift' || true

Repository: BCSDLab/KOIN_iOS

Length of output: 1546


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== NotificationHistoryService.swift (1-220) ==\n'
cat -n Common/NotificationHistory/NotificationHistoryService.swift

printf '\n== NotificationService.swift (1-220) ==\n'
cat -n NotificationService/NotificationService.swift

Repository: BCSDLab/KOIN_iOS

Length of output: 10492


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

for path in [
    Path('Common/NotificationHistory/NotificationHistoryService.swift'),
    Path('NotificationService/NotificationService.swift'),
]:
    print(f"\n== {path} ==")
    for i, line in enumerate(path.read_text().splitlines(), 1):
        if 1 <= i <= 220:
            print(f"{i:4d}: {line}")
PY

Repository: BCSDLab/KOIN_iOS

Length of output: 10278


Drop Sendable from NotificationRecord, and make messageId unique.

  • NotificationRecord is a mutable SwiftData @Model class, so Sendable only masks actor-isolation diagnostics when the instance crosses into Task {}/MainActor.run.
  • messageId is the lookup key for markAsRead and delete; without a uniqueness constraint, duplicate deliveries can create duplicate rows and inconsistent history.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Common/NotificationHistory/NotificationRecord.swift` at line 12, Remove
Sendable from NotificationRecord since it is a mutable SwiftData `@Model` class
and should not be treated as thread-safe across Task/MainActor boundaries. Also
update NotificationRecord so messageId is declared as a unique lookup key in the
model definition, ensuring markAsRead and delete operate on a single row and
duplicate deliveries cannot create duplicate records.

var body: String
var title: String
var category: AppPath
var schemeUri: String
var messageId: String

var isRead: Bool = false
var createdAt: Date = Date()

Comment on lines +13 to +21

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

git ls-files | rg 'Common/NotificationHistory|NotificationItem|insert\(record:|NotificationRecord'

Repository: BCSDLab/KOIN_iOS

Length of output: 315


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== NotificationRecord.swift ==\n'
cat -n Common/NotificationHistory/NotificationRecord.swift

printf '\n== NotificationHistoryService.swift ==\n'
cat -n Common/NotificationHistory/NotificationHistoryService.swift

printf '\n== NotificationItem.swift ==\n'
cat -n Koin/Domain/Model/NewHome/NotificationItem.swift

Repository: BCSDLab/KOIN_iOS

Length of output: 9785


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "messageId|NotificationRecord|insert\\(record:|delete\\(messageId:|markAsRead\\(messageId:" Common Koin -g '!**/build/**' -g '!**/DerivedData/**'

Repository: BCSDLab/KOIN_iOS

Length of output: 3411


Make messageId unique in NotificationRecord. Push retries can create duplicate rows, and messageId is already the lookup key for read/delete plus NotificationItem.id, so duplicates will surface as repeated list items and ambiguous targeting.

🛡️ Proposed fix
 final class NotificationRecord: Sendable {
     var body: String
     var title: String
     var category: AppPath
     var schemeUri: String
-    var messageId: String
+    `@Attribute`(.unique) var messageId: String
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
var body: String
var title: String
var category: AppPath
var schemeUri: String
var messageId: String
var isRead: Bool = false
var createdAt: Date = Date()
var body: String
var title: String
var category: AppPath
var schemeUri: String
`@Attribute`(.unique) var messageId: String
var isRead: Bool = false
var createdAt: Date = Date()
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Common/NotificationHistory/NotificationRecord.swift` around lines 13 - 21,
`NotificationRecord` currently allows duplicate `messageId` values, which can
lead to repeated rows and ambiguous read/delete targeting. Update the
`NotificationRecord` model and any persistence path that creates or inserts it
so `messageId` is enforced as unique, and add deduplication logic where records
are loaded or saved using `messageId` as the key (including the
`NotificationItem.id` mapping) to prevent multiple entries for the same
notification.

init(
body: String,
title: String,
category: AppPath,
schemeUri: String,
messageId: String
) {
self.body = body
self.title = title
self.category = category
self.schemeUri = schemeUri
self.messageId = messageId
}
}
44 changes: 44 additions & 0 deletions Koin/Data/Repository/DefaultNotificationHistoryRepository.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
//
// DefaultNotificationHistoryRepository.swift
// koin
//
// Created by 홍기정 on 7/7/26.
//

import Foundation

final class DefaultNotificationHistoryRepository: NotificationHistoryRepository {

private let service: NotificationHistoryService

init(service: NotificationHistoryService) {
self.service = service
}

func insert(record: NotificationRecord) async throws {
try await service.insert(record: record)
}

func fetchAll() async throws -> [NotificationItem] {
try await service.fetchAll()
.compactMap {
NotificationItem.init(from: $0)
}
}

func deleteAll() async throws {
try await service.deleteAll()
}

func delete(id: String) async throws {
try await service.delete(messageId: id)
}

func markAsRead(id: String) async throws {
try await service.markAsRead(messageId: id)
}

func makrAllAsRead() async throws {
try await service.markAllAsRead()
}
Comment on lines +41 to +43

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Typo: makrAllAsReadmarkAllAsRead.

Naming typo in the public repository API. Since this must match the NotificationHistoryRepository protocol signature to compile, the same typo likely exists there too — worth fixing in both places while it's still low-diff.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Koin/Data/Repository/DefaultNotificationHistoryRepository.swift` around lines
41 - 43, The public repository API has a naming typo in
DefaultNotificationHistoryRepository: rename makrAllAsRead to markAllAsRead so
it matches the NotificationHistoryRepository protocol and compiles cleanly.
Update the same method name anywhere it is declared or referenced, including the
protocol signature and any conforming implementations, so the async
mark-all-read flow remains consistent.

}
15 changes: 0 additions & 15 deletions Koin/Domain/Model/NewHome/NotificationIconType.swift

This file was deleted.

62 changes: 59 additions & 3 deletions Koin/Domain/Model/NewHome/NotificationItem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
import Foundation

struct NotificationItem {
let id: Int
let isRead: Bool
let id: String
var isRead: Bool
let icon: ImageAsset
let appPath: AppPath
let uri: String?
Expand All @@ -18,7 +18,7 @@ struct NotificationItem {
let dateText: String

init(
id: Int,
id: String,
isRead: Bool,
icon: ImageAsset,
appPath: AppPath,
Expand All @@ -37,3 +37,59 @@ struct NotificationItem {
self.dateText = dateText
}
}

extension NotificationItem {

init?(from record: NotificationRecord) {
guard let icon = NotificationItem.icon(for: record.category) else {
return nil
}
self.id = record.messageId
self.isRead = record.isRead
self.icon = icon
self.appPath = record.category
self.uri = record.schemeUri
self.title = record.title
self.content = record.body
self.dateText = NotificationItem.dateText(for: record.createdAt)
}

static func icon(for appPath: AppPath) -> ImageAsset? {
switch appPath {
case .shop:
return ImageAsset.notificationShop
case .dining:
return ImageAsset.notificationDining
case .keyword:
return ImageAsset.notificationLostItem
case .chat:
return ImageAsset.notificationChat
case .callvan:
return ImageAsset.notificationCallVan
case .callvanChat:
return ImageAsset.notificationChat
default:
return nil
}
}
Comment on lines +40 to +74

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Notifications for .home, .login, .club, .timeTable categories are silently dropped from history.

icon(for:)'s default: return nil causes init?(from:) to return nil, and DefaultNotificationHistoryRepository.fetchAll() uses compactMap, so any persisted NotificationRecord with one of the unhandled AppPath cases (.home, .login, .club, .timeTable) is stored but never shown to the user, with no error surfaced. Since these are valid AppPath cases that NotificationService.saveNotificationIfAvailable will happily persist, this isn't just a future-proofing concern.

Consider either providing a fallback icon for the remaining cases, or making the switch exhaustive (dropping default) so adding a new AppPath case forces this mapping to be revisited at compile time.

♻️ Example fix
     static func icon(for appPath: AppPath) -> ImageAsset? {
         switch appPath {
         case .shop:
             return ImageAsset.notificationShop
         case .dining:
             return ImageAsset.notificationDining
         case .keyword:
             return ImageAsset.notificationLostItem
         case .chat:
             return ImageAsset.notificationChat
         case .callvan:
             return ImageAsset.notificationCallVan
         case .callvanChat:
             return ImageAsset.notificationChat
-        default:
-            return nil
+        case .home, .login, .club, .timeTable:
+            return ImageAsset.notificationDefault // or an appropriate fallback
         }
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
extension NotificationItem {
init?(from record: NotificationRecord) {
guard let icon = NotificationItem.icon(for: record.category) else {
return nil
}
self.id = record.messageId
self.isRead = record.isRead
self.icon = icon
self.appPath = record.category
self.uri = record.schemeUri
self.title = record.title
self.content = record.body
self.dateText = NotificationItem.dateText(for: record.createdAt)
}
static func icon(for appPath: AppPath) -> ImageAsset? {
switch appPath {
case .shop:
return ImageAsset.notificationShop
case .dining:
return ImageAsset.notificationDining
case .keyword:
return ImageAsset.notificationLostItem
case .chat:
return ImageAsset.notificationChat
case .callvan:
return ImageAsset.notificationCallVan
case .callvanChat:
return ImageAsset.notificationChat
default:
return nil
}
}
extension NotificationItem {
init?(from record: NotificationRecord) {
guard let icon = NotificationItem.icon(for: record.category) else {
return nil
}
self.id = record.messageId
self.isRead = record.isRead
self.icon = icon
self.appPath = record.category
self.uri = record.schemeUri
self.title = record.title
self.content = record.body
self.dateText = NotificationItem.dateText(for: record.createdAt)
}
static func icon(for appPath: AppPath) -> ImageAsset? {
switch appPath {
case .shop:
return ImageAsset.notificationShop
case .dining:
return ImageAsset.notificationDining
case .keyword:
return ImageAsset.notificationLostItem
case .chat:
return ImageAsset.notificationChat
case .callvan:
return ImageAsset.notificationCallVan
case .callvanChat:
return ImageAsset.notificationChat
case .home, .login, .club, .timeTable:
return ImageAsset.notificationDefault // or an appropriate fallback
}
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Koin/Domain/Model/NewHome/NotificationItem.swift` around lines 40 - 74, The
notification mapping in NotificationItem.icon(for:) is dropping valid AppPath
cases because the default branch returns nil, which makes init?(from:) fail and
causes DefaultNotificationHistoryRepository.fetchAll() to skip persisted records
via compactMap. Update the icon mapping so .home, .login, .club, and .timeTable
are either assigned appropriate fallback icons or handled explicitly, and remove
the catch-all default if possible so future AppPath additions are forced to be
mapped at compile time.


static func dateText(for createdAt: Date) -> String {
let now = Date()
let compareComponents = Calendar.current.dateComponents(
[.day, .hour, .minute],
from: createdAt,
to: now
)

if let day = compareComponents.day, 1 <= day {
return "\(day)일 전"
}
if let hour = compareComponents.hour, 1 <= hour {
return "\(hour)시간 전"
}
if let minute = compareComponents.minute, 1 <= minute {
return "\(minute)분 전"
}
return "지금"
}
}
16 changes: 16 additions & 0 deletions Koin/Domain/Repository/NotificationHistoryRepository.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//
// NotificationHistoryRepository.swift
// koin
//
// Created by 홍기정 on 7/7/26.
//

import Combine

protocol NotificationHistoryRepository {
func fetchAll() async throws -> [NotificationItem]
func deleteAll() async throws
func delete(id: String) async throws
func markAsRead(id: String) async throws
func makrAllAsRead() async throws

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Typo: makrAllAsReadmarkAllAsRead.

This new protocol method name is misspelled. It's consistently used in DefaultNotificationHistoryRepository and DefaultUpdateNotificationHistoryUseCase, so fixing it now (before this API is more widely adopted) is cheap.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Koin/Domain/Repository/NotificationHistoryRepository.swift` at line 15,
Rename the misspelled repository API from makrAllAsRead to markAllAsRead in
NotificationHistoryRepository and update every matching implementation/call
site, including DefaultNotificationHistoryRepository and
DefaultUpdateNotificationHistoryUseCase, so the protocol and its uses all share
the corrected method name.

}
30 changes: 30 additions & 0 deletions Koin/Domain/UseCase/NewHome/DeleteNotificationHistoryUseCase.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
//
// DeleteNotificationHistoryUseCase.swift
// koin
//
// Created by 홍기정 on 7/7/26.
//

import Foundation

protocol DeleteNotificationHistoryUseCase {
func delete(id: String) async throws
func deleteAll() async throws
}

final class DefaultDeleteNotificationHistoryUseCase: DeleteNotificationHistoryUseCase {

private let repository: NotificationHistoryRepository

init(repository: NotificationHistoryRepository) {
self.repository = repository
}

func delete(id: String) async throws {
try await repository.delete(id: id)
}

func deleteAll() async throws {
try await repository.deleteAll()
}
}
Loading