enhance/tools-dummy-data-react #3002
Conversation
… and enhanced navigation
WalkthroughAdds a new Dummy Data importer page and components, registers a /dummy-data admin route, updates the legacy import-dummy-data menu URL to point to the Dokan dashboard, and extends the admin panel switcher supported keys to include Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Dashboard
participant DummyPage as Dummy Data Page
participant CSV
participant API
User->>Dashboard: Navigate to /dummy-data
Dashboard->>DummyPage: Render DummyData component
DummyPage->>CSV: Fetch CSV URL
CSV-->>DummyPage: Return rows
DummyPage->>DummyPage: Parse → vendors & products
User->>DummyPage: Click "Run Import"
loop per vendor
DummyPage->>API: POST vendor + products
API-->>DummyPage: Response
DummyPage->>DummyPage: Update progress
end
DummyPage->>User: Show Result card
User->>DummyPage: Click "Clear Data"
DummyPage->>API: DELETE dummy data
API-->>DummyPage: Confirm deletion
DummyPage->>DummyPage: Reset state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 2 inconclusive)
✅ Passed checks (1 passed)
✨ 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
🧹 Nitpick comments (3)
src/admin/dashboard/pages/dummy-data/Importer.tsx (1)
39-40: Simplify boolean coercion.The double negation
!! loadingis unnecessary since thedisabledandloadingprops already expect boolean values.Apply this diff:
- <DokanButton - disabled={ !! loading } - loading={ !! loading } - onClick={ onRun } + <DokanButton + disabled={ loading } + loading={ loading } + onClick={ onRun }src/admin/dashboard/pages/dummy-data/HeaderImage.tsx (1)
1-1: Consider adding explicit prop types.While the inline destructuring with default value works, defining an explicit TypeScript interface would improve type safety and maintainability.
Apply this diff:
+type HeaderImageProps = { + className?: string; +}; + -function HeaderImage( { className = '' } ) { +function HeaderImage( { className = '' }: HeaderImageProps ) {src/admin/dashboard/pages/dummy-data/index.tsx (1)
337-341: Document or remove ts-ignore directive.The
ts-ignorecomment suppresses TypeScript errors without explanation. Consider properly typingdokanAdminDashboardor adding a comment explaining why the ignore is necessary.Consider creating a proper type declaration or adding a comment:
<DokanLink href={ - // @ts-ignore `${ dokanAdminDashboard.urls.adminRoot }admin.php?page=dokan-dashboard#/tools` }If
dokanAdminDashboardis a global, add it to your type declarations file:// In types/externals.d.ts or similar declare const dokanAdminDashboard: { urls: { adminRoot: string; dummy_data: string; }; nonce: string; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
includes/Admin/Dashboard/Dashboard.php(1 hunks)src/admin/dashboard/components/Dashboard.tsx(2 hunks)src/admin/dashboard/pages/dummy-data/HeaderImage.tsx(1 hunks)src/admin/dashboard/pages/dummy-data/Importer.tsx(1 hunks)src/admin/dashboard/pages/dummy-data/Result.tsx(1 hunks)src/admin/dashboard/pages/dummy-data/StatusSkeleton.tsx(1 hunks)src/admin/dashboard/pages/dummy-data/index.tsx(1 hunks)src/admin/panel-switcher/PanelSwitch.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/admin/dashboard/pages/dummy-data/index.tsx (1)
includes/DummyData/Importer.php (1)
Importer(26-356)
src/admin/dashboard/pages/dummy-data/Result.tsx (1)
types/externals.d.ts (1)
DokanModal(65-65)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: api tests (1, 1)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (1, 3)
🔇 Additional comments (5)
src/admin/panel-switcher/PanelSwitch.tsx (1)
17-17: LGTM!The addition of 'tools' and 'dummy-data' to the supported panel keys correctly enables navigation to the new dummy-data route introduced in this PR.
includes/Admin/Dashboard/Dashboard.php (1)
220-220: LGTM!The menu link correctly points to the Dokan dashboard page with the new /dummy-data route, aligning with the routing changes in this PR.
src/admin/dashboard/components/Dashboard.tsx (2)
13-13: LGTM!The import correctly references the new dummy-data page component.
64-68: LGTM!The route configuration is consistent with other routes and properly integrates the new DummyData page.
src/admin/dashboard/pages/dummy-data/StatusSkeleton.tsx (1)
3-28: LGTM!The skeleton loading component is well-structured with appropriate accessibility considerations (sr-only text).
| const [ dummyData, setDummyData ] = useState< any[] >( [] ); | ||
| const [ loading, setLoading ] = useState< boolean >( true ); | ||
| const [ allVendors, setAllVendors ] = useState< any[] >( [] ); | ||
| const [ allProducts, setAllProducts ] = useState< any[] >( [] ); | ||
| const [ done, setDone ] = useState< boolean >( false ); | ||
| const [ statusLoader, setStatusLoader ] = useState< boolean >( true ); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| { /* eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions */ } | ||
| <div | ||
| className="mb-[14px] border border-[#E9E9E9] rounded-[6px] flex items-center gap-[12px] p-[12px] hover:bg-[#F8F9F8]" | ||
| role="button" | ||
| tabIndex={ 0 } | ||
| onClick={ () => navigate( '/vendors' ) } | ||
| > | ||
| <div className="h-[44px] w-[44px] border border-[#E9E9E9] bg-[#F8F9F8] flex justify-center items-center rounded-[5px]"> | ||
| <Users size="24" color="#828282" /> | ||
| </div> | ||
| <p className="text-[14px] font-[600] text-[#25252D]"> | ||
| { __( 'View Dummy Vendors', 'dokan-lite' ) } | ||
| </p> | ||
| </div> |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| { /* eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions */ } | ||
| <div | ||
| className="mb-[14px] border border-[#E9E9E9] rounded-[6px] flex items-center gap-[12px] p-[12px] hover:bg-[#F8F9F8]" | ||
| role="button" | ||
| tabIndex={ 0 } | ||
| onClick={ () => | ||
| window.open( | ||
| `${ dokanAdminDashboard.urls.adminRoot }edit.php?post_type=product`, | ||
| '_self' | ||
| ) | ||
| } | ||
| > | ||
| <div className="h-[44px] w-[44px] border border-[#E9E9E9] bg-[#F8F9F8] flex justify-center items-center rounded-[5px]"> | ||
| <Box size="24" color="#828282" /> | ||
| </div> | ||
| <p className="text-[14px] font-[600] text-[#25252D]"> | ||
| { __( 'View Dummy Products', 'dokan-lite' ) } | ||
| </p> | ||
| </div> |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… in Result component
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/admin/dashboard/pages/dummy-data/index.tsx (2)
335-344: Replace @ts-ignore with proper type handling.The
@ts-ignoredirective on line 337 suppresses TypeScript errors when accessing the globaldokanAdminDashboard. This hides potential runtime issues if the global is undefined.Consider using optional chaining with a fallback, similar to line 23:
<DokanLink href={ - // @ts-ignore - `${ dokanAdminDashboard.urls.adminRoot }admin.php?page=dokan-dashboard#/tools` + `${ ( window as any )?.dokanAdminDashboard?.urls?.adminRoot || '' }admin.php?page=dokan-dashboard#/tools` }Or define a proper type for the window object to avoid casting to
anythroughout the component.
18-19: Consider defining explicit types for vendor and product data.Both
allVendorsandallProductsuseany[], which bypasses type checking. While this works for dynamic CSV data, defining proper interfaces would improve type safety and code maintainability.Consider creating interfaces based on the CSV structure:
interface VendorData { id: string; email: string; password: string; store_name: string; // ... other vendor fields } interface ProductData { id: string; name: string; sku: string; vendor: string; raw_attributes?: Array<{ name: string; value: string[]; visible: string; taxonomy: string; }>; // ... other product fields }Then update the state declarations:
const [ allVendors, setAllVendors ] = useState<VendorData[]>( [] ); const [ allProducts, setAllProducts ] = useState<ProductData[]>( [] );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/admin/dashboard/pages/dummy-data/Importer.tsx(1 hunks)src/admin/dashboard/pages/dummy-data/Result.tsx(1 hunks)src/admin/dashboard/pages/dummy-data/index.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/admin/dashboard/pages/dummy-data/Result.tsx
- src/admin/dashboard/pages/dummy-data/Importer.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/admin/dashboard/pages/dummy-data/index.tsx (2)
src/definitions/RouterProps.ts (1)
RouterProps(10-18)includes/DummyData/Importer.php (1)
Importer(26-356)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (3)
src/admin/dashboard/pages/dummy-data/index.tsx (3)
263-284: LGTM - attribute formatting and data cleanup.The function correctly:
- Builds structured
raw_attributesfrom CSV columns- Converts
manage_stockto boolean- Removes vendor-specific fields from product data
The approach of splitting attribute values by comma and converting
manage_stockusingBoolean()is appropriate for CSV-based dummy data.
210-253: LGTM - sequential import with recursive chaining.The recursive pattern for sequential vendor imports is appropriate here:
- Ensures vendors are imported one at a time in order
- Progress updates after each vendor
- Stops gracefully on error
- Shows success toast only when all vendors complete (line 217-223)
The use of the nullish coalescing operator on line 239 correctly handles the response index.
115-119: LGTM - intentional mount-only effect.The
useEffectwith an empty dependency array correctly runs initialization logic once on mount. The eslint-disable comment is appropriate for this intentional pattern.
| const [ progress, setProgress ] = useState( 0 ); | ||
| const [ loading, setLoading ] = useState< boolean >( true ); | ||
| const [ allVendors, setAllVendors ] = useState< any[] >( [] ); | ||
| const [ allProducts, setAllProducts ] = useState< any[] >( [] ); | ||
| const [ done, setDone ] = useState< boolean >( false ); | ||
| const [ statusLoader, setStatusLoader ] = useState< boolean >( true ); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/admin/dashboard/pages/dummy-data/RemovableDatasObj.ts (1)
1-51: Remove duplicate'status'entry fromvendorsDataToRemove.
vendorsDataToRemovecontains'status'twice (Lines 3 and 36). This is harmless at runtime but redundant and suggests a copy‑paste oversight. It’s better to keep each key only once for clarity and to avoid confusion in future edits.You can safely drop the second occurrence:
'menu_order', 'virtual', 'downloadable', - 'status', 'attribute_1_name',src/admin/dashboard/pages/dummy-data/index.tsx (1)
210-243: Move success toast out offinallyso it only fires on successful clear.Right now the success toast is inside the
finallyblock ofclearAllDummyData. That block runs for both success and failure, so users will see a “success” message even when the clear request fails (and an error toast has already been shown), which is misleading.Move the success toast into the
.thenhandler and keep only the loading/confirm state resets infinally:apiFetch( { path: 'dokan/v1/dummy-data/clear', method: 'DELETE', data: { nonce: ( window as any )?.dokanAdminDashboard?.nonce }, } ) - .then( () => { - resetToImport(); - // reload CSV and status so user can import again - loadImportStatus(); - loadCsvFile(); - } ) + .then( () => { + resetToImport(); + // reload CSV and status so user can import again + loadImportStatus(); + loadCsvFile(); + + toast( { + type: 'success', + title: __( + 'All dummy data remove successfully', + 'dokan-lite' + ), + } ); + } ) .catch( ( err: any ) => { toast( { type: 'error', title: err?.message || __( 'Something went wrong', 'dokan-lite' ), } ); } ) .finally( () => { setLoading( false ); setIsConfirmOpen( false ); - - toast( { - type: 'success', - title: __( - 'All dummy data remove successfully', - 'dokan-lite' - ), - } ); } );
🧹 Nitpick comments (1)
src/admin/dashboard/pages/dummy-data/index.tsx (1)
88-120: Consider handling the “no vendors in CSV” case explicitly.If
allVendorsis empty (e.g., unexpected CSV contents) and the user triggers import,handleImport( 0 )will immediately setdonetotruewithout importing anything, and the UI will move to the “completed” state silently.Not critical, but you may want to surface a clearer message (e.g., “No vendors found in CSV”) instead of treating this as a successful run.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/admin/dashboard/pages/dummy-data/RemovableDatasObj.ts(1 hunks)src/admin/dashboard/pages/dummy-data/index.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/admin/dashboard/pages/dummy-data/index.tsx (3)
src/definitions/RouterProps.ts (1)
RouterProps(10-18)src/admin/dashboard/pages/dummy-data/RemovableDatasObj.ts (2)
vendorsDataToRemove(1-51)productsDataToRemove(52-82)includes/DummyData/Importer.php (1)
Importer(26-356)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (2)
src/admin/dashboard/pages/dummy-data/index.tsx (2)
15-22: State typing and use ofany[]look acceptable for this UI layer.The state definitions rely on inference for
progressand useany[]for CSV-derived vendor/product collections. Given the dynamic CSV shape and downstream PHP importer, this is a reasonable compromise here, and the overall state wiring (progress, loading, done, statusLoader) is consistent with the intended UX.
245-307: Importer body and layout wiring look coherent with the dummy-data flow.
loadImporterBodycleanly switches between skeleton, importer, and result views based onstatusLoaderanddone, and the surrounding layout (back link, explanatory copy, header image, Card wrapper, andDokanToaster) is wired correctly.No functional issues spotted in this section.
Aunshon
added
Needs: Testing
All Submissions:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry