Api connecting-Edite problems in AddListing page #58
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR introduces draft-saving capability to the listing creation flow, enriches product data models with location/coordinates/seller/rejection information, refactors product detail page to use a view-model layer, and normalizes product data transformation across API endpoints with consistent mappers. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/pages/AddListingPage/AddListingPage.tsx (2)
445-458:⚠️ Potential issue | 🟠 MajorFail fast before pending submit when no attributes were mapped.
src/services/product.service.ts:16-24documentsattributesas required for pending listings, but this branch now turns an empty mapper result intoattributes: undefined. That shifts a predictable validation failure to the backend and makes the review flow fail late. Bail out locally when no attributes were resolved instead of callingpendingMutation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/AddListingPage/AddListingPage.tsx` around lines 445 - 458, The branch builds payloadWithAttributes from resolveListingAttributes (mappedAttrs) and then calls pendingMutation when isPendingRoute; because attributes are required for pending listings, add a guard before calling pendingMutation that fails fast when mappedAttrs is empty (e.g., surface a validation error/throw/return early) instead of turning attributes into undefined and calling pendingMutation; update the logic around resolveListingAttributes, mappedAttrs, payloadWithAttributes, isPendingRoute and pendingMutation to validate mappedAttrs.length > 0 and abort the pending submit path if not.
251-265:⚠️ Potential issue | 🟠 MajorUse a symmetric category matcher and stop falling back to an arbitrary ID.
The current synonym check only accepts backend labels containing
group[0], so valid aliases like default"Audio"against a backend"Headphones"category can miss.resolveCategoryIdthen degrades tolocalCategories[0]/"1", which saves the listing under the wrong category and resolves attributes against the wrong schema. Returnundefinedon no-match and let callers block submit or omitcategoryIdfor drafts instead of silently picking a fallback.Also applies to: 343-365
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/AddListingPage/AddListingPage.tsx` around lines 251 - 265, The category matcher currently only checks whether the input name contains a synonym and the backend label contains group[0], and when no match is found resolveCategoryId falls back to localCategories[0]/"1"; change findCategory to perform a symmetric synonym match (treat any synonym in CATEGORY_SYNONYM_GROUPS as matching if either the input name contains a group member and the candidate label contains any group member, or vice versa), and update resolveCategoryId (and the duplicate logic at the other occurrence around the 343-365 block) to return undefined on no-match instead of selecting localCategories[0] so callers can handle missing categoryId for drafts or block submission for required categories.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/locales/en.json`:
- Around line 104-106: The translation file defines two very similar keys
("goHome" and "goToHome") which can cause confusion and duplication; choose one
canonical key (e.g., keep "goHome" or "goToHome"), remove the duplicate entry,
and update any code references to use the chosen key (search for usages of
goHome and goToHome in components/containers to replace the deprecated one);
ensure the remaining key's value matches project tone and run i18n tests/lint to
catch any leftover references.
In `@src/pages/AddListingPage/AddListingPage.tsx`:
- Around line 373-403: The draft save currently blocks on resolveCategoryId and
resolveListingAttributes; change the flow in the submit handler so you build
draftPayload opportunistically (using title, price, condition, images, etc.) and
call draftMutation.mutateAsync immediately, then attempt to enrich the payload
with categoryId and attributes only if resolveCategoryId and
resolveListingAttributes succeed; specifically, compute attributeInputs and call
resolveCategoryId/resolveListingAttributes inside try/catch (or
Promise.allSettled), and only set draftPayload.categoryId and
draftPayload.attributes from mappedAttrs when those calls succeed (leave them
undefined on failure) before calling draftMutation.mutateAsync (or call
mutateAsync first and patch later if you prefer), referencing resolveCategoryId,
resolveListingAttributes, attributeInputs, mappedAttrs, draftPayload,
CreateProductPayload, and draftMutation.mutateAsync to locate the code to
update.
In `@src/pages/AddListingPage/components/BasicDetailsStep.tsx`:
- Around line 699-717: The CTA row in BasicDetailsStep currently uses fixed
widths on the two Button components which causes overflow; update the two Button
classNames to use responsive fluid sizing by replacing w-[358px] with w-full and
adding sm:flex-1, and remove the fixed horizontal padding token px-[119px] from
the "Next" button so both buttons can shrink with the container (keep existing
justify-center, height, rounded and disabled/bg conditional logic, and handlers
onSaveDraft/onNext unchanged).
In `@src/pages/IdentityVerificationPage/IdentityVerificationSelectPage.tsx`:
- Around line 277-292: handleSubmit currently ignores fileSlots and locally
marks identity approved; instead, read front/back images from fileSlots, call
the verification upload mutation in src/services/verification.service.ts (e.g.,
the uploadIdentityDocuments/submitIdentityVerification function used by the
service at lines ~87-111), setPageStatus("submitting") and keep local
verification.state as pending (do not set identity.status = "approved" or
reviewedAt locally), await the upload response, then update setVerification with
the server response and setPageStatus based on that result; follow the
backend-sync pattern used in IdentityVerificationStatusPage (the fetch/merge
flow at lines ~19-29) so reviewedAt only comes from the backend.
In `@src/pages/ProductDetailPage/ProductDetailPage.tsx`:
- Around line 226-228: The ternary assigning viewModel uses a type assertion
(serverViewModel as ProductDetailViewModel) that is safe because of the earlier
guard that validated serverViewModel; add a brief inline comment next to this
assignment (near viewModel, isDemo, demoViewModel, serverViewModel, and
ProductDetailViewModel) stating that the prior guard guarantees serverViewModel
conforms to ProductDetailViewModel, so the assertion is intentional and safe.
- Around line 52-55: formatConditionLabel currently special-cases "like-new" and
falls back to naive capitalization for other Product["condition"] values;
replace this with an explicit mapping object inside formatConditionLabel (e.g.,
map keys for "new", "like-new", "good", "fair" and any future conditions) and
return map[condition] || a sensible default so labels are clear and
maintainable; update references to formatConditionLabel accordingly.
In `@src/services/verification.service.ts`:
- Around line 27-28: The persisted defaults ("not_started"/"not_verified") from
getVerification() are masking real verification transitions; update the logic
that assigns storedVerification (and the similar block at lines 53-66) to treat
those default sentinel values as absent so later states like "pending" or the
is*Verified flags can overwrite them: when calling getVerification(), if the
returned object has status fields equal to "not_started" or "not_verified" (or
all fields are defaults), treat it as null/undefined rather than using the
nullish coalescing (the current (getVerification() as VerificationState | null)
?? null), and ensure uploadIdentityDocument() and any code reading
storedVerification will accept and persist non-default transitions. Reference
symbols: storedVerification, getVerification(), uploadIdentityDocument(), and
the is*Verified flags.
---
Outside diff comments:
In `@src/pages/AddListingPage/AddListingPage.tsx`:
- Around line 445-458: The branch builds payloadWithAttributes from
resolveListingAttributes (mappedAttrs) and then calls pendingMutation when
isPendingRoute; because attributes are required for pending listings, add a
guard before calling pendingMutation that fails fast when mappedAttrs is empty
(e.g., surface a validation error/throw/return early) instead of turning
attributes into undefined and calling pendingMutation; update the logic around
resolveListingAttributes, mappedAttrs, payloadWithAttributes, isPendingRoute and
pendingMutation to validate mappedAttrs.length > 0 and abort the pending submit
path if not.
- Around line 251-265: The category matcher currently only checks whether the
input name contains a synonym and the backend label contains group[0], and when
no match is found resolveCategoryId falls back to localCategories[0]/"1"; change
findCategory to perform a symmetric synonym match (treat any synonym in
CATEGORY_SYNONYM_GROUPS as matching if either the input name contains a group
member and the candidate label contains any group member, or vice versa), and
update resolveCategoryId (and the duplicate logic at the other occurrence around
the 343-365 block) to return undefined on no-match instead of selecting
localCategories[0] so callers can handle missing categoryId for drafts or block
submission for required categories.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4e5e21f4-c3a1-4deb-9104-310e4b1fb8cd
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.json
📒 Files selected for processing (13)
src/locales/en.jsonsrc/pages/AddListingPage/AddListingPage.tsxsrc/pages/AddListingPage/components/BasicDetailsStep.tsxsrc/pages/AddListingPage/components/MoreDetailsStep.tsxsrc/pages/HomePage/HomePage.tsxsrc/pages/IdentityVerificationPage/IdentityVerificationSelectPage.tsxsrc/pages/MyListingsPage/MyListingsPage.tsxsrc/pages/ProductDetailPage/ProductDetailPage.tsxsrc/pages/ProfilePage/ProfileDetails.tsxsrc/pages/SearchPage/SearchPage.tsxsrc/services/product.service.tssrc/services/verification.service.tssrc/types/product.ts
Comment on lines
104
to
+106
| "goHome": "Go Home", | ||
| "goToHome": "Go to Home", | ||
| "addAnother": "Add another listing", |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Potential duplication: "goHome" vs "goToHome".
Line 104 has "goHome": "Go Home" and line 105 adds "goToHome": "Go to Home". These are semantically similar. Consider consolidating to avoid confusion about which key to use.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/locales/en.json` around lines 104 - 106, The translation file defines two
very similar keys ("goHome" and "goToHome") which can cause confusion and
duplication; choose one canonical key (e.g., keep "goHome" or "goToHome"),
remove the duplicate entry, and update any code references to use the chosen key
(search for usages of goHome and goToHome in components/containers to replace
the deprecated one); ensure the remaining key's value matches project tone and
run i18n tests/lint to catch any leftover references.
Comment on lines
+373
to
+403
| const currentValues = getValues(); | ||
| const categoryId = await resolveCategoryId(currentValues.category ?? ""); | ||
| const images = photos.map((p) => p.file); | ||
| const resolvedLocation = | ||
| currentValues.location?.trim() || | ||
| [locationValue.city, locationValue.country].filter(Boolean).join(", "); | ||
| const attributeInputs: AttributeValueMap = { | ||
| brand: currentValues.brand, | ||
| model: currentValues.model, | ||
| storage: currentValues.storage, | ||
| battery: currentValues.batteryHealth, | ||
| description: | ||
| typeof currentValues.description === "string" | ||
| ? currentValues.description | ||
| : undefined, | ||
| location: resolvedLocation, | ||
| }; | ||
| const { attributes: mappedAttrs } = await resolveListingAttributes( | ||
| categoryId, | ||
| attributeInputs, | ||
| ); | ||
| const draftPayload: CreateProductPayload = { | ||
| title: currentValues.title?.trim() || "Untitled listing", | ||
| categoryId, | ||
| condition: normalizeCondition(currentValues.condition || "good"), | ||
| price: Number(currentValues.price) || 0, | ||
| isNegotiable: Boolean(currentValues.isNegotiable), | ||
| images: images.length ? images : undefined, | ||
| attributes: mappedAttrs.length ? mappedAttrs : undefined, | ||
| }; | ||
| await draftMutation.mutateAsync(draftPayload); |
There was a problem hiding this comment.
Don't make draft creation depend on category metadata APIs.
src/services/product.service.ts:260-304 already serializes drafts without categoryId or attributes, but this flow always awaits resolveCategoryId and resolveListingAttributes before sending anything. A transient categories/category-detail failure now blocks saving an otherwise valid partial draft. Build the draft payload opportunistically and only enrich it with category/attributes when those lookups succeed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/AddListingPage/AddListingPage.tsx` around lines 373 - 403, The
draft save currently blocks on resolveCategoryId and resolveListingAttributes;
change the flow in the submit handler so you build draftPayload
opportunistically (using title, price, condition, images, etc.) and call
draftMutation.mutateAsync immediately, then attempt to enrich the payload with
categoryId and attributes only if resolveCategoryId and resolveListingAttributes
succeed; specifically, compute attributeInputs and call
resolveCategoryId/resolveListingAttributes inside try/catch (or
Promise.allSettled), and only set draftPayload.categoryId and
draftPayload.attributes from mappedAttrs when those calls succeed (leave them
undefined on failure) before calling draftMutation.mutateAsync (or call
mutateAsync first and patch later if you prefer), referencing resolveCategoryId,
resolveListingAttributes, attributeInputs, mappedAttrs, draftPayload,
CreateProductPayload, and draftMutation.mutateAsync to locate the code to
update.
Comment on lines
+699
to
+717
| <div className="flex w-full flex-col items-center justify-center gap-3 sm:flex-row"> | ||
| <Button | ||
| type="button" | ||
| onClick={onSaveDraft} | ||
| disabled={isSavingDraft} | ||
| className="flex h-14 w-[358px] items-center justify-center rounded-xl border border-[#e4e4e4] bg-white px-10 py-4 text-[#212121] hover:bg-neutral-5 disabled:cursor-not-allowed disabled:opacity-60" | ||
| > | ||
| <Text className="font-['Poppins'] text-base font-medium leading-normal"> | ||
| {isSavingDraft ? "Saving draft..." : "Save as draft"} | ||
| </Text> | ||
| </div> | ||
| </Button> | ||
| </Button> | ||
| <Button | ||
| type="button" | ||
| onClick={onNext} | ||
| disabled={isNextDisabled} | ||
| className={`flex h-14 w-[358px] items-center justify-center rounded-xl px-[119px] py-4 ${ | ||
| isNextDisabled ? "cursor-not-allowed bg-[#e4e4e4]" : "bg-[#2563eb]" | ||
| }`} | ||
| > |
There was a problem hiding this comment.
Make the new CTA row fluid instead of fixed-width.
At sm this switches to a horizontal layout, but the two w-[358px] buttons need ~728px before gap/padding. Inside the current Add Listing form that overflows on common viewport widths, so one action can end up off-screen. Use w-full + sm:flex-1 and drop the fixed px-[119px] so the row can shrink with its container.
📐 Suggested fix
- <div className="flex w-full flex-col items-center justify-center gap-3 sm:flex-row">
+ <div className="flex w-full flex-col gap-3 sm:flex-row">
<Button
type="button"
onClick={onSaveDraft}
disabled={isSavingDraft}
- className="flex h-14 w-[358px] items-center justify-center rounded-xl border border-[`#e4e4e4`] bg-white px-10 py-4 text-[`#212121`] hover:bg-neutral-5 disabled:cursor-not-allowed disabled:opacity-60"
+ className="flex h-14 w-full items-center justify-center rounded-xl border border-[`#e4e4e4`] bg-white px-6 py-4 text-[`#212121`] hover:bg-neutral-5 disabled:cursor-not-allowed disabled:opacity-60 sm:flex-1"
>
@@
- className={`flex h-14 w-[358px] items-center justify-center rounded-xl px-[119px] py-4 ${
+ className={`flex h-14 w-full items-center justify-center rounded-xl px-6 py-4 sm:flex-1 ${
isNextDisabled ? "cursor-not-allowed bg-[`#e4e4e4`]" : "bg-[`#2563eb`]"
}`}📝 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.
Suggested change
| <div className="flex w-full flex-col items-center justify-center gap-3 sm:flex-row"> | |
| <Button | |
| type="button" | |
| onClick={onSaveDraft} | |
| disabled={isSavingDraft} | |
| className="flex h-14 w-[358px] items-center justify-center rounded-xl border border-[#e4e4e4] bg-white px-10 py-4 text-[#212121] hover:bg-neutral-5 disabled:cursor-not-allowed disabled:opacity-60" | |
| > | |
| <Text className="font-['Poppins'] text-base font-medium leading-normal"> | |
| {isSavingDraft ? "Saving draft..." : "Save as draft"} | |
| </Text> | |
| </div> | |
| </Button> | |
| </Button> | |
| <Button | |
| type="button" | |
| onClick={onNext} | |
| disabled={isNextDisabled} | |
| className={`flex h-14 w-[358px] items-center justify-center rounded-xl px-[119px] py-4 ${ | |
| isNextDisabled ? "cursor-not-allowed bg-[#e4e4e4]" : "bg-[#2563eb]" | |
| }`} | |
| > | |
| <div className="flex w-full flex-col gap-3 sm:flex-row"> | |
| <Button | |
| type="button" | |
| onClick={onSaveDraft} | |
| disabled={isSavingDraft} | |
| className="flex h-14 w-full items-center justify-center rounded-xl border border-[`#e4e4e4`] bg-white px-6 py-4 text-[`#212121`] hover:bg-neutral-5 disabled:cursor-not-allowed disabled:opacity-60 sm:flex-1" | |
| > | |
| <Text className="font-['Poppins'] text-base font-medium leading-normal"> | |
| {isSavingDraft ? "Saving draft..." : "Save as draft"} | |
| </Text> | |
| </Button> | |
| <Button | |
| type="button" | |
| onClick={onNext} | |
| disabled={isNextDisabled} | |
| className={`flex h-14 w-full items-center justify-center rounded-xl px-6 py-4 sm:flex-1 ${ | |
| isNextDisabled ? "cursor-not-allowed bg-[`#e4e4e4`]" : "bg-[`#2563eb`]" | |
| }`} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/AddListingPage/components/BasicDetailsStep.tsx` around lines 699 -
717, The CTA row in BasicDetailsStep currently uses fixed widths on the two
Button components which causes overflow; update the two Button classNames to use
responsive fluid sizing by replacing w-[358px] with w-full and adding sm:flex-1,
and remove the fixed horizontal padding token px-[119px] from the "Next" button
so both buttons can shrink with the container (keep existing justify-center,
height, rounded and disabled/bg conditional logic, and handlers
onSaveDraft/onNext unchanged).
Comment on lines
277
to
+292
| const handleSubmit = () => { | ||
| setPageStatus("submitting"); | ||
| // Simulate network request | ||
| const submissionTimestamp = new Date().toISOString(); | ||
|
|
||
| // Simulate a short review process before marking identity as verified locally | ||
| setTimeout(() => { | ||
| // Randomly succeed or fail | ||
| const isSuccess = Math.random() > 0.5; | ||
| setPageStatus(isSuccess ? "success" : "failed"); | ||
| }, 3000); | ||
| setVerification({ | ||
| identity: { | ||
| status: "approved", | ||
| type: docType, | ||
| submittedAt: submissionTimestamp, | ||
| reviewedAt: submissionTimestamp, | ||
| }, | ||
| }); | ||
| setPageStatus("success"); | ||
| }, 1500); |
There was a problem hiding this comment.
handleSubmit() never uploads the documents and self-approves the user.
This block ignores fileSlots, waits 1.5s, then persists identity.status = "approved" and reviewedAt locally. That bypasses the real upload path in src/services/verification.service.ts Lines 87-111 and the backend-sync pattern in src/pages/IdentityVerificationPage/IdentityVerificationStatusPage.tsx Lines 19-29, so the UI can show a verified identity without any server review.
🛠️ Wire submission to the upload mutation and keep the local state pending
- const handleSubmit = () => {
+ const handleSubmit = async () => {
setPageStatus("submitting");
const submissionTimestamp = new Date().toISOString();
-
- // Simulate a short review process before marking identity as verified locally
- setTimeout(() => {
+ try {
+ await uploadIdentityDocument.mutateAsync({
+ type: docType,
+ frontImage,
+ backImage,
+ });
setVerification({
identity: {
- status: "approved",
+ status: "pending",
type: docType,
submittedAt: submissionTimestamp,
- reviewedAt: submissionTimestamp,
},
});
setPageStatus("success");
- }, 1500);
+ } catch {
+ setPageStatus("failed");
+ }
};frontImage / backImage should come from the selected slots, and reviewedAt should only be populated from the backend once the review actually completes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/IdentityVerificationPage/IdentityVerificationSelectPage.tsx` around
lines 277 - 292, handleSubmit currently ignores fileSlots and locally marks
identity approved; instead, read front/back images from fileSlots, call the
verification upload mutation in src/services/verification.service.ts (e.g., the
uploadIdentityDocuments/submitIdentityVerification function used by the service
at lines ~87-111), setPageStatus("submitting") and keep local verification.state
as pending (do not set identity.status = "approved" or reviewedAt locally),
await the upload response, then update setVerification with the server response
and setPageStatus based on that result; follow the backend-sync pattern used in
IdentityVerificationStatusPage (the fetch/merge flow at lines ~19-29) so
reviewedAt only comes from the backend.
Comment on lines
+52
to
+55
| const formatConditionLabel = (condition: Product["condition"]): string => { | ||
| if (condition === "like-new") return "Like New"; | ||
| return condition.charAt(0).toUpperCase() + condition.slice(1); | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Condition label formatting may not handle all valid conditions correctly.
The formatConditionLabel function handles "like-new" explicitly but uses a generic capitalize for others. Given that Product["condition"] is "new" | "like-new" | "good" | "fair", this works correctly. However, if other conditions are added in the future, consider using an explicit mapping for clarity and maintainability.
♻️ Optional: Use explicit mapping for all conditions
const formatConditionLabel = (condition: Product["condition"]): string => {
- if (condition === "like-new") return "Like New";
- return condition.charAt(0).toUpperCase() + condition.slice(1);
+ const labels: Record<Product["condition"], string> = {
+ "new": "New",
+ "like-new": "Like New",
+ "good": "Good",
+ "fair": "Fair",
+ };
+ return labels[condition] ?? condition;
};📝 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.
Suggested change
| const formatConditionLabel = (condition: Product["condition"]): string => { | |
| if (condition === "like-new") return "Like New"; | |
| return condition.charAt(0).toUpperCase() + condition.slice(1); | |
| }; | |
| const formatConditionLabel = (condition: Product["condition"]): string => { | |
| const labels: Record<Product["condition"], string> = { | |
| "new": "New", | |
| "like-new": "Like New", | |
| "good": "Good", | |
| "fair": "Fair", | |
| }; | |
| return labels[condition] ?? condition; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/ProductDetailPage/ProductDetailPage.tsx` around lines 52 - 55,
formatConditionLabel currently special-cases "like-new" and falls back to naive
capitalization for other Product["condition"] values; replace this with an
explicit mapping object inside formatConditionLabel (e.g., map keys for "new",
"like-new", "good", "fair" and any future conditions) and return map[condition]
|| a sensible default so labels are clear and maintainable; update references to
formatConditionLabel accordingly.
Comment on lines
+226
to
+228
| const viewModel: ProductDetailViewModel = isDemo | ||
| ? demoViewModel | ||
| : (serverViewModel as ProductDetailViewModel); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Type assertion is safe but could be clearer.
The assertion serverViewModel as ProductDetailViewModel is safe due to the guard on line 216. Consider adding a brief comment explaining why the assertion is valid to aid future readers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/ProductDetailPage/ProductDetailPage.tsx` around lines 226 - 228,
The ternary assigning viewModel uses a type assertion (serverViewModel as
ProductDetailViewModel) that is safe because of the earlier guard that validated
serverViewModel; add a brief inline comment next to this assignment (near
viewModel, isDemo, demoViewModel, serverViewModel, and ProductDetailViewModel)
stating that the prior guard guarantees serverViewModel conforms to
ProductDetailViewModel, so the assertion is intentional and safe.
Comment on lines
+27
to
+28
| const storedVerification = | ||
| (getVerification() as VerificationState | null) ?? null; |
There was a problem hiding this comment.
Stored default statuses are masking later verification transitions.
src/lib/storage.ts Lines 18-25 persists these values in localStorage, and the ?? checks here treat "not_started" / "not_verified" as authoritative. After the initial profile sync writes those defaults, uploadIdentityDocument() can no longer surface pending, and later is*Verified user flags are ignored until something else rewrites storage.
🛠️ Suggested precedence fix
- const resolvedIdentityStatus =
- identityFromStore?.status ??
- (isIdentityVerified
- ? "approved"
- : pendingIdentityFlag
- ? "pending"
- : "not_started");
+ const resolvedIdentityStatus =
+ isIdentityVerified
+ ? "approved"
+ : identityFromStore?.status && identityFromStore.status !== "not_started"
+ ? identityFromStore.status
+ : pendingIdentityFlag
+ ? "pending"
+ : "not_started";
- const resolvedPhoneStatus =
- phoneFromStore?.status ??
- (isPhoneVerified ? "verified" : "not_verified");
+ const resolvedPhoneStatus =
+ isPhoneVerified || phoneFromStore?.status === "verified"
+ ? "verified"
+ : phoneFromStore?.status === "pending"
+ ? "pending"
+ : "not_verified";
- const resolvedEmailStatus =
- emailFromStore?.status ??
- (isEmailVerified ? "verified" : "not_verified");
+ const resolvedEmailStatus =
+ isEmailVerified || emailFromStore?.status === "verified"
+ ? "verified"
+ : emailFromStore?.status === "pending"
+ ? "pending"
+ : "not_verified";Also applies to: 53-66
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/verification.service.ts` around lines 27 - 28, The persisted
defaults ("not_started"/"not_verified") from getVerification() are masking real
verification transitions; update the logic that assigns storedVerification (and
the similar block at lines 53-66) to treat those default sentinel values as
absent so later states like "pending" or the is*Verified flags can overwrite
them: when calling getVerification(), if the returned object has status fields
equal to "not_started" or "not_verified" (or all fields are defaults), treat it
as null/undefined rather than using the nullish coalescing (the current
(getVerification() as VerificationState | null) ?? null), and ensure
uploadIdentityDocument() and any code reading storedVerification will accept and
persist non-default transitions. Reference symbols: storedVerification,
getVerification(), uploadIdentityDocument(), and the is*Verified flags.
MahmoudHamo1
approved these changes
Mar 6, 2026
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
Release Notes
New Features
Bug Fixes