feat(sdk): read post tips over GET and align pro-members freshness - #1273
Conversation
Post tips were fetched with a POST, which no cache may store, so the same totals were re-requested on every mount. The endpoint keys off nothing but author and permlink and needs no auth, so it is now read as a GET on a URL and can be served from the browser cache. - post-tips: POST body -> GET path segments, both encodeURIComponent'd so a permlink cannot alter which resource is addressed. staleTime added; it was unset, meaning stale on every mount and window refocus. - pro-members: staleTime 5m -> 10m to match the roster's fresh window, so react-query stops re-asking for a list the browser already holds. Requires the GET route to be live server-side first: without it the request falls through to the unmatched-GET template page, which is a 200 that fails to parse as JSON. Web and mobile both consume these through the SDK.
![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: cbec038cd2
ℹ️ 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".
| return response.json() as Promise<ProMembersResponse>; | ||
| }, | ||
| staleTime: 5 * 60 * 1000, | ||
| staleTime: 10 * 60 * 1000, |
There was a problem hiding this comment.
When a page reload creates a new QueryClient while the browser already has, for example, a nine-minute-old pro-members response, fetch returns that cached response but React Query records it as freshly fetched and waits another ten minutes. A membership change can therefore remain hidden for nearly twenty minutes—well beyond the endpoint's ten-minute window, and up to five minutes longer than with the previous setting. Keep the shorter staleTime or derive the remaining freshness from the response's Age/cache metadata instead of restarting the full server window.
Useful? React with 👍 / 👎.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (7)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe SDK now retrieves post tips through an encoded, cacheable GET request with expanded tests. Pro-members caching documentation and SDK and wallets release metadata were updated. ChangesSDK query update
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant QueryOptions
participant Fetch
participant PostTipsEndpoint
QueryOptions->>Fetch: GET encoded author/permlink path
Fetch->>PostTipsEndpoint: Request post tips
PostTipsEndpoint-->>Fetch: JSON response or HTTP error
Fetch-->>QueryOptions: Parsed response or thrown error
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 `@packages/sdk/src/modules/accounts/queries/get-pro-members-query-options.ts`:
- Around line 17-18: Update the staleTime explanation near the cache-control
comment to state that a longer staleTime keeps the cached roster fresh longer
and delays refetches after the endpoint’s server-side cache window expires;
remove the implication that it causes stale badges sooner.
In `@packages/sdk/src/modules/posts/queries/get-post-tips-query-options.spec.ts`:
- Around line 49-51: Update the test for getPostTipsQueryOptions to assert that
staleTime is exactly 60 seconds, rather than merely greater than zero, so it
verifies the required server-cache freshness window.
- Line 17: Update the fetchMock definition in the get-post-tips query options
test to avoid unused _url and _init parameters, using a typed zero-argument mock
while preserving the existing typed call inspection and mock response behavior.
🪄 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: 84571ecb-003f-43ae-90dc-a57a522aa493
📒 Files selected for processing (3)
packages/sdk/src/modules/accounts/queries/get-pro-members-query-options.tspackages/sdk/src/modules/posts/queries/get-post-tips-query-options.spec.tspackages/sdk/src/modules/posts/queries/get-post-tips-query-options.ts
| // an argument-less mock infers an empty tuple and asserting on it needs a cast | ||
| // that would pass whatever it was given. | ||
| function captureRequest(response: unknown = { tips: [] }) { | ||
| const fetchMock = vi.fn(async (_url: string, _init?: RequestInit) => ({ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove the unused mock parameters.
_url and _init trigger the reported ESLint errors, so this suite will not meet the clean-lint requirement. Consume them or replace the implementation with a typed zero-argument mock while retaining typed call inspection.
Proposed fix
- const fetchMock = vi.fn(async (_url: string, _init?: RequestInit) => ({
- ok: true,
- status: 200,
- json: async () => response,
- }));
+ const fetchMock = vi.fn(async (_url: string, _init?: RequestInit) => {
+ void _url;
+ void _init;
+ return {
+ ok: true,
+ status: 200,
+ json: async () => response,
+ };
+ });📝 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.
| const fetchMock = vi.fn(async (_url: string, _init?: RequestInit) => ({ | |
| const fetchMock = vi.fn(async (_url: string, _init?: RequestInit) => { | |
| void _url; | |
| void _init; | |
| return { | |
| ok: true, | |
| status: 200, | |
| json: async () => response, | |
| }; | |
| }); |
🧰 Tools
🪛 ESLint
[error] 17-17: '_url' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 17-17: '_init' is defined but never used.
(@typescript-eslint/no-unused-vars)
🤖 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 `@packages/sdk/src/modules/posts/queries/get-post-tips-query-options.spec.ts`
at line 17, Update the fetchMock definition in the get-post-tips query options
test to avoid unused _url and _init parameters, using a typed zero-argument mock
while preserving the existing typed call inspection and mock response behavior.
Source: Linters/SAST tools
Raising pro-members staleTime to match the server window was wrong. react-query cannot see how old a response already was when fetch served it from the browser cache, so it treats a nine-minute-old body as freshly fetched and starts its own window from zero. Worst-case staleness is the two windows added together, so matching them roughly doubles it instead of aligning them. Reverted to 5m and documented the reasoning on both queries. Also assert the exact post-tips window in the spec: toBeGreaterThan(0) passes for a 1ms staleTime, which is the very bug the test is meant to catch.
|
Thanks both. Triage: Codex, CodeRabbit, exact freshness assertion — valid, fixed. CodeRabbit, unused mock parameters cause ESLint errors — not reproducible, no change. CodeRabbit, stale-time wording — superseded. Both comments were rewritten for the revert above. |
1 participant
Client half of #1272. Server half is ecency/vision-api#61.
Post tips were fetched with a
POST. A POST response is uncacheable by definition, so the same totals were re-requested on every mount, andgetPostTipsQueryOptionsalso had nostaleTime(defaulting to 0, i.e. stale on every mount and window refocus). The endpoint keys off nothing but author and permlink and needs no auth, so there was never a reason for it to be a POST.Changes
post-tipsnow reads asGET /private-api/post-tips/{author}/{permlink}. Both segments go throughencodeURIComponent, so a permlink cannot change which resource is addressed.staleTimeset to the endpoint's fresh window.pro-membersstaleTime5m to 10m, matching its fresh window. A shorter one only makes react-query re-ask for a roster the browser already holds now that the response carries aCache-Control.announcementsneeds no change here; its 1hstaleTimealready sits above the server window.staleTimealone would not have helped much: it dedupes within a session, while theCache-Controlfrom #61 is what stops the refetch on the next page load. The two are paired on purpose.Merge order
This must not be published before ecency/vision-api#61 is deployed. Without the GET route, the request falls through to the unmatched-GET template page, which is a
200whose body fails to parse as JSON, so the query errors rather than degrading. The POST route stays in place server-side, so nothing breaks in the other direction.Note both web and mobile consume these through
@ecency/sdk(tipsQueries.tsandproQueries.tsin ecency-mobile), so mobile picks the change up on its next SDK bump rather than needing its own patch.Verification
entry-tip-btnandpro-configspecs pass.The new spec pins the verb and URL shape deliberately: a regression back to a POST, or an unencoded permlink, is invisible in the UI and costs every reader a round trip per mount.
Summary by CodeRabbit