Skip to content

Ongoing bug fixes#43

Open
AndrewG828 wants to merge 7 commits intomainfrom
andrew/bug-fixes
Open

Ongoing bug fixes#43
AndrewG828 wants to merge 7 commits intomainfrom
andrew/bug-fixes

Conversation

@AndrewG828
Copy link
Copy Markdown
Contributor

@AndrewG828 AndrewG828 commented Mar 18, 2026

Look at the commit messages for specific bug fixes.

Summary by CodeRabbit

  • New Features

    • Save/unsave with notification support and local optimistic syncing; new listing events post notifications to refresh profiles.
  • Improvements

    • Refined product details layout, save button behavior, draggable sheet styling and z-ordering.
    • Tighter options menu spacing, improved "For You" empty-state and horizontal list behavior.
    • Onboarding now triggers account creation during Venmo step.
    • Image picker uses confirmation dialogs; new scrollable request layout.
    • Added user profile update helper.
  • Bug Fixes

    • Fixed JSON key mappings for original/altered price fields.
  • Chores

    • Project/scheme metadata updated for newer tooling.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 18, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Project/scheme version bumps and test build setting removals; model JSON CodingKeys changed from snake_case to camelCase for price fields; multiple SwiftUI view refactors and view-model ownership changes to shared singletons; onboarding/user-creation made async; new notification pub/sub for new listings and optimistic local save syncing.

Changes

Cohort / File(s) Summary
Xcode project & scheme
Resell.xcodeproj/project.pbxproj, Resell.xcodeproj/xcshareddata/xcschemes/Resell.xcscheme
Bumped project/scheme version to 1620; removed ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = YES from test target build configurations.
Model CodingKeys
Resell/Models/Listing.swift, Resell/Models/Post.swift, Resell/Models/Transaction.swift
Changed JSON key mappings for price fields from snake_case (original_price, altered_price) to camelCase (originalPrice, alteredPrice).
Draggable sheet & options
Resell/Views/Components/DraggableSheetView.swift, Resell/Views/Components/OptionsMenuView.swift
Replaced background/cornerRadius with RoundedRectangle.fill, moved dragOffset reset into animation block, and set VStack(spacing: 0) for options layout.
Product details & save flow
Resell/Views/ProductDetails/ProductDetailsView.swift, Resell/ViewModels/ProductDetailsViewModel.swift, Resell/ViewModels/HomeViewModel.swift
Added homeViewModel as @ObservedObject, layered views with zIndex, implemented saveButton flow that toggles isSaved, calls async updateItemSaved(), uses toggleLocalSaveStatus(...) for optimistic local updates and re-sync, and conditionally requests/sends notifications; updateItemSaved() converted to async; toggleLocalSaveStatus added.
Home views: ownership & layout
Resell/Views/Home/...
Resell/Views/Home/ForYouView.swift, Resell/Views/Home/HomeView.swift, Resell/Views/Home/SavedView.swift, Resell/Views/Home/FilterView.swift, Resell/Views/Home/DetailedFilterView.swift
Converted several views from @StateObject/@EnvironmentObject to @ObservedObject using HomeViewModel.shared; ForYouView now conditionally renders the horizontal ScrollView vs empty state and adjusted paddings.
Onboarding: profile & Venmo flow
Resell/ViewModels/SetupProfileViewModel.swift, Resell/Views/Onboarding/SetupProfileView.swift, Resell/Views/Onboarding/VenmoView.swift
createNewUser() became async -> Bool with stricter validation and explicit error returns; SetupProfile Next no longer calls creation; VenmoView invokes createNewUser() in async tasks on Continue/Skip and navigates on result.
New listing & notifications
Resell/ViewModels/NewListingViewModel.swift, Resell/ViewModels/CurrentUserProfileManager.swift, Resell/Utils/Constants.swift
After creating a listing, NewListingCreated notification is posted; CurrentUserProfileManager subscribes and triggers loadProfile(forceRefresh: true) on receipt; added Constants.Notifications.NewListingCreated.
New listing UI & request views
Resell/Views/NewListing/NewListingImagesView.swift, Resell/Views/NewListing/NewRequestView.swift
Replaced .actionSheet with .confirmationDialog for image picker actions and wrapped NewRequestView content in ScrollView; added centralized dialogButtons view builder.
User model helper
Resell/Models/User.swift
Added updatingProfile(newUsername:newBio:newVenmoHandle:newPhotoUrl:) to produce an updated User value preserving non-profile fields.

Sequence Diagram

sequenceDiagram
    participant User
    participant ProductDetailsView
    participant ProductDetailsVM
    participant HomeViewModel
    participant Network
    participant NotificationService

    User->>ProductDetailsView: Tap Save button
    ProductDetailsView->>ProductDetailsVM: updateItemSaved() (async)
    ProductDetailsVM->>Network: send save/unsave request
    Network-->>ProductDetailsVM: response
    alt success
        ProductDetailsVM-->>ProductDetailsView: update isSaved
        ProductDetailsView->>HomeViewModel: toggleLocalSaveStatus(post, isSaving) (async)
        HomeViewModel->>Network: getSavedPosts(forceRefresh: true)
        Network-->>HomeViewModel: saved posts
        HomeViewModel-->>ProductDetailsView: confirm local state
        ProductDetailsView->>NotificationService: check authorization / request if needed
        alt authorized
            ProductDetailsView->>NotificationService: schedule/display notification
            NotificationService-->>User: show confirmation
        else not authorized
            NotificationService-->>ProductDetailsView: prompt result
        end
    else failure
        ProductDetailsVM-->>ProductDetailsView: log/error
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I hopped through keys and camel-case trails,

Rounded sheets and async sails,
A save that nudged the local store,
A bell to ring and then explore,
I thumped my foot — new listing tales.

🚥 Pre-merge checks | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description does not follow the required template. It lacks Overview, detailed Changes Made sections, Test Coverage, and other required sections, providing only a vague instruction to check commit messages. Follow the template by filling in the Overview, detailed Changes Made with specific subsections, Test Coverage, and other required sections instead of deferring to commit messages.
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title "Ongoing bug fixes" is too vague and generic, providing no meaningful information about the specific changes or primary purpose of this changeset. Use a more specific title that describes the primary change, such as "Fix profile display after onboarding" or "Add notification system for profile refresh on new listings".

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch andrew/bug-fixes

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.

@AndrewG828 AndrewG828 requested review from MrPeterss and Xhether March 18, 2026 20:40
Copy link
Copy Markdown

@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

Caution

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

⚠️ Outside diff range comments (1)
Resell/Views/ProductDetails/ProductDetailsView.swift (1)

381-397: ⚠️ Potential issue | 🟡 Minor

Thread-safety concern with @AppStorage inside function.

Declaring @AppStorage inside requestNotificationAuthorization() (line 382) is unconventional. The property wrapper is typically declared at the view level. While this may work, the closure captures it by reference after the function returns, which could lead to unexpected behavior.

Consider using the existing view-level isNotificationAuthorized property (line 399) instead:

Proposed fix
 func requestNotificationAuthorization() {
-    `@AppStorage`("isNotificationAuthorized") var isNotificationAuthorized = false
     UNUserNotificationCenter.current().requestAuthorization(options: [.alert, .sound, .badge]) { (granted, error) in
         if let error = error {
             print("Error sending notification: \(error.localizedDescription)")
             return
         }

         if granted {
-            isNotificationAuthorized = true
+            DispatchQueue.main.async {
+                self.isNotificationAuthorized = true
+            }
             print("Notification permission granted.")
         } else {
-            isNotificationAuthorized = false
+            DispatchQueue.main.async {
+                self.isNotificationAuthorized = false
+            }
             print("Notification permission denied.")
         }
     }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resell/Views/ProductDetails/ProductDetailsView.swift` around lines 381 - 397,
The local declaration of `@AppStorage` inside requestNotificationAuthorization()
is unsafe; remove the in-function "@AppStorage(\"isNotificationAuthorized\") var
isNotificationAuthorized" and instead update the view-level
isNotificationAuthorized property already declared on the view
(isNotificationAuthorized) from within the UNUserNotificationCenter completion
handler; ensure you marshal UI/state changes to the main thread (e.g., wrap
assignments to isNotificationAuthorized inside DispatchQueue.main.async) and
keep the existing error handling and prints in the same closure.
🧹 Nitpick comments (3)
Resell/Views/ProductDetails/ProductDetailsView.swift (1)

401-440: Reduce code duplication in saveButton and remove debug print.

The saveButton has duplicated UI code between the if and else branches—only the action differs. Additionally, line 425 contains a debug print("Test1") statement that should be removed.

♻️ Proposed refactor
 private var saveButton: some View {
-    if isNotificationAuthorized {
-        Button {
-            viewModel.isSaved.toggle()
-            viewModel.updateItemSaved()
+    Button {
+        viewModel.isSaved.toggle()
+        viewModel.updateItemSaved()
+        if isNotificationAuthorized {
             sendNotification()
-        } label: {
-            ZStack {
-                Circle()
-                    .frame(width: 72, height: 72)
-                    .foregroundStyle(Constants.Colors.white)
-                    .opacity(viewModel.isSaved ? 1.0 : 0.9)
-                    .shadow(radius: 2)
-
-                Image(viewModel.isSaved ? "saved.fill" : "saved")
-                    .resizable()
-                    .frame(width: 21, height: 27)
-            }
+        } else {
+            requestNotificationAuthorization()
         }
-    } else {
-        Button {
-            viewModel.isSaved.toggle()
-            viewModel.updateItemSaved()
-            requestNotificationAuthorization()
-            print("Test1")
-        } label: {
-            ZStack {
-                Circle()
-                    .frame(width: 72, height: 72)
-                    .foregroundStyle(Constants.Colors.white)
-                    .opacity(viewModel.isSaved ? 1.0 : 0.9)
-                    .shadow(radius: 2)
-
-                Image(viewModel.isSaved ? "saved.fill" : "saved")
-                    .resizable()
-                    .frame(width: 21, height: 27)
-            }
+    } label: {
+        ZStack {
+            Circle()
+                .frame(width: 72, height: 72)
+                .foregroundStyle(Constants.Colors.white)
+                .opacity(viewModel.isSaved ? 1.0 : 0.9)
+                .shadow(radius: 2)
+
+            Image(viewModel.isSaved ? "saved.fill" : "saved")
+                .resizable()
+                .frame(width: 21, height: 27)
         }
     }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resell/Views/ProductDetails/ProductDetailsView.swift` around lines 401 - 440,
The saveButton currently duplicates the button label UI in both branches and
contains a debug print; refactor by creating one Button whose label is the
shared ZStack (reuse the Circle and Image UI from both branches) and make the
action choose between calling sendNotification() or
requestNotificationAuthorization() based on isNotificationAuthorized while still
toggling viewModel.isSaved and calling viewModel.updateItemSaved(); remove the
debug print("Test1") and ensure the unique symbols referenced are saveButton,
viewModel.isSaved.toggle(), viewModel.updateItemSaved(), sendNotification(), and
requestNotificationAuthorization().
Resell/Models/Transaction.swift (1)

130-131: Redundant explicit CodingKey mappings.

These CodingKey mappings are redundant since originalPrice and alteredPrice property names already match their JSON key values. They could be simplified by adding them to line 129:

 private enum CodingKeys: String, CodingKey {
-    case id, title, images, description, condition, sold
-    case originalPrice = "originalPrice"
-    case alteredPrice = "alteredPrice"
+    case id, title, images, description, condition, sold, originalPrice, alteredPrice
 }

This is a minor stylistic preference—the current code is functionally correct.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resell/Models/Transaction.swift` around lines 130 - 131, The CodingKeys enum
in Transaction redundantly maps originalPrice and alteredPrice to identical JSON
keys; simplify by removing their explicit case lines and instead include
originalPrice and alteredPrice in the existing CodingKeys declaration (or rely
on synthesized keys) so Transaction.CodingKeys no longer contains redundant
mappings for originalPrice and alteredPrice.
Resell/Models/Post.swift (1)

104-104: Lines 28-29 contain redundant CodingKey mappings that can be simplified.

In the Post struct, lines 28-29 explicitly map originalPrice and alteredPrice to JSON keys with the same names:

case originalPrice = "originalPrice"
case alteredPrice = "alteredPrice"

Since the Swift property names already match the JSON key names, these explicit mappings are redundant. Simplify to:

case originalPrice, alteredPrice

Additionally, the PostBody struct (line 95) uses a snake_case property name (original_price) that maps to camelCase JSON ("originalPrice"). While this works, consider aligning property naming conventions with the JSON format for consistency across the models.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resell/Models/Post.swift` at line 104, In the Post struct update the
CodingKeys to remove redundant explicit mappings by collapsing `case
originalPrice = "originalPrice"` and `case alteredPrice = "alteredPrice"` into a
single line `case originalPrice, alteredPrice` within the `CodingKeys` enum;
also review the `PostBody` struct and rename the snake_case property
`original_price` to `originalPrice` (and adjust its CodingKeys or remove the
mapping if names now match) so property naming aligns with the JSON camelCase
convention and avoids unnecessary key mappings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Resell/Views/ProductDetails/ProductDetailsView.swift`:
- Around line 69-70: The MARK comment in ProductDetailsView.swift is using an
invalid format; update the line "// MARK - Need to get a real shareable url (put
in post or viewmodel)" to a valid SwiftLint pattern such as "// MARK: - Need to
get a real shareable url (put in post or viewmodel)" or replace it with a task
comment like "// TODO: Need to get a real shareable url (put in post or
viewmodel)"; ensure the nearby .share(url: "TEST", itemName:
viewModel.item?.title ?? "cool item") comment stays intact.

---

Outside diff comments:
In `@Resell/Views/ProductDetails/ProductDetailsView.swift`:
- Around line 381-397: The local declaration of `@AppStorage` inside
requestNotificationAuthorization() is unsafe; remove the in-function
"@AppStorage(\"isNotificationAuthorized\") var isNotificationAuthorized" and
instead update the view-level isNotificationAuthorized property already declared
on the view (isNotificationAuthorized) from within the UNUserNotificationCenter
completion handler; ensure you marshal UI/state changes to the main thread
(e.g., wrap assignments to isNotificationAuthorized inside
DispatchQueue.main.async) and keep the existing error handling and prints in the
same closure.

---

Nitpick comments:
In `@Resell/Models/Post.swift`:
- Line 104: In the Post struct update the CodingKeys to remove redundant
explicit mappings by collapsing `case originalPrice = "originalPrice"` and `case
alteredPrice = "alteredPrice"` into a single line `case originalPrice,
alteredPrice` within the `CodingKeys` enum; also review the `PostBody` struct
and rename the snake_case property `original_price` to `originalPrice` (and
adjust its CodingKeys or remove the mapping if names now match) so property
naming aligns with the JSON camelCase convention and avoids unnecessary key
mappings.

In `@Resell/Models/Transaction.swift`:
- Around line 130-131: The CodingKeys enum in Transaction redundantly maps
originalPrice and alteredPrice to identical JSON keys; simplify by removing
their explicit case lines and instead include originalPrice and alteredPrice in
the existing CodingKeys declaration (or rely on synthesized keys) so
Transaction.CodingKeys no longer contains redundant mappings for originalPrice
and alteredPrice.

In `@Resell/Views/ProductDetails/ProductDetailsView.swift`:
- Around line 401-440: The saveButton currently duplicates the button label UI
in both branches and contains a debug print; refactor by creating one Button
whose label is the shared ZStack (reuse the Circle and Image UI from both
branches) and make the action choose between calling sendNotification() or
requestNotificationAuthorization() based on isNotificationAuthorized while still
toggling viewModel.isSaved and calling viewModel.updateItemSaved(); remove the
debug print("Test1") and ensure the unique symbols referenced are saveButton,
viewModel.isSaved.toggle(), viewModel.updateItemSaved(), sendNotification(), and
requestNotificationAuthorization().

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 65b35089-82ba-421a-8836-c80cf89b7bef

📥 Commits

Reviewing files that changed from the base of the PR and between 11e7997 and 5173552.

⛔ Files ignored due to path filters (1)
  • .DS_Store is excluded by !**/.DS_Store
📒 Files selected for processing (8)
  • Resell.xcodeproj/project.pbxproj
  • Resell.xcodeproj/xcshareddata/xcschemes/Resell.xcscheme
  • Resell/Models/Listing.swift
  • Resell/Models/Post.swift
  • Resell/Models/Transaction.swift
  • Resell/Views/Components/DraggableSheetView.swift
  • Resell/Views/Components/OptionsMenuView.swift
  • Resell/Views/ProductDetails/ProductDetailsView.swift

Comment on lines +69 to +70
// MARK - Need to get a real shareable url (put in post or viewmodel)
// .share(url: "TEST", itemName: viewModel.item?.title ?? "cool item"),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix MARK comment format.

The MARK comment on line 69 uses an invalid format. Per SwiftLint, it should follow the pattern // MARK: ... or // MARK: - ....

Proposed fix
-                        // MARK - Need to get a real shareable url (put in post or viewmodel)
+                        // MARK: - TODO: Need to get a real shareable url (put in post or viewmodel)

Or simply use a regular // TODO: comment since this is a task reminder rather than a section marker.

📝 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
// MARK - Need to get a real shareable url (put in post or viewmodel)
// .share(url: "TEST", itemName: viewModel.item?.title ?? "cool item"),
// MARK: - TODO: Need to get a real shareable url (put in post or viewmodel)
// .share(url: "TEST", itemName: viewModel.item?.title ?? "cool item"),
🧰 Tools
🪛 SwiftLint (0.63.2)

[Warning] 69-69: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'

(mark)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resell/Views/ProductDetails/ProductDetailsView.swift` around lines 69 - 70,
The MARK comment in ProductDetailsView.swift is using an invalid format; update
the line "// MARK - Need to get a real shareable url (put in post or viewmodel)"
to a valid SwiftLint pattern such as "// MARK: - Need to get a real shareable
url (put in post or viewmodel)" or replace it with a task comment like "// TODO:
Need to get a real shareable url (put in post or viewmodel)"; ensure the nearby
.share(url: "TEST", itemName: viewModel.item?.title ?? "cool item") comment
stays intact.

Copy link
Copy Markdown

@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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Resell/Views/Home/ForYouView.swift`:
- Around line 56-57: The empty-state container in ForYouView currently uses a
fixed .frame(height: 110) which can clip at larger Dynamic Type sizes; remove
the fixed height and make the container flexible (for example replace the fixed
height with a .frame(minHeight: 110) or let the view size itself by removing the
frame and using padding/flexible layout), ensure the empty-state view (the view
with the modifier chain containing .frame(height: 110) and .padding(.horizontal,
24)) can grow for accessibility by using minHeight or intrinsic sizing so text
isn't clipped.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7c34137b-6af2-4afa-9d8c-33d14016a1cc

📥 Commits

Reviewing files that changed from the base of the PR and between 5173552 and 16bcfbf.

📒 Files selected for processing (1)
  • Resell/Views/Home/ForYouView.swift

Comment on lines +56 to +57
.frame(height: 110)
.padding(.horizontal, 24)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Avoid fixed-height empty-state container for Dynamic Type.

frame(height: 110) can clip text at larger accessibility sizes. Prefer a flexible height so the empty state remains readable.

Suggested adjustment
-                .frame(height: 110)
+                .frame(minHeight: 110)
+                .frame(maxWidth: .infinity, alignment: .leading)
📝 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
.frame(height: 110)
.padding(.horizontal, 24)
.frame(minHeight: 110)
.frame(maxWidth: .infinity, alignment: .leading)
.padding(.horizontal, 24)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resell/Views/Home/ForYouView.swift` around lines 56 - 57, The empty-state
container in ForYouView currently uses a fixed .frame(height: 110) which can
clip at larger Dynamic Type sizes; remove the fixed height and make the
container flexible (for example replace the fixed height with a
.frame(minHeight: 110) or let the view size itself by removing the frame and
using padding/flexible layout), ensure the empty-state view (the view with the
modifier chain containing .frame(height: 110) and .padding(.horizontal, 24)) can
grow for accessibility by using minHeight or intrinsic sizing so text isn't
clipped.

Copy link
Copy Markdown

@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

🧹 Nitpick comments (4)
Resell/Views/Onboarding/SetupProfileView.swift (1)

39-42: Remove commented-out code.

The commented-out viewModel.createNewUser() call is dead code since user creation has been moved to VenmoView. Remove it to keep the codebase clean and avoid confusion for future readers.

♻️ Proposed cleanup
 PurpleButton(isActive: viewModel.checkInputIsValid(), text: "Next", horizontalPadding: 80) {
-//                viewModel.createNewUser()
     router.push(.venmo)
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resell/Views/Onboarding/SetupProfileView.swift` around lines 39 - 42, Remove
the dead commented-out call to viewModel.createNewUser() inside the PurpleButton
action in SetupProfileView; the button currently calls router.push(.venmo) and
user creation was moved to VenmoView, so delete the commented line to clean up
the code and avoid confusion while leaving the PurpleButton(...){
router.push(.venmo) } logic intact.
Resell/Views/Onboarding/VenmoView.swift (2)

85-92: Remove commented-out observer code.

The .onChange(of: viewModel.isLoading) observer is no longer needed since the navigation logic is now driven by the createNewUser() result. Remove this commented-out block to keep the code clean.

♻️ Proposed cleanup
         }
-//        .onChange(of: viewModel.isLoading) { newValue in
-//            if !newValue {
-//                withAnimation {
-//                    router.popToRoot()
-//                    userDidLogin = true
-//                }
-//            }
-//        }
         .endEditingOnTap()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resell/Views/Onboarding/VenmoView.swift` around lines 85 - 92, Remove the
dead/commented observer block that watches viewModel.isLoading; delete the
commented .onChange(of: viewModel.isLoading) { ... } block (which contains
router.popToRoot() and userDidLogin assignment) because navigation is now driven
by the createNewUser() result—leave createNewUser(), router.popToRoot(),
viewModel, and userDidLogin intact but remove the commented code to clean up the
file.

34-64: Duplicated async flow logic between Continue and Skip handlers.

Both button handlers contain identical Task/success handling logic. Consider extracting the common flow to reduce duplication and simplify future maintenance.

♻️ Proposed extraction

Add a helper method and simplify both handlers:

private func performUserCreation() {
    viewModel.isLoading = true
    
    Task {
        let success = await viewModel.createNewUser()
        viewModel.isLoading = false
        
        if success {
            withAnimation {
                router.popToRoot()
                userDidLogin = true
            }
        }
    }
}

Then the button handlers become:

 PurpleButton(isLoading: viewModel.isLoading, isActive: !viewModel.venmoHandle.cleaned().isEmpty, text: "Continue") {
-    viewModel.isLoading = true
-    
-    Task {
-        let success = await viewModel.createNewUser()
-        viewModel.isLoading = false
-        
-        if success {
-            withAnimation {
-                router.popToRoot()
-                userDidLogin = true
-            }
-        }
-    }
+    performUserCreation()
 }

 Button(action: {
     viewModel.venmoHandle = ""
-    viewModel.isLoading = true
-    
-    Task {
-        let success = await viewModel.createNewUser()
-        viewModel.isLoading = false
-        
-        if success {
-            withAnimation {
-                router.popToRoot()
-                userDidLogin = true
-            }
-        }
-    }
+    performUserCreation()
 }, label: {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resell/Views/Onboarding/VenmoView.swift` around lines 34 - 64, Extract the
duplicated async flow from both the PurpleButton and the Skip Button into a
single helper on the VenmoView (e.g., performUserCreation) that sets
viewModel.isLoading = true, runs the Task that awaits viewModel.createNewUser(),
sets isLoading = false, and on success runs withAnimation { router.popToRoot();
userDidLogin = true }; then call this helper from the PurpleButton action and
from the Button(action:) after resetting viewModel.venmoHandle so both handlers
reuse the same logic.
Resell/ViewModels/SetupProfileViewModel.swift (1)

38-41: UIImage reference comparison is fragile.

Comparing UIImage instances with != relies on UIKit's image caching behavior to return the same instance for UIImage(named:). This is an implementation detail and may not hold in all scenarios (e.g., memory pressure purging the cache).

Consider tracking image selection state explicitly with a boolean flag, or comparing against nil only since updateUserProfile already handles the selection flow.

♻️ Alternative approach using explicit state
+    `@Published` var hasSelectedCustomImage: Bool = false
     `@Published` var selectedImage: UIImage? = nil

Then in checkInputIsValid():

     func checkInputIsValid() -> Bool {
-        let hasImage = selectedImage != nil && selectedImage != UIImage(named: "emptyProfile")
-        
-        return !(username.cleaned().isEmpty || bio.cleaned().isEmpty) && didAgreeWithEULA && hasImage
+        return !(username.cleaned().isEmpty || bio.cleaned().isEmpty) && didAgreeWithEULA && hasSelectedCustomImage
     }

And set hasSelectedCustomImage = true in updateUserProfile when a valid image is loaded.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resell/ViewModels/SetupProfileViewModel.swift` around lines 38 - 41, The
current checkInputIsValid() compares selectedImage to UIImage(named:
"emptyProfile"), which is fragile; instead add an explicit Bool property (e.g.,
hasSelectedCustomImage) to track whether the user picked a custom image, update
that flag to true inside updateUserProfile (or wherever a custom image is
successfully loaded) and set it false for the default/no-image state, then
change checkInputIsValid() to use hasSelectedCustomImage (and only compare
selectedImage to nil if needed) rather than relying on UIImage(named:) identity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Resell/ViewModels/SetupProfileViewModel.swift`:
- Around line 83-85: The method currently returns false silently if
selectedImage?.resizedToMaxDimension(256).toBase64() is nil; update the flow in
the function that performs image encoding (reference the
selectedImage?.resizedToMaxDimension(256).toBase64() call) to detect a nil
result, call presentError(...) with a user-facing message about image
encoding/processing failure, and then return false—ensuring presentError() is
invoked before exiting so the user gets feedback when image encoding fails.

---

Nitpick comments:
In `@Resell/ViewModels/SetupProfileViewModel.swift`:
- Around line 38-41: The current checkInputIsValid() compares selectedImage to
UIImage(named: "emptyProfile"), which is fragile; instead add an explicit Bool
property (e.g., hasSelectedCustomImage) to track whether the user picked a
custom image, update that flag to true inside updateUserProfile (or wherever a
custom image is successfully loaded) and set it false for the default/no-image
state, then change checkInputIsValid() to use hasSelectedCustomImage (and only
compare selectedImage to nil if needed) rather than relying on UIImage(named:)
identity.

In `@Resell/Views/Onboarding/SetupProfileView.swift`:
- Around line 39-42: Remove the dead commented-out call to
viewModel.createNewUser() inside the PurpleButton action in SetupProfileView;
the button currently calls router.push(.venmo) and user creation was moved to
VenmoView, so delete the commented line to clean up the code and avoid confusion
while leaving the PurpleButton(...){ router.push(.venmo) } logic intact.

In `@Resell/Views/Onboarding/VenmoView.swift`:
- Around line 85-92: Remove the dead/commented observer block that watches
viewModel.isLoading; delete the commented .onChange(of: viewModel.isLoading) {
... } block (which contains router.popToRoot() and userDidLogin assignment)
because navigation is now driven by the createNewUser() result—leave
createNewUser(), router.popToRoot(), viewModel, and userDidLogin intact but
remove the commented code to clean up the file.
- Around line 34-64: Extract the duplicated async flow from both the
PurpleButton and the Skip Button into a single helper on the VenmoView (e.g.,
performUserCreation) that sets viewModel.isLoading = true, runs the Task that
awaits viewModel.createNewUser(), sets isLoading = false, and on success runs
withAnimation { router.popToRoot(); userDidLogin = true }; then call this helper
from the PurpleButton action and from the Button(action:) after resetting
viewModel.venmoHandle so both handlers reuse the same logic.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 06963465-15f6-4564-a120-1437231a519b

📥 Commits

Reviewing files that changed from the base of the PR and between 16bcfbf and c272fe9.

📒 Files selected for processing (3)
  • Resell/ViewModels/SetupProfileViewModel.swift
  • Resell/Views/Onboarding/SetupProfileView.swift
  • Resell/Views/Onboarding/VenmoView.swift

Comment thread Resell/ViewModels/SetupProfileViewModel.swift
… with db, and fixed homeViewModel object passing within the codebase.
Copy link
Copy Markdown

@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

Caution

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

⚠️ Outside diff range comments (1)
Resell/Views/Home/FilterView.swift (1)

248-254: ⚠️ Potential issue | 🟡 Minor

Sheet dismisses before filters finish applying.

The isPresented = false on line 253 executes immediately after spawning the Task, not after applyFilters completes. This can cause the sheet to dismiss while the network request is still in flight, potentially leading to a jarring UX if the filter results appear later.

Proposed fix
 Button{
     Task {
         try await filtersVM.applyFilters(homeViewModel: homeViewModel)
+        isPresented = false
     }
-    // MARK: This should wait for the above request to complete
-    isPresented = false
 } label: {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resell/Views/Home/FilterView.swift` around lines 248 - 254, The sheet is
dismissed immediately because isPresented = false runs synchronously after
spawning Task; move the dismissal into the Task after the await so it runs only
when filtersVM.applyFilters completes. Concretely, inside the Button action wrap
the call in Task { do { try await filtersVM.applyFilters(homeViewModel:
homeViewModel); await MainActor.run { isPresented = false } } catch { await
MainActor.run { /* handle error, optionally keep sheet open or show error */ } }
} or simply place isPresented = false directly after the try await inside the
Task so dismissal occurs only after applyFilters returns; reference
filtersVM.applyFilters, Task and isPresented.
🧹 Nitpick comments (3)
Resell/ViewModels/HomeViewModel.swift (1)

146-153: Minor formatting: missing space before {.

-    func getSavedPosts(forceRefresh: Bool = false) async  {
-        if !forceRefresh{
+    func getSavedPosts(forceRefresh: Bool = false) async {
+        if !forceRefresh {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resell/ViewModels/HomeViewModel.swift` around lines 146 - 153, In
getSavedPosts(forceRefresh: Bool = false) async the opening brace is missing a
space; update the function declaration to include a space before the `{` (i.e.,
change "async  {" to "async {") so formatting is consistent—verify near symbols
lastSavedFetchTime, cacheValidityDuration, and savedItems to ensure no other
stray spacing issues.
Resell/Views/Home/SavedView.swift (1)

13-14: Remove commented-out code.

The commented @EnvironmentObject line should be removed to keep the codebase clean.

Proposed fix
 `@EnvironmentObject` var router: Router
-//    `@EnvironmentObject` private var viewModel: HomeViewModel
 `@ObservedObject` private var viewModel = HomeViewModel.shared
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resell/Views/Home/SavedView.swift` around lines 13 - 14, Remove the
commented-out declaration "@EnvironmentObject private var viewModel:
HomeViewModel" to keep the file clean; keep the active "@ObservedObject private
var viewModel = HomeViewModel.shared" as-is, and ensure no leftover commented
references to EnvironmentObject remain in the file.
Resell/Views/ProductDetails/ProductDetailsView.swift (1)

403-451: Reduce code duplication in save button and add rollback on failure.

The two branches of saveButton are nearly identical, differing only in the notification handling. Consider extracting the common logic. Additionally, if updateItemSaved() fails, viewModel.isSaved remains toggled but the server state is unchanged, causing UI desynchronization.

Proposed refactor
 private var saveButton: some View {
-    if isNotificationAuthorized {
-        Button {
-            viewModel.isSaved.toggle()
-            
-            Task {
-                await viewModel.updateItemSaved()
-                await homeViewModel.toggleLocalSaveStatus(for: post, isSaving: viewModel.isSaved)
-            }
-            
-            sendNotification()
-        } label: {
-            ZStack {
-                Circle()
-                    .frame(width: 72, height: 72)
-                    .foregroundStyle(Constants.Colors.white)
-                    .opacity(viewModel.isSaved ? 1.0 : 0.9)
-                    .shadow(radius: 2)
-
-                Image(viewModel.isSaved ? "saved.fill" : "saved")
-                    .resizable()
-                    .frame(width: 21, height: 27)
-            }
-        }
-    } else {
-        Button {
-            viewModel.isSaved.toggle()
-            
-            Task {
+    Button {
+        let previousState = viewModel.isSaved
+        viewModel.isSaved.toggle()
+        
+        Task {
+            do {
                 await viewModel.updateItemSaved()
                 await homeViewModel.toggleLocalSaveStatus(for: post, isSaving: viewModel.isSaved)
+            } catch {
+                // Rollback on failure
+                viewModel.isSaved = previousState
             }
-            
-            requestNotificationAuthorization()
-        } label: {
-            ZStack {
-                Circle()
-                    .frame(width: 72, height: 72)
-                    .foregroundStyle(Constants.Colors.white)
-                    .opacity(viewModel.isSaved ? 1.0 : 0.9)
-                    .shadow(radius: 2)
-
-                Image(viewModel.isSaved ? "saved.fill" : "saved")
-                    .resizable()
-                    .frame(width: 21, height: 27)
-            }
         }
+        
+        if isNotificationAuthorized {
+            sendNotification()
+        } else {
+            requestNotificationAuthorization()
+        }
+    } label: {
+        ZStack {
+            Circle()
+                .frame(width: 72, height: 72)
+                .foregroundStyle(Constants.Colors.white)
+                .opacity(viewModel.isSaved ? 1.0 : 0.9)
+                .shadow(radius: 2)
+
+            Image(viewModel.isSaved ? "saved.fill" : "saved")
+                .resizable()
+                .frame(width: 21, height: 27)
+        }
     }
 }

Note: The updateItemSaved() method currently doesn't throw errors to the caller. Consider having it rethrow for proper error handling, or check success internally.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resell/Views/ProductDetails/ProductDetailsView.swift` around lines 403 - 451,
The two branches of saveButton duplicate the toggle/update/send flow; extract
the common save action into a single helper (e.g., a private func
performSaveAction(notify: Bool) or closure) that toggles viewModel.isSaved,
calls await viewModel.updateItemSaved() and await
homeViewModel.toggleLocalSaveStatus(for: post, isSaving: viewModel.isSaved),
then either sendNotification() or requestNotificationAuthorization() based on a
notify flag; also add rollback logic: make updateItemSaved() surface failure
(either rethrow or return Bool) so performSaveAction can detect failure and
revert viewModel.isSaved and local state if the server update fails. Ensure UI
button uses the single helper and only differs by the notify flag.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Resell/ViewModels/HomeViewModel.swift`:
- Around line 168-188: The optimistic-update rollback is vulnerable to race
conditions in toggleLocalSaveStatus(for:isSaving:): capture of prevSavedItems
can overwrite newer changes if multiple calls run concurrently; fix by
serializing/guarding operations—e.g., add a monotonic operation counter or UUID
property (e.g., saveOperationID or currentSaveToken) on the ViewModel,
increment/generate it at the start of toggleLocalSaveStatus, capture the token
in the async call, and on error only restore prevSavedItems if the captured
token matches the current token (otherwise ignore rollback); alternatively (or
additionally) disable the save button or debounce it while an operation is in
flight so NetworkManager.shared.getSavedPosts results and lastSavedFetchTime are
only applied when the operation token is still current.

---

Outside diff comments:
In `@Resell/Views/Home/FilterView.swift`:
- Around line 248-254: The sheet is dismissed immediately because isPresented =
false runs synchronously after spawning Task; move the dismissal into the Task
after the await so it runs only when filtersVM.applyFilters completes.
Concretely, inside the Button action wrap the call in Task { do { try await
filtersVM.applyFilters(homeViewModel: homeViewModel); await MainActor.run {
isPresented = false } } catch { await MainActor.run { /* handle error,
optionally keep sheet open or show error */ } } } or simply place isPresented =
false directly after the try await inside the Task so dismissal occurs only
after applyFilters returns; reference filtersVM.applyFilters, Task and
isPresented.

---

Nitpick comments:
In `@Resell/ViewModels/HomeViewModel.swift`:
- Around line 146-153: In getSavedPosts(forceRefresh: Bool = false) async the
opening brace is missing a space; update the function declaration to include a
space before the `{` (i.e., change "async  {" to "async {") so formatting is
consistent—verify near symbols lastSavedFetchTime, cacheValidityDuration, and
savedItems to ensure no other stray spacing issues.

In `@Resell/Views/Home/SavedView.swift`:
- Around line 13-14: Remove the commented-out declaration "@EnvironmentObject
private var viewModel: HomeViewModel" to keep the file clean; keep the active
"@ObservedObject private var viewModel = HomeViewModel.shared" as-is, and ensure
no leftover commented references to EnvironmentObject remain in the file.

In `@Resell/Views/ProductDetails/ProductDetailsView.swift`:
- Around line 403-451: The two branches of saveButton duplicate the
toggle/update/send flow; extract the common save action into a single helper
(e.g., a private func performSaveAction(notify: Bool) or closure) that toggles
viewModel.isSaved, calls await viewModel.updateItemSaved() and await
homeViewModel.toggleLocalSaveStatus(for: post, isSaving: viewModel.isSaved),
then either sendNotification() or requestNotificationAuthorization() based on a
notify flag; also add rollback logic: make updateItemSaved() surface failure
(either rethrow or return Bool) so performSaveAction can detect failure and
revert viewModel.isSaved and local state if the server update fails. Ensure UI
button uses the single helper and only differs by the notify flag.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f5a62770-0aff-4543-8d14-76d673f6bd1b

📥 Commits

Reviewing files that changed from the base of the PR and between c272fe9 and 31fef14.

📒 Files selected for processing (8)
  • Resell/ViewModels/HomeViewModel.swift
  • Resell/ViewModels/ProductDetailsViewModel.swift
  • Resell/Views/Home/DetailedFilterView.swift
  • Resell/Views/Home/FilterView.swift
  • Resell/Views/Home/ForYouView.swift
  • Resell/Views/Home/HomeView.swift
  • Resell/Views/Home/SavedView.swift
  • Resell/Views/ProductDetails/ProductDetailsView.swift
🚧 Files skipped from review as they are similar to previous changes (1)
  • Resell/Views/Home/ForYouView.swift

Comment on lines +168 to 188
func toggleLocalSaveStatus(for post: Post, isSaving: Bool) async {
let prevSavedItems = self.savedItems

if isSaving {
if !savedItems.contains(where: { $0.id == post.id }) {
savedItems.insert(post, at: 0)
}
} else {
savedItems.removeAll(where: { $0.id == post.id })
}

do {
let postsResponse = try await NetworkManager.shared.getSavedPosts()
self.savedItems = Post.sortPostsByDate(postsResponse.posts)
self.lastSavedFetchTime = Date()
} catch {
print("Network failed, rolling back local state...")
self.savedItems = prevSavedItems
NetworkManager.shared.logger.error("Sync failed: \(error)")
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if there's any debounce or rate-limiting on save button interactions
rg -n -C3 'toggleLocalSaveStatus|isSaved.toggle' --type=swift

Repository: cuappdev/resell-ios

Length of output: 2565


🏁 Script executed:

# Check HomeViewModel class definition and `@MainActor` annotation
rg -n '@MainActor|class HomeViewModel' --type=swift Resell/ViewModels/HomeViewModel.swift -B2 -A2

Repository: cuappdev/resell-ios

Length of output: 749


🏁 Script executed:

# Look for the full toggleLocalSaveStatus implementation and surrounding code
cat -n Resell/ViewModels/HomeViewModel.swift | sed -n '150,200p'

Repository: cuappdev/resell-ios

Length of output: 1973


🏁 Script executed:

# Check if there's any button state management or tap prevention in ProductDetailsView
rg -n 'saveButton|\.disabled|isLoading|isSaving' --type=swift Resell/Views/ProductDetails/ProductDetailsView.swift -B2 -A2

Repository: cuappdev/resell-ios

Length of output: 1374


Consider potential race condition in optimistic update rollback.

If toggleLocalSaveStatus is called multiple times in rapid succession (e.g., user double-taps the save button), each call captures prevSavedItems at the start. If the first call's network request fails and rolls back while a second call is in flight, the rollback could restore stale state, overwriting changes from the second call.

Although @MainActor serializes synchronous code, the await on line 180 yields control, allowing task interleaving. The save button has no .disabled() or loading state protection to prevent concurrent calls. Consider adding a debounce mechanism on the save button or using a serial task queue to ensure only one save operation proceeds at a time.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resell/ViewModels/HomeViewModel.swift` around lines 168 - 188, The
optimistic-update rollback is vulnerable to race conditions in
toggleLocalSaveStatus(for:isSaving:): capture of prevSavedItems can overwrite
newer changes if multiple calls run concurrently; fix by serializing/guarding
operations—e.g., add a monotonic operation counter or UUID property (e.g.,
saveOperationID or currentSaveToken) on the ViewModel, increment/generate it at
the start of toggleLocalSaveStatus, capture the token in the async call, and on
error only restore prevSavedItems if the captured token matches the current
token (otherwise ignore rollback); alternatively (or additionally) disable the
save button or debounce it while an operation is in flight so
NetworkManager.shared.getSavedPosts results and lastSavedFetchTime are only
applied when the operation token is still current.

Copy link
Copy Markdown

@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

♻️ Duplicate comments (1)
Resell/ViewModels/SetupProfileViewModel.swift (1)

69-98: ⚠️ Potential issue | 🟠 Major

Still returning silently when image encoding fails.

If selectedImage?.resizedToMaxDimension(256).toBase64() returns nil, the method falls through to Line 98 and returns false without calling presentError(...), so the onboarding flow just stops loading with no feedback.

🐛 Suggested fix
-            return false
+            presentError("Failed to process profile image. Please try again.")
+            return false
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resell/ViewModels/SetupProfileViewModel.swift` around lines 69 - 98, The code
silently returns false when selectedImage?.resizedToMaxDimension(256).toBase64()
is nil; update the flow in the block around
selectedImage/resizedToMaxDimension/toBase64 so that when imageBase64 is nil you
call presentError(...) with a clear message (e.g., "Image processing failed.
Please try a different image.") and then return false, rather than falling
through silently—make the check explicit before creating ImageBody/uploadImage
and ensure presentError is used for any early exit related to image encoding.
🧹 Nitpick comments (1)
Resell/Views/NewListing/NewListingImagesView.swift (1)

101-114: Remove the commented-out .actionSheet block.

Keeping the old implementation commented here adds noise and can confuse future edits now that .confirmationDialog is the active path.

Suggested cleanup
-//        .actionSheet(isPresented: $viewModel.didShowActionSheet) {
-//            ActionSheet(
-//                title: Text("Select Image Source"),
-//                buttons: [
-//                    .default(Text("Photo Library")) {
-//                        viewModel.didShowPhotosPicker = true
-//                    },
-//                    .default(Text("Camera")) {
-//                        viewModel.didShowCamera = true
-//                    },
-//                    .cancel()
-//                ]
-//            )
-//        }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resell/Views/NewListing/NewListingImagesView.swift` around lines 101 - 114,
Remove the commented-out .actionSheet block to avoid dead code and confusion:
delete the entire commented section that references .actionSheet and ActionSheet
(the block toggling viewModel.didShowActionSheet, viewModel.didShowPhotosPicker,
viewModel.didShowCamera), since the active UI uses .confirmationDialog instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Resell/ViewModels/CurrentUserProfileManager.swift`:
- Around line 169-178: The notification observer in setupNotificationObservers
currently calls loadProfile(forceRefresh: true) unconditionally, allowing
overlapping fetches; change the observer closure to first check the view model's
isLoading flag (accessing it on the MainActor) and return early if a load is
already in progress, then only call loadProfile when not loading—this prevents
concurrent network requests and nondeterministic UI updates; reference
setupNotificationObservers, isLoading, and loadProfile (and consider
ProfileViewModel.loadCurrentUser callers) when implementing the guard.

In `@Resell/ViewModels/SetupProfileViewModel.swift`:
- Around line 99-104: The catch in SetupProfileViewModel.createNewUser only
shows an error for ErrorResponse.usernameAlreadyExists and otherwise just logs
then returns false; update the catch to map other error cases to user-facing
messages by inspecting the caught error (e.g., ErrorResponse cases,
URLError/network errors, or a generic fallback) and call presentError(...) with
a contextual message before returning false; retain
NetworkManager.shared.logger.error("Error in
SetupProfileViewModel.createNewUser: \(error)") for logging but ensure
presentError is invoked for non-username failures so users get feedback.
- Around line 57-60: createNewUser() currently only rejects a nil selectedImage
and can still submit the placeholder image that clear() assigns; update
createNewUser() to perform the same image validation as checkInputIsValid():
ensure selectedImage is non-nil and not equal to the placeholder image instance
assigned in clear() (or call checkInputIsValid() at the start), and call
presentError("Please select a profile picture.") if the selectedImage is the
placeholder so the default asset cannot be uploaded.
- Around line 68-75: The current flow uploads the image before verifying
authentication, which can leave orphaned images; update the method in
SetupProfileViewModel (the block using selectedImage, ImageBody and calling
NetworkManager.shared.uploadImage) to check GoogleAuthManager.shared.user first
with a guard let user = GoogleAuthManager.shared.user and return false/present
the auth error if nil, then proceed to resize/encode and call
NetworkManager.shared.uploadImage; ensure no NetworkManager.shared.uploadImage
call happens unless the auth guard passes.

In `@Resell/Views/NewListing/NewRequestView.swift`:
- Around line 43-46: Remove the immediate router.pop() call from the
PurpleButton action so you only call viewModel.createNewRequest() (and UI
animations) there; rely on the existing onChange(of: viewModel.isLoading)
observer to call router.pop() once the async request finishes. Specifically,
edit the PurpleButton closure (where viewModel.createNewRequest() and
router.pop() are invoked) to delete the synchronous router.pop() invocation and
keep the remainder, leaving the onChange handler to perform the single
navigation pop.

---

Duplicate comments:
In `@Resell/ViewModels/SetupProfileViewModel.swift`:
- Around line 69-98: The code silently returns false when
selectedImage?.resizedToMaxDimension(256).toBase64() is nil; update the flow in
the block around selectedImage/resizedToMaxDimension/toBase64 so that when
imageBase64 is nil you call presentError(...) with a clear message (e.g., "Image
processing failed. Please try a different image.") and then return false, rather
than falling through silently—make the check explicit before creating
ImageBody/uploadImage and ensure presentError is used for any early exit related
to image encoding.

---

Nitpick comments:
In `@Resell/Views/NewListing/NewListingImagesView.swift`:
- Around line 101-114: Remove the commented-out .actionSheet block to avoid dead
code and confusion: delete the entire commented section that references
.actionSheet and ActionSheet (the block toggling viewModel.didShowActionSheet,
viewModel.didShowPhotosPicker, viewModel.didShowCamera), since the active UI
uses .confirmationDialog instead.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 41198267-f2ce-445b-a8fb-0f5e9d1b36f0

📥 Commits

Reviewing files that changed from the base of the PR and between 31fef14 and b697f49.

📒 Files selected for processing (7)
  • Resell/Models/User.swift
  • Resell/Utils/Constants.swift
  • Resell/ViewModels/CurrentUserProfileManager.swift
  • Resell/ViewModels/NewListingViewModel.swift
  • Resell/ViewModels/SetupProfileViewModel.swift
  • Resell/Views/NewListing/NewListingImagesView.swift
  • Resell/Views/NewListing/NewRequestView.swift
✅ Files skipped from review due to trivial changes (1)
  • Resell/Utils/Constants.swift

Comment on lines +169 to +178
private func setupNotificationObservers() {
NotificationCenter.default.addObserver(
forName: Constants.Notifications.NewListingCreated,
object: nil,
queue: .main
) { [weak self] _ in
Task { @MainActor in
print("New listing detected. Refreshing profile...")
self?.loadProfile(forceRefresh: true)
}
Copy link
Copy Markdown

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:

#!/bin/bash
# Verify potential overlapping load paths for CurrentUserProfileManager.
rg -n -C3 'func loadProfile\(|isLoading|Task \{|setupNotificationObservers|NewListingCreated' Resell/ViewModels/CurrentUserProfileManager.swift
rg -n -C2 'loadCurrentUser\(forceRefresh' Resell/ViewModels/ProfileViewModel.swift

Repository: cuappdev/resell-ios

Length of output: 2174


Guard notification-triggered refreshes to avoid overlapping profile fetches.

The observer fires without checking isLoading, allowing concurrent loadProfile calls when a refresh is already in progress (including from ProfileViewModel.loadCurrentUser). This can cause duplicate network requests and nondeterministic last-write UI state.

Suggested fix
     private func setupNotificationObservers() {
         NotificationCenter.default.addObserver(
             forName: Constants.Notifications.NewListingCreated,
             object: nil,
             queue: .main
         ) { [weak self] _ in
             Task { `@MainActor` in
-                print("New listing detected. Refreshing profile...")
-                self?.loadProfile(forceRefresh: true)
+                guard let self else { return }
+                guard !self.isLoading else { return }
+                self.loadProfile(forceRefresh: true)
             }
         }
     }
📝 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
private func setupNotificationObservers() {
NotificationCenter.default.addObserver(
forName: Constants.Notifications.NewListingCreated,
object: nil,
queue: .main
) { [weak self] _ in
Task { @MainActor in
print("New listing detected. Refreshing profile...")
self?.loadProfile(forceRefresh: true)
}
private func setupNotificationObservers() {
NotificationCenter.default.addObserver(
forName: Constants.Notifications.NewListingCreated,
object: nil,
queue: .main
) { [weak self] _ in
Task { `@MainActor` in
guard let self else { return }
guard !self.isLoading else { return }
self.loadProfile(forceRefresh: true)
}
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resell/ViewModels/CurrentUserProfileManager.swift` around lines 169 - 178,
The notification observer in setupNotificationObservers currently calls
loadProfile(forceRefresh: true) unconditionally, allowing overlapping fetches;
change the observer closure to first check the view model's isLoading flag
(accessing it on the MainActor) and return early if a load is already in
progress, then only call loadProfile when not loading—this prevents concurrent
network requests and nondeterministic UI updates; reference
setupNotificationObservers, isLoading, and loadProfile (and consider
ProfileViewModel.loadCurrentUser callers) when implementing the guard.

Comment thread Resell/ViewModels/SetupProfileViewModel.swift
Comment thread Resell/ViewModels/SetupProfileViewModel.swift
Comment thread Resell/ViewModels/SetupProfileViewModel.swift
Comment thread Resell/Views/NewListing/NewRequestView.swift
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