-
Notifications
You must be signed in to change notification settings - Fork 108
Unvoting #325
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unvoting #325
Conversation
Implement the ability to unvote posts and comments within the 1-hour window that Hacker News allows. The unvote option appears automatically when an unvote link is available from the server. Changes: - Add unvote methods to VoteUseCase protocol - Implement unvote in PostRepository for posts and comments - Update VotingState to track unvote availability with canUnvote property - Add unvote to VotingStateProvider protocol and DefaultVotingStateProvider - Update VotingViewModel with unvote methods and optimistic UI updates - Update VoteButton to enable interaction when unvote is available - Add unvote options to VotingContextMenuItems for posts and comments - Update PostDisplayView to support unvote with toggle between upvote/unvote - Add unvote swipe actions and context menus in FeedView - Add unvote swipe actions and context menus in CommentsView for posts and comments The feature includes: - Optimistic UI updates with automatic rollback on error - Swipe actions (orange arrow.uturn.down icon for unvote) - Context menu options - Proper accessibility labels and hints - Automatic detection of unvote availability based on server response
|
Warning Rate limit exceeded@weiran has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 44 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
WalkthroughAdds unvote support across layers: repository methods, domain use‑case and state, voting state provider, view models, UI components (buttons, context menus, post display), and tests; introduces VotingState.canUnvote and wires unvote flows with optimistic UI updates and error rollback. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as PostDisplayView
participant VM as VotingViewModel
participant UseCase as VoteUseCase
participant Repo as PostRepository
participant API as Remote API
User->>UI: Tap Unvote
UI->>VM: unvote(post: inout Post)
Note over VM: Optimistic update — upvoted=false, score -= 1
VM->>UseCase: unvote(post)
UseCase->>Repo: unvote(post)
Repo->>API: GET unvote URL
alt Success
API-->>Repo: 200 OK
Repo-->>UseCase: success
UseCase-->>VM: success
VM-->>UI: confirm update (post replaced)
UI->>User: shows unvoted state
else Failure / Unauthenticated
API-->>Repo: Login form / hint or error
Repo-->>UseCase: throws
UseCase-->>VM: throws
Note over VM: Revert optimistic update — restore upvoted and score
VM-->>UI: error + reverted state
UI->>User: error indication, state restored
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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. Comment |
There was a problem hiding this 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
DesignSystem/Sources/DesignSystem/Components/VoteButton.swift (1)
103-110: Fix interactive tint when only unvote is available.With the new
.disabled((!votingState.canVote && !votingState.canUnvote) || votingState.isVoting)the button stays enabled during the unvote window, butforegroundColor(for:)still returnsdisabledColorwhenevercanVoteis false. For items that are upvoted with only an unvote link, the control is now actionable yet rendered in the disabled tint, which is misleading and hurts accessibility. Please gate the disabled tint on bothcanVoteandcanUnvote, e.g.:public func foregroundColor(for state: VotingState) -> Color { - if !state.canVote { + if !state.canVote && !state.canUnvote { disabledColor } else if state.isUpvoted { upvotedColor } else { defaultColor } }Features/Feed/Sources/Feed/FeedView.swift (1)
215-276: Local unvote still re-applies the upvote
Both the leading swipe unvote action and the context-menu unvote branch callviewModel.applyLocalUpvote(to:)afterVotingViewModel.unvote. That helper unconditionally setsupvoted = trueand bumps the score, so the list row reverts to “upvoted” immediately even though the backend call succeeded. Please introduce a symmetric helper (e.g.applyLocalUnvote) that clears the flag and decrements the score, and invoke it from the unvote paths instead ofapplyLocalUpvote.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
Data/Sources/Data/PostRepository+Voting.swift(2 hunks)DesignSystem/Sources/DesignSystem/Components/PostDisplayView.swift(8 hunks)DesignSystem/Sources/DesignSystem/Components/VoteButton.swift(1 hunks)DesignSystem/Sources/DesignSystem/Components/VotingContextMenuItems.swift(4 hunks)Domain/Sources/Domain/Models.swift(1 hunks)Domain/Sources/Domain/VoteUseCase.swift(1 hunks)Domain/Sources/Domain/VotingStateProvider.swift(3 hunks)Features/Comments/Sources/Comments/CommentsComponents.swift(7 hunks)Features/Feed/Sources/Feed/FeedView.swift(5 hunks)Shared/Sources/Shared/ViewModels/VotingViewModel.swift(4 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
DesignSystem/**/Sources/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
DesignSystem contains reusable UI components and styling only (no feature logic)
Files:
DesignSystem/Sources/DesignSystem/Components/VoteButton.swiftDesignSystem/Sources/DesignSystem/Components/PostDisplayView.swiftDesignSystem/Sources/DesignSystem/Components/VotingContextMenuItems.swift
**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
**/*.swift: Use Swift concurrency (async/await) instead of completion handlers where appropriate
Adopt Sendable where needed to ensure thread safety across concurrency boundaries
Files:
DesignSystem/Sources/DesignSystem/Components/VoteButton.swiftDesignSystem/Sources/DesignSystem/Components/PostDisplayView.swiftDomain/Sources/Domain/VoteUseCase.swiftDesignSystem/Sources/DesignSystem/Components/VotingContextMenuItems.swiftDomain/Sources/Domain/VotingStateProvider.swiftDomain/Sources/Domain/Models.swiftData/Sources/Data/PostRepository+Voting.swiftShared/Sources/Shared/ViewModels/VotingViewModel.swiftFeatures/Comments/Sources/Comments/CommentsComponents.swiftFeatures/Feed/Sources/Feed/FeedView.swift
Domain/**/{*UseCase,PostUseCase,CommentUseCase,SettingsUseCase,VoteUseCase}.swift
📄 CodeRabbit inference engine (AGENTS.md)
Declare use cases (PostUseCase, CommentUseCase, SettingsUseCase, VoteUseCase) in the Domain layer
Files:
Domain/Sources/Domain/VoteUseCase.swift
Domain/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
Provide a VotingService protocol (and its implementation) within Domain
Files:
Domain/Sources/Domain/VoteUseCase.swiftDomain/Sources/Domain/VotingStateProvider.swiftDomain/Sources/Domain/Models.swift
Data/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
Data/**/*.swift: Data layer implements Domain protocols (e.g., repositories implement use case contracts)
Use protocol-based UserDefaults wrappers in Data for testability
Files:
Data/Sources/Data/PostRepository+Voting.swift
Features/*/Sources/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
Features/*/Sources/**/*.swift: SwiftUI views in Features should use @EnvironmentObject for navigation
Annotate UI code with @mainactor
Files:
Features/Comments/Sources/Comments/CommentsComponents.swiftFeatures/Feed/Sources/Feed/FeedView.swift
Features/*/Sources/**/*View.swift
📄 CodeRabbit inference engine (AGENTS.md)
Use @StateObject for view-owned ViewModels in SwiftUI Views
Files:
Features/Feed/Sources/Feed/FeedView.swift
🧠 Learnings (5)
📚 Learning: 2025-09-14T17:42:10.615Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T17:42:10.615Z
Learning: Applies to Domain/**/*.swift : Provide a VotingService protocol (and its implementation) within Domain
Applied to files:
DesignSystem/Sources/DesignSystem/Components/VoteButton.swiftDesignSystem/Sources/DesignSystem/Components/PostDisplayView.swiftDomain/Sources/Domain/VoteUseCase.swiftDesignSystem/Sources/DesignSystem/Components/VotingContextMenuItems.swiftDomain/Sources/Domain/VotingStateProvider.swiftDomain/Sources/Domain/Models.swiftData/Sources/Data/PostRepository+Voting.swiftShared/Sources/Shared/ViewModels/VotingViewModel.swiftFeatures/Feed/Sources/Feed/FeedView.swift
📚 Learning: 2025-09-14T17:42:10.615Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T17:42:10.615Z
Learning: Applies to Domain/**/{*UseCase,PostUseCase,CommentUseCase,SettingsUseCase,VoteUseCase}.swift : Declare use cases (PostUseCase, CommentUseCase, SettingsUseCase, VoteUseCase) in the Domain layer
Applied to files:
Domain/Sources/Domain/VoteUseCase.swiftDomain/Sources/Domain/VotingStateProvider.swiftData/Sources/Data/PostRepository+Voting.swift
📚 Learning: 2025-09-14T17:42:10.615Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T17:42:10.615Z
Learning: Applies to Domain/**/{Post,Comment,User,TextSize}.swift : Place models Post, Comment, User, and TextSize in the Domain layer
Applied to files:
Domain/Sources/Domain/VoteUseCase.swiftData/Sources/Data/PostRepository+Voting.swift
📚 Learning: 2025-09-14T17:42:10.615Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T17:42:10.615Z
Learning: Applies to Features/*/Sources/**/*ViewModel.swift : Feature ViewModels must be ObservableObject and expose state via Published properties
Applied to files:
Shared/Sources/Shared/ViewModels/VotingViewModel.swift
📚 Learning: 2025-09-14T17:42:10.615Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T17:42:10.615Z
Learning: Applies to Features/*/Sources/**/*View.swift : Use StateObject for view-owned ViewModels in SwiftUI Views
Applied to files:
Shared/Sources/Shared/ViewModels/VotingViewModel.swift
🧬 Code graph analysis (10)
DesignSystem/Sources/DesignSystem/Components/VoteButton.swift (4)
Domain/Sources/Domain/VotingStateProvider.swift (1)
votingState(27-36)Shared/Sources/Shared/ViewModels/VotingViewModel.swift (3)
votingState(160-170)canVote(172-174)canUnvote(176-178)DesignSystem/Sources/DesignSystem/Components/VoteIndicator.swift (1)
votingState(11-63)Domain/Sources/Domain/VotingService.swift (1)
votingState(12-15)
DesignSystem/Sources/DesignSystem/Components/PostDisplayView.swift (4)
Shared/Sources/Shared/ViewModels/VotingViewModel.swift (6)
canUnvote(176-178)unvote(72-101)unvote(132-156)canVote(172-174)upvote(41-70)upvote(106-130)Data/Sources/Data/PostRepository+Parsing.swift (1)
post(54-111)Data/Sources/Data/PostRepository+Voting.swift (5)
voteLinks(97-145)unvote(40-56)unvote(77-93)upvote(21-38)upvote(58-75)Domain/Sources/Domain/VotingStateProvider.swift (2)
unvote(50-60)upvote(38-48)
Domain/Sources/Domain/VoteUseCase.swift (4)
Data/Sources/Data/PostRepository+Voting.swift (2)
unvote(40-56)unvote(77-93)Domain/Sources/Domain/VotingStateProvider.swift (1)
unvote(50-60)Shared/Sources/Shared/ViewModels/VotingViewModel.swift (2)
unvote(72-101)unvote(132-156)Domain/Sources/Domain/VotingService.swift (5)
upvoteComment(51-53)upvoteComment(55-59)voteUseCase(19-47)upvoteComment(56-58)upvote(36-46)
DesignSystem/Sources/DesignSystem/Components/VotingContextMenuItems.swift (4)
Data/Sources/Data/PostRepository+Parsing.swift (1)
post(54-111)Data/Sources/Data/PostRepository+Voting.swift (5)
voteLinks(97-145)upvote(21-38)upvote(58-75)unvote(40-56)unvote(77-93)Domain/Sources/Domain/VotingStateProvider.swift (2)
upvote(38-48)unvote(50-60)Shared/Sources/Shared/ViewModels/VotingViewModel.swift (4)
upvote(41-70)upvote(106-130)unvote(72-101)unvote(132-156)
Domain/Sources/Domain/VotingStateProvider.swift (3)
Data/Sources/Data/PostRepository+Voting.swift (5)
unvote(40-56)unvote(77-93)voteLinks(97-145)upvote(21-38)upvote(58-75)Shared/Sources/Shared/ViewModels/VotingViewModel.swift (5)
unvote(72-101)unvote(132-156)canUnvote(176-178)upvote(41-70)upvote(106-130)Domain/Sources/Domain/VotingService.swift (3)
voteUseCase(19-47)upvote(36-46)votingState(12-15)
Domain/Sources/Domain/Models.swift (3)
Shared/Sources/Shared/ViewModels/VotingViewModel.swift (2)
canUnvote(176-178)canVote(172-174)Domain/Sources/Domain/VotingService.swift (3)
votingState(12-15)votingState(26-34)voteUseCase(19-47)Shared/Tests/SharedTests/VotingViewModelTests.swift (1)
votingState(33-39)
Data/Sources/Data/PostRepository+Voting.swift (5)
Domain/Sources/Domain/VotingStateProvider.swift (1)
unvote(50-60)Shared/Sources/Shared/ViewModels/VotingViewModel.swift (2)
unvote(72-101)unvote(132-156)Data/Sources/Data/PostRepository+Parsing.swift (1)
post(54-111)Data/Tests/DataTests/PostRepositoryTests.swift (2)
post(57-71)get(42-55)Networking/Sources/Networking/NetworkManager.swift (2)
post(49-62)get(40-47)
Shared/Sources/Shared/ViewModels/VotingViewModel.swift (4)
Data/Sources/Data/PostRepository+Voting.swift (3)
unvote(40-56)unvote(77-93)voteLinks(97-145)Domain/Sources/Domain/VotingStateProvider.swift (2)
unvote(50-60)unvoteComment(75-77)Data/Sources/Data/PostRepository+Parsing.swift (1)
post(54-111)Domain/Sources/Domain/VotingService.swift (1)
voteUseCase(19-47)
Features/Comments/Sources/Comments/CommentsComponents.swift (4)
Data/Sources/Data/PostRepository+Parsing.swift (1)
post(54-111)Data/Sources/Data/PostRepository+Voting.swift (5)
voteLinks(97-145)upvote(21-38)upvote(58-75)unvote(40-56)unvote(77-93)Domain/Sources/Domain/VotingStateProvider.swift (2)
upvote(38-48)unvote(50-60)Shared/Sources/Shared/ViewModels/VotingViewModel.swift (5)
upvote(41-70)upvote(106-130)unvote(72-101)unvote(132-156)canUnvote(176-178)
Features/Feed/Sources/Feed/FeedView.swift (4)
Data/Sources/Data/PostRepository+Voting.swift (5)
voteLinks(97-145)upvote(21-38)upvote(58-75)unvote(40-56)unvote(77-93)Domain/Sources/Domain/VotingStateProvider.swift (2)
upvote(38-48)unvote(50-60)Shared/Sources/Shared/ViewModels/VotingViewModel.swift (5)
upvote(41-70)upvote(106-130)unvote(72-101)unvote(132-156)canUnvote(176-178)Features/Feed/Sources/Feed/FeedViewModel.swift (2)
applyLocalUpvote(242-248)vote(86-92)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
There was a problem hiding this 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Features/Feed/Sources/Feed/FeedViewModel.swift (1)
155-161: Unvote tap currently does nothingThe new unvote paths never reach the data layer because we still bail out when
upvote == false. When the UI offers “Unvote” in the one‑hour window, the action completes silently and the server state never changes. Please wire this through tovoteUseCase.unvote(post:).if upvote { try await voteUseCase.upvote(post: post) - } - // Unvote removed; do nothing when upvote == false + } else { + try await voteUseCase.unvote(post: post) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
Features/Comments/Sources/Comments/CommentsComponents.swift(9 hunks)Features/Comments/Tests/CommentsTests/CommentsViewModelTests.swift(1 hunks)Features/Feed/Sources/Feed/FeedView.swift(7 hunks)Features/Feed/Sources/Feed/FeedViewModel.swift(1 hunks)Features/Feed/Tests/FeedTests/FeedViewModelTests.swift(1 hunks)Features/Feed/Tests/FeedTests/FeedViewTests.swift(1 hunks)Shared/Sources/Shared/ViewModels/VotingViewModel.swift(9 hunks)Shared/Tests/SharedTests/DependencyContainerTests.swift(2 hunks)Shared/Tests/SharedTests/VotingViewModelTests.swift(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
Shared/Tests/SharedTests/DependencyContainerTests.swift (4)
Shared/Sources/Shared/ViewModels/VotingViewModel.swift (2)
unvote(75-104)unvote(138-162)Data/Sources/Data/PostRepository+Voting.swift (3)
unvote(40-56)unvote(77-93)upvote(12-114)Domain/Sources/Domain/VotingStateProvider.swift (2)
unvote(50-60)unvoteComment(75-77)Domain/Sources/Domain/VoteUseCase.swift (1)
upvote(10-13)
Features/Feed/Tests/FeedTests/FeedViewModelTests.swift (4)
Features/Comments/Tests/CommentsTests/CommentsViewModelTests.swift (2)
unvote(635-639)unvote(641-645)Features/Feed/Tests/FeedTests/FeedViewTests.swift (2)
unvote(116-116)unvote(118-118)Shared/Tests/SharedTests/DependencyContainerTests.swift (3)
unvote(124-124)unvote(125-125)unvote(150-150)Domain/Sources/Domain/VoteUseCase.swift (1)
upvote(10-13)
Features/Feed/Sources/Feed/FeedViewModel.swift (2)
Data/Tests/DataTests/PostRepositoryTests.swift (1)
parseUpvotedPostFromFeed(288-312)Data/Sources/Data/PostRepository+Voting.swift (1)
upvote(12-114)
Features/Comments/Tests/CommentsTests/CommentsViewModelTests.swift (4)
Features/Feed/Tests/FeedTests/FeedViewModelTests.swift (3)
unvote(363-365)unvote(367-369)post(468-480)Shared/Sources/Shared/ViewModels/VotingViewModel.swift (2)
unvote(75-104)unvote(138-162)Shared/Tests/SharedTests/DependencyContainerTests.swift (3)
unvote(124-124)unvote(125-125)unvote(150-150)Data/Sources/Data/PostRepository+Voting.swift (2)
unvote(40-56)unvote(77-93)
Shared/Tests/SharedTests/VotingViewModelTests.swift (4)
Shared/Sources/Shared/ViewModels/VotingViewModel.swift (4)
unvote(75-104)unvote(138-162)upvote(41-73)upvote(109-136)Data/Sources/Data/PostRepository+Voting.swift (5)
unvote(40-56)unvote(77-93)voteLinks(97-145)upvote(21-38)upvote(58-75)Domain/Sources/Domain/VotingStateProvider.swift (3)
unvote(50-60)unvoteComment(75-77)upvote(38-48)Data/Sources/Data/PostRepository+Parsing.swift (1)
post(54-111)
Features/Comments/Sources/Comments/CommentsComponents.swift (3)
Data/Sources/Data/PostRepository+Voting.swift (7)
voteLinks(97-145)upvote(21-38)upvote(58-75)unvote(40-56)unvote(77-93)upvote(12-114)upvote(13-17)Shared/Sources/Shared/ViewModels/VotingViewModel.swift (5)
upvote(41-73)upvote(109-136)unvote(75-104)unvote(138-162)canUnvote(182-184)DesignSystem/Sources/DesignSystem/Components/VotingContextMenuItems.swift (2)
postVotingMenuItems(11-62)postVotingMenuItems(14-27)
Features/Feed/Tests/FeedTests/FeedViewTests.swift (5)
Features/Comments/Tests/CommentsTests/CommentsViewModelTests.swift (2)
unvote(635-639)unvote(641-645)Features/Feed/Tests/FeedTests/FeedViewModelTests.swift (3)
unvote(363-365)unvote(367-369)post(468-480)Shared/Sources/Shared/ViewModels/VotingViewModel.swift (2)
unvote(75-104)unvote(138-162)Shared/Tests/SharedTests/DependencyContainerTests.swift (3)
unvote(124-124)unvote(125-125)unvote(150-150)Data/Sources/Data/PostRepository+Voting.swift (2)
unvote(40-56)unvote(77-93)
Shared/Sources/Shared/ViewModels/VotingViewModel.swift (3)
Data/Sources/Data/PostRepository+Parsing.swift (1)
post(54-111)Data/Sources/Data/PostRepository+Voting.swift (7)
voteLinks(97-145)unvote(40-56)unvote(77-93)upvote(21-38)upvote(58-75)upvote(12-114)upvote(13-17)Domain/Sources/Domain/VotingStateProvider.swift (3)
unvote(50-60)upvote(38-48)voteUseCase(19-47)
Features/Feed/Sources/Feed/FeedView.swift (5)
Features/Feed/Sources/Feed/FeedViewModel.swift (1)
replacePost(235-243)Data/Sources/Data/PostRepository+Voting.swift (7)
voteLinks(97-145)upvote(21-38)upvote(58-75)unvote(40-56)unvote(77-93)upvote(12-114)upvote(13-17)Shared/Sources/Shared/ViewModels/VotingViewModel.swift (5)
upvote(41-73)upvote(109-136)unvote(75-104)unvote(138-162)canUnvote(182-184)Domain/Sources/Domain/VotingStateProvider.swift (2)
upvote(38-48)unvote(50-60)Domain/Sources/Domain/VoteUseCase.swift (1)
upvote(10-13)
| guard votingViewModel.canUnvote(item: post), post.upvoted else { return true } | ||
|
|
||
| var mutablePost = post | ||
| await votingViewModel.unvote(post: &mutablePost) | ||
| let wasUnvoted = !mutablePost.upvoted | ||
|
|
||
| if wasUnvoted { | ||
| await MainActor.run { | ||
| onPostUpdated(mutablePost) | ||
| } | ||
| } | ||
|
|
||
| return wasUnvoted | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Return false when unvote is skipped
If the guard fails we never fire an unvote, yet the method returns true, so callers will think the unvote succeeded and flip their UI state. Mirror the upvote helper and return false when we bail out of the guard.
- guard votingViewModel.canUnvote(item: post), post.upvoted else { return true }
+ guard votingViewModel.canUnvote(item: post), post.upvoted else { return false }📝 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.
| guard votingViewModel.canUnvote(item: post), post.upvoted else { return true } | |
| var mutablePost = post | |
| await votingViewModel.unvote(post: &mutablePost) | |
| let wasUnvoted = !mutablePost.upvoted | |
| if wasUnvoted { | |
| await MainActor.run { | |
| onPostUpdated(mutablePost) | |
| } | |
| } | |
| return wasUnvoted | |
| } | |
| guard votingViewModel.canUnvote(item: post), post.upvoted else { return false } | |
| var mutablePost = post | |
| await votingViewModel.unvote(post: &mutablePost) | |
| let wasUnvoted = !mutablePost.upvoted | |
| if wasUnvoted { | |
| await MainActor.run { | |
| onPostUpdated(mutablePost) | |
| } | |
| } | |
| return wasUnvoted |
🤖 Prompt for AI Agents
In Features/Comments/Sources/Comments/CommentsComponents.swift around lines 272
to 285 the guard currently returns true when unvoting is skipped, misleading
callers; change the early return to return false (matching the upvote helper
behavior) so callers know the unvote did not occur, leaving the rest of the
function unchanged.
| private func handleUnvoteTap() async -> Bool { | ||
| guard votingViewModel.canUnvote(item: post), post.upvoted else { return true } | ||
|
|
||
| var mutablePost = post | ||
| await votingViewModel.unvote(post: &mutablePost) | ||
| let wasUnvoted = !mutablePost.upvoted | ||
|
|
||
| if wasUnvoted { | ||
| await MainActor.run { | ||
| onPostUpdated?(mutablePost) | ||
| } | ||
| } | ||
|
|
||
| return wasUnvoted | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signal failure when unvote cannot run
Here too, a guard failure means no unvote request is sent, but we still return true. The parent view will incorrectly assume success and toggle the row. Return false on the early exit.
- guard votingViewModel.canUnvote(item: post), post.upvoted else { return true }
+ guard votingViewModel.canUnvote(item: post), post.upvoted else { return false }🤖 Prompt for AI Agents
In Features/Feed/Sources/Feed/FeedView.swift around lines 441 to 455, the early
guard in handleUnvoteTap currently returns true when unvoting is not allowed,
causing the parent to think the action succeeded; change the guard's early
return to false so the method signals failure when no unvote request is sent,
leaving the rest of the method unchanged.
This reverts commit ebdf485.
The comments pill was appearing disabled in the comments view because it had no action handler. Updated the pill rendering logic to show static content (instead of a disabled button) when enabled but with no action, preventing the disabled styling from being applied. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
DesignSystem/Sources/DesignSystem/Components/PostDisplayView.swift (2)
223-275: Consider extracting unvote logic for readability.The upvote/unvote logic is correct with proper optimistic updates and error rollback. However, the function is quite long (52 lines) and handles two distinct flows. Consider extracting the unvote logic (lines 232-251) into a separate helper method
handleUnvoteTap()for improved readability and maintainability.Example structure:
private func handleUnvoteTap() async -> Bool { // Lines 233-251 logic } private func makeUpvoteAction() -> (() -> Void)? { return { if isCurrentlyUpvoted && canUnvote { Task { await handleUnvoteTap() } } else { // Existing upvote logic } } }
277-297: Document URL format assumptions.The
derivedVoteLinksfunction makes assumptions about Hacker News URL patterns (how=up→how=un). While this logic matches the repository layer (PostRepository+Voting.swift lines 96-144) and provides optimistic UI, consider adding a brief comment explaining that this derives an unvote URL for optimistic updates and that the authoritative state comes from server responses.Example:
/// Derives an unvote URL from the upvote URL for optimistic UI updates. /// The authoritative voteLinks come from server responses parsed by the repository. /// Assumes HN URL format: "how=up" or "how%3Dup" in upvote URLs. private func derivedVoteLinks(afterUpvoteFrom voteLinks: VoteLinks?) -> VoteLinks? {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
DesignSystem/Sources/DesignSystem/Components/PostDisplayView.swift(12 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
DesignSystem/Sources/DesignSystem/Components/PostDisplayView.swift (4)
Features/Feed/Tests/FeedTests/FeedViewModelTests.swift (5)
post(468-480)upvote(354-357)upvote(359-361)unvote(363-365)unvote(367-369)Data/Sources/Data/PostRepository+Parsing.swift (1)
post(54-111)Data/Sources/Data/PostRepository+Voting.swift (5)
voteLinks(97-145)upvote(21-38)upvote(58-75)unvote(40-56)unvote(77-93)Shared/Sources/Shared/ViewModels/VotingViewModel.swift (4)
upvote(41-73)upvote(109-136)unvote(75-104)unvote(138-162)
🔇 Additional comments (5)
DesignSystem/Sources/DesignSystem/Components/PostDisplayView.swift (5)
19-19: LGTM! Clean property additions for unvote support.The new
onUnvoteTapcallback anddisplayedVoteLinksstate follow the established patterns for optimistic UI updates. The initializer properly accepts and stores these values.Also applies to: 29-29, 38-38, 48-48, 54-54
106-106: Good state synchronisation.The onChange handlers properly sync
displayedVoteLinkswithpost.voteLinks, ensuring optimistic updates are overwritten by authoritative server state. This follows the pattern established for score, upvoted, and bookmarked states.Also applies to: 117-119
136-176: Excellent upvote/unvote interaction logic and accessibility.The
canInteractlogic correctly enables interaction when either upvote (not yet voted) or unvote (already voted) is available. The accessibility labels and hints properly distinguish between scenarios, including the helpful "Double tap to unvote" hint when unvote is available.
360-394: Loading overlays implemented well.The dual overlay approach (semi-transparent background + progress indicator) provides clear visual feedback during operations. The conditional rendering logic correctly distinguishes between static views and interactive buttons to avoid unintended disabled styling.
401-436: Context menu unvote integration looks correct.The unvote option properly appears when
post.voteLinks?.unvote != nilandpost.upvotedis true. The "arrow.uturn.down" icon is semantically appropriate, and the default empty closure foronUnvotemaintains backward compatibility with existing call sites.
When post data changes in the comments view (e.g., votes, title, comment count), those changes are now reflected in the feed view when navigating back. This is achieved by updating the NavigationStore's selectedPost property whenever the post changes in CommentsViewModel, and having FeedView observe and apply those updates. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
Features/Comments/Sources/Comments/CommentsComponents.swift (1)
282-283: Return false when unvote is skipped.Both guard statements incorrectly return
truewhen the unvote cannot proceed. This misleads callers into thinking the unvote succeeded. The return value should befalseto indicate the action didn't occur, mirroringhandleUpvoteat lines 265-266.Apply this diff to fix the return values:
- guard !isLoadingComments else { return true } - guard votingViewModel.canUnvote(item: post), post.upvoted else { return true } + guard !isLoadingComments else { return false } + guard votingViewModel.canUnvote(item: post), post.upvoted else { return false }
🧹 Nitpick comments (1)
Features/Comments/Sources/Comments/CommentsComponents.swift (1)
67-69: Consider consolidating unvote link clearing logic.The logic to clear the unvote link after successful unvoting is duplicated between the swipe action (lines 67-69) and
handleUnvote(lines 290-292). Whilst the duplication is minimal, consolidating it would improve maintainability.One option is to have the swipe action call
handleUnvote()instead of directly callingvotingViewModel.unvote(), though this would require adjusting the pattern to match context menu usage.Also applies to: 290-292
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Features/Comments/Sources/Comments/CommentsComponents.swift(10 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Features/Comments/Sources/Comments/CommentsComponents.swift (3)
Data/Sources/Data/PostRepository+Parsing.swift (1)
post(54-111)Data/Sources/Data/PostRepository+Voting.swift (5)
voteLinks(97-145)upvote(21-38)upvote(58-75)unvote(40-56)unvote(77-93)Shared/Sources/Shared/ViewModels/VotingViewModel.swift (5)
upvote(41-73)upvote(109-136)unvote(75-104)unvote(138-162)canUnvote(182-184)
🔇 Additional comments (3)
Features/Comments/Sources/Comments/CommentsComponents.swift (3)
41-46: LGTM! Improved API design.The refactoring from
onUpvoteAppliedtoonPostUpdatedis a cleaner design that handles both upvote and unvote through a single callback. The addition ofisLoadingCommentsappropriately prevents vote interactions during loading states.
57-100: Unvote swipe actions implemented correctly.The swipe actions for both posts and comments properly:
- Conditionally display based on vote state and available links
- Use the orange "arrow.uturn.down" icon as specified
- Guard against loading states
- Include appropriate accessibility labels
The difference in post vs comment handling (manual state updates for posts, automatic for comments) correctly reflects the underlying state management patterns where comments are
ObservableObjects updated directly by the repository.Also applies to: 184-208
226-262: Integration points properly wired.The remaining changes correctly wire the unvote feature through the component hierarchy:
PostHeadersignature updated to acceptisLoadingCommentsandonPostUpdated- Context menus for both posts and comments now include unvote options
VoteIndicatorreceivescanUnvotestate for proper display- Loading checks consistently guard against concurrent operations
Also applies to: 346-379
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
Features/Feed/Sources/Feed/FeedView.swift (1)
454-471: Guard must signal failure when unvote cannot runWhen the guard fails, no unvote is attempted, but the method currently returns true, causing the caller to assume the unvote succeeded and potentially toggle UI state incorrectly. This matches the earlier review comment and should return false instead.
- private func handleUnvoteTap() async -> Bool { - guard votingViewModel.canUnvote(item: post), post.upvoted else { return true } + private func handleUnvoteTap() async -> Bool { + guard votingViewModel.canUnvote(item: post), post.upvoted else { return false }The rest of the method (computing wasUnvoted from mutablePost and only invoking onPostUpdated on success) is aligned with the upvote path.
🧹 Nitpick comments (2)
Features/Feed/Sources/Feed/FeedView.swift (2)
210-214: Swipe visibility predicate is correct but could be extractedThe condition accurately reflects “can upvote or can unvote” based on voteLinks and upvoted state, but it’s a bit dense inline. Consider extracting to a helper like canShowVoteSwipe(for:) and reusing it where needed for clarity.
222-259: Duplicate unvote handling logic could be centralisedThe unvote swipe path duplicates the pattern used elsewhere (row tap and context menu): call unvote, then clear voteLinks.unvote and replacePost only when the final state is not upvoted. Behaviour is sound, but extracting this into a shared helper (e.g. applyUnvoteResult(_:)) would reduce the risk of the flows drifting apart in future changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Features/Comments/Sources/Comments/CommentsView.swift(1 hunks)Features/Comments/Sources/Comments/CommentsViewModel.swift(1 hunks)Features/Feed/Sources/Feed/FeedView.swift(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Features/Feed/Sources/Feed/FeedView.swift (3)
Features/Feed/Sources/Feed/FeedViewModel.swift (1)
replacePost(235-243)Data/Sources/Data/PostRepository+Voting.swift (5)
voteLinks(97-145)upvote(21-38)upvote(58-75)unvote(40-56)unvote(77-93)Shared/Sources/Shared/ViewModels/VotingViewModel.swift (5)
upvote(41-73)upvote(109-136)unvote(75-104)unvote(138-162)canUnvote(182-184)
🔇 Additional comments (9)
Features/Comments/Sources/Comments/CommentsViewModel.swift (2)
17-21: LGTM! Clean implementation of the observer pattern.The
didSetobserver correctly propagates post updates to interested observers through the callback. This enables the navigation layer to stay synchronized with post changes during voting operations.
27-28: LGTM! Well-documented callback property.The optional callback property provides a clean communication pathway for post updates without introducing tight coupling.
Features/Comments/Sources/Comments/CommentsView.swift (1)
117-122: LGTM! Proper callback setup with correct memory management.The callback setup correctly uses weak capture to prevent retain cycles, and the conditional unwrapping ensures only valid post updates are propagated to the navigation store. The placement in
.taskis appropriate for the view lifecycle.Features/Feed/Sources/Feed/FeedView.swift (6)
100-106: Feed stays in sync with navigation-driven post updatesThis onChange hook correctly propagates navigationStore.selectedPost changes back into the feed via replacePost, keeping list and detail views consistent without extra fetches.
192-194: onPostUpdated callback wiring keeps the feed model consistentPassing an onPostUpdated closure that delegates to viewModel.replacePost cleanly centralises post mutation handling and avoids duplicating update logic in the row.
272-290: Context menu voting flows are consistent with swipe behaviourThe context menu’s upvote and unvote handlers mirror the swipe actions’ semantics (optimistic call via VotingViewModel and only replacing the post when the final state matches the intended action, plus clearing unvote links on success). This keeps behaviour consistent across interaction modalities.
390-407: onPostUpdated plumbing in PostRowView is straightforward and flexibleAdding an onPostUpdated callback and threading it through the initialiser gives FeedView (and other callers) a clean hook to react to post mutations without tightly coupling PostRowView to a specific model update strategy.
418-418: Unvote tap wiring matches the upvote pathPassing onUnvoteTap through to PostDisplayView alongside onUpvoteTap keeps the voting surface symmetrical and lets the shared voting UI drive both actions without duplicating logic in the row.
438-452: Upvote handler correctly reports success and propagates updateshandleUpvoteTap still guards on canVote and the current upvoted state, then only notifies onPostUpdated when the final state is upvoted, so the caller can safely rely on the Bool return value to reflect the actual outcome.
Move the logic to clear the unvote link after a successful unvote operation from the UI layer into VotingViewModel.unvote(). This eliminates duplicate code that was present in both the swipe action and handleUnvote() method. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Resolved conflict in CommentsComponents.swift by keeping the unvote swipe action logic and hidden row separators from the feature branch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Implement the ability to unvote posts and comments within the 1-hour window that Hacker News allows. The unvote option appears automatically when an unvote link is available from the server.
Changes:
The feature includes:
Summary by CodeRabbit
New Features
Bug Fixes
Tests