Phase 5: business-logic and product-decision findings - #7
Open
subhanlone wants to merge 7 commits into
Open
Conversation
…s (BV-047) Consumes the backend's new fulfilment state machine (COMPLETED -> SHIPPED -> DELIVERED, or DISPUTED -> DELIVERED/REFUNDED): - PaymentModal collects delivery address and phone -- now required by create-intent, and previously never collected at all. - BuyerMyWins shows the full lifecycle: awaiting shipment, a shipped item's confirm-or-dispute countdown with Confirm Receipt / Report a Problem actions (new DisputeModal), under-review and refunded states. Review eligibility moved from COMPLETED to DELIVERED. - New SellerMySales screen -- sellers previously had no view of their own sales at all. Shows the buyer's delivery address and a Mark as Shipped action, gated on payout setup. - SellerProfile gained a Payout Setup card (Stripe Connect onboarding). - AdminTransactions gained a Disputes tab with a resolve (refund / release) modal. Breaking to match the backend's 3.0.0 -> 4.0.0 bump: create-intent's request now requires deliveryAddress/deliveryPhone. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jr8E6jcYCAeognvqvBD45Y
…ings and invoice screens
Matches the backend's payment-gateway migration (see backend
PAYMENT-GATEWAY-MIGRATION.md and DECISIONS.md #12). PaymentModal no longer
loads Stripe.js or renders CardNumberElement/CardExpiryElement/
CardCvcElement -- plain controlled inputs for card number/expiry/CVC, and
a single synchronous POST /payments/{id}/pay call replaces the
create-intent + stripe.confirmCardPayment two-step.
SellerProfile's "Payout Setup" card (Stripe Connect onboarding, the
?connect= redirect handling) is replaced with an "Earnings" card showing
the seller's ledger balance and recent credited sales, each linking to its
invoice. SellerMySales drops the onboarding-incomplete banner and the
gate it put on "Mark as Shipped" -- there is no onboarding concept left.
New: TransactionInvoice screen (GET /payments/{id}/invoice) at
/transactions/:transactionId/invoice, reachable from both BuyerMyWins and
SellerMySales for any transaction past SHIPPED.
Breaking (major, 3.0.0 -> 5.0.0 to match the backend): the create-intent
request/response shape is gone; openapi.d.ts regenerated from the new
contract (70 schemas, 55 operations). @stripe/react-stripe-js and
@stripe/stripe-js are removed; VITE_STRIPE_PUBLISHABLE_KEY is gone from
.env.example.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jr8E6jcYCAeognvqvBD45Y
… (BV-013) Mirrors the backend fix: minNext now equals currentBid itself when bidCount === 0, instead of currentBid + minIncrement -- the starting price was otherwise displayed but not actually biddable. Also relabels "Current Bid" as "Starting At" (and the confirm-bid modal's "Current highest bid" as "Starting price") before any bid exists, so the copy stops implying a bid that hasn't happened. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018BUBrF1UQ5ShLy4fAiHK4n
Regenerated openapi.d.ts: SubmitListingRequest.category is now a literal union of the seven recognised categories instead of string. ListingDraft's category field gains the matching ListingCategory | '' type (the same pattern condition already used), and the two call sites that produce a raw string -- the category <select>'s onChange and the pre-submit validation guard -- are narrowed accordingly. No behaviour change: the dropdown already only ever offered these seven values. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018BUBrF1UQ5ShLy4fAiHK4n
Regenerated openapi.d.ts: GET /:auctionId/bids items lose buyerId and gain isMine; buyerName on both bids and reviews is a pseudonym now, not the real name. applyBidToCache and its two writers (usePlaceBid's onSuccess, the bid:placed socket handler) are rebuilt around the new PublicBid shape: the mutation's own response is known-mine and gets isMine:true / 'You' directly; the masked socket broadcast can't know that about itself, so it writes isMine:false and applyBidToCache upgrades an existing entry in place if a later write knows it's mine -- the two can arrive over two different connections in either order. BuyerLiveBidding's isTopBidder/myBids/bid-history rendering all switch from comparing buyerId to reading isMine, including the avatar-initial letter (caught live in browser testing: it kept reading the raw masked buyerName even after the "You" label was already fixed, so a bidder's own row showed a mismatched initial for a moment after a fresh page load). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018BUBrF1UQ5ShLy4fAiHK4n
…nfirmed (BV-048) BuyerLiveBidding navigates here the instant its own countdown hits zero, but the worker settles the auction independently and can be seconds -- or, after a missed scheduled job, until the reconciliation sweep catches it -- behind. The screen used to assume a transaction already existed; nothing backed that assumption. Now polls GET /payments/my-wins every 2s for up to 60s, showing "Confirming your win..." with the payment button disabled meanwhile, then a fallback notice past 60s. Caught live in browser testing (a real closeAuction() run against a seeded short-lived auction): the first version stopped polling entirely once it hit the 60s mark, so a transaction that appeared after that point was never picked up without a manual refresh. Fixed to back off to a 10s interval instead of stopping -- the timeout is a UI signal, not a reason to give up, since the mechanism it's covering for can legitimately take minutes. Also stops showing the "taking longer than usual" notice once confirmation actually lands, rather than leaving it beside a ready-to-pay screen. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018BUBrF1UQ5ShLy4fAiHK4n
…all finishes it (BV-049) The backend now caps each call at 50 and reports `remaining`. usePendingListings' approveAll loops calls until remaining is 0, accumulating totals across batches, so a backlog bigger than one batch still resolves in a single button click rather than needing the admin to notice and retry. AdminListingReviews shows a running "Approving... (N so far)" count on the confirm button while a multi-batch run is in progress. Browser-verified end-to-end against a real pending listing: approve, toast, queue empties. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018BUBrF1UQ5ShLy4fAiHK4n
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
🟡 Changes recommended
There are several concrete correctness/operational and contract-drift issues (unused payment fields, unmount-safe polling/state updates, and duplicated wire types) that should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the frontend to match Phase 5 backend contract and business-logic decisions, especially around masked bidding, category enum wiring, post-auction payment/shipping/dispute flows, and bulk listing approval batching.
Changes:
- Updates bid handling/UI to consume masked
PublicBid+isMine, including cache reconciliation for socket vs mutation arrival order. - Adds transaction lifecycle UX (confirming win polling, invoices, buyer disputes, seller shipment + sales/earnings views, admin dispute resolution).
- Threads the new category enum contract through listing draft + create-listing wizard and implements batched approve-all progress.
File summaries
| File | Description |
|---|---|
| src/types/openapi.d.ts | Regenerated/updated API contract types and endpoint maps (bids, payments, disputes, listings). |
| src/types/index.ts | Introduces ListingCategory derived from the wire request type; updates ListingDraft to use it. |
| src/types/api.ts | Re-exports API shapes used by the app (now also includes PublicBid). |
| src/screens/TransactionInvoice.tsx | New shared invoice screen for buyer/seller (and route allows admin). |
| src/screens/seller/SellerProfile.tsx | Adds earnings ledger snippet with invoice links on seller profile. |
| src/screens/seller/SellerMySales.tsx | New seller “My Sales” screen with shipment actions and invoice links. |
| src/screens/seller/SellerCreateListingStep3.tsx | Narrows category validation to the new enum-based draft type. |
| src/screens/seller/SellerCreateListingStep1.tsx | Uses literal-typed category list to enforce wire-enum correctness. |
| src/screens/buyer/BuyerMyWins.tsx | Expands win transaction statuses and adds confirm-receipt/dispute/invoice UX. |
| src/screens/buyer/BuyerLiveBidding.tsx | Updates min-next-bid logic and masked-bidder display using isMine. |
| src/screens/buyer/BuyerAuctionWon.tsx | Adds polling UX for delayed transaction settlement after countdown ends. |
| src/screens/admin/AdminTransactions.tsx | Adds disputes tab and dispute resolution modal; refactors loading per tab. |
| src/screens/admin/AdminListingReviews.tsx | Adds approve-all running progress UI and threads progress callback. |
| src/queries/RealtimeBridge.tsx | Updates bid:placed socket payload handling to masked bid shape. |
| src/queries/auctions.ts | Rebuilds bid cache update logic around PublicBid and isMine upgrade semantics. |
| src/hooks/usePendingListings.ts | Loops approve-all calls across backend batch cap and reports progress. |
| src/components/ui/SellerNavbar.tsx | Adds “My Sales” nav entry. |
| src/components/ui/PaymentModal.tsx | Removes Stripe Elements flow and uses dummy gateway pay endpoint with delivery info. |
| src/components/ui/index.ts | Exports new DisputeModal. |
| src/components/ui/DisputeModal.tsx | New buyer dispute submission modal for shipped-window reporting. |
| src/App.tsx | Adds routes for seller sales and shared transaction invoice. |
| README.md | Updates docs to reflect dummy gateway and new invoice screen. |
| package.json | Removes Stripe dependencies. |
| package-lock.json | Removes Stripe packages from lockfile. |
| .env.example | Removes Stripe publishable key from example env. |
Review details
Suppressed comments (3)
src/screens/seller/SellerProfile.tsx:71
- The earnings fetch effect doesn’t guard against unmounts (unlike the listings/stats effect above). If the request resolves after navigation, it can still call
setEarnings. Add acancelledflag (or AbortController) and check it before setting state.
useEffect(() => {
if (!user) return;
api.get('/payments/earnings').then(setEarnings).catch(() => undefined);
}, [user?.userId]); // eslint-disable-line react-hooks/exhaustive-deps
src/screens/TransactionInvoice.tsx:43
- This route is allowed for ADMINs, but
Navbarfalls back toBuyerNavbarfor any non-SELLER role. That means admins will see buyer navigation on this shared invoice screen. Consider rendering an admin-appropriate wrapper (or no navbar) whenuser.role === 'ADMIN'.
const Navbar = user?.role === 'SELLER' ? SellerNavbar : BuyerNavbar;
return (
<div className="min-h-screen bg-bg">
<Navbar userName={user?.name} onLogout={logout} />
src/types/api.ts:49
- New API wire types were added in
openapi.d.ts(e.g.Invoice,Earnings,SellerSale,PaginatedSellerSales,AdminDispute,PayResult), but they aren’t re-exported fromtypes/api.ts. As a result, new code is forced to import directly fromtypes/openapior redefine shapes locally, which conflicts with this file’s own guidance (“anything new should import from here”). Re-export the new types to keep consumers consistent and reduce drift.
export type {
Auction,
Listing,
User,
Bid,
PublicBid,
BidWithAuction,
PaginatedAuctions,
PaginatedBids,
PaginatedBidsWithAuction,
PaginatedListings,
NotificationPrefs,
Review,
SellerReviews,
WonTransaction,
SellerStats,
Analytics,
PlatformSettings,
PublicSettings,
PlatformStats,
Session,
Registration,
UserRole,
AuctionStatus,
ListingStatus,
ItemCondition,
TransactionStatus,
NotificationType,
CategoryAttributes,
RegisterRequest,
LoginRequest,
SubmitListingRequest,
CreateReviewRequest,
UpdateSettingsRequest,
ErrorResponse,
ValidationError,
} from './openapi';
- Files reviewed: 23/25 changed files
- Comments generated: 8
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+48
to
+52
| const result = await api.post( | ||
| `/payments/${transactionId}/pay`, | ||
| { | ||
| cardNumber: digits, | ||
| deliveryAddress: deliveryAddress.trim(), |
Comment on lines
+30
to
+36
| useEffect(() => { | ||
| if (!transactionId) return; | ||
| api.get(`/payments/${transactionId}/invoice`) | ||
| .then(setInvoice) | ||
| .catch((err: unknown) => setError(err instanceof Error ? err.message : 'Could not load invoice.')) | ||
| .finally(() => setLoading(false)); | ||
| }, [transactionId]); |
Comment on lines
+47
to
+50
| async function poll() { | ||
| try { | ||
| const wins = await api.get('/payments/my-wins'); | ||
| if (cancelled) return; |
Comment on lines
+51
to
+73
| function loadSales() { | ||
| return (async () => { | ||
| const all: SellerSale[] = []; | ||
| let cursor: string | null = null; | ||
| do { | ||
| const page: { items: SellerSale[]; nextCursor: string | null } = await api.get( | ||
| cursor ? `/payments/my-sales?limit=100&cursor=${encodeURIComponent(cursor)}` : '/payments/my-sales?limit=100', | ||
| ); | ||
| all.push(...page.items); | ||
| cursor = page.nextCursor; | ||
| } while (cursor); | ||
| setSales(all.filter(s => RELEVANT_STATUSES.includes(s.status))); | ||
| })(); | ||
| } | ||
|
|
||
| useEffect(() => { | ||
| if (!user) return; | ||
| let cancelled = false; | ||
| loadSales() | ||
| .catch(() => { if (!cancelled) setError('Could not load your sales. Please try again.'); }) | ||
| .finally(() => { if (!cancelled) setLoading(false); }); | ||
| return () => { cancelled = true; }; | ||
| }, [user?.userId]); // eslint-disable-line react-hooks/exhaustive-deps |
Comment on lines
+26
to
+37
| interface Dispute { | ||
| disputeId: string; | ||
| transactionId: string; | ||
| auctionTitle: string; | ||
| buyerId: string; | ||
| buyerName: string; | ||
| sellerId: string; | ||
| sellerName: string; | ||
| finalAmount: number; | ||
| reason: string; | ||
| createdAt: string; | ||
| } |
Comment on lines
12
to
36
| @@ -17,20 +21,29 @@ interface WinTransaction { | |||
| auctionImageUrl: string; | |||
| sellerName: string; | |||
| finalAmount: number; | |||
| status: 'PENDING' | 'COMPLETED' | 'FAILED' | 'VOIDED'; | |||
| status: TransactionStatus; | |||
| // Why the last payment attempt failed, if one did. The transaction stays PENDING rather | |||
| // than moving to FAILED so a decline is retryable — this is the explanation that goes with | |||
| // that PENDING state instead of a separate terminal one. | |||
| lastPaymentError?: string; | |||
| // BV-047: present once the seller has shipped. reviewDeadlineAt is the confirm-or-dispute | |||
| // deadline, computed server-side so this screen never needs reviewTimeoutHours itself. | |||
| shippedAt?: string; | |||
| reviewDeadlineAt?: string; | |||
| disputeReason?: string; | |||
| createdAt: string; | |||
| reviewed: boolean; | |||
| } | |||
Comment on lines
+10
to
+29
| type TransactionStatus = | ||
| | 'PENDING' | 'COMPLETED' | 'FAILED' | 'VOIDED' | ||
| | 'SHIPPED' | 'DELIVERED' | 'DISPUTED' | 'REFUNDED'; | ||
|
|
||
| interface SellerSale { | ||
| transactionId: string; | ||
| auctionId: string; | ||
| auctionTitle: string; | ||
| auctionEmoji: string; | ||
| auctionImageUrl: string; | ||
| buyerName: string; | ||
| finalAmount: number; | ||
| status: TransactionStatus; | ||
| deliveryAddress?: string; | ||
| deliveryPhone?: string; | ||
| shippedAt?: string; | ||
| reviewDeadlineAt?: string; | ||
| disputeReason?: string; | ||
| createdAt: string; | ||
| } |
Comment on lines
1
to
5
| import { useState, useEffect } from 'react'; | ||
| import { Link, useNavigate } from 'react-router-dom'; | ||
| import { Check, Package, Shield, Mail, Calendar, Gavel, PackageCheck, Clock, Banknote, Eye, EyeOff } from 'lucide-react'; | ||
| import { Check, Package, Shield, Mail, Calendar, Gavel, PackageCheck, Clock, Banknote, Eye, EyeOff, Wallet, Receipt } from 'lucide-react'; | ||
| import { useAuth } from '../../context/AuthContext'; | ||
| import { useToast } from '../../context/ToastContext'; |
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
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
Frontend half of Phase 5 (see the paired backend PR,
bidvault-backend#8, for the full finding list and rationale). This branch already carried BV-047 and the Stripe-to-dummy-gateway migration; this PR's new commits are the frontend side of:minNextallows the first bid atstartPrice; "Current Bid"/"Current highest bid" relabel to "Starting At"/"Starting price" before any bid existscategoryenum contract change;ListingCategorytype threaded throughListingDraftand the create-listing wizardapplyBidToCacherebuilt around the newPublicBid/isMineshape, reconciling the mutation's own known-mine response against the masked socket broadcast regardless of arrival orderBuyerAuctionWonpollsGET /payments/my-winsfor the real transaction instead of assuming one exists the instant the client's own countdown hits zero, with a "Confirming your win…" state and a backed-off (not abandoned) poll past 60susePendingListings'approveAllloops across the backend's 50-per-call cap, with a running-total progress label on the confirm buttonTest plan
npm run buildcleannpm run lint— 0 errors🤖 Generated with Claude Code