fix(feed): deleting a post from a list no longer pops the feed - #3410
Conversation
postsListContainer mounted the options sheet with no onDelete, so deleting a post from a feed ran the sheet's own path: navigation.goBack() popped the feed screen, and nothing removed the post, so it stayed visible until something refetched. The container receives posts as a prop and does not own the list, so the removal belongs with the query that does. Adds deletePost to useFeedQuery, modelled on wavesQueries.deleteWave: broadcast, then prune the post from that feed's infinite-query cache, matching author as well as permlink since a permlink is only unique per author. The cache is patched rather than invalidated because the delete broadcasts async, so a refetch issued at that point returns pre-transaction state and would bring the post straight back. Wired through postsTabContent and postsListContainer as onDeletePost. Every PostOptionsModal consumer that owns a surrounding list now delegates. The two that do not, postScreen and editorScreen, render the content as the screen itself, so the fallback pop is correct for them.
Greptile SummaryThe PR delegates feed deletion to
Confidence Score: 4/5The PR is not yet safe to merge because deleting a pinned post can still be visually undone by restarting the app before the backend reflects the asynchronous deletion. The current implementation stores deleted identities only in module memory; after a process restart, the independently fetched pinned-post query can return and render the stale post because no durable suppression state remains. Files Needing Attention: src/providers/queries/postQueries/feedQueries.ts
|
| Filename | Overview |
|---|---|
| src/providers/queries/postQueries/feedQueries.ts | Adds feed-owned deletion and cache/display pruning, but the pinned-post suppression remains process-local and can be lost before remote indexing catches up. |
| src/components/postOptionsModal/container/postOptionsModal.tsx | Prevents parsed cross-post cards from exposing deletion that would otherwise target the original post. |
| src/components/postsList/container/postsListContainer.tsx | Forwards the list owner's optional deletion handler to the post options modal. |
| src/components/tabbedPosts/view/postsTabContent.tsx | Connects the feed query's deletion callback to the rendered posts list. |
Sequence Diagram
sequenceDiagram
participant User
participant Modal as PostOptionsModal
participant Feed as useFeedQuery
participant Chain as Hive mutation
participant Cache as React Query cache
User->>Modal: Delete post
Modal->>Feed: onDelete(content)
Feed->>Chain: Broadcast async deletion
Chain-->>Feed: Mempool acceptance
Feed->>Cache: Prune author/permlink
Feed->>Feed: Record deleted identity
Feed-->>User: Updated feed and toast
Reviews (6): Last reviewed commit: "fix(post): do not offer delete on a cros..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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: 04fae0a031
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // Match author as well as permlink: a permlink is only unique per author, | ||
| // so filtering on it alone could drop someone else's post. | ||
| queryClient.setQueryData<InfiniteData<any[]>>(queryOptions.queryKey, (oldData) => { |
There was a problem hiding this comment.
When an eligible pinned post is deleted from the user's profile, this only patches the infinite-feed cache, but the displayed list is assembled with pinnedPostQuery.data ahead of those pages at lines 187–190. Consequently the successful deletion and toast leave the pinned post visible until that separate post query happens to refetch; clear or update the pinned query when it identifies the deleted post.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in a19af5c. You are right: _data prepends pinnedPostQuery.data via unionBy before the feed pages, so patching the infinite-feed cache alone left a deleted pinned post on screen.
Rather than patch a second cache, the fix records the deleted post's author/permlink and applies it where the list is assembled, next to the existing mute filter. That is the displayed identity, so one rule covers the pinned post, an ordinary feed row, and the cross-post case raised separately on this PR.
The cache patch stays alongside it: it is what makes the removal durable across a remount, while this covers what that cache cannot express.
| pages: oldData.pages.map((page) => | ||
| page.filter( | ||
| (post) => | ||
| !(post?.author === currentAccount.name && post?.permlink === content.permlink), |
There was a problem hiding this comment.
When the current user's deletable post is displayed through another author's cross-post, parsePost replaces the rendered author and permlink with original_entry values, so the menu deletes the original; however, this callback filters the raw query page, where those top-level fields still identify the cross-post wrapper. The predicate therefore retains the item and the deleted post remains visible, so it must also match the wrapper's original_entry identity.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in a19af5c. This one was more subtle than it looks and you read it correctly.
select does parsePost({ ...post }, ...) on a shallow copy specifically so it does not mutate the cache, which means the cache holds raw posts while the rendered list holds parsed ones. For a cross-post parsePost swaps author and permlink to the original_entry, so the sheet acts on the original while the cached row still carries the wrapper's identity. The predicate matched neither, exactly as you describe.
Fixed by filtering on the displayed identity instead of trying to reconcile the two shapes: the deleted author/permlink is recorded and applied where the list is assembled. Verified the predicate against all the cases, including that a same-permlink-different-author post survives:
plain deleted post kept? false (want false)
pinned deleted post kept? false (want false)
crosspost row (parsed=orig) false (want false)
other user's post kept? true (want true)
same permlink diff author true (want true)
One nuance worth stating rather than hiding: the wrapper is @bob's own post and still exists on chain, so a refetch can legitimately bring that row back showing a deleted original. Removing it here matches what the user just acted on, but the underlying oddity is that delete-post is offered on a cross-post card at all. Mute and pin are already withheld there (_canModerateCommunityPost) precisely because the identity is ambiguous. Extending that to delete is arguably the better answer, but it changes which actions are offered on the post detail screen too, so I have not folded it into this PR.
…e list Two review findings, both cases the cache patch alone could not reach. The pinned post comes from its own query and is prepended to the feed data, so patching the infinite-feed cache left a deleted pinned post visible. And select() parses a shallow copy, so the cache holds raw posts: for a cross-post the cached author and permlink are the wrapper's while the rendered ones are the original's, and the predicate matched neither. Records the author/permlink of posts deleted this session and applies it where the list is assembled, alongside the existing mute filter. That is the displayed identity, which is what the user acted on, so it covers a plain feed row, a pinned post and a cross-post row with one rule. The cache patch stays: it is what keeps the removal durable across a remount, while this covers what the cache cannot express. Author is matched as well as permlink throughout, since a permlink is only unique per author.
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe feed query now exposes post deletion. The posts list passes the callback to ChangesFeed post deletion
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PostOptionsModal
participant PostsList
participant useFeedQuery
participant SDK
participant FeedCache
PostOptionsModal->>PostsList: invoke onDeletePost(content)
PostsList->>useFeedQuery: call deletePost(content)
useFeedQuery->>SDK: submit deletion
SDK-->>useFeedQuery: return deletion result
useFeedQuery->>FeedCache: remove matching author/permlink
Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@src/providers/queries/postQueries/feedQueries.ts`:
- Around line 227-240: Update the pages mapping in the queryClient.setQueryData
callback to check Array.isArray(page) before calling filter, returning non-array
pages unchanged. Keep the existing post author/permlink filtering behavior for
array pages and match the guard used by the hook’s select callback.
- Around line 218-242: Wrap the sdkDeleteMutation.mutateAsync call in the
deletePost flow with error handling so broadcast failures do not proceed to the
cache update or success toast. On failure, dispatch an appropriate error toast
using the existing notification and intl patterns, while preserving the current
success path after a successful mutation.
- Around line 212-216: Update deletePost in the pinned-post deletion path to
also clear or invalidate pinnedPostQuery’s cache for the same author and
permlink when content.permlink matches pinnedPermlink, before the cached result
is rebuilt. Preserve the existing feed infinite-query pruning and ensure _data
cannot re-add the deleted pinned post.
🪄 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 Plus
Run ID: 4e11c2f1-8205-42db-b444-166a2f50cce1
📒 Files selected for processing (3)
src/components/postsList/container/postsListContainer.tsxsrc/components/tabbedPosts/view/postsTabContent.tsxsrc/providers/queries/postQueries/feedQueries.ts
select() in this hook already treats a page as possibly not an array. The cache patch filtered unconditionally, so the same case would throw and abort the whole update rather than skipping one page. Matches the guard the hook already applies.
deletedKeys is component state, so it only hides the pinned post for the life of the hook. Leaving the profile and returning remounts with an empty set while pinnedPostQuery still holds the post, and the union prepends it again. The feed cache patch did not cover this either: the pinned post is served from its own query and was never in the feed cache to begin with, so my earlier claim that the patch made the removal durable was only true for ordinary feed rows. Clears that query when the deleted post is the pinned one. The key is built with the same getPostQueryOptions call useGetPostQuery uses, and the union already guards on falsy data.
… caches Clearing the pinned post's query was the wrong move: emptying it forces a refetch, and the delete broadcasts async, so that refetch returns the not-yet-indexed post and puts it straight back. Greptile caught this on the previous commit. Records deleted author/permlink in a module-level set instead. Being module level rather than component state is the point: the previous version was lost when the screen unmounted, so leaving a profile and returning brought the post back. It now suppresses the row whatever any cache still holds, without forcing a fetch that would lose the race. Being shared across feeds is deliberate too: deleting from a profile tab hides the post on the main feed as well. The feed cache patch stays, since it provides data rather than emptying it and so does not trigger a fetch.
parsePost swaps author and permlink to the original entry, so the sheet's content is the ORIGINAL rather than the wrapper the user is looking at. On a self cross-post the author check therefore passes and deleting destroys the original post while leaving the wrapper behind, which is not what delete on that card means. This PR made it worse rather than causing it: hiding the row and toasting success told the user the cross-post was gone when their original had been deleted instead. Withholds delete on cross-posts, matching mute and pin, which are already withheld through _canModerateCommunityPost for exactly this ambiguity. The original is still deletable from its own card or screen.
1 participant
Closes #3407
postsListContainermounted the options sheet with noonDelete, so deleting a post from a feed ran the sheet's own path:navigation.goBack()popped the feed screen, and nothing removed the post, so it stayed visible until something refetched. Reachable on your own profile feed, where_canDeletePostis satisfied.Where the removal belongs
The container receives
postsas a prop and does not own the list, so it cannot remove anything itself. The owner isuseFeedQuery, which is wheredeletePostnow lives, modelled onwavesQueries.deleteWave:Wired through as
onDeletePost:postsTabContent->postsListContainer-> the sheet'sonDelete.The cache is patched, not invalidated. The delete broadcasts async, so
mutateAsyncresolves on mempool acceptance and a refetch issued at that point returns pre-transaction state, bringing the post straight back. Same reasondeleteWavepatches, and the same trap as thesetRolecache work in #3397.Consumer audit
This is the last list consumer. I checked what each screen actually renders this time, not just which pass the prop, since that narrower check is what let the #3408 regression through.
onDeletewavesScreenwavesQuery.deleteWavecommentsViewpostCommentspostsListContainerpostScreeneditorScreenEvery consumer owning a surrounding list now delegates, and the fallback is only reached where popping is genuinely correct.
Deliberately not in this PR
The sheet still pops by default, which is the footgun behind all three occurrences. Inverting it to an explicit opt-in is filed as #3409 rather than bundled here: there is no active bug once this lands, and the last attempt to harden that default inside a bug-fix PR (#3408's
parent_authorgate) caused a regression. It deserves its own change and its own review.Testing
yarn test:ci: 730 passed, 1 skipped, 49 suites.yarn lint: 0 errors.Device checks:
Summary by CodeRabbit
New Features
Bug Fixes