Wave overview cards to show description drop and take last drop time from ApiWave itself#2111
Merged
GelatoGenesis merged 1 commit intomainfrom Mar 12, 2026
Merged
Wave overview cards to show description drop and take last drop time from ApiWave itself#2111GelatoGenesis merged 1 commit intomainfrom
GelatoGenesis merged 1 commit intomainfrom
Conversation
…from ApiWave itself Signed-off-by: GelatoGenesis <tarmokalling@gmail.com>
📝 WalkthroughWalkthroughThis PR refactors wave preview rendering to use the existing Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan for PR comments
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/waves/utils/getOptimisticDrop.ts (1)
40-50:⚠️ Potential issue | 🟠 Major
last_drop_timeis stale in the optimistic drop.After this PR, cards read recency from
wave.last_drop_time. Reusing the pre-submit value here means optimistic posts can keep the old activity time and ordering until the round-trip finishes. Capture onenowvalue and use it forserial_no,created_at, andwave.last_drop_time.Suggested fix
const replyTo = getReplyTo(); const replyToObj = replyTo ? { reply_to: replyTo } : {}; + const now = Date.now(); return { id: getOptimisticDropId(), - serial_no: Date.now(), + serial_no: now, ...replyToObj, wave: { id: wave.id, @@ picture: wave.picture ?? "", description_drop_id: wave.description_drop.id, - last_drop_time: wave.last_drop_time, + last_drop_time: now, @@ - created_at: Date.now(), + created_at: now,Also applies to: 91-91
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/utils/getOptimisticDrop.ts` around lines 40 - 50, The optimistic drop creation is using stale wave.last_drop_time; capture a single timestamp (e.g., const now = Date.now()) inside getOptimisticDrop (use getOptimisticDropId and the returned object construction) and replace the separate Date.now() calls so that serial_no, created_at, and wave.last_drop_time all use that same now value; update both occurrences (the serial_no/created_at assignment and the wave.last_drop_time field) so the optimistic drop and wave recency are consistent until the server round-trip completes.
🧹 Nitpick comments (3)
openapi.yaml (1)
4710-4719: Keep this endpoint’s page size aligned with the existing overview payload.This route returns the same heavyweight
ApiWaveshape as/waves-overview, but it raises the default/max page size from 10/20 to 50/100. That will make first-page responses much larger than the current overview API. If you really need bigger pages here, it would be safer to introduce a slimmer response model instead of increasing the cap onApiWave.♻️ Suggested limit change
- name: limit in: query - description: How many waves to return (50 by default) + description: How many waves to return (10 by default) required: false schema: type: integer format: int64 minimum: 1 - maximum: 100 - default: 50 + maximum: 20 + default: 10🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi.yaml` around lines 4710 - 4719, The page-size for this operation is too large compared to the existing /waves-overview which returns the heavyweight ApiWave; either reduce the query-parameter "limit" default and max to match the overview (set limit.default = 10 and limit.maximum = 20) or keep the larger caps but change the response schema from ApiWave to a slimmer model (e.g., ApiWaveOverviewLite) and update the operation response to use that new schema; modify the "limit" parameter values or replace the response schema accordingly and ensure any referenced models (ApiWave or the new slim model) are updated consistently.__tests__/components/home/explore-waves/ExploreWaveCard.test.tsx (2)
46-50: Redundant mock clearing.
jest.clearAllMocks()already clears all mocks includingmockContentDisplay, so the subsequentmockContentDisplay.mockClear()call is unnecessary.♻️ Suggested simplification
beforeEach(() => { jest.clearAllMocks(); - mockContentDisplay.mockClear(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/home/explore-waves/ExploreWaveCard.test.tsx` around lines 46 - 50, In the ExploreWaveCard describe block's beforeEach, remove the redundant mock clearing call: keep jest.clearAllMocks() and delete the explicit mockContentDisplay.mockClear() invocation so mocks are only cleared once; locate this in the beforeEach within the ExploreWaveCard test file where mockContentDisplay.mockClear() is currently called and remove that line.
115-203: Consider improving type safety for the test fixture helper.The
as anycast on line 189 is pragmatic for test fixtures but could mask issues if theApiWaveschema evolves. The current shallow-merge approach also won't deep-merge other nested objects likechatorvotingif overrides are provided.This is acceptable for the current test scope, but if more complex overrides are needed later, consider using a type-safe factory library or stricter typing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/home/explore-waves/ExploreWaveCard.test.tsx` around lines 115 - 203, The test fixture helper createWave uses a blanket "as any" on baseWave and shallow-merges overrides which can hide type mismatches and won't deep-merge nested objects like chat or voting; remove the "as any" cast, type baseWave explicitly as ApiWave, and change the merge logic to strongly-typed merging (for example, recursively merge or explicitly merge nested fields) so overrides for nested properties (metrics, description_drop, chat, voting, etc.) are preserved and type-checked against ApiWave; ensure function signature still accepts overrides: Partial<ApiWave> and return value is typed ApiWave.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/home/explore-waves/ExploreWaveCard.tsx`:
- Around line 87-96: The card shows a preview from wave.description_drop via
descriptionPreview while still rendering the empty-state “No drops yet” based on
drops_count, causing a contradiction; in ExploreWaveCard update the render logic
so that when descriptionPreview (used by MessagePreviewContent) is truthy you
either (A) suppress/replace the empty-state text by gating the “No drops yet”
copy behind a condition that requires drops_count === 0 AND no
descriptionPreview, or (B) change the copy when descriptionPreview exists (e.g.,
“Description only — no drops yet”) so the UI distinguishes description content
from user drops; update the conditional that reads drops_count and
descriptionPreview to implement the chosen behavior.
In `@components/waves/memes/submission/utils/buildPreviewDrop.ts`:
- Around line 99-100: In buildPreviewDrop, the new preview drop is incorrectly
reusing wave.last_drop_time (description_drop_id: wave.description_drop.id,
last_drop_time: wave.last_drop_time) so the preview shows stale recency; update
the construction so last_drop_time is set to the preview submission’s timestamp
(e.g., preview.timestamp or preview.created_at) instead of wave.last_drop_time
and ensure any reference to description_drop_id remains
wave.description_drop.id; locate this in buildPreviewDrop and replace the
last_drop_time assignment accordingly.
---
Outside diff comments:
In `@components/waves/utils/getOptimisticDrop.ts`:
- Around line 40-50: The optimistic drop creation is using stale
wave.last_drop_time; capture a single timestamp (e.g., const now = Date.now())
inside getOptimisticDrop (use getOptimisticDropId and the returned object
construction) and replace the separate Date.now() calls so that serial_no,
created_at, and wave.last_drop_time all use that same now value; update both
occurrences (the serial_no/created_at assignment and the wave.last_drop_time
field) so the optimistic drop and wave recency are consistent until the server
round-trip completes.
---
Nitpick comments:
In `@__tests__/components/home/explore-waves/ExploreWaveCard.test.tsx`:
- Around line 46-50: In the ExploreWaveCard describe block's beforeEach, remove
the redundant mock clearing call: keep jest.clearAllMocks() and delete the
explicit mockContentDisplay.mockClear() invocation so mocks are only cleared
once; locate this in the beforeEach within the ExploreWaveCard test file where
mockContentDisplay.mockClear() is currently called and remove that line.
- Around line 115-203: The test fixture helper createWave uses a blanket "as
any" on baseWave and shallow-merges overrides which can hide type mismatches and
won't deep-merge nested objects like chat or voting; remove the "as any" cast,
type baseWave explicitly as ApiWave, and change the merge logic to
strongly-typed merging (for example, recursively merge or explicitly merge
nested fields) so overrides for nested properties (metrics, description_drop,
chat, voting, etc.) are preserved and type-checked against ApiWave; ensure
function signature still accepts overrides: Partial<ApiWave> and return value is
typed ApiWave.
In `@openapi.yaml`:
- Around line 4710-4719: The page-size for this operation is too large compared
to the existing /waves-overview which returns the heavyweight ApiWave; either
reduce the query-parameter "limit" default and max to match the overview (set
limit.default = 10 and limit.maximum = 20) or keep the larger caps but change
the response schema from ApiWave to a slimmer model (e.g., ApiWaveOverviewLite)
and update the operation response to use that new schema; modify the "limit"
parameter values or replace the response schema accordingly and ensure any
referenced models (ApiWave or the new slim model) are updated consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8646cb93-1756-40f0-832c-8b0ac611a00a
⛔ Files ignored due to path filters (3)
generated/models/ApiWave.tsis excluded by!**/generated/**generated/models/ApiWaveMin.tsis excluded by!**/generated/**generated/models/ObjectSerializer.tsis excluded by!**/generated/**
📒 Files selected for processing (7)
__tests__/components/home/explore-waves/ExploreWaveCard.test.tsxcomponents/home/explore-waves/ExploreWaveCard.tsxcomponents/home/explore-waves/useWaveLatestDrop.tscomponents/waves/memes/submission/utils/buildPreviewDrop.tscomponents/waves/utils/getOptimisticDrop.tshooks/useWaveDropsSearch.tsopenapi.yaml
💤 Files with no reviewable changes (1)
- components/home/explore-waves/useWaveLatestDrop.ts
ragnep
approved these changes
Mar 12, 2026
This was referenced Apr 6, 2026
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Tests