[feat]: unify review components between listings and pc-listings#357
Merged
Conversation
…ility report actions
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThis PR consolidates approval modal UI across handheld and PC listings into a single ChangesListing Approval Modal Consolidation
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/components/compatibility/review/CompatibilityReportReviewModal.tsx`:
- Around line 294-303: The map in CompatibilityReportReviewModal uses
customFieldDefinition.label as the React key which can collide; update the key
to use a stable unique identifier (e.g., fieldValue.customFieldDefinition.id or
fieldValue.id) and fall back to the loop index if no unique id exists so rows
reconcile correctly; locate the remainingFields.map(...) (and the
CompatibilityReportCustomFieldValue usage) and replace the key with the
unique-id-with-index-fallback approach.
🪄 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
Run ID: f19d7d85-2660-4dde-a642-52ce6e9475a2
📒 Files selected for processing (27)
src/app/admin/approvals/components/ApprovalModal.tsxsrc/app/admin/pc-listing-approvals/components/ApprovalModal.tsxsrc/app/listings/[id]/components/ListingDetailsClient.tsxsrc/app/listings/components/shared/CustomFieldRenderer.tsxsrc/app/listings/components/shared/CustomFieldsSection.tsxsrc/app/listings/components/shared/approval/ApprovalModalSharedComponents.tsxsrc/app/listings/components/shared/approval/ListingApprovalModal.tsxsrc/app/listings/components/shared/utils/isDefaultValue.tssrc/app/pc-listings/[id]/components/PcListingDetailsClient.tsxsrc/components/admin/review-risk/ReviewRiskIndicator.tsxsrc/components/admin/review-risk/ReviewRiskWarningBanner.tsxsrc/components/compatibility/review/CompatibilityReportApprovalActions.tsxsrc/components/compatibility/review/CompatibilityReportCustomFieldValue.tsxsrc/components/compatibility/review/CompatibilityReportReviewModal.test.tsxsrc/components/compatibility/review/CompatibilityReportReviewModal.tsxsrc/components/compatibility/review/ReviewRiskWarningBanner.tsxsrc/components/compatibility/review/index.tssrc/components/compatibility/review/reviewRiskDisplay.tssrc/constants/customFields.tssrc/server/api/routers/listings/core.test.tssrc/server/api/routers/listings/core.tssrc/server/api/routers/pcListings.test.tssrc/server/api/routers/pcListings.tssrc/server/services/review-risk.service.test.tssrc/server/services/review-risk.service.tstests/helpers/data-factory.tstests/listing-approval.spec.ts
💤 Files with no reviewable changes (2)
- src/app/listings/components/shared/approval/ApprovalModalSharedComponents.tsx
- src/app/listings/components/shared/approval/ListingApprovalModal.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/app/admin/approvals/page.tsx (1)
216-223: ⚡ Quick winAvoid refetching the admin datasets twice after bulk actions.
Both bulk mutations already run
invalidateQueries()inonSuccess, so these extra calls immediately trigger the same refetch fan-out again. That adds avoidable network churn and loading flicker on every bulk approve/reject.♻️ Proposed cleanup
const handleBulkApprovalWithConfirmation = async (listingIds: string[]) => { const confirmed = await confirmBulkApproval(listings, listingIds, confirm, 'listings') if (!confirmed) return await bulkApproveMutation.mutateAsync({ listingIds }) - await invalidateQueries() approvalModal.close() }reject: { label: 'Reject Selected', onAction: async (listingIds, notes) => { await bulkRejectMutation.mutateAsync({ listingIds, notes }) - await invalidateQueries() }, },Also applies to: 352-355
🤖 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 `@src/app/admin/approvals/page.tsx` around lines 216 - 223, The extra call to invalidateQueries() inside handleBulkApprovalWithConfirmation is redundant because bulkApproveMutation.mutateAsync already triggers invalidateQueries() in its onSuccess; remove the await invalidateQueries() line from handleBulkApprovalWithConfirmation (keep the confirm call, mutateAsync call and approvalModal.close()), and do the same cleanup in the other bulk action handler (e.g., the bulk reject handler such as handleBulkRejectionWithConfirmation or wherever bulkRejectMutation.mutateAsync is used) so you don't double-invalidate and refetch.
🤖 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/components/compatibility/review/CompatibilityReportReviewModal.tsx`:
- Around line 127-130: The Link element in CompatibilityReportReviewModal (the
<Link href={`/users/${props.author.id}`} target="_blank` />) opens an external
tab without tab-isolation attributes; add rel="noopener noreferrer" to that Link
(the element rendering the author profile link) so external pages opened with
target="_blank" cannot access window.opener or leak referrer information.
In `@src/components/compatibility/review/reviewItem.ts`:
- Around line 3-8: The import currently brings ApprovalStatus in using a
type-only import which removes its runtime value needed by the
CompatibilityReportReviewDecision union; change the import so ApprovalStatus is
imported as a value (not only with the `type` modifier) and update
CompatibilityReportReviewDecision to use the actual enum/value members (e.g.,
ApprovalStatus.APPROVED and ApprovalStatus.REJECTED) instead of typeof
expressions; keep CustomFieldType as a type-only import if it’s only used in
type positions.
---
Nitpick comments:
In `@src/app/admin/approvals/page.tsx`:
- Around line 216-223: The extra call to invalidateQueries() inside
handleBulkApprovalWithConfirmation is redundant because
bulkApproveMutation.mutateAsync already triggers invalidateQueries() in its
onSuccess; remove the await invalidateQueries() line from
handleBulkApprovalWithConfirmation (keep the confirm call, mutateAsync call and
approvalModal.close()), and do the same cleanup in the other bulk action handler
(e.g., the bulk reject handler such as handleBulkRejectionWithConfirmation or
wherever bulkRejectMutation.mutateAsync is used) so you don't double-invalidate
and refetch.
🪄 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
Run ID: 3645c56b-4224-4784-965b-e29c28886468
📒 Files selected for processing (17)
src/app/admin/approvals/components/ApprovalModal.tsxsrc/app/admin/approvals/page.tsxsrc/app/admin/pc-listing-approvals/components/ApprovalModal.tsxsrc/app/admin/pc-listing-approvals/page.tsxsrc/components/compatibility/review/CompatibilityReportApprovalActions.tsxsrc/components/compatibility/review/CompatibilityReportCustomFieldValue.tsxsrc/components/compatibility/review/CompatibilityReportReviewModal.test.tsxsrc/components/compatibility/review/CompatibilityReportReviewModal.tsxsrc/components/compatibility/review/CompatibilityReportReviewModalAdapter.tsxsrc/components/compatibility/review/index.tssrc/components/compatibility/review/reviewItem.tssrc/components/compatibility/review/useCompatibilityReportReviewDecisionModal.tssrc/server/api/routers/listings/core.test.tssrc/server/api/routers/listings/core.tssrc/server/api/routers/pcListings.tssrc/server/services/review-risk.service.test.tssrc/server/services/review-risk.service.ts
💤 Files with no reviewable changes (2)
- src/app/admin/pc-listing-approvals/components/ApprovalModal.tsx
- src/app/admin/approvals/components/ApprovalModal.tsx
✅ Files skipped from review due to trivial changes (1)
- src/components/compatibility/review/useCompatibilityReportReviewDecisionModal.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/server/services/review-risk.service.test.ts
- src/components/compatibility/review/index.ts
- src/components/compatibility/review/CompatibilityReportCustomFieldValue.tsx
…on and clean up unused approvals logic
…ypes for dynamic custom field rendering
…place FieldValueLike with CompatibilityCustomFieldValue across codebase
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
1 participant
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.
unify review components between listings and pc-listings
Type of change
How Has This Been Tested?
Checklist
Notes for reviewers
Summary by CodeRabbit
Release Notes
New Features
Improvements