Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughClient-side filtering and owner-aware UI were added to hide listings marked as hidden: browse lists filter out hidden items; detail and edit pages gate visibility and editing based on ownership and hidden status; two new UI components surface unavailable/hidden states. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Browse as Browse Page
participant Detail as Listing Detail Page
participant Client as Client Data Layer
participant Router as Router
User->>Browse: Open browse
Browse->>Client: Fetch listings
Client-->>Browse: Return listings (includes isHidden flags)
Browse->>Browse: Filter out listings where isHidden === true
Browse-->>User: Show filtered listings
User->>Detail: Open listing [id]
Detail->>Client: Fetch listing by id
Client-->>Detail: Return listing
alt listing.isHidden === true and currentUser != sellerId
Detail->>Router: Render ListingUnavailable
Router-->>User: "This listing is no longer available"
else listing.isHidden === true and currentUser === sellerId
Detail->>Detail: Render listing + HiddenBanner
Detail-->>User: Show listing with hidden banner and no Edit button
else listing.isHidden !== true
Detail->>Detail: Render normal listing view (Edit allowed for owner)
Detail-->>User: Show listing details
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
frontend/app/listings/[id]/edit.tsx (2)
145-151: Two consecutive early-return blocks render the same component — merge them.Both
listing === nullandisHiddenguards return<ListingUnavailable />. Combining them removes redundancy and makes the intent clearer.♻️ Proposed refactor
- if (listing === null) { - return <ListingUnavailable />; - } - - if (isHidden) { - return <ListingUnavailable />; - } + if (listing === null || isHidden) { + return <ListingUnavailable />; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/listings/`[id]/edit.tsx around lines 145 - 151, The two consecutive early-return checks both render ListingUnavailable; combine them into a single guard that returns <ListingUnavailable /> when either listing === null or isHidden is true (e.g., if (listing === null || isHidden) return <ListingUnavailable />) to remove redundancy and clarify intent in the edit component where listing, isHidden, and ListingUnavailable are used.
17-17: Import alias inconsistency with the rest of the file.Line 16 uses a relative path (
'../../../components/TagInput'); Line 17 switches to the@/alias forListingUnavailable. Pick one style and apply it consistently.♻️ Proposed fix (normalize to relative path)
-import ListingUnavailable from '@/components/ListingUnavailable'; +import ListingUnavailable from '../../../components/ListingUnavailable';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/listings/`[id]/edit.tsx at line 17, The import for ListingUnavailable uses the alias '@/components/ListingUnavailable' while other imports in this file (e.g., the relative '../../../components/TagInput') use relative paths; make them consistent by switching the ListingUnavailable import to the same relative style (use the appropriate relative path to components/ListingUnavailable) so all imports (including TagInput and ListingUnavailable) follow the same pattern.frontend/components/HiddenBanner.tsx (1)
3-4: Placeholder support email must be replaced before shipping.The comment on Line 3 explicitly flags that
'support@polybuys.com'is not the real address. This will go to production as-is if not resolved.Do you want me to open a tracking issue to ensure this is replaced with the verified support address before release?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/HiddenBanner.tsx` around lines 3 - 4, Replace the placeholder SUPPORT_EMAIL constant in HiddenBanner.tsx with the verified support address (or load it from configuration/env if you need different values per environment); update the const SUPPORT_EMAIL = 'support@polybuys.com' to the real email or to a process.env or config lookup (and ensure any components using SUPPORT_EMAIL continue to receive the updated value).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/index.tsx`:
- Around line 115-116: Move the hidden-listing filter from the client into the
backend query so pagination counts (isDone and continueCursor) are computed
after excluding hidden items: update the backend getListings implementation (see
backend/convex/listings.ts and the logic around the existing search filter at
line ~795) to add isHidden !== true to the query and ensure
continueCursor/isDone are derived from the filtered result; keep the client-side
defensive guard in frontend/app/index.tsx (the allListings -> listings filter)
as a last-resort safety net but do not rely on it for pagination correctness.
In `@frontend/app/listings/`[id].tsx:
- Around line 36-42: The hidden-listing check has a race when currentUserSubject
is still undefined; update the loading guard to wait for both subscriptions
before rendering the hidden-owner logic: specifically treat listing ===
undefined OR currentUserSubject === undefined as "still loading" (but allow
currentUserSubject === null to proceed), so that isOwner (currentUserSubject ===
listing.sellerId), isHidden, and isHiddenOwnerView are only evaluated after both
values are defined and then only non-owners see <ListingUnavailable />; update
the component where currentUserSubject, listing, isOwner, isHidden,
isHiddenOwnerView and ListingUnavailable are referenced.
In `@frontend/components/HiddenBanner.tsx`:
- Line 7: The onAppealPress handler currently calls
Linking.openURL(`mailto:${SUPPORT_EMAIL}`) and discards the returned Promise;
update onAppealPress to handle rejections by appending a .catch(...) to
Linking.openURL and present a user-friendly Alert on failure, and ensure Alert
is added to the component imports so the Alert API is available; keep the mailto
call and SUPPORT_EMAIL usage the same, but surface errors via Alert.alert(...)
instead of letting the rejection be unhandled.
---
Nitpick comments:
In `@frontend/app/listings/`[id]/edit.tsx:
- Around line 145-151: The two consecutive early-return checks both render
ListingUnavailable; combine them into a single guard that returns
<ListingUnavailable /> when either listing === null or isHidden is true (e.g.,
if (listing === null || isHidden) return <ListingUnavailable />) to remove
redundancy and clarify intent in the edit component where listing, isHidden, and
ListingUnavailable are used.
- Line 17: The import for ListingUnavailable uses the alias
'@/components/ListingUnavailable' while other imports in this file (e.g., the
relative '../../../components/TagInput') use relative paths; make them
consistent by switching the ListingUnavailable import to the same relative style
(use the appropriate relative path to components/ListingUnavailable) so all
imports (including TagInput and ListingUnavailable) follow the same pattern.
In `@frontend/components/HiddenBanner.tsx`:
- Around line 3-4: Replace the placeholder SUPPORT_EMAIL constant in
HiddenBanner.tsx with the verified support address (or load it from
configuration/env if you need different values per environment); update the
const SUPPORT_EMAIL = 'support@polybuys.com' to the real email or to a
process.env or config lookup (and ensure any components using SUPPORT_EMAIL
continue to receive the updated value).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
frontend/app/index.tsxfrontend/app/listings/[id].tsxfrontend/app/listings/[id]/edit.tsxfrontend/components/HiddenBanner.tsxfrontend/components/ListingUnavailable.tsx
Comment on lines
+115
to
+116
| // Defensive client-side guard in case stale cache includes hidden listings. | ||
| const listings = allListings.filter((listing) => listing.isHidden !== true); |
Contributor
There was a problem hiding this comment.
Client-side hidden filter can produce visible pages shorter than PAGE_SIZE, potentially starving the infinite-scroll trigger.
isDone and continueCursor are determined by the backend before this filter runs. If the backend returns a full page of 20 but some are hidden, FlatList receives fewer than 20 items. With onEndReachedThreshold={0.5}, if the resulting list is too short to fill the viewport, onEndReached may never fire and the user is stuck with an incomplete page.
The comment calls this a defensive guard for stale cache, which is fair — but the ideal fix is to push the isHidden filter into the getListings backend query (consistent with how the search endpoint already does it per backend/convex/listings.ts:795). The client-side guard can then remain as a true last-resort safety net without affecting pagination arithmetic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/index.tsx` around lines 115 - 116, Move the hidden-listing
filter from the client into the backend query so pagination counts (isDone and
continueCursor) are computed after excluding hidden items: update the backend
getListings implementation (see backend/convex/listings.ts and the logic around
the existing search filter at line ~795) to add isHidden !== true to the query
and ensure continueCursor/isDone are derived from the filtered result; keep the
client-side defensive guard in frontend/app/index.tsx (the allListings ->
listings filter) as a last-resort safety net but do not rely on it for
pagination correctness.
Collaborator
|
Created clean companion branch Feedback:
|
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.
Linked Issues
Closes #22
Linear: POLY-22
Summary
Implemented hidden-listing handling in the frontend so moderated content is not shown in normal browse flows.
Changes:
Added ListingUnavailable component with:
Added reusable HiddenBanner for owner-only hidden listing view
Updated listing detail screen to:
Added defensive client-side filtering in home feed to exclude hidden listings
How to Test
Steps to verify locally:
npm run lintnpm run typechecknpm testnpm run dev:backend(in terminal A)npm run dev(in terminal B)Checklist
npm run lint)devScreenshots / Demos
Still requires testing
Summary by CodeRabbit