fix(tinyplace): replace native window.confirm with in-app modal on feed delete#4214
Conversation
…ation Feed delete used native window.confirm(), which renders the OS dialog titled "tauri.localhost says — Delete this post?", exposing the internal hostname and breaking from the app's design system. Add a reusable ConfirmDialog (built on the shared ModalShell) and use it for both the post delete (FeedSection.handleDeletePost) and the comment delete (CommentRow). The destructive RPC now runs only after the user confirms, with a busy state that disables the buttons while the delete is in flight. Adds a Vitest suite for ConfirmDialog.
📝 WalkthroughWalkthroughAdds an in-app ChangesFeed delete confirmation
Sequence Diagram(s)Comment deletion flow sequenceDiagram
participant User
participant CommentRow
participant ConfirmDialog
participant apiClient.feeds
participant FeedSection
User->>CommentRow: click Delete
CommentRow->>ConfirmDialog: open confirmation modal
User->>ConfirmDialog: click confirm-dialog-confirm
ConfirmDialog-->>CommentRow: onConfirm()
CommentRow->>apiClient.feeds: deleteComment(...)
apiClient.feeds-->>CommentRow: success
CommentRow->>FeedSection: onCommentDeleted()
CommentRow->>CommentRow: clear confirmingDelete and deleting
Post deletion flow sequenceDiagram
participant User
participant FeedSection
participant ConfirmDialog
participant apiClient.feeds
participant homeFeed
User->>FeedSection: click Delete
FeedSection->>ConfirmDialog: open confirmation modal
User->>ConfirmDialog: click confirm-dialog-confirm
ConfirmDialog-->>FeedSection: onConfirm()
FeedSection->>apiClient.feeds: deletePost(...)
apiClient.feeds-->>FeedSection: success
FeedSection->>homeFeed: refetch()
FeedSection->>FeedSection: clear postPendingDelete and deletingPost
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c8f797039
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| title="Delete comment" | ||
| message="Delete this comment? This can't be undone." | ||
| confirmLabel="Delete" |
There was a problem hiding this comment.
AGENTS.md requires all UI text to go through useT() with locale entries, but this new delete dialog copy is hard-coded in English here (and the post dialog below repeats the same pattern). In any non-English locale, users will still see English text for the destructive confirmation modal, so these labels/messages should be moved into the i18n catalog and passed through translations.
Useful? React with 👍 / 👎.
![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)
app/src/agentworld/pages/FeedSection.tsx (1)
742-758: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep post deletion pending until the refresh actually succeeds.
This handler has two contract gaps: it ignores the
{ ok: boolean }result fromdeletePost(), and it detaches thehomeFeed()refresh withvoid. A backendok: falseor a refresh rejection both fall through tofinally, closing the modal and leaving stale feed state behind.Suggested fix
const confirmDeletePost = () => { const post = postPendingDelete; if (!post) return; setDeletingPost(true); void apiClient.feeds .deletePost(post.postId) - .then(() => { - void apiClient.graphql.homeFeed({ limit: 50, includeSelf: true }).then(result => { + .then(({ ok }) => { + if (!ok) throw new Error('Post deletion was not accepted by the backend'); + return apiClient.graphql.homeFeed({ limit: 50, includeSelf: true }).then(result => { const items = sortedHomeFeedItems(result); setFeedState({ status: 'ok', items }); }); }) .catch(err => console.error('[FeedSection] delete post failed:', err)) .finally(() => { setDeletingPost(false); setPostPendingDelete(null); }); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/agentworld/pages/FeedSection.tsx` around lines 742 - 758, The confirmDeletePost handler in FeedSection should keep the delete flow pending until both deletePost() and the homeFeed refresh succeed. Check the { ok: boolean } result returned by apiClient.feeds.deletePost(post.postId) and treat ok: false as a failure instead of proceeding to refresh. Also remove the detached void from the apiClient.graphql.homeFeed(...) refresh so its rejection is awaited and can prevent the cleanup in finally. Only clear deletingPost and postPendingDelete after the refresh succeeds; on any failure, log the error and leave the modal/state intact for retry.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/agentworld/pages/FeedSection.tsx`:
- Around line 538-547: The delete flow in confirmDeleteComment currently treats
any resolved deleteComment() call as success, but apiClient.feeds.deleteComment
returns an object with an ok flag that must be checked. Update the promise
handling in confirmDeleteComment to inspect the deleteComment result and call
onCommentDeleted() only when ok is true; if ok is false, skip the success path
and keep the failure handling consistent with the existing error logging and
state reset logic in FeedSection.
---
Outside diff comments:
In `@app/src/agentworld/pages/FeedSection.tsx`:
- Around line 742-758: The confirmDeletePost handler in FeedSection should keep
the delete flow pending until both deletePost() and the homeFeed refresh
succeed. Check the { ok: boolean } result returned by
apiClient.feeds.deletePost(post.postId) and treat ok: false as a failure instead
of proceeding to refresh. Also remove the detached void from the
apiClient.graphql.homeFeed(...) refresh so its rejection is awaited and can
prevent the cleanup in finally. Only clear deletingPost and postPendingDelete
after the refresh succeeds; on any failure, log the error and leave the
modal/state intact for retry.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 033acff0-3667-43f6-95ea-ec22d35befdf
📒 Files selected for processing (3)
app/src/agentworld/components/ConfirmDialog.test.tsxapp/src/agentworld/components/ConfirmDialog.tsxapp/src/agentworld/pages/FeedSection.tsx
…nding until refresh
- Update the two delete tests to confirm via the in-app modal (they relied on
window.confirm, which the modal replaced) — the broken CI tests.
- Address CodeRabbit: check the {ok} result from deletePost/deleteComment and
chain (not detach) the feed/detail refresh so a backend rejection or refresh
failure is caught instead of silently closing the modal on stale state.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/agentworld/pages/FeedSection.test.tsx (1)
687-691: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winWait for the refetch, not just the delete RPC.
These assertions only wait for
feeds.deletePost/feeds.deleteCommentto be invoked. The follow-uphomeFeed/graphql.postrefetch happens later in the promise chain, so the immediatetoHaveBeenCalledTimes(2)can race and flake. Fold the refetch assertion intowaitFor(or assert on refreshed UI) so the test waits for the whole delete flow.Suggested change
- await waitFor(() => { - expect(vi.mocked(apiClient.feeds.deletePost)).toHaveBeenCalledWith(ownPost.postId); - }); - // Feed refetched after delete - expect(vi.mocked(apiClient.graphql.homeFeed)).toHaveBeenCalledTimes(2); + await waitFor(() => { + expect(vi.mocked(apiClient.feeds.deletePost)).toHaveBeenCalledWith(ownPost.postId); + expect(vi.mocked(apiClient.graphql.homeFeed)).toHaveBeenCalledTimes(2); + });- await waitFor(() => { - expect(vi.mocked(apiClient.feeds.deleteComment)).toHaveBeenCalledWith( - samplePost.author.handle, - samplePost.postId, - myComment.commentId - ); - }); - // Detail refetched after delete - expect(vi.mocked(apiClient.graphql.post)).toHaveBeenCalledTimes(2); + await waitFor(() => { + expect(vi.mocked(apiClient.feeds.deleteComment)).toHaveBeenCalledWith( + samplePost.author.handle, + samplePost.postId, + myComment.commentId + ); + expect(vi.mocked(apiClient.graphql.post)).toHaveBeenCalledTimes(2); + });Based on the delete promise chains in
app/src/agentworld/pages/FeedSection.tsx, each refetch is triggered after the delete RPC resolves.Also applies to: 731-739
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/agentworld/pages/FeedSection.test.tsx` around lines 687 - 691, The delete-flow tests in FeedSection are only waiting for deletePost/deleteComment to be called, but the homeFeed/graphql.post refetch happens later in the promise chain. Update the affected tests in FeedSection.test.tsx to wait for the full delete sequence by moving the refetch assertion into waitFor, or assert on the refreshed UI state after the delete RPC resolves. Use the existing delete-related assertions around apiClient.feeds.deletePost, apiClient.feeds.deleteComment, apiClient.graphql.homeFeed, and apiClient.graphql.post to locate and adjust both cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@app/src/agentworld/pages/FeedSection.test.tsx`:
- Around line 687-691: The delete-flow tests in FeedSection are only waiting for
deletePost/deleteComment to be called, but the homeFeed/graphql.post refetch
happens later in the promise chain. Update the affected tests in
FeedSection.test.tsx to wait for the full delete sequence by moving the refetch
assertion into waitFor, or assert on the refreshed UI state after the delete RPC
resolves. Use the existing delete-related assertions around
apiClient.feeds.deletePost, apiClient.feeds.deleteComment,
apiClient.graphql.homeFeed, and apiClient.graphql.post to locate and adjust both
cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b9e941de-4d3b-4dca-a7f8-6a604f47f4e7
📒 Files selected for processing (2)
app/src/agentworld/pages/FeedSection.test.tsxapp/src/agentworld/pages/FeedSection.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/agentworld/pages/FeedSection.tsx
2 participants
Feed delete used native
window.confirm(), which renders the OS dialog titled "tauri.localhost says — Delete this post?" — exposing the internal hostname and breaking from the app's design system.Adds a reusable
ConfirmDialog(built on the sharedModalShell) and uses it for both the post delete (FeedSection.handleDeletePost) and the comment delete (CommentRow). The delete RPC runs only after the user confirms, with a busy state disabling the buttons while in flight. Includes a Vitest suite forConfirmDialog.Closes #4197
Summary by CodeRabbit
New Features
Bug Fixes
Tests