Skip to content

Commit 7fe8824

Browse files
authored
[#773] PushNotificationListView에서 refreshable이 끝나지 않는 현상을 해결한다 (#775)
* fix: PushNotificationList refresh와 listener 분리 * refactor: refreshable 활성화 조건 분리 * fix: 푸시 알림 시간 필터 기준 시각 갱신 * refactor: 푸시 알림 다음 페이지 판별 책임 이동 * fix: 푸시 알림 refresh와 listener 갱신 직렬화
1 parent f2cab68 commit 7fe8824

9 files changed

Lines changed: 401 additions & 106 deletions

Application/Core/Sources/PushNotificationQuery.swift

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,20 @@ public struct PushNotificationQuery: Equatable {
2323
public var timeFilter: TimeFilter
2424
public var unreadOnly: Bool
2525
public var pageSize: Int
26+
public var referenceDate: Date
2627

2728
public init(
2829
sortOrder: SortOrder,
2930
timeFilter: TimeFilter,
3031
unreadOnly: Bool,
31-
pageSize: Int
32+
pageSize: Int,
33+
referenceDate: Date = Date()
3234
) {
3335
self.sortOrder = sortOrder
3436
self.timeFilter = timeFilter
3537
self.unreadOnly = unreadOnly
3638
self.pageSize = pageSize
39+
self.referenceDate = referenceDate
3740
}
3841

3942
public static let `default` = PushNotificationQuery(
@@ -67,14 +70,14 @@ public extension PushNotificationQuery.TimeFilter {
6770
}
6871
}
6972

70-
var thresholdDate: Date? {
73+
func thresholdDate(relativeTo referenceDate: Date) -> Date? {
7174
switch self {
7275
case .none:
7376
return nil
7477
case .hours(let value):
75-
return Date().addingTimeInterval(-Double(value) * 3600.0)
78+
return referenceDate.addingTimeInterval(-Double(value) * 3600.0)
7679
case .days(let value):
77-
return Date().addingTimeInterval(-Double(value) * 86400.0)
80+
return referenceDate.addingTimeInterval(-Double(value) * 86400.0)
7881
}
7982
}
8083
}

Application/Infra/Sources/Service/PushNotificationServiceImpl.swift

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -135,25 +135,12 @@ final class PushNotificationServiceImpl: PushNotificationService {
135135
])
136136
}
137137

138+
let pageLimit = notificationQuery.pageSize
138139
let snapshot = try await firestoreQuery
139-
.limit(to: notificationQuery.pageSize)
140+
.limit(to: pageLimit + 1)
140141
.getDocuments()
141142

142-
let items = snapshot.documents.compactMap { makeResponse(from: $0) }
143-
144-
let nextCursor: PushNotificationCursorDTO? = snapshot.documents.last.map { document in
145-
guard let receivedAt = document.data()[PushNotificationFieldKey.receivedAt.rawValue] as? Timestamp
146-
else {
147-
return nil
148-
}
149-
150-
return PushNotificationCursorDTO(
151-
receivedAt: receivedAt.dateValue(),
152-
documentID: document.documentID
153-
)
154-
} ?? nil
155-
156-
return PushNotificationPageResponse(items: items, nextCursor: nextCursor)
143+
return makePageResponse(from: snapshot.documents, limit: pageLimit)
157144
} catch {
158145
logger.error("Failed to request notifications", error: error)
159146
record(error, code: .requestNotifications)
@@ -170,7 +157,7 @@ final class PushNotificationServiceImpl: PushNotificationService {
170157
let subject = PassthroughSubject<PushNotificationPageResponse, Error>()
171158
let pageLimit = max(query.pageSize, limit)
172159
let listener = makeQuery(uid: uid, query: query)
173-
.limit(to: pageLimit)
160+
.limit(to: pageLimit + 1)
174161
.addSnapshotListener { [weak self] snapshot, error in
175162
if let error {
176163
Self.record(error, code: .observeNotifications)
@@ -180,14 +167,7 @@ final class PushNotificationServiceImpl: PushNotificationService {
180167

181168
guard let self, let snapshot else { return }
182169

183-
let items = snapshot.documents.compactMap { self.makeResponse(from: $0) }
184-
let nextCursor = self.makeNextCursor(from: snapshot.documents.last)
185-
subject.send(
186-
PushNotificationPageResponse(
187-
items: items,
188-
nextCursor: nextCursor
189-
)
190-
)
170+
subject.send(self.makePageResponse(from: snapshot.documents, limit: pageLimit))
191171
}
192172

193173
return subject
@@ -317,7 +297,9 @@ private extension PushNotificationServiceImpl {
317297
var firestoreQuery: Query = store.collection(FirestorePath.notifications(uid))
318298
.whereField(PushNotificationFieldKey.isDeleted.rawValue, isEqualTo: false)
319299

320-
if let thresholdDate = query.timeFilter.thresholdDate {
300+
if let thresholdDate = query.timeFilter.thresholdDate(
301+
relativeTo: query.referenceDate
302+
) {
321303
firestoreQuery = firestoreQuery.whereField(
322304
"receivedAt",
323305
isGreaterThanOrEqualTo: Timestamp(date: thresholdDate)
@@ -347,6 +329,19 @@ private extension PushNotificationServiceImpl {
347329
)
348330
}
349331

332+
func makePageResponse(
333+
from documents: [QueryDocumentSnapshot],
334+
limit: Int
335+
) -> PushNotificationPageResponse {
336+
let pageDocuments = Array(documents.prefix(limit))
337+
let items = pageDocuments.compactMap { makeResponse(from: $0) }
338+
let nextCursor = limit < documents.count
339+
? makeNextCursor(from: pageDocuments.last)
340+
: nil
341+
342+
return PushNotificationPageResponse(items: items, nextCursor: nextCursor)
343+
}
344+
350345
func makeResponse(from snapshot: QueryDocumentSnapshot) -> PushNotificationResponse? {
351346
let data = snapshot.data()
352347
if (data[PushNotificationFieldKey.isDeleted.rawValue] as? Bool) == true {

Application/Presentation/NotificationTab/Sources/PushNotification/PushNotificationListFeature.swift

Lines changed: 48 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ struct PushNotificationListFeature {
1818
@Presents var alert: AlertState<Never>?
1919
@Presents var sheet: SheetState?
2020
var notifications: [PushNotificationItem] = []
21-
var hasMore = false
2221
var nextCursor: PushNotificationCursor?
2322
var query: PushNotificationQuery
2423
var selectedNotificationId: String?
@@ -58,6 +57,8 @@ struct PushNotificationListFeature {
5857
case loading(LoadingFeature.Action)
5958

6059
enum ViewAction: Equatable {
60+
case stopObserving
61+
case startObserving
6162
case refresh
6263
case fetchNotifications
6364
case loadNextPage
@@ -79,9 +80,7 @@ struct PushNotificationListFeature {
7980
enum StoreAction: Equatable {
8081
case setAlert
8182
case appendNotifications([PushNotificationItem], nextCursor: PushNotificationCursor?)
82-
case resetPagination
83-
case setHasMore(Bool)
84-
case syncNotifications([PushNotificationItem], nextCursor: PushNotificationCursor?, hasMore: Bool)
83+
case replaceNotifications([PushNotificationItem], nextCursor: PushNotificationCursor?)
8584
case setNotificationHidden(String, Bool)
8685
case setNotificationRead(String, Bool)
8786
}
@@ -98,6 +97,7 @@ struct PushNotificationListFeature {
9897
@Dependency(\.undoDeletePushNotificationUseCase) var undoDeletePushNotificationUseCase
9998
@Dependency(\.togglePushNotificationReadUseCase) var togglePushNotificationReadUseCase
10099
@Dependency(\.updatePushNotificationQueryUseCase) var updatePushNotificationQueryUseCase
100+
@Dependency(\.date.now) var now
101101

102102
var body: some ReducerOf<Self> {
103103
Scope(state: \.loading, action: \.loading) {
@@ -116,6 +116,7 @@ struct PushNotificationListFeature {
116116
break
117117
case .binding(\.query.timeFilter):
118118
state.nextCursor = nil
119+
state.query.referenceDate = now
119120
return refreshForQueryChangeEffect(query: state.query)
120121
case .binding:
121122
break
@@ -151,23 +152,32 @@ private extension PushNotificationListFeature {
151152
state: inout State
152153
) -> Effect<Action> {
153154
switch action {
155+
case .stopObserving:
156+
return .cancel(id: CancelID.observeNotifications)
157+
case .startObserving:
158+
return observeNotificationsEffect(
159+
query: state.query,
160+
limit: state.query.pageSize
161+
)
154162
case .refresh:
155163
state.nextCursor = nil
156-
return fetchNotificationsEffect(
157-
query: state.query,
158-
cursor: nil,
159-
existingCount: 0,
160-
showsIndicator: false
164+
state.query.referenceDate = now
165+
return .concatenate(
166+
.cancel(id: CancelID.observeNotifications),
167+
fetchNotificationsPageEffect(
168+
query: state.query,
169+
cursor: nil,
170+
showsIndicator: false
171+
)
161172
)
162173
case .fetchNotifications:
163174
state.nextCursor = nil
164-
return fetchNotificationsEffect(query: state.query, cursor: nil, existingCount: 0)
175+
return fetchNotificationsPageEffect(query: state.query, cursor: nil)
165176
case .loadNextPage:
166-
guard state.hasMore, !state.isLoading else { return .none }
167-
return fetchNotificationsEffect(
177+
guard state.nextCursor != nil, !state.isLoading else { return .none }
178+
return fetchNotificationsPageEffect(
168179
query: state.query,
169-
cursor: state.nextCursor,
170-
existingCount: state.notifications.count
180+
cursor: state.nextCursor
171181
)
172182
case .deleteNotification(let item):
173183
guard state.notifications.contains(where: { $0.id == item.id }) else { return .none }
@@ -193,14 +203,17 @@ private extension PushNotificationListFeature {
193203
}
194204
case .toggleSortOption:
195205
state.query.sortOrder = state.query.sortOrder == .latest ? .oldest : .latest
206+
state.query.referenceDate = now
196207
state.nextCursor = nil
197208
return refreshForQueryChangeEffect(query: state.query)
198209
case .toggleUnreadOnly:
199210
state.query.unreadOnly.toggle()
211+
state.query.referenceDate = now
200212
state.nextCursor = nil
201213
return refreshForQueryChangeEffect(query: state.query)
202214
case .resetFilters:
203215
state.query = .default
216+
state.query.referenceDate = now
204217
state.nextCursor = nil
205218
return refreshForQueryChangeEffect(query: state.query)
206219
case .selectNotification(let notificationId):
@@ -242,18 +255,12 @@ private extension PushNotificationListFeature {
242255
incomingNotifications: notifications
243256
))
244257
state.nextCursor = nextCursor
245-
case .resetPagination:
246-
state.notifications = []
247-
state.nextCursor = nil
248-
case .setHasMore(let value):
249-
state.hasMore = value
250-
case .syncNotifications(let notifications, let nextCursor, let hasMore):
258+
case .replaceNotifications(let notifications, let nextCursor):
251259
state.notifications = Self.mergedHiddenNotifications(
252260
currentNotifications: state.notifications,
253261
incomingNotifications: notifications
254262
)
255263
state.nextCursor = nextCursor
256-
state.hasMore = hasMore
257264
case .setNotificationHidden(let notificationId, let isHidden):
258265
Self.setNotificationHidden(&state, notificationId: notificationId, isHidden: isHidden)
259266
case .setNotificationRead(let notificationId, let isRead):
@@ -268,32 +275,12 @@ private extension PushNotificationListFeature {
268275
func refreshForQueryChangeEffect(query: PushNotificationQuery) -> Effect<Action> {
269276
.merge(
270277
updateQueryEffect(query: query),
271-
fetchNotificationsEffect(query: query, cursor: nil, existingCount: 0)
272-
)
273-
}
274-
275-
func fetchNotificationsEffect(
276-
query: PushNotificationQuery,
277-
cursor: PushNotificationCursor?,
278-
existingCount: Int,
279-
showsIndicator: Bool = true
280-
) -> Effect<Action> {
281-
let limit = max(query.pageSize, existingCount)
282-
let fetchEffect = fetchNotificationsPageEffect(query: query, cursor: cursor, showsIndicator: showsIndicator)
283-
let observeEffect = observeNotificationsEffect(
284-
query: query,
285-
limit: max(limit, existingCount + query.pageSize)
286-
)
287-
288-
if cursor == nil {
289-
return .concatenate(
290-
.cancel(id: CancelID.observeNotifications),
291-
fetchEffect,
292-
observeEffect
278+
.concatenate(
279+
.send(.view(.stopObserving)),
280+
fetchNotificationsPageEffect(query: query, cursor: nil),
281+
.send(.view(.startObserving))
293282
)
294-
}
295-
296-
return fetchEffect
283+
)
297284
}
298285

299286
func fetchNotificationsPageEffect(
@@ -307,16 +294,22 @@ private extension PushNotificationListFeature {
307294
}
308295
do {
309296
let page = try await fetchPushNotificationsUseCase.execute(query, cursor: cursor)
297+
let notifications = page.items.map(PushNotificationItem.init(from:))
310298
if cursor == nil {
311-
await send(.store(.resetPagination))
299+
await send(
300+
.store(.replaceNotifications(
301+
notifications,
302+
nextCursor: page.nextCursor
303+
))
304+
)
305+
} else {
306+
await send(
307+
.store(.appendNotifications(
308+
notifications,
309+
nextCursor: page.nextCursor
310+
))
311+
)
312312
}
313-
await send(
314-
.store(.appendNotifications(
315-
page.items.map(PushNotificationItem.init(from:)),
316-
nextCursor: page.nextCursor
317-
))
318-
)
319-
await send(.store(.setHasMore(page.items.count == query.pageSize && page.nextCursor != nil)))
320313
if showsIndicator {
321314
await send(.loading(.end(target: .default, mode: .delayed)))
322315
}
@@ -339,8 +332,7 @@ private extension PushNotificationListFeature {
339332
let publisher = try fetchPushNotificationsUseCase.observe(query, limit: limit)
340333
for try await page in publisher.values {
341334
let items = page.items.map(PushNotificationItem.init(from:))
342-
let hasMore = items.count == max(query.pageSize, limit) && page.nextCursor != nil
343-
await send(.store(.syncNotifications(items, nextCursor: page.nextCursor, hasMore: hasMore)))
335+
await send(.store(.replaceNotifications(items, nextCursor: page.nextCursor)))
344336
}
345337
} catch is CancellationError {
346338
} catch {

Application/Presentation/NotificationTab/Sources/PushNotification/PushNotificationListView.swift

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,11 @@ public struct PushNotificationListView: View {
3838
headerOffset = max(0, -offset)
3939
}
4040
.safeAreaInset(edge: .top) { safeAreaHeader }
41-
.refreshable { await store.send(.view(.refresh)).finish() }
41+
.refreshable(isEnabled: PullToRefreshAvailability.isEnabled) {
42+
let task = store.send(.view(.refresh))
43+
await task.finish()
44+
store.send(.view(.startObserving))
45+
}
4246
.navigationTitle(String(localized: "nav_push_notifications"))
4347
.listStyle(.plain)
4448
}
@@ -124,7 +128,7 @@ public struct PushNotificationListView: View {
124128
)
125129
.onAppear {
126130
let lastId = notifications.last?.id
127-
if notification.id == lastId, store.hasMore {
131+
if notification.id == lastId, store.nextCursor != nil {
128132
store.send(.view(.loadNextPage))
129133
}
130134
}

Application/Presentation/NotificationTab/Sources/PushNotification/PushNotificationListViewCoordinator.swift

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ public final class PushNotificationListViewCoordinator {
1717
private let container: DIContainer
1818
@ObservationIgnored
1919
private var todoDetailStore: StoreOf<TodoDetailFeature>?
20+
@ObservationIgnored
21+
private var fetchNotificationsTask: Task<Void, Never>?
2022

2123
public init(container: DIContainer) {
2224
self.container = container
@@ -42,7 +44,15 @@ public final class PushNotificationListViewCoordinator {
4244
}
4345

4446
public func fetchData() {
45-
store.send(.view(.fetchNotifications))
47+
fetchNotificationsTask?.cancel()
48+
store.send(.view(.stopObserving))
49+
let query = store.query
50+
let task = store.send(.view(.fetchNotifications))
51+
fetchNotificationsTask = Task { [store] in
52+
await task.finish()
53+
guard !Task.isCancelled, store.query == query else { return }
54+
store.send(.view(.startObserving))
55+
}
4656
}
4757

4858
public func makeTodoDetailStore(todoId: String) -> StoreOf<TodoDetailFeature> {

0 commit comments

Comments
 (0)