Skip to content

Conversation

@leeari95
Copy link
Owner

@leeari95 leeari95 commented Feb 1, 2026

๐Ÿ“˜ ์ž‘์—… ์œ ํ˜•

  • ์‹ ๊ทœ ๊ธฐ๋Šฅ ์ถ”๊ฐ€
  • ๋ฒ„๊ทธ ์ˆ˜์ •
  • ๋ฆฌํŽ™ํ† ๋ง
  • ๋ฌธ์„œ ์—…๋ฐ์ดํŠธ

๐Ÿ“‹ ์ฒดํฌ๋ฆฌ์ŠคํŠธ

  • Merge ํ•˜๋Š” ๋ธŒ๋žœ์น˜๊ฐ€ ์˜ฌ๋ฐ”๋ฅธ๊ฐ€?
  • PR๊ณผ ๊ด€๋ จ์—†๋Š” ๋ณ€๊ฒฝ์‚ฌํ•ญ์ด ์—†๋Š”๊ฐ€?
  • ๋‚ด ์ฝ”๋“œ์— ๋Œ€ํ•œ ์ž๊ธฐ ๊ฒ€ํ† ๊ฐ€ ๋˜์—ˆ๋Š”๊ฐ€?
  • ์‚ฌ์šฉ์ž ๊ฐ€์ด๋“œ ์—…๋ฐ์ดํŠธ๊ฐ€ ํ•„์š”ํ•œ๊ฐ€?
    • ์‚ฌ์šฉ์ž ๊ฐ€์ด๋“œ๋ฅผ ์—…๋ฐ์ดํŠธ ํ•˜์˜€๋Š”๊ฐ€?

Summary by CodeRabbit

  • New Features

    • Hotel Tickets added as an exchange currency shown in lists and item details.
    • Collect, toggle and view specific item variants across catalog and detail screens.
  • Localization

    • Added English and Korean strings for new locations, housewares themes and currency labels.
  • Chores

    • App target bumped to 3.1.0 and new icon asset added.
  • Stability

    • Improved persistent storage migration, background-save handling and variant persistence.

โœ๏ธ Tip: You can customize this high-level summary in your review settings.

leeari95 and others added 3 commits February 1, 2026 16:02
* โœจ [feat] Hotel Tickets ๊ตํ™˜ ํ†ตํ™” UI ์ง€์› ์ถ”๊ฐ€

- Item์— canExchangeHotelTickets computed property ์ถ”๊ฐ€
- ItemBellsView์— hotelTickets ๋ชจ๋“œ ์ถ”๊ฐ€ (์•„์ด์ฝ˜: icon-hoteltickets)
- ItemDetailInfoView์— Hotel Tickets ๊ตํ™˜ ๊ฐ€๊ฒฉ ํ‘œ์‹œ ๋กœ์ง ์ถ”๊ฐ€

* ๐Ÿ”ง [chore] ์ƒˆ๋กœ์šด ํƒœ๊ทธ ๋‹ค๊ตญ์–ด ์ถ”๊ฐ€
* โœจ [feat] Variant ์ˆ˜์ง‘ ์ถ”์ ์„ ์œ„ํ•œ CoreData ์Šคํ‚ค๋งˆ ๋ฐ Storage ๋ ˆ์ด์–ด ์ถ”๊ฐ€
- VariantCollectionEntity ์ถ”๊ฐ€ (variantId, itemName ์ €์žฅ)
- VariantsStorage ํ”„๋กœํ† ์ฝœ ์ •์˜ (fetch, add, remove, removeAll)
- CoreDataVariantsStorage ๊ตฌํ˜„ (RxSwift ๊ธฐ๋ฐ˜)
- CoreDataStorage์— ์ž๋™ ๋ณ‘ํ•ฉ ์„ค์ • ์ถ”๊ฐ€ (automaticallyMergesChangesFromParent)
- CoreDataItemsStorage์— viewContext ์ฆ‰์‹œ ์ €์žฅ ๋กœ์ง ์ถ”๊ฐ€ (๋ฐ์ดํ„ฐ ์†์‹ค ๋ฐฉ์ง€)

* ๐Ÿ› [fix] ์˜ท ์•„์ดํ…œ variants๊ฐ€ ๋™์ผํ•œ variantId๋ฅผ ์‚ฌ์šฉํ•˜๋˜ ๋ฒ„๊ทธ ์ˆ˜์ •
- WardrobeVariat.toVariat()์—์„œ ํ•˜๋“œ์ฝ”๋”ฉ๋œ "1_0_0" ๋Œ€์‹  filename๊ณผ internalId๋ฅผ ์กฐํ•ฉํ•œ ๊ณ ์œ  ID ์ƒ์„ฑ
- ํ˜•์‹: "{filename}_{internalId}_0"์œผ๋กœ ๋ณ€๊ฒฝํ•˜์—ฌ ๊ฐ variant๋งˆ๋‹ค ๊ณ ์œ ์„ฑ ๋ณด์žฅ
- variationsWithColor ํ•„ํ„ฐ ํ†ต๊ณผ๋ฅผ ์œ„ํ•ด "_0" ์ ‘๋ฏธ์‚ฌ ์œ ์ง€

* โœจ [feat] Variant ์ˆ˜์ง‘ ๊ธฐ๋Šฅ - Reactor ๋ฐ ๋น„์ฆˆ๋‹ˆ์Šค ๋กœ์ง ์ถ”๊ฐ€
- ItemDetailReactor์— variant collection actions/mutations/state ์ถ”๊ฐ€
  - fetchCollectedVariants, toggleVariantCollection ์•ก์…˜
  - collectedVariantIds ์ƒํƒœ ๊ด€๋ฆฌ
- CatalogCellReactor์— ์•„์ดํ…œ ์ˆ˜์ง‘ ํ•ด์ œ ์‹œ variants ์ผ๊ด„ ์ œ๊ฑฐ ๋กœ์ง ์ถ”๊ฐ€
- Items ์‹ฑ๊ธ€ํ„ด์— variant ๊ด€๋ฆฌ ๊ธฐ๋Šฅ ์ถ”๊ฐ€
  - collectedVariants BehaviorRelay ์ถ”๊ฐ€
  - updateVariant, getCollectedVariants ๋ฉ”์„œ๋“œ ์ถ”๊ฐ€
  - CoreDataVariantsStorage ์ดˆ๊ธฐํ™” ๋ฐ ๋กœ๋“œ
- ์•„์ดํ…œ ์ฒดํฌ ํ•ด์ œ ์‹œ ํ•ด๋‹น ์•„์ดํ…œ์˜ ๋ชจ๋“  variants ์ž๋™ ์ œ๊ฑฐ
- Variant ์ฒดํฌ ์‹œ ์•„์ดํ…œ๋„ ์ž๋™ ์ˆ˜์ง‘ ์ฒ˜๋ฆฌ

* โœจ [feat] Variant ์ˆ˜์ง‘ ๊ธฐ๋Šฅ - UI ๋ ˆ์ด์–ด ๊ตฌํ˜„
- VariantCell์— ์ฒดํฌ๋ฐ•์Šค UI ์ถ”๊ฐ€
  - checkButton (checkmark.seal ์•„์ด์ฝ˜)
  - showCheckbox ํ”Œ๋ž˜๊ทธ๋กœ ๋ฆฌํผ ๊ฐ€๋Šฅ ์•„์ดํ…œ ํ•„ํ„ฐ๋ง
  - checkboxObservable๋กœ ์ฒดํฌ ์ด๋ฒคํŠธ ์ „๋‹ฌ
- ItemVariantsView์— ์ˆ˜์ง‘ ์ƒํƒœ ๊ด€๋ฆฌ ์ถ”๊ฐ€
  - collectedVariantIds BehaviorRelay๋กœ reactive ์—…๋ฐ์ดํŠธ
  - isReformable ํ”Œ๋ž˜๊ทธ๋กœ ์ฒดํฌ๋ฐ•์Šค ํ‘œ์‹œ ์ œ์–ด
  - didToggleVariantCollection Observable ์ถ”๊ฐ€
- ItemDetailViewController ๋ฐ”์ธ๋”ฉ ์ถ”๊ฐ€
  - viewDidAppear์—์„œ ์ˆ˜์ง‘ ์ƒํƒœ ์ƒˆ๋กœ๊ณ ์นจ
  - reactor์™€ ์–‘๋ฐฉํ–ฅ ๋ฐ”์ธ๋”ฉ (collectedVariantIds โ†” UI)
  - setUpVariant์—์„œ ๋ฆฌํผ ๊ฐ€๋Šฅ ์—ฌ๋ถ€ ์ „๋‹ฌ

* ๐Ÿ”ง [chore] ์•ฑ ์ข…๋ฃŒ ์‹œ CoreData ์ €์žฅ ๋ณด์žฅ ๋กœ์ง ์ถ”๊ฐ€
- sceneDidEnterBackground์—์„œ background task ์‹œ์ž‘
- ๋น„๋™๊ธฐ CoreData ์ž‘์—… ์™„๋ฃŒ๋ฅผ ์œ„ํ•ด 2์ดˆ ๋Œ€๊ธฐ ํ›„ viewContext ์ €์žฅ
- ์•ฑ ๊ฐ•์ œ ์ข…๋ฃŒ ์‹œ ๋ฐ์ดํ„ฐ ์†์‹ค ๋ฐฉ์ง€
@coderabbitai
Copy link

coderabbitai bot commented Feb 1, 2026

๐Ÿ“ Walkthrough

Walkthrough

Adds variant-collection support (model, storage, UI, reactor/viewmodel wiring), hotel-tickets exchange support (model, UI, asset, localization), Core Data migration/config tweaks, Rx wrapper for networking, and minor scene/background and layout adjustments.

Changes

Cohort / File(s) Summary
Config & Repo URL
Animal-Crossing-Wiki/Configurations/TargetVersion.xcconfig, Projects/App/Sources/Networking/Utilities/EnvironmentsVariable.swift
Bumped TARGET_VERSION to 3.1.0; updated repoURL to the release/3.0.0 data path.
Localization
Projects/App/Resources/en.lproj/Localizable.strings, Projects/App/Resources/ko.lproj/Localizable.strings
Added new localized keys including hotel/shop names, housewares concepts, and exchange currency labels (including "Hotel Tickets").
Assets
Projects/App/Resources/Assets.xcassets/.../icon-hoteltickets.imageset/Contents.json
Added Contents.json for the hotel-tickets icon asset.
Core Data model & config
Projects/App/Sources/CoreDataStorage/CoreDataStorage.swift, .../CoreDataStorage.xcdatamodeld/.../UserCollectionEntity.xcdatamodel/contents
Enabled lightweight migration and auto-merge/merge policy; added VariantCollectionEntity and a to-many variants relationship on UserCollectionEntity.
Variants storage API & impl.
Projects/App/Sources/CoreDataStorage/VariantsStorage/VariantsStorage.swift, .../CoreDataStorage/VariantsStorage/CoreDataVariantsStorage.swift
Introduced VariantsStorage protocol and CoreDataVariantsStorage implementation (background tasks; fetch/add/remove/fetchAll; map errors).
Domain & DTO changes
Projects/App/Sources/Models/Items/Item.swift, .../Networking/Response/Clothes/TopsResponseDTO.swift, .../Networking/Response/Furniture/HousewaresResponseDTO.swift
Added canExchangeHotelTickets prop, made variantId generation dynamic in DTO, and added hotelTickets exchange currency case.
Global items state
Projects/App/Sources/Utility/Items.swift
Added collectedVariants BehaviorRelay, variantList observable, getCollectedVariants, updateVariant, initialization from storage, and reset integration.
Reactors / ViewModels
Projects/App/Sources/Presentation/Catalog/ViewModels/ItemDetailReactor.swift, .../CatalogCellReactor.swift
Injected VariantsStorage; added actions/mutations/state for collected variant IDs; clear variants on uncollect and toggle handling.
Views / UI
Projects/App/Sources/Presentation/Catalog/Views/ItemVariantsView.swift, .../VariantCell.swift, .../ItemDetailViewController.swift
Variant UI supports checkboxes, exposes toggle events and collectedVariantIds, accepts isReformable flag; ItemDetailViewController wired to reactor and collects variant state.
Exchange UI
Projects/App/Sources/Presentation/Catalog/Views/CatalogCell.swift, .../ItemBellsView.swift, .../ItemDetailInfoView.swift
Added hotel-tickets display branch, new mode case, and icon mapping for hotel tickets.
Networking
Projects/App/Sources/Networking/DefaultAPIProvider.swift, .../Protocol/APIProvider.swift
Added RxSwift import and a reactive request โ†’ Single<T.Response> API alongside existing completion API.
Misc / Background & Layout
.../CoreDataStorage/ItemsStorage/CoreDataItemsStorage.swift, Projects/App/Sources/SceneDelegate.swift, .../Views/VillagerHouseView.swift, .../Utility/AboutItem.swift
Minor formatting; bounded background task on scene backgrounding; adjusted house image top constraint; updated publicly shown version text.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant VariantCell
    participant ItemVariantsView
    participant Reactor as ItemDetailReactor
    participant VariantsStorage as CoreDataVariantsStorage
    participant Items as Items

    User->>VariantCell: Tap checkbox
    VariantCell->>ItemVariantsView: emit checkboxObservable(variant, isCollected)
    ItemVariantsView->>Reactor: dispatch Action.toggleVariantCollection(variant)
    Reactor->>VariantsStorage: add/remove(variantId, itemName)
    VariantsStorage->>VariantsStorage: persist change (background context)
    Reactor->>Items: updateVariant(variantId, itemName, isAdding)
    Items->>ItemVariantsView: collectedVariantIds observable updates
    ItemVariantsView->>VariantCell: refresh checkbox state
Loading

Estimated code review effort

๐ŸŽฏ 4 (Complex) | โฑ๏ธ ~45 minutes

Possibly related PRs

Suggested labels

feature, fix

Poem

๐Ÿฐ
I hopped through code with twitchy paws,
Checkboxes stitched without a pause,
Hotel tickets gleam in the list,
Variants countedโ€”none are missed,
Hoppity cheer for every saved twist! ๐ŸŽจ๐Ÿงพ

๐Ÿšฅ Pre-merge checks | โœ… 1 | โŒ 2
โŒ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check โš ๏ธ Warning The PR description is incomplete. While it provides checked work-type items and a partial checklist, it is missing the mandatory 'Issue', 'Work Details', and narrative description sections from the required template. Only structure fragments are filled, with minimal substantive content beyond checkbox selections. Complete the missing sections: add Issue URL, provide detailed ์ž‘์—… ๋‚ด์—ญ (Work Details) describing the implemented features/fixes/refactoring, and include a narrative summary of changes. Reference the commit messages provided (updated game data, layout fix, RxSwift retry logic) to populate the missing content.
Docstring Coverage โš ๏ธ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
โœ… Passed checks (1 passed)
Check name Status Explanation
Title check โœ… Passed The title 'Merge release/3.1.0 into develop' accurately describes the primary intent of the pull requestโ€”merging a release branch into the development branch. It is clear, specific, and directly reflects the changeset.

โœ๏ธ Tip: You can configure your own custom pre-merge checks in the settings.

โœจ Finishing touches
  • ๐Ÿ“ Generate docstrings
๐Ÿงช Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch release/3.1.0

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

โค๏ธ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

Caution

Some comments are outside the diff and canโ€™t be posted inline due to platform limitations.

โš ๏ธ Outside diff range comments (1)
Animal-Crossing-Wiki/Projects/App/Sources/Presentation/Catalog/Views/VariantCell.swift (1)

39-48: โš ๏ธ Potential issue | ๐ŸŸก Minor

Minor: Checkbox briefly visible during reuse for non-checkbox cells.

Setting checkButton.isHidden = false in prepareForReuse means cells that should hide the checkbox will briefly show it until setUp(showCheckbox: false) is called. Consider defaulting to hidden in prepareForReuse:

๐Ÿ”ง Suggested fix
     override func prepareForReuse() {
         super.prepareForReuse()
         imageView.kf.cancelDownloadTask()
         imageView.image = nil
         titleLabel.text = nil
         isCollected = false
-        checkButton.isHidden = false
+        checkButton.isHidden = true
         updateCheckboxImage()
         disposeBag = DisposeBag()
     }
๐Ÿค– Fix all issues with AI agents
In
`@Animal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/CoreDataStorage.xcdatamodeld/UserCollectionEntity.xcdatamodel/contents`:
- Around line 91-99: The NSPersistentCloudKitContainer initialization in
CoreDataStorage.swift must enable automatic lightweight migration so adding
VariantCollectionEntity doesn't crash existing stores; update the container
configuration (the container.persistentStoreDescriptions handling) to set
NSMigratePersistentStoresAutomaticallyOption and
NSInferMappingModelAutomaticallyOption to true for each
persistentStoreDescription before loading stores, or alternatively add explicit
.xcmappingmodel files if lightweight migration is insufficient. Ensure the
changes are applied prior to calling loadPersistentStores on the
NSPersistentCloudKitContainer.

In
`@Animal-Crossing-Wiki/Projects/App/Sources/Presentation/Catalog/ViewModels/ItemDetailReactor.swift`:
- Around line 105-111: fetchCollectedVariants currently snapshots
Items.shared.variantList with .take(1) and can race with async hydration from
Items.setUpUserCollection(); instead, read the authoritative source directly or
wait for hydration: replace the Items.shared.variantList.take(1) usage in the
.fetchCollectedVariants case with a direct synchronous/awaited fetch from
VariantsStorage (e.g. call the VariantsStorage method that returns collected
variant ids or await its hydrated/completion publisher) or subscribe to a
VariantsStorage.hydrated publisher before mapping to
Mutation.setCollectedVariantIds โ€” reference fetchCollectedVariants,
Items.shared.variantList, Items.setUpUserCollection(), and VariantsStorage to
locate and update the logic so the action uses the hydrated storage rather than
a potentially-empty snapshot.

In
`@Animal-Crossing-Wiki/Projects/App/Sources/Presentation/Catalog/Views/ItemVariantsView.swift`:
- Line 71: Replace the single-line guard used in the closure with a multi-line
guard block to satisfy SwiftLint's conditional_returns_on_newline rule: change
the line "guard let self = self else { return }" to a block-style guard (guard
let self = self else { newline return newline }) so the return is on its own
line; update the occurrence inside ItemVariantsView (the closure capturing self)
accordingly.
- Around line 79-84: The subscription on cell.checkboxObservable currently uses
take(1) which unsubscribes after the first toggle; remove the take(1) so the
cell can emit multiple toggles before reuse, i.e. subscribe directly to
cell.checkboxObservable and keep disposing into cell.disposeBag; ensure
VariantCell.prepareForReuse() continues to reset disposeBag so subscriptions are
cleared on reuse and the handler variantCollectionToggled.accept((item,
isCollected)) remains the callback.

In `@Animal-Crossing-Wiki/Projects/App/Sources/SceneDelegate.swift`:
- Around line 52-67: Remove the arbitrary 2-second delay and perform the Core
Data save immediately when entering background: call beginBackgroundTask to
obtain backgroundTaskID, save using
CoreDataStorage.shared.persistentContainer.viewContext synchronously (try
viewContext.save()), and then end the task with
UIApplication.shared.endBackgroundTask(backgroundTaskID). Also update the
expiration handler passed to UIApplication.shared.beginBackgroundTask to capture
the same backgroundTaskID and safely call endBackgroundTask only if
backgroundTaskID != .invalid, then set backgroundTaskID = .invalid after ending
to avoid double-ending. Ensure you remove the DispatchQueue.main.asyncAfter
usage and guard against calling endBackgroundTask on an already-ended task.
๐Ÿงน Nitpick comments (3)
Animal-Crossing-Wiki/Projects/App/Sources/Presentation/Catalog/Views/VariantCell.swift (1)

17-19: Consider making disposeBag private.

The disposeBag property is exposed publicly but appears to only be used internally for subscription cleanup. Making it private would better encapsulate the cell's implementation.

โ™ป๏ธ Suggested change
-    var disposeBag = DisposeBag()
+    private var disposeBag = DisposeBag()
Animal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/VariantsStorage/CoreDataVariantsStorage.swift (1)

110-134: Use saveViewContext() helper for consistency.

The remove() method has inline view context save logic (lines 121-128) that duplicates the saveViewContext() helper. The add() and removeAll() methods already use the helper.

โ™ป๏ธ Proposed refactor
     func remove(_ variantId: String) {
         coreDataStorage.performBackgroundTask { context in
             do {
                 let object = try self.coreDataStorage.getUserCollection(context)
                 let variants = object.variants?.allObjects as? [VariantCollectionEntity] ?? []

                 if let variant = variants.first(where: { $0.variantId == variantId }) {
                     object.removeFromVariants(variant)

                     context.saveContext()
-
-                    DispatchQueue.main.async {
-                        let viewContext = self.coreDataStorage.persistentContainer.viewContext
-                        do {
-                            try viewContext.save()
-                        } catch {
-                            debugPrint(error)
-                        }
-                    }
+                    self.saveViewContext()
                 }
             } catch {
                 debugPrint(error)
             }
         }
     }
Animal-Crossing-Wiki/Projects/App/Sources/Presentation/Catalog/ViewControllers/ItemDetailViewController.swift (1)

40-45: Avoid double fetch on first appearance.
viewDidLoad already dispatches fetch actions; viewDidAppear will fire immediately after and repeat them. Consider gating the first appearance or moving the refresh to subsequent appearances only.

โ™ป๏ธ One way to gate the first appearance
-    private var reactor: ItemDetailReactor?
+    private var reactor: ItemDetailReactor?
+    private var didInitialFetch = false
@@
     override func viewDidAppear(_ animated: Bool) {
         super.viewDidAppear(animated)
-        // ํ™”๋ฉด์ด ๋‹ค์‹œ ๋‚˜ํƒ€๋‚  ๋•Œ ์ˆ˜์ง‘ ์ƒํƒœ ๊ฐฑ์‹ 
-        reactor?.action.onNext(.fetch)
-        reactor?.action.onNext(.fetchCollectedVariants)
+        // ํ™”๋ฉด์ด ๋‹ค์‹œ ๋‚˜ํƒ€๋‚  ๋•Œ ์ˆ˜์ง‘ ์ƒํƒœ ๊ฐฑ์‹ 
+        if didInitialFetch {
+            reactor?.action.onNext(.fetch)
+            reactor?.action.onNext(.fetchCollectedVariants)
+        } else {
+            didInitialFetch = true
+        }
     }

Also applies to: 73-83

- automaticallyMergesChangesFromParent๋กœ ์ž๋™ ๋ณ‘ํ•ฉ๋˜๋ฏ€๋กœ ๋ช…์‹œ์  viewContext.save() ๋ถˆํ•„์š”
- CoreDataItemsStorage, CoreDataVariantsStorage์—์„œ saveViewContext() ๋ฉ”์„œ๋“œ ์ œ๊ฑฐ
- SceneDelegate์—์„œ ๋ถˆํ•„์š”ํ•œ viewContext.save() ์ œ๊ฑฐ (2์ดˆ ๋Œ€๊ธฐ๋งŒ ์œ ์ง€)
- VariantsStorage์—์„œ removeFromVariants ํ›„ context.delete() ์ถ”๊ฐ€ (orphaned entities ๋ฐฉ์ง€)
- ItemVariantsView์—์„œ .take(1) ์ œ๊ฑฐ (disposeBag์ด prepareForReuse์—์„œ ๋ฆฌ์…‹๋˜๋ฏ€๋กœ ๋ถˆํ•„์š”)
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

๐Ÿค– Fix all issues with AI agents
In
`@Animal-Crossing-Wiki/Projects/App/Sources/Presentation/Catalog/Views/ItemVariantsView.swift`:
- Around line 65-84: In setUpItems(by:), ensure the UI binding runs on the main
thread by inserting an observe(on: MainScheduler.instance) into the
collectedVariantIds observable chain before binding to collectionView.rx.items;
specifically, after the collectedVariantIds.map { _ in variations } call and
before .bind(to: collectionView.rx.items(...)), add the observe operator so the
subscription that creates VariantCell via collectionView.rx.items (and uses
cell.setUp and cell.checkboxObservable) receives events on the main scheduler.

In `@Animal-Crossing-Wiki/Projects/App/Sources/SceneDelegate.swift`:
- Around line 52-61: Remove the no-op background task and its 2-second delayed
end: delete the backgroundTaskID variable and the calls to
UIApplication.shared.beginBackgroundTask /
UIApplication.shared.endBackgroundTask as well as the
DispatchQueue.main.asyncAfter block; the app already uses background contexts
via performBackgroundTask with immediate saveContext() and the viewContext
merges changes, so no explicit backgroundTask management is needed.
๐Ÿงน Nitpick comments (2)
Animal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/VariantsStorage/CoreDataVariantsStorage.swift (2)

35-44: Avoid loading all variants for per-item fetches.
Filtering in-memory scales poorly as the collection grows. Prefer a predicate-based fetch for just the itemโ€™s variants.

โ™ป๏ธ Proposed refactor
-                let variantEntities = object.variants?.allObjects as? [VariantCollectionEntity] ?? []
-                let variantIds = Set(variantEntities
-                    .filter { $0.itemName == itemName }
-                    .compactMap { $0.variantId })
+                let request = VariantCollectionEntity.fetchRequest()
+                request.predicate = NSPredicate(format: "itemName == %@", itemName)
+                request.fetchBatchSize = 200
+                let variantEntities = try context.fetch(request)
+                let variantIds = Set(variantEntities.compactMap { $0.variantId })

75-129: Surface write failures instead of debugPrint-only.
Add/remove/removeAll can fail silently; consider returning a Completable (or similar) so callers can handle errors and keep UI state consistent.

- CoreData ์ž๋™ ๋งˆ์ด๊ทธ๋ ˆ์ด์…˜ ์„ค์ • ์ถ”๊ฐ€ (๊ธฐ์กด ์‚ฌ์šฉ์ž ์•ฑ ์—…๋ฐ์ดํŠธ ์‹œ ํฌ๋ž˜์‹œ ๋ฐฉ์ง€)
  - NSMigratePersistentStoresAutomaticallyOption ํ™œ์„ฑํ™”
  - NSInferMappingModelAutomaticallyOption ํ™œ์„ฑํ™”
- ItemDetailReactor.fetchCollectedVariants race condition ์ˆ˜์ •
  - Items.shared.variantList ๋Œ€์‹  variantsStorage.fetchByItem() ์ง์ ‘ ํ˜ธ์ถœ
  - ๋น„๋™๊ธฐ ์ดˆ๊ธฐํ™” ์™„๋ฃŒ ์ „ ๋นˆ ๊ฐ’ ๋ฐ˜ํ™˜ ๋ฌธ์ œ ํ•ด๊ฒฐ
- ItemVariantsView UI ๋ฐ”์ธ๋”ฉ์— MainScheduler ๋ช…์‹œ
  - .observe(on: MainScheduler.instance) ์ถ”๊ฐ€๋กœ ๋ฉ”์ธ ์Šค๋ ˆ๋“œ ๋ณด์žฅ
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

๐Ÿค– Fix all issues with AI agents
In
`@Animal-Crossing-Wiki/Projects/App/Sources/Presentation/Catalog/ViewModels/ItemDetailReactor.swift`:
- Around line 112-137: The toggleVariantCollection handler currently calls
variantsStorage.add/remove (and related Items.shared.updateVariant /
storage.update / Items.shared.updateItem) which each performBackgroundTask on
separate private contexts and can complete out of order; change these calls so
writes are serialized: either update variantsStorage to expose a single shared
background context or serial DispatchQueue and perform the add/remove on that
single queue/context, or change variantsStorage.add/remove to return a
Completable/Single and chain those futures so toggleVariantCollection waits for
the prior write before issuing the next; ensure the same serialization is
applied to Items.shared.updateVariant and storage.update calls so the final
toggle action wins and mutations return only after the serialized write
completes.

Comment on lines +112 to +137
case .toggleVariantCollection(let variant):
HapticManager.shared.impact(style: .medium)
let isCurrentlyCollected = currentState.collectedVariantIds.contains(variant.variantId)
let newState = !isCurrentlyCollected

if newState {
variantsStorage.add(variant.variantId, itemName: currentState.item.name)
Items.shared.updateVariant(variant.variantId, itemName: currentState.item.name, isAdding: true)

// ์•„์ดํ…œ์ด ์ˆ˜์ง‘๋˜์ง€ ์•Š์•˜๋‹ค๋ฉด ํ•จ๊ป˜ ์ˆ˜์ง‘ ์ฒ˜๋ฆฌ
if !currentState.isAcquired {
Items.shared.updateItem(currentState.item)
storage.update(currentState.item)
}
} else {
variantsStorage.remove(variant.variantId)
Items.shared.updateVariant(variant.variantId, itemName: currentState.item.name, isAdding: false)
}

// Variant ์ฒดํฌ ์‹œ ์•„์ดํ…œ๋„ ํ•จ๊ป˜ ์ฒดํฌ๋œ ๊ฒฝ์šฐ ๋‘ mutation ๋ฐ˜ํ™˜
var mutations: [Mutation] = [.updateVariantCollection(variantId: variant.variantId, isCollected: newState)]
if newState && !currentState.isAcquired {
mutations.append(.updateAcquired)
}

return .from(mutations)
Copy link

Choose a reason for hiding this comment

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

โš ๏ธ Potential issue | ๐ŸŸ  Major

๐Ÿงฉ Analysis chain

๐Ÿ Script executed:

find . -name "CoreDataVariantsStorage.swift" -type f

Repository: leeari95/ACNH-wiki

Length of output: 167


๐Ÿ Script executed:

fd "VariantsStorage" --type f

Repository: leeari95/ACNH-wiki

Length of output: 261


๐ŸŒ Web query:

NSPersistentContainer performBackgroundTask execution order guarantee concurrent

๐Ÿ’ก Result:

NSPersistentContainer.performBackgroundTask(_:) does not guarantee execution order across multiple calls, and blocks may run concurrently.

  • Each call creates a new NSManagedObjectContext configured with .privateQueueConcurrencyType (i.e., its own private queue). Because these are separate contexts/queues, two performBackgroundTask blocks can overlap in time and complete in any order. Apple documents the concurrency types and that performBackgroundTask uses private-queue contexts, but does not specify any FIFO/serial ordering guarantee across tasks. [1]
  • In contrast, ordering is guaranteed per context: multiple context.perform { ... } calls on the same NSManagedObjectContext are queued and executed serially (FIFO) on that contextโ€™s private queue. [1][2]

If you need strict ordering / no concurrent writers:

  1. Create one background context (container.newBackgroundContext()), and submit work via bgContext.perform { ... } (or performAndWait). This serializes work on that single contextโ€™s queue. [1][2]
  2. Or, keep using performBackgroundTask, but add your own serial DispatchQueue / OperationQueue to enqueue tasks and only start the next when the previous one finishes (often via completion or performBackgroundTask + DispatchGroup). [2]

Implication: if you fire many performBackgroundTask writes, you should assume concurrent saves are possible, and handle conflicts (merge policy, re-fetch by objectID, etc.) rather than relying on an internal serial queue. [2]

Sources:
[1] Apple Core Data Concurrency Programming Guide (archived): private-queue contexts, and use of performBlock/performBlockAndWait; NSPersistentContainer background contexts are .privateQueueConcurrencyType. (developer.apple.com)
[2] Discussion/confirmation that performBackgroundTask calls can overlap, while perform on the same context is serialized. (stackoverflow.com)

Citations:


๐Ÿ Script executed:

cat -n "Animal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/VariantsStorage/CoreDataVariantsStorage.swift" | sed -n '74,111p'

Repository: leeari95/ACNH-wiki

Length of output: 1723


๐Ÿ Script executed:

cat -n "Animal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/VariantsStorage/CoreDataVariantsStorage.swift" | head -120

Repository: leeari95/ACNH-wiki

Length of output: 5507


๐Ÿ Script executed:

cat -n "Animal-Crossing-Wiki/Projects/App/Sources/Presentation/Catalog/ViewModels/ItemDetailReactor.swift" | sed -n '112,137p'

Repository: leeari95/ACNH-wiki

Length of output: 1492


๐Ÿ Script executed:

grep -A 5 -B 5 "variantsStorage.add\|variantsStorage.remove" "Animal-Crossing-Wiki/Projects/App/Sources/Presentation/Catalog/ViewModels/ItemDetailReactor.swift"

Repository: leeari95/ACNH-wiki

Length of output: 1621


Serialize variant write operations to avoid out-of-order persistence.
variantsStorage.add/remove use performBackgroundTask without ordering guaranteesโ€”each call creates its own private-queue context and can complete in any order. Rapid toggles (collect โ†’ uncollect โ†’ recollect) may persist the wrong final state. Serialize writes using a single background context with context.perform { ... } or return Completable/Single to chain operations so the last action wins.

๐Ÿค– Prompt for AI Agents
In
`@Animal-Crossing-Wiki/Projects/App/Sources/Presentation/Catalog/ViewModels/ItemDetailReactor.swift`
around lines 112 - 137, The toggleVariantCollection handler currently calls
variantsStorage.add/remove (and related Items.shared.updateVariant /
storage.update / Items.shared.updateItem) which each performBackgroundTask on
separate private contexts and can complete out of order; change these calls so
writes are serialized: either update variantsStorage to expose a single shared
background context or serial DispatchQueue and perform the add/remove on that
single queue/context, or change variantsStorage.add/remove to return a
Completable/Single and chain those futures so toggleVariantCollection waits for
the prior write before issuing the next; ensure the same serialization is
applied to Items.shared.updateVariant and storage.update calls so the final
toggle action wins and mutations return only after the serialized write
completes.

- topAnchor constant๋ฅผ -40์—์„œ 0์œผ๋กœ ๋ณ€๊ฒฝํ•˜์—ฌ ๋ ˆ์ด์•„์›ƒ ๊ฐœ์„ 
- APIProvider์— RxSwift Single ๋ฐ˜ํ™˜ ๋ฉ”์„œ๋“œ ์ถ”๊ฐ€
- DefaultAPIProvider์—์„œ Alamofire๋ฅผ RxSwift๋กœ ๋ž˜ํ•‘
- Items.swift์—์„œ retry(when:) ์˜คํผ๋ ˆ์ดํ„ฐ ์‚ฌ์šฉ
- ์ง€์ˆ˜ ๋ฐฑ์˜คํ”„ ์ „๋žต ์ ์šฉ (2์ดˆ, 4์ดˆ, 6์ดˆ ๊ฐ„๊ฒฉ, ์ตœ๋Œ€ 3ํšŒ)
- ์„ ์–ธ์  ์ฝ”๋“œ๋กœ ๊ฐœ์„  (์ˆ˜๋™ ์žฌ๊ท€ ํ˜ธ์ถœ ์ œ๊ฑฐ)
@leeari95 leeari95 merged commit e76f6b2 into develop Feb 6, 2026
5 checks passed
@leeari95 leeari95 deleted the release/3.1.0 branch February 6, 2026 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant