fix(self-hosted): reader-facing UI and content-filtering defects - #1322
Conversation
`{user?.username && loading ? sending : send}` binds as
`(user?.username && loading) ? sending : send`, so a logged-out visitor
fell into the send branch and the button rendered the sign-in prompt and
the send label side by side. Replaced both expressions with a single
getTipSubmitLabelKey() call that returns exactly one key per state.
The button receives the raw post body, but only stripped HTML tags, so image URLs, link targets and the # and * markers were all spoken. The correct strip already existed as a private helper in blog-post-header; moved it to features/blog/utils/strip-markdown and used it in both places instead of duplicating it.
… ownership /publish lets any authenticated user compose on a community instance, but /edit required isBlogOwner as well as a username match, so a member who published there could never reopen their post. The Edit control had the opposite problem: it rendered on isBlogOwner alone, so the owner saw it on every post and following it just redirected to /blog. Both now use canEditEntry(user, author), which is the authority an edit actually needs.
html/body carried overflow-x: hidden, which makes the element a scroll container on both axes. body then became the nearest scrollport for the sidebar's lg:sticky, and since body's height is content-driven it never scrolls, so the sidebar never stuck. overflow-x: clip clips the same content without creating a scroll container, so the horizontal-scroll guard stays in place and sticky resolves against the viewport again.
modern-gradient, the default template, sets --theme-tag-bg to a linear-gradient, but .tag-theme applied it through background-color. A gradient is not a color, so the declaration was dropped and the chips had no background. The template's own compensating rule targeted .theme-tag, a class no component renders. .tag-theme now uses the background shorthand, which takes a color or an image, and the template rule keeps only its border and points at the real class. Also adds the .border-theme-accent utility the tipping currency cards already referenced; it was undefined, so the selected card fell back to a currentColor border.
bg-theme-bg on the sidebar tip button and text-theme-contrast on the tip send button are not defined anywhere, so both were silent no-ops: the button had no surface colour and the label fell back to the inherited one. Replaced with bg-theme-primary and text-theme-primary, matching the other tipping buttons.
The three post links on a feed card were raw <a href>, so every feed-to-post click reloaded the document: the SPA booted again, refetched the instance config and dropped the query cache the feed had just filled. They now use the router Link against the declared /$author/$permlink route. The author link stays a plain anchor, it leaves the instance. The href loses the @ prefix (/alice/post rather than /@alice/post). The router percent-encodes path params, so keeping the prefix would have produced /%40alice/post; both forms resolve to the same route and the post page already strips a leading @, so existing links keep working. The dead-route guard's segment-safety entries for this file went with the templates they described; a stale entry fails that test, so they cannot be left behind.
House copy rule: no em or en dashes in user-facing text. The post title separator now matches the pipe the main app already uses.
Every SDK post query runs its results through filterDmcaEntry, but that reads lists which stay empty until ConfigManager.setDmcaLists is called, and nothing in this app called it. Takedown-listed content was served unfiltered. The lists are now fetched at bootstrap from the same three published files the main app bundles. The fetch is started at module load and never awaited, and a failed or blocked request leaves that list empty, so an unreachable list source cannot stop the blog rendering. The community feed also called bridge.get_ranked_posts directly, which skipped filtering and tag sanitisation regardless of the lists. It now uses the SDK's getPostsRankedInfiniteQueryOptions, the same path the rest of the app's queries take. That query also keeps pinned posts at the head of the first page, which the bespoke one did not.
|
Warning Review limit reached
Next review available in: 17 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (25)
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 |
![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: 8b52d1ba46
ℹ️ 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".
| // Load the DMCA lists the SDK's post queries filter against. Started here, at | ||
| // module load, so the request is in flight before React mounts and well before | ||
| // any post query can resolve. Not awaited: the lists are fetched from a remote | ||
| // origin and nothing about rendering the blog may depend on that origin being | ||
| // reachable. | ||
| loadDmcaLists(); |
There was a problem hiding this comment.
When a Hive RPC resolves before these three cross-origin requests—especially when one request takes the 10-second timeout—the SDK filters that response against its initially empty DMCA configuration and React Query caches and displays the uncensored result. Completing loadDmcaLists() only calls ConfigManager.setDmcaLists; it neither refilters nor invalidates existing post queries, so listed content can remain visible for the session. Await the fail-open loader before mounting post routes, or invalidate/refilter all affected queries after installing the lists.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed. Post queries are now refetched once the lists are installed, so the filtering applies to whatever resolved during the window. Every post key starts with "posts", so one invalidation reaches them all.
Kept the loader unawaited rather than blocking the routes: nothing about rendering a blog may depend on a remote origin being reachable, and on a timing-out list request that would be a ten second blank page for every visitor. The refetch is skipped when no list loaded, since there is then nothing to filter and it would cost every visitor a round of requests for no change.
| const communityOptions = getPostsRankedInfiniteQueryOptions( | ||
| communitySort, | ||
| communityId, | ||
| limit, | ||
| '', | ||
| !!communityId && isCommunityMode, |
There was a problem hiding this comment.
On every community feed, switching to this helper prevents hasNextPage from ever becoming false: its getNextPageParam at packages/sdk/src/modules/posts/queries/get-posts-ranked-query-options.ts:94-100 returns an object even for an empty page, whereas React Query requires undefined/null to signal completion. Because this component keeps rendering DetectBottom and passes a newly created callback on each render, an intersecting end marker repeatedly appends empty pages, causing continuous query-state churn and unbounded cache growth after the user reaches the end.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed, and it is a pre-existing bug in the SDK rather than in this PR, so it is fixed separately in #1324.
getNextPageParam returned an object for every page including an empty one, and React Query reads "there is no next page" from undefined alone, so hasNextPage was permanently true. The hasNextPage field inside the returned param was only ever read by the query function, which is why no extra RPCs went out, but each fetchNextPage still appended an empty page.
Worth noting this PR does make self-hosted newly subject to it, since the bespoke query it replaces returned undefined on a short page. That is the trade for DMCA filtering, which the bespoke query skipped entirely. #1324 removes the trade.
| const communityOptions = getPostsRankedInfiniteQueryOptions( | ||
| communitySort, | ||
| communityId, | ||
| limit, | ||
| '', | ||
| !!communityId && isCommunityMode, |
There was a problem hiding this comment.
For trending, payout, and muted community filters, this helper sorts each bridge response by creation time before getNextPageParam selects the displayed page's last item (packages/sdk/src/modules/posts/queries/get-posts-ranked-query-options.ts:74-99). That item is generally not the last item in the bridge's ranked order, so the next RPC starts from the middle of the previous ranked page and scrolling produces duplicates or skips posts. Preserve the final raw-response cursor separately or avoid reordering ranked pages before pagination.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed, and also pre-existing in the SDK. Filed as #1325 rather than fixed, because the fix is a product decision.
The cursor is wrong because the page order was changed, but the order change is itself questionable: a trending feed re-sorted by creation date is not a trending feed, and the bridge already returns posts in the order the sort asked for. Stopping the re-sort fixes the ordering and the cursor together and is the smaller change, but it visibly alters community feed ordering on the main app, so someone should choose it deliberately. The alternative keeps the sort and carries the raw cursor alongside the sorted page, which the current Entry[] page type has nowhere to put.
The bespoke query this PR replaces did not re-sort, so its cursor was correct. Same trade as the other thread: this is the cost of routing community feeds through the filtered path, and #1325 is how it gets paid back.
Routing the feed cards through Link fixed the SPA reboot but changed the URL: the router percent-encodes path params, so the canonical /@author/permlink became /%40author/permlink, and dropping the '@' to avoid that made every feed link non-canonical instead. Non-canonical links across the whole feed is a wider version of a defect already on the list, and '@author/permlink' is the form the rest of the Hive ecosystem links with. The router now allows '@' through unencoded, so the links keep their canonical shape and still route client-side. The two halves only work together: allowing the character without passing it changes nothing, and passing it without allowing it produces %40. Guarded as a pair, since neither is expressible in the type system. Reverting either half fails.
…landed Nothing about rendering the blog may wait on a remote origin, so the list fetch is not awaited. That leaves a window: a post query can resolve first, filterDmcaEntry runs against an empty configuration, and the unfiltered result is cached and shown for the rest of the session. On a slow or timing-out list request that window is seconds wide. Post queries are now refetched once the lists are installed, so the filtering applies to what is already on screen. Every post key starts with "posts", so one invalidation reaches them all. Skipped when nothing was loaded. There is then nothing to filter, and refetching would cost every visitor a round of requests for no change whenever the lists are unreachable.
getNextPageParam reads the cursor from the last entry of the page the query function returns, and the bridge continues a ranked feed from that entry in ITS ranking. The page was re-sorted by creation date first, so for trending, payout and muted the cursor named the oldest entry by date rather than the last by rank: the next request started from the middle of the previous ranked page, and scrolling repeated some posts and skipped others. The display ordering, pinned entries first then created descending, moves into `select`, which React Query applies after pagination. Nothing a reader sees changes; the cursor is now taken from the bridge's own order. This closes the second half of the ranked-posts problem, so #1322 can move community feeds onto this query without inheriting it.
|
Unblocked. #1324 now fixes both halves of the ranked-posts problem, so moving community feeds onto that query no longer inherits a cursor defect. The termination fix was already there. The cursor fix landed after: the created-date re-sort moved out of the query function and into Merge #1324 before this one. |
1 participant
Reader-facing defects in the self-hosted blog app. One commit per defect.
Tip button rendered two labels.
{user?.username && loading ? sending : send}binds as(user?.username && loading) ? sending : send, so a logged-out visitor got the sign-in prompt and the send label side by side. Both expressions are replaced by a singlegetTipSubmitLabelKey()call that returns exactly one key per state.Text-to-speech read markdown aloud. The button gets the raw post body but only stripped HTML tags, so image URLs, link targets and the
#/*markers were spoken. The correct strip already existed as a private helper inblog-post-header; it moved tofeatures/blog/utils/strip-markdownand both call sites use it.Community authors could not edit their own posts.
/publishlets any authenticated user compose on a community instance, but/editrequiredisBlogOwneron top of a username match, so a member who published there could never reopen the post. The Edit control had the mirror problem: it rendered onisBlogOwneralone, so the instance owner saw it on every post and following it just redirected to/blog. Both now usecanEditEntry(user, author), which is the authority an edit actually needs, since the operation is broadcast under the author's own account.Sticky sidebar never stuck.
html, body { overflow-x: hidden }makes the element a scroll container on both axes, so body became the nearest scrollport for the sidebar'slg:sticky; body's height is content-driven and never scrolls, so it never stuck.overflow-x: clipclips the same content without creating a scroll container, so the horizontal-scroll guard stays and sticky resolves against the viewport.Tag chips had no background under modern-gradient. That template, which is the default, sets
--theme-tag-bgto alinear-gradient, but.tag-themeapplied it throughbackground-color. A gradient is not a color, so the declaration was dropped. The template's compensating rule targeted.theme-tag, a class no component renders..tag-themenow uses thebackgroundshorthand and the template rule keeps only its border, against the real class. Also adds the.border-theme-accentutility the tipping currency cards already referenced but which was never defined.Undefined utility classes.
bg-theme-bg(sidebar tip button) andtext-theme-contrast(tip send button) are not defined anywhere, so both were silent no-ops. Replaced withbg-theme-primaryandtext-theme-primary, matching the other tipping buttons.Feed cards reloaded the document. The three post links on a card were raw anchors, so every feed-to-post click booted the SPA again, refetched the instance config and dropped the query cache the feed had just filled. They now use the router
Linkagainst the declared/$author/$permlinkroute. The author link stays a plain anchor because it leaves the instance. Note the href loses the@prefix (/alice/postrather than/@alice/post): the router percent-encodes path params, so keeping it would have produced/%40alice/post. Both forms resolve to the same route and the post page already strips a leading@, so existing links keep working. The dead-route guard's segment-safety entries for this file went with the templates they described.Document title separator. Em dash replaced with the pipe the main app already uses.
DMCA filtering was a no-op. Every SDK post query runs results through
filterDmcaEntry, but that reads lists which stay empty untilConfigManager.setDmcaListsis called, and nothing in this app called it. The lists are now fetched at bootstrap from the same three published files the main app bundles. The fetch starts at module load and is never awaited, and a failed or blocked request leaves that list empty, so an unreachable source cannot stop the blog rendering. Separately, the community feed calledbridge.get_ranked_postsdirectly, skipping filtering and tag sanitization regardless of the lists; it now usesgetPostsRankedInfiniteQueryOptions. That query also keeps pinned posts at the head of the first page, which the bespoke one did not, and sorts non-hot feeds by creation date.Tests: new unit tests for the tip label, the markdown strip, the edit predicate and the DMCA loader, plus a guard that no module makes its own ranked-posts bridge call. Each was mutation-verified by breaking the fix and confirming the expected failure. The CSS fixes were checked by reading the compiled stylesheet after a build. Full suite 153 passing, typecheck and rsbuild build clean.