Open
Conversation
added 2 commits
March 4, 2026 23:33
…ignment and preliminary share functionality.
📝 WalkthroughWalkthroughProject and scheme Xcode metadata updated to 1620; model CodingKeys changed from snake_case to camelCase for price fields; UI tweaks to draggable sheet and options menu; ProductDetailsView refactored with a new save button that handles notification authorization and persistence. Changes
Sequence DiagramsequenceDiagram
participant User
participant ProductDetailsView
participant NotificationCenter
participant Persistence
User->>ProductDetailsView: Tap Save button
ProductDetailsView->>NotificationCenter: Check authorization
alt authorized
ProductDetailsView->>ProductDetailsView: Toggle saved state
ProductDetailsView->>Persistence: Persist saved state
ProductDetailsView->>NotificationCenter: Schedule/display notification
NotificationCenter->>User: Show save confirmation
else not authorized
ProductDetailsView->>NotificationCenter: Request permission
NotificationCenter->>User: Present permission prompt
User->>NotificationCenter: Approve/Deny
NotificationCenter->>ProductDetailsView: Authorization result
ProductDetailsView->>ProductDetailsView: Retain or update state accordingly
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip You can disable the changed files summary in the walkthrough.Disable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 | 🟡 MinorThread-safety concern with
@AppStorageinside function.Declaring
@AppStorageinsiderequestNotificationAuthorization()(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
isNotificationAuthorizedproperty (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
saveButtonhas duplicated UI code between theifandelsebranches—only the action differs. Additionally, line 425 contains a debugprint("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
originalPriceandalteredPriceproperty 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
Poststruct, lines 28-29 explicitly maporiginalPriceandalteredPriceto 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, alteredPriceAdditionally, the
PostBodystruct (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
⛔ Files ignored due to path filters (1)
.DS_Storeis excluded by!**/.DS_Store
📒 Files selected for processing (8)
Resell.xcodeproj/project.pbxprojResell.xcodeproj/xcshareddata/xcschemes/Resell.xcschemeResell/Models/Listing.swiftResell/Models/Post.swiftResell/Models/Transaction.swiftResell/Views/Components/DraggableSheetView.swiftResell/Views/Components/OptionsMenuView.swiftResell/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"), |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
📒 Files selected for processing (1)
Resell/Views/Home/ForYouView.swift
Comment on lines
+56
to
+57
| .frame(height: 110) | ||
| .padding(.horizontal, 24) |
There was a problem hiding this comment.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Look at the commit messages for specific bug fixes.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes