[Re-Enginrring] withdraw page #2506
Conversation
…ure-convert-withdraw # Conflicts: # composer.lock
…andle responsive preview from base components.
…ce-dataviews-from-dokan-free
…te/vendor-dashboard-structure-convert-withdraw # Conflicts: # package-lock.json
…ataviews-from-dokan-free' into update/vendor-dashboard-structure-convert-withdraw # Conflicts: # includes/Assets.php # package.json
…& add documentation for components, utilities.
… into update/vendor-dashboard-structure-convert-withdraw # Conflicts: # src/hooks/useCurrentUser.ts
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request introduces a comprehensive set of changes to the Dokan plugin, focusing on modernizing the frontend architecture, enhancing REST API capabilities, and implementing a new status management system. The changes span multiple areas including routing, component design, utility functions, and backend infrastructure, with a significant emphasis on React-based dashboard components and improved type safety. Changes
Sequence DiagramsequenceDiagram
participant User
participant Dashboard
participant API
participant WithdrawHooks
participant PaymentMethods
User->>Dashboard: Access Withdraw Page
Dashboard->>WithdrawHooks: Fetch Balance
WithdrawHooks->>API: Request Balance Data
API-->>WithdrawHooks: Return Balance
WithdrawHooks-->>Dashboard: Update Balance State
Dashboard->>PaymentMethods: Render Payment Methods
PaymentMethods->>API: Fetch Payment Settings
API-->>PaymentMethods: Return Payment Methods
Dashboard->>User: Display Withdraw Interface
Possibly related PRs
Suggested Labels
Suggested ReviewersPoem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…e/vendor-dashboard-structure-convert-withdraw # Conflicts: # package-lock.json # package.json # src/routing/index.tsx
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 34
🧹 Nitpick comments (42)
src/definitions/RouterProps.ts (1)
13-13: Consider replacinganywith a more specific type.The
Locationgeneric parameter is set toany, which reduces type safety. Consider defining a specific type for the location state if possible.- location: Location< any >; + location: Location< unknown >;src/routing/index.tsx (3)
46-55: Remove commented code.Commented-out code should be removed if it's no longer needed. If this is intended for future use, consider tracking it in a separate issue or documentation.
9-41: Enhance type safety in withRouter HOC.The HOC could benefit from proper TypeScript generics and component type definitions.
-export function withRouter(Component) { +export function withRouter<P extends RouterProps>( + Component: React.ComponentType<P> | React.ReactElement +): React.ComponentType<Omit<P, keyof RouterProps>> {
57-77: Consider grouping related routes.The withdraw-related routes could be grouped together in a separate configuration object for better maintainability.
const withdrawRoutes = { base: { id: 'dokan-withdraw', title: __('Withdraw', 'dokan-lite'), element: <Withdraw/>, path: '/withdraw', exact: true, order: 10, }, requests: { id: 'dokan-withdraw-requests', title: __('Withdraw Requests', 'dokan-lite'), element: <WithdrawRequests/>, path: '/withdraw-requests', exact: true, order: 10, }, }; routes.push(withdrawRoutes.base, withdrawRoutes.requests);src/components/DataTable.tsx (2)
14-28: Consider accessibility and maintainability improvements.While the header implementation is solid, consider these enhancements:
- Add
aria-sortattribute for sortable columns to improve accessibility- Move hardcoded color values (e.g., #828282) to Tailwind config for better maintainability
Example implementation:
<th key={header.id} scope="col" + aria-sort={header.column.getIsSorted() ? header.column.getIsSorted() : 'none'} - className="px-4 py-4 text-left text-xs font-normal uppercase text-[#828282] last:ps-0 last:text-right" + className="px-4 py-4 text-left text-xs font-normal uppercase text-table-header last:ps-0 last:text-right" >
30-55: Enhance empty state handling.Instead of rendering null for empty state, consider showing a "No data" message to improve user experience.
{table.getRowModel().rows.length ? table.getRowModel().rows.map((row) => ( // ... existing row rendering ... )) - : null} + : ( + <tr> + <td + colSpan={table.getAllColumns().length} + className="px-4 py-4 text-center text-sm text-gray-500" + > + No data available + </td> + </tr> + )}🧰 Tools
🪛 Biome (1.9.4)
[error] 36-36: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/definitions/Product.ts (4)
1-5: Consider using number type for dimensions.The dimensions (length, width, height) are currently typed as strings but would be better represented as numbers, possibly with an additional unit field.
interface ProductDimensions { - length: string; - width: string; - height: string; + length: number; + width: number; + height: number; + unit: 'cm' | 'in' | 'm'; }
25-29: Strengthen type safety for metadata value.The
valueproperty uses a union withany[]which reduces type safety. Consider defining a more specific union type based on expected values.interface ProductMetaData { id: number; key: string; - value: string | number | any[]; + value: string | number | boolean | Record<string, unknown>[] | null; }
55-61: Consider making RowActions more flexible.The interface currently hardcodes specific action types. Consider making it more flexible to accommodate dynamic action types.
-interface RowActions { - edit: RowAction; - delete: RowAction; - view: RowAction; - 'quick-edit': RowAction; - duplicate: RowAction; -} +type ActionType = 'edit' | 'delete' | 'view' | 'quick-edit' | 'duplicate' | string; +interface RowActions { + [key: ActionType]: RowAction; +}
146-148: Enhance subscription product type guard.The type guard could be more robust by checking for subscription-specific properties and handling edge cases.
-const isSubscriptionProduct = (product: Product): boolean => { - return product.type === 'subscription'; -}; +const isSubscriptionProduct = (product: Product): boolean => { + if (!product) return false; + return product.type === 'subscription' && + product.meta_data.some(meta => + meta.key === '_subscription_period' || + meta.key === '_subscription_price' + ); +};src/dashboard/Withdraw/index.tsx (3)
1-11: Add TypeScript type definitions for better type safety.Consider adding type definitions for the imported hooks and components to improve type safety and code maintainability.
+import type { WithdrawSettings } from './types'; +import type { WithdrawRequest } from './types'; +import type { BalanceData } from './types';
13-13: Improve hook variable naming.The variable name
useWithdrawRequestHookis redundant with the 'Hook' suffix since it's already a hook. Consider renaming it towithdrawRequestsfor better readability.- const useWithdrawRequestHook = useWithdrawRequests( true ); + const withdrawRequests = useWithdrawRequests( true );
35-61: Optimize loading state and enhance accessibility.The loading state logic is duplicated across components and the wrapper div lacks accessibility attributes.
- <div className="dokan-withdraw-wrapper dokan-react-withdraw space-y-6"> + <div + className="dokan-withdraw-wrapper dokan-react-withdraw space-y-6" + role="main" + aria-label="Withdrawal Management" + > + {const masterLoading = currentUser.isLoading || useWithdrawRequestHook.isLoading;} <Balance - masterLoading={ - currentUser.isLoading || - useWithdrawRequestHook.isLoading - } + masterLoading={masterLoading} bodyData={balance} settings={withdrawSettings} withdrawRequests={useWithdrawRequestHook} /> <PaymentDetails - masterLoading={ - currentUser.isLoading || - useWithdrawRequestHook.isLoading - } + masterLoading={masterLoading} bodyData={balance} withdrawRequests={useWithdrawRequestHook} settings={withdrawSettings} /> <PaymentMethods - masterLoading={ - currentUser.isLoading || - useWithdrawRequestHook.isLoading - } + masterLoading={masterLoading} bodyData={withdrawSettings} /> </div>src/dashboard/Withdraw/TableSkeleton.tsx (2)
34-34: Extract magic number to prop or constant.The hardcoded value of 10 for skeleton rows should be configurable.
-{ [...Array(10)].map((_, index) => ( +{ [...Array(rowCount)].map((_, index) => (
3-81: Consider extracting reusable skeleton components.The skeleton UI elements are repeated with similar patterns. Consider extracting them into reusable components for better maintainability.
Example refactor:
const SkeletonCell: FC<{ width: string; height?: string }> = ({ width, height = 'h-4' }) => ( <div className={`${height} ${width} bg-gray-200 rounded animate-pulse`}></div> ); const TableRowSkeleton: FC = () => ( <tr className="border-t"> <td className="py-4 px-6"> <SkeletonCell width="w-20" height="h-6" /> </td> {/* ... other cells */} </tr> );src/dashboard/Withdraw/tailwind.scss (4)
15-16: Use consistent CSS variable naming convention.The CSS variable names are inconsistent.
hover-colorvshover-background-colormight cause confusion.- background-color: var(--dokan-button-hover-color, #F05025); - border-color: var(--dokan-button-hover-background-color, #F05025); + background-color: var(--dokan-button-hover-bg-color, #F05025); + border-color: var(--dokan-button-hover-border-color, #F05025);
2-17: Consolidate selectors for better maintainability.The long list of selectors for button states can be simplified using attribute selectors.
- button:focus, - .menu-toggle:hover, - button:hover, - .button:hover, - .ast-custom-button:hover, - input[type=reset]:hover, - input[type=reset]:focus, - input#submit:hover, - input#submit:focus, - input[type="button"]:hover, - input[type="button"]:focus, - input[type="submit"]:hover, - input[type="submit"]:focus { + button, + .menu-toggle, + .button, + .ast-custom-button, + input[type="reset"], + input[type="button"], + input[type="submit"] { + &:hover, + &:focus {
28-39: Consider using Tailwind's reset utilities.Instead of writing custom table reset styles, consider using Tailwind's built-in reset utilities.
- table, th, td { - margin: 0; - padding: 0; - border: 0; - border-spacing: 0; - border-collapse: collapse; - font-size: inherit; - font-weight: inherit; - text-align: inherit; - vertical-align: inherit; - box-sizing: border-box; - } + @layer base { + table, th, td { + @apply m-0 p-0 border-0 border-collapse text-inherit align-inherit box-border; + } + }
70-72: Document import dependencies.Add comments to explain the purpose of each import and their dependencies.
+// Base Tailwind configuration for consistent styling @config './../../../base-tailwind.config.js'; +// Core Tailwind utilities and custom styles @import '../../base-tailwind'; +// Dokan UI components and utilities @import "@getdokan/dokan-ui/dist/dokan-ui.css";src/components/DokanTooltip.tsx (1)
9-22: Make the component more flexible and robust.A few suggestions to improve the implementation:
- The empty string default for
contentmight mask missing content. Consider making it required.- Hardcoded classes reduce reusability. Consider making them configurable.
Consider this improved implementation:
-function DokanTooltip( { children, content = '', direction = 'top' }: Props ) { +function DokanTooltip( { + children, + content, + direction = 'top', + className = "block text-sm text-gray-500 mb-1" +}: Props & { className?: string } ) { return ( <Tooltip content={ content } direction={ direction }> { ( toolTipTriggerProps ) => ( <div - className="block text-sm text-gray-500 mb-1" + className={className} { ...toolTipTriggerProps } > { children } </div> ) } </Tooltip> ); }src/components/Pagination.tsx (2)
5-12: Add JSDoc comments to document the Props interface.Consider adding JSDoc comments to document the purpose and usage of each prop, especially for className customization props.
+/** + * Props for the Pagination component + * @template T The type of data being paginated + */ type Props<T> = { + /** The table instance to paginate */ table: Table<T>; + /** Additional CSS classes for the container */ className?: string; + /** Loading state of the pagination */ isLoading?: boolean; + /** Additional CSS classes for pagination buttons */ btnClasses?: string; + /** Additional CSS classes for text elements */ textClasses?: string; + /** Additional CSS classes for the page input */ inputClasses?: string; };
16-64: Extract repeated button styles into a constant.The pagination buttons share identical styles. Consider extracting these into a constant to improve maintainability.
+const baseButtonClasses = 'relative h-[35px] w-[35px] inline-flex items-center rounded-md bg-white px-3 py-2 text-sm font-semibold text-gray-900 ring-1 ring-inset ring-gray-300 hover:bg-gray-50 focus-visible:outline-offset-0'; + +const getButtonClasses = (isLoading: boolean, btnClasses: string) => + twMerge( + isLoading ? 'cursor-progress' : 'disabled:cursor-not-allowed cursor-pointer', + baseButtonClasses, + btnClasses + ); + <div className={twMerge('flex flex-1 items-center justify-between sm:justify-end py-3', className)}> {/* ... */} <button - className={twMerge(isLoading ? 'cursor-progress' : 'disabled:cursor-not-allowed cursor-pointer', 'relative ml-3 h-[35px] w-[35px] inline-flex items-center rounded-md bg-white px-3 py-2 text-sm font-semibold text-gray-900 ring-1 ring-inset ring-gray-300 hover:bg-gray-50 focus-visible:outline-offset-0', btnClasses)} + className={twMerge('ml-3', getButtonClasses(isLoading, btnClasses))} onClick={() => table.firstPage()} disabled={!table.getCanPreviousPage() || isLoading} >src/definitions/CountryState.ts (1)
1-5: Enhance API documentation.The API documentation could be more comprehensive. Consider adding:
- Expected response format
- Authentication requirements
- Available HTTP methods
- Query parameters
src/definitions/window-types.ts (1)
18-18: Consider using a more specific return type formoment.Instead of using
any, consider using the actual moment.js types for better type safety:moment: (date: string) => moment.Moment;src/definitions/Customers.ts (2)
1-13: Consider making more address fields optional.Fields like
company,address_2, andphoneare often optional in practice. Consider updating the interface:export interface CustomerAddress { first_name: string; last_name: string; - company: string; + company?: string; address_1: string; - address_2: string; + address_2?: string; city: string; postcode: string; country: string; state: string; email?: string; - phone: string; + phone?: string; }
20-36: Consider using more specific date types.The date fields could benefit from a more specific type to ensure proper date string format:
date_created: `${number}-${number}-${number}T${number}:${number}:${number}`;src/components/PriceHtml.tsx (1)
15-42: Simplify default value assignments using destructuring.The multiple if-statements for default values can be simplified:
const PriceHtml = ({ price = 0, - currencySymbol = '', - precision = null, - thousand = '', - decimal = '', - format = '', + currencySymbol = window.dokanCurrency.symbol, + precision = window.dokanCurrency.precision, + thousand = window.dokanCurrency.thousand, + decimal = window.dokanCurrency.decimal, + format = window.dokanCurrency.format, }: PriceHtmlProps) => { - if (!currencySymbol) { - currencySymbol = window.dokanCurrency.symbol; - } - // Remove other if statements...src/dashboard/Withdraw/Hooks/useMakeDefaultMethod.ts (2)
14-37: Consider adding dependencies to useCallback.The
makeDefaultMethodfunction is memoized with an empty dependency array, but it uses external functions (setIsLoading,setError) from useState. While these setState functions are stable across renders, it's a good practice to explicitly list all dependencies.); - [] + [setIsLoading, setError] );
20-24: Add success notification for better UX.Consider adding a success notification after successfully setting the default method to provide better user feedback.
await apiFetch( { path: '/dokan/v2/withdraw/make-default-method', method: 'POST', data: { method }, } ); + // Assuming you have a notification system + dokan.hooks.doAction('dokan_notify', { + type: 'success', + message: 'Default withdrawal method updated successfully' + });src/components/DateTimeHtml.tsx (1)
16-22: Extract common date formatting logic to reduce duplication.The date formatting logic is duplicated across all three variants. Consider extracting it into a utility function.
const formatDate = (value: string, format: string): string => { return dateI18n( format, value, getSettings().timezone.string ); };Also applies to: 37-43, 57-63
src/dashboard/Withdraw/Hooks/useCharge.ts (1)
4-7: Consider using numeric types for charge values.The
ChargeDatainterface uses string types for numeric values. Consider using number type for better type safety and to avoid type coercion issues.interface ChargeData { - fixed: string; - percentage: string; + fixed: number; + percentage: number; }src/definitions/Continents.ts (1)
65-65: Remove redundant type alias.The
Continentstype alias is redundant as it's identical to theContinenttype. Consider removing it or documenting why it exists if there's a specific reason.-export type Continents = Continentsrc/dashboard/Withdraw/Hooks/useWithdrawSettings.ts (2)
18-22: Consider adding validation for setup_url.The
setup_urlinWithdrawSettingsinterface should be validated to ensure it's a valid URL.export interface WithdrawSettings { withdraw_method: string; payment_methods: PaymentMethod[]; active_methods: Record< string, WithdrawMethod >; - setup_url: string; + setup_url: URL | string; }
47-54: Enhance error handling with specific error types.Consider creating custom error types for different failure scenarios to improve error handling.
+interface WithdrawSettingsError extends Error { + code: 'NETWORK_ERROR' | 'INVALID_RESPONSE' | 'UNAUTHORIZED'; +} + const response = await apiFetch< WithdrawSettings >( { path: '/dokan/v2/withdraw/settings', method: 'GET', } );src/definitions/woocommerce-accounting.d.ts (1)
35-43: Add JSDoc comments for better documentation.The formatting methods lack proper documentation explaining their purpose and parameters.
+ /** + * Formats a number into a currency string. + * @param number - The number to format + * @param symbol - Currency symbol + * @param precision - Decimal precision + * @param thousand - Thousand separator + * @param decimal - Decimal separator + * @param format - Output format + * @returns Formatted currency string + */ formatMoney( number: number | string, symbol?: string,src/definitions/Order.ts (1)
76-85: Consider using a strict union type forOrderStatus.The current type allows any string value, which defeats the purpose of using a union type. Consider removing the
stringtype to enforce strict type checking.export type OrderStatus = | 'pending' | 'processing' | 'on-hold' | 'completed' | 'cancelled' | 'refunded' | 'failed' - | 'trash' - | string; + | 'trash';src/dashboard/Withdraw/Balance.tsx (1)
64-70: Use nullish coalescing operator for better null checks.The nullish coalescing operator (
??) is more appropriate here as it only falls back to the default value when the left-hand side isnullorundefined.- bodyData?.data?.current_balance ?? - '' + bodyData?.data?.current_balance ?? '' - bodyData?.data?.withdraw_limit ?? '' + bodyData?.data?.withdraw_limit ?? ''Also applies to: 81-86
src/dashboard/Withdraw/WithdrawRequests.tsx (1)
41-42: Simplify optional chaining.The optional chaining could be simplified for better readability.
- let page = useWithdrawRequestHook?.view?.page; - if ( status !== useWithdrawRequestHook?.lastPayload?.status ) { + let page = useWithdrawRequestHook.view.page; + if ( status !== useWithdrawRequestHook.lastPayload?.status ) {src/dashboard/Withdraw/PaymentMethods.tsx (1)
56-108: Simplify action button logic.The current implementation uses nested conditions which could be simplified for better readability and maintainability.
Consider refactoring to use early returns:
const actionButton = (activemethod: WithdrawMethod) => { - if ( - activemethod.has_information && - activemethod?.value === bodyData?.data?.withdraw_method - ) { - return ( - <Button - color="secondary" - className="bg-gray-50 hover:bg-gray-100" - disabled={true} - label={__('Default', 'dokan-lite')} - /> - ); - } else if ( - activemethod.has_information && - activemethod?.value !== bodyData?.data?.withdraw_method - ) { - return ( - <Button - color="secondary" - className="bg-dokan-btn hover:bg-dokan-btn-hover text-white" - onClick={() => { - makeDefaultMethodHook - .makeDefaultMethod(activemethod.value) - .then(() => { - toast({ - type: 'success', - title: __('Default method updated', 'dokan-lite'), - }); - bodyData.refresh(); - }); - }} - disabled={makeDefaultMethodHook.isLoading} - loading={makeDefaultMethodHook.isLoading} - label={__('Make Default', 'dokan-lite')} - /> - ); - } - return ( - <Button - type="button" - color="secondary" - className="bg-dokan-btn hover:bg-dokan-btn-hover text-white" - onClick={() => { - window.location.href = bodyData?.data?.setup_url; - }} - label={__('Setup', 'dokan-lite')} - /> - ); + if (!activemethod.has_information) { + return ( + <Button + type="button" + color="secondary" + className="bg-dokan-btn hover:bg-dokan-btn-hover text-white" + onClick={() => { + window.location.href = bodyData?.data?.setup_url; + }} + label={__('Setup', 'dokan-lite')} + /> + ); + } + + const isDefault = activemethod?.value === bodyData?.data?.withdraw_method; + + if (isDefault) { + return ( + <Button + color="secondary" + className="bg-gray-50 hover:bg-gray-100" + disabled={true} + label={__('Default', 'dokan-lite')} + /> + ); + } + + return ( + <Button + color="secondary" + className="bg-dokan-btn hover:bg-dokan-btn-hover text-white" + onClick={() => { + makeDefaultMethodHook + .makeDefaultMethod(activemethod.value) + .then(() => { + toast({ + type: 'success', + title: __('Default method updated', 'dokan-lite'), + }); + bodyData.refresh(); + }); + }} + disabled={makeDefaultMethodHook.isLoading} + loading={makeDefaultMethodHook.isLoading} + label={__('Make Default', 'dokan-lite')} + /> + ); };src/dashboard/Withdraw/RequestList.tsx (2)
20-31: Consider using TypeScript enum for status mapping.The
allStatusobject could benefit from being defined as a TypeScript enum for better type safety and maintainability.- const allStatus = { - pending: __( 'Pending Review', 'dokan' ), - approved: __( 'Approved', 'dokan' ), - cancelled: __( 'Cancelled', 'dokan' ), - }; + enum WithdrawStatus { + Pending = 'pending', + Approved = 'approved', + Cancelled = 'cancelled' + } + + const allStatus: Record<WithdrawStatus, string> = { + [WithdrawStatus.Pending]: __( 'Pending Review', 'dokan' ), + [WithdrawStatus.Approved]: __( 'Approved', 'dokan' ), + [WithdrawStatus.Cancelled]: __( 'Cancelled', 'dokan' ), + };
33-174: Consider extracting field definitions to a separate configuration file.The fields array is quite large and contains complex rendering logic. Consider moving it to a separate configuration file for better maintainability and reusability.
Create a new file
src/dashboard/Withdraw/config/fields.tsxand move the field definitions there:import { __ } from '@wordpress/i18n'; import PriceHtml from '../../../Components/PriceHtml'; import DateTimeHtml from '../../../Components/DateTimeHtml'; export const getWithdrawFields = (status: string, handlers: { setCancelRequestId: (id: string) => void; setIsOpen: (isOpen: boolean) => void; }) => { const baseFields = [ // ... move existing field definitions here ]; return [ ...baseFields, ...(status === 'pending' ? getPendingFields(handlers) : []), ...(status === 'cancelled' ? getCancelledFields() : []), ]; };src/dashboard/Withdraw/RequestWithdrawBtn.tsx (1)
160-313: Consider enhancing form validation and UI consistency.The UI implementation is good but could benefit from:
- Form validation before submission
- Consistent class naming
- Loading state feedback for the main button
+ const isFormValid = () => { + const amount = unformatNumber(withdrawAmount); + return Boolean( + withdrawMethod && + amount && + amount >= (settings?.data?.minimum_withdraw_amount || 0) + ); + }; return ( <> <Button color="gray" className="bg-dokan-btn hover:bg-dokan-btn-hover" onClick={ () => setIsOpen( true ) } + disabled={hasWithdrawRequests} label={ __( 'Request Withdraw', 'dokan' ) } />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
includes/Dashboard/Templates/NewDashboard.php(1 hunks)src/components/DataTable.tsx(1 hunks)src/components/DateTimeHtml.tsx(1 hunks)src/components/DokanTooltip.tsx(1 hunks)src/components/Pagination.tsx(1 hunks)src/components/PriceHtml.tsx(1 hunks)src/dashboard/Withdraw/Balance.tsx(1 hunks)src/dashboard/Withdraw/Hooks/useBalance.ts(1 hunks)src/dashboard/Withdraw/Hooks/useCharge.ts(1 hunks)src/dashboard/Withdraw/Hooks/useMakeDefaultMethod.ts(1 hunks)src/dashboard/Withdraw/Hooks/useWithdraw.ts(1 hunks)src/dashboard/Withdraw/Hooks/useWithdrawRequests.ts(1 hunks)src/dashboard/Withdraw/Hooks/useWithdrawSettings.ts(1 hunks)src/dashboard/Withdraw/PaymentDetails.tsx(1 hunks)src/dashboard/Withdraw/PaymentMethods.tsx(1 hunks)src/dashboard/Withdraw/RequestList.tsx(1 hunks)src/dashboard/Withdraw/RequestWithdrawBtn.tsx(1 hunks)src/dashboard/Withdraw/TableSkeleton.tsx(1 hunks)src/dashboard/Withdraw/WithdrawRequests.tsx(1 hunks)src/dashboard/Withdraw/index.tsx(1 hunks)src/dashboard/Withdraw/tailwind.scss(1 hunks)src/definitions/Continents.ts(1 hunks)src/definitions/CountryState.ts(1 hunks)src/definitions/Customers.ts(1 hunks)src/definitions/Order.ts(1 hunks)src/definitions/Product.ts(1 hunks)src/definitions/RouterProps.ts(1 hunks)src/definitions/window-types.ts(1 hunks)src/definitions/woocommerce-accounting.d.ts(1 hunks)src/routing/index.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- includes/Dashboard/Templates/NewDashboard.php
🧰 Additional context used
🪛 Biome (1.9.4)
src/definitions/window-types.ts
[error] 30-31: This empty export is useless because there's another export or import.
This import makes useless the empty export.
Safe fix: Remove this useless empty export.
(lint/complexity/noUselessEmptyExport)
src/dashboard/Withdraw/PaymentMethods.tsx
[error] 111-111: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
src/dashboard/Withdraw/Balance.tsx
[error] 43-43: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
src/components/DataTable.tsx
[error] 36-36: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/dashboard/Withdraw/PaymentDetails.tsx
[error] 46-46: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
src/definitions/Order.ts
[error] 21-21: Expected a class, a function, or a variable declaration but instead found 'export'.
Expected a class, a function, or a variable declaration here.
(parse)
[error] 65-65: Expected a class, a function, or a variable declaration but instead found 'export'.
Expected a class, a function, or a variable declaration here.
(parse)
🔇 Additional comments (22)
src/definitions/RouterProps.ts (1)
10-18: Well-structured router props interface!The interface is comprehensive and follows TypeScript best practices with proper type imports and readonly parameters where appropriate.
src/routing/index.tsx (1)
1-8: LGTM: Clean imports and proper organization.The imports are well-organized and properly separated between external libraries and internal components.
src/components/DataTable.tsx (2)
1-7: LGTM! Clean type definitions and appropriate imports.The Props type is well-defined with proper generic type parameter, and the imports are appropriate for the component's functionality.
9-9: LGTM! Well-structured component declaration.The component is properly declared as a generic function component with appropriate prop destructuring.
src/definitions/Product.ts (2)
31-46: LGTM! Well-structured store interfaces.The Store and StoreAddress interfaces are well-defined with clear, typed properties.
150-164: LGTM! Clean export statements.The exports are well-organized and follow TypeScript best practices.
src/components/DokanTooltip.tsx (3)
1-1: LGTM!Clean and focused import statement.
24-24: LGTM!Clean default export following React conventions.
1-24: Verify component usage and potential duplicates.Let's ensure this new component is being used consistently and there aren't similar existing tooltip implementations.
src/components/Pagination.tsx (2)
14-14: LGTM! Well-structured component definition.The component is properly typed and uses appropriate default values for optional props.
68-68: LGTM! Appropriate export statement.The default export is suitable for this component.
src/definitions/CountryState.ts (2)
7-26: LGTM! Well-structured interfaces for API response types.The interfaces
Link,Links,State, andCountryare well-defined and follow REST API conventions with proper typing.
30-34: LGTM! Redux-style state management structure.The
CountryStateStateinterface follows best practices for state management with clear separation of data, loading states, and errors.src/definitions/window-types.ts (1)
30-31: Keep the empty export.Although flagged by static analysis, this empty export is necessary to make the file a module, enabling proper TypeScript module resolution.
🧰 Tools
🪛 Biome (1.9.4)
[error] 30-31: This empty export is useless because there's another export or import.
This import makes useless the empty export.
Safe fix: Remove this useless empty export.
(lint/complexity/noUselessEmptyExport)
src/components/PriceHtml.tsx (1)
44-53: Verify XSS protection in price formatting.Using RawHTML with formatMoney output could potentially expose XSS vulnerabilities if the currency symbol or other inputs contain malicious HTML.
✅ Verification successful
No XSS vulnerability in price formatting implementation.
The price formatting is secure because:
- Currency symbols are system-controlled configuration values
- The RawHTML component from @wordpress/element is used appropriately for rendering formatted prices
- The accounting.js formatMoney() function safely handles numeric formatting
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for currency symbol sanitization rg -l "dokanCurrency.symbol.*sanitize" || echo "No sanitization found for currency symbol" # Search for formatMoney usage to check for patterns rg "formatMoney.*RawHTML" -A 3Length of output: 166
Script:
#!/bin/bash # Find PriceHtml component definition and usage ast-grep --pattern 'const PriceHtml = $_' -l rg "PriceHtml" -B 2 -A 2 # Look for currency symbol handling rg "currencySymbol" -B 2 -A 2 # Check for sanitization functions rg -i "(sanitize|escape).*currency" -B 2 -A 2 rg "dangerouslySetInnerHTML" -B 2 -A 2Length of output: 74384
Script:
#!/bin/bash # Find RawHTML component definition and usage ast-grep --pattern 'const RawHTML = $_' --lang typescript rg -B 2 -A 2 "RawHTML" # Check for any sanitization functions in the component rg -B 2 -A 2 "sanitize.*html|escapeHtml|DOMPurify" # Look for accounting.js initialization or configuration rg -B 2 -A 2 "window.accounting"Length of output: 81659
src/components/DateTimeHtml.tsx (1)
16-22: Verify HTML sanitization in dateI18n output.Using RawHTML with dateI18n output could potentially expose XSS vulnerabilities if the date format contains HTML. Verify that dateI18n properly escapes its output.
✅ Verification successful
The implementation is secure as is
The code uses official WordPress components where:
dateI18nonly processes date format tokens, not HTML- Date formats are controlled through WordPress settings
RawHTMLis the appropriate component for this use case🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if dateI18n sanitizes its output rg -A 5 "dateI18n" | grep -i "esc_"Length of output: 33
Script:
#!/bin/bash # Find dateI18n implementation and imports rg -l "dateI18n" # Find RawHTML component definition rg -l "RawHTML" # Look for datetime format definitions rg -l "formats\.datetime"Length of output: 185
Script:
#!/bin/bash # Check imports and package details cat src/components/DateTimeHtml.tsx # Look for package.json to verify dependencies cat package.jsonLength of output: 4086
src/dashboard/Withdraw/Hooks/useWithdrawSettings.ts (1)
41-44: Verify API version consistency.The hook uses
/v2/API endpoint whileuseBalanceuses/v1/. This inconsistency might cause issues with API versioning.src/dashboard/Withdraw/PaymentDetails.tsx (2)
13-30: Well-implemented loading state with skeleton UI!The loading state implementation follows best practices by using animated placeholders that maintain the layout, providing a smooth user experience.
53-151: Excellent plugin integration and extensibility!The implementation follows WordPress best practices:
- Uses slot/fill pattern for extensible UI.
- Provides plugin area for third-party integrations.
src/dashboard/Withdraw/RequestList.tsx (2)
1-19: Well-structured imports and type definitions!The imports are properly organized, and TypeScript interfaces are well-defined for the component props.
221-289: Well-implemented DataViews and Modal components!The implementation shows good practices:
- Proper handling of loading states
- Accessible modal implementation
- Clear confirmation messages
src/dashboard/Withdraw/RequestWithdrawBtn.tsx (1)
1-37: Well-structured component setup with proper TypeScript usage!The component demonstrates good practices:
- Proper type definitions for props
- Well-organized imports
- Clear state management setup
src/routing/index.tsx
Outdated
| id: 'dokan-base', | ||
| title: __( 'Withdraw', 'dokan-lite' ), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use unique route IDs.
Multiple routes are using the same ID 'dokan-base'. Each route should have a unique identifier for proper tracking and debugging.
- id: 'dokan-base',
+ id: 'dokan-withdraw',- id: 'dokan-base',
+ id: 'dokan-withdraw-requests',Also applies to: 70-71
src/routing/index.tsx
Outdated
| // @ts-ignore | ||
| routes = wp.hooks.applyFilters('dokan-dashboard-routes', routes) as Array<DokanRoute>; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove @ts-ignore and fix type issues.
Using @ts-ignore masks potential type safety issues. Consider properly typing the WordPress hooks or creating appropriate type definitions.
declare global {
interface Window {
wp: {
hooks: {
applyFilters<T>(hookName: string, value: T, ...args: any[]): T;
};
};
}
}
src/components/DataTable.tsx
Outdated
| onClick={() => { | ||
| onRowClick && onRowClick(row.original); | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve the click handler implementation.
The click handler can be simplified using optional chaining as suggested by static analysis.
-onClick={() => {
- onRowClick && onRowClick(row.original);
-}}
+onClick={() => onRowClick?.(row.original)}📝 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.
| onClick={() => { | |
| onRowClick && onRowClick(row.original); | |
| }} | |
| onClick={() => onRowClick?.(row.original)} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 36-36: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/definitions/Product.ts
Outdated
| interface Product { | ||
| id: number; | ||
| name: string; | ||
| slug: string; | ||
| post_author: string; | ||
| permalink: string; | ||
| date_created: string; | ||
| date_created_gmt: string; | ||
| date_modified: string; | ||
| date_modified_gmt: string; | ||
| type: string; | ||
| status: string; | ||
| featured: boolean; | ||
| catalog_visibility: string; | ||
| description: string; | ||
| short_description: string; | ||
| sku: string; | ||
| price: string; | ||
| regular_price: string; | ||
| sale_price: string; | ||
| date_on_sale_from: string | null; | ||
| date_on_sale_from_gmt: string | null; | ||
| date_on_sale_to: string | null; | ||
| date_on_sale_to_gmt: string | null; | ||
| price_html: string; | ||
| on_sale: boolean; | ||
| purchasable: boolean; | ||
| total_sales: number; | ||
| virtual: boolean; | ||
| downloadable: boolean; | ||
| downloads: any[]; // You can define a specific type if needed | ||
| download_limit: number; | ||
| download_expiry: number; | ||
| external_url: string; | ||
| button_text: string; | ||
| tax_status: string; | ||
| tax_class: string; | ||
| manage_stock: boolean; | ||
| stock_quantity: number | null; | ||
| low_stock_amount: string; | ||
| in_stock: boolean; | ||
| backorders: string; | ||
| backorders_allowed: boolean; | ||
| backordered: boolean; | ||
| sold_individually: boolean; | ||
| weight: string; | ||
| dimensions: ProductDimensions; | ||
| shipping_required: boolean; | ||
| shipping_taxable: boolean; | ||
| shipping_class: string; | ||
| shipping_class_id: number; | ||
| reviews_allowed: boolean; | ||
| average_rating: string; | ||
| rating_count: number; | ||
| related_ids: number[]; | ||
| upsell_ids: number[]; | ||
| cross_sell_ids: number[]; | ||
| parent_id: number; | ||
| purchase_note: string; | ||
| categories: ProductCategory[]; | ||
| tags: any[]; // You can define a specific type if needed | ||
| images: ProductImage[]; | ||
| attributes: any[]; // You can define a specific type if needed | ||
| default_attributes: any[]; // You can define a specific type if needed | ||
| variations: any[]; // You can define a specific type if needed | ||
| grouped_products: any[]; // You can define a specific type if needed | ||
| menu_order: number; | ||
| meta_data: ProductMetaData[]; | ||
| store: Store; | ||
| row_actions: RowActions; | ||
| _links: ApiLinks; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety and documentation in Product interface.
- Several properties use
any[]which reduces type safety:
// Define specific types for these arrays
interface ProductDownload {
id: string;
name: string;
file: string;
}
interface ProductAttribute {
id: number;
name: string;
position: number;
visible: boolean;
variation: boolean;
options: string[];
}
interface ProductTag {
id: number;
name: string;
slug: string;
}- Consider adding JSDoc documentation for better developer experience.
- Make null handling consistent across properties.
Apply these improvements:
+/** Represents a WooCommerce product with all its properties */
interface Product {
// ... existing properties ...
- downloads: any[];
+ downloads: ProductDownload[];
- tags: any[];
+ tags: ProductTag[];
- attributes: any[];
+ attributes: ProductAttribute[];
- variations: any[];
+ variations: number[]; // Array of variation IDs
- grouped_products: any[];
+ grouped_products: number[]; // Array of grouped product IDs
// ... existing properties ...
+ stock_quantity: number | null;
+ external_url: string | null;
+ button_text: string | null;
}
src/dashboard/Withdraw/index.tsx
Outdated
| useEffect( () => { | ||
| currentUser.fetchCurrentUser(); | ||
| }, [] ); |
There was a problem hiding this comment.
Add currentUser to effect dependencies.
The effect uses currentUser.fetchCurrentUser() but doesn't include it in the dependency array. This could lead to stale closures.
useEffect( () => {
currentUser.fetchCurrentUser();
- }, [] );
+ }, [currentUser.fetchCurrentUser] );📝 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.
| useEffect( () => { | |
| currentUser.fetchCurrentUser(); | |
| }, [] ); | |
| useEffect( () => { | |
| currentUser.fetchCurrentUser(); | |
| }, [currentUser.fetchCurrentUser] ); |
| onClick={ () => { | ||
| makeDefaultMethodHook | ||
| .makeDefaultMethod( activemethod.value ) | ||
| .then( () => { | ||
| toast( { | ||
| type: 'success', | ||
| title: __( | ||
| 'Default method updated', | ||
| 'dokan-lite' | ||
| ), | ||
| } ); | ||
| bodyData.refresh(); | ||
| } ); | ||
| } } |
There was a problem hiding this comment.
Add error handling for makeDefaultMethod.
The promise chain is missing error handling, which could lead to unhandled rejections and poor user experience when errors occur.
Add error handling:
makeDefaultMethodHook
.makeDefaultMethod(activemethod.value)
.then(() => {
toast({
type: 'success',
title: __('Default method updated', 'dokan-lite'),
});
bodyData.refresh();
+ })
+ .catch((error) => {
+ toast({
+ type: 'error',
+ title: error?.message ?? __('Failed to update default method', 'dokan-lite'),
+ });
});📝 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.
| onClick={ () => { | |
| makeDefaultMethodHook | |
| .makeDefaultMethod( activemethod.value ) | |
| .then( () => { | |
| toast( { | |
| type: 'success', | |
| title: __( | |
| 'Default method updated', | |
| 'dokan-lite' | |
| ), | |
| } ); | |
| bodyData.refresh(); | |
| } ); | |
| } } | |
| onClick={ () => { | |
| makeDefaultMethodHook | |
| .makeDefaultMethod( activemethod.value ) | |
| .then( () => { | |
| toast( { | |
| type: 'success', | |
| title: __( | |
| 'Default method updated', | |
| 'dokan-lite' | |
| ), | |
| } ); | |
| bodyData.refresh(); | |
| } ) | |
| .catch((error) => { | |
| toast({ | |
| type: 'error', | |
| title: error?.message ?? __('Failed to update default method', 'dokan-lite'), | |
| }); | |
| }); | |
| } } |
| if ( | ||
| ! bodyData || | ||
| ! bodyData.hasOwnProperty( 'isLoading' ) || | ||
| bodyData.isLoading || | ||
| masterLoading | ||
| ) { | ||
| return <Loader />; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve data validation checks.
Similar to PaymentDetails.tsx, replace the unsafe hasOwnProperty check with Object.hasOwn().
Apply this diff:
- if (
- ! bodyData ||
- ! bodyData.hasOwnProperty('isLoading') ||
- bodyData.isLoading ||
- masterLoading
- ) {
+ if ( ! bodyData || ! Object.hasOwn(bodyData, 'isLoading') || bodyData.isLoading || masterLoading ) {
return <Loader />;
}📝 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.
| if ( | |
| ! bodyData || | |
| ! bodyData.hasOwnProperty( 'isLoading' ) || | |
| bodyData.isLoading || | |
| masterLoading | |
| ) { | |
| return <Loader />; | |
| } | |
| if ( ! bodyData || ! Object.hasOwn(bodyData, 'isLoading') || bodyData.isLoading || masterLoading ) { | |
| return <Loader />; | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 111-111: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
| const [ cancelRequestId, setCancelRequestId ] = useState( '' ); | ||
| const withdrawHook = useWithdraw(); | ||
|
|
||
| const actions = []; | ||
|
|
||
| const toast = useToast(); | ||
|
|
||
| const fallbackData = []; | ||
| const canclePendingRequest = () => { | ||
| withdrawHook | ||
| .updateWithdraw( Number( cancelRequestId ), { | ||
| status: 'cancelled', | ||
| } ) | ||
| .then( () => { | ||
| toast( { | ||
| type: 'success', | ||
| title: __( 'Request cancelled successfully', 'dokan' ), | ||
| } ); | ||
| withdrawRequests.refresh(); | ||
| } ) | ||
| .catch( () => { | ||
| toast( { | ||
| type: 'error', | ||
| title: __( 'Failed to cancel request', 'dokan' ), | ||
| } ); | ||
| } ) | ||
| .finally( () => { | ||
| setIsOpen( false ); | ||
| } ); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety and error handling in cancellation logic.
- The
cancelRequestIdis stored as a string but used as a number in the API call. - Error handling could be more specific to provide better user feedback.
- const [ cancelRequestId, setCancelRequestId ] = useState( '' );
+ const [ cancelRequestId, setCancelRequestId ] = useState<number | null>(null);
const canclePendingRequest = () => {
+ if (cancelRequestId === null) return;
withdrawHook
- .updateWithdraw( Number( cancelRequestId ), {
+ .updateWithdraw( cancelRequestId, {
status: 'cancelled',
} )
.then( () => {
toast( {
type: 'success',
title: __( 'Request cancelled successfully', 'dokan' ),
} );
withdrawRequests.refresh();
} )
- .catch( () => {
+ .catch( (error: Error) => {
toast( {
type: 'error',
- title: __( 'Failed to cancel request', 'dokan' ),
+ title: error.message || __( 'Failed to cancel request', 'dokan' ),
} );
} )📝 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.
| const [ cancelRequestId, setCancelRequestId ] = useState( '' ); | |
| const withdrawHook = useWithdraw(); | |
| const actions = []; | |
| const toast = useToast(); | |
| const fallbackData = []; | |
| const canclePendingRequest = () => { | |
| withdrawHook | |
| .updateWithdraw( Number( cancelRequestId ), { | |
| status: 'cancelled', | |
| } ) | |
| .then( () => { | |
| toast( { | |
| type: 'success', | |
| title: __( 'Request cancelled successfully', 'dokan' ), | |
| } ); | |
| withdrawRequests.refresh(); | |
| } ) | |
| .catch( () => { | |
| toast( { | |
| type: 'error', | |
| title: __( 'Failed to cancel request', 'dokan' ), | |
| } ); | |
| } ) | |
| .finally( () => { | |
| setIsOpen( false ); | |
| } ); | |
| }; | |
| const [ cancelRequestId, setCancelRequestId ] = useState<number | null>(null); | |
| const withdrawHook = useWithdraw(); | |
| const actions = []; | |
| const toast = useToast(); | |
| const fallbackData = []; | |
| const canclePendingRequest = () => { | |
| if (cancelRequestId === null) return; | |
| withdrawHook | |
| .updateWithdraw( cancelRequestId, { | |
| status: 'cancelled', | |
| } ) | |
| .then( () => { | |
| toast( { | |
| type: 'success', | |
| title: __( 'Request cancelled successfully', 'dokan' ), | |
| } ); | |
| withdrawRequests.refresh(); | |
| } ) | |
| .catch( (error: Error) => { | |
| toast( { | |
| type: 'error', | |
| title: error.message || __( 'Failed to cancel request', 'dokan' ), | |
| } ); | |
| } ) | |
| .finally( () => { | |
| setIsOpen( false ); | |
| } ); | |
| }; |
| const unformatNumber = ( value ) => { | ||
| if ( value === '' ) { | ||
| return value; | ||
| } | ||
| return window.accounting.unformat( | ||
| value, | ||
| window.dokanCurrency.decimal | ||
| ); | ||
| }; | ||
|
|
||
| function calculateWithdrawCharge( method, value ) { | ||
| fetchCharge( method, value ); | ||
| } | ||
|
|
||
| const formatNumber = ( value ) => { | ||
| if ( value === '' ) { | ||
| return value; | ||
| } | ||
| return window.accounting.formatNumber( | ||
| value, | ||
| // @ts-ignore | ||
| window.dokanCurrency.precision, | ||
| window.dokanCurrency.thousand, | ||
| window.dokanCurrency.decimal | ||
| ); | ||
| }; | ||
|
|
||
| const formatMoney = ( money ) => { | ||
| // @ts-ignore | ||
| return window.accounting.formatMoney( money, { | ||
| symbol: '', | ||
| decimal: window.dokanCurrency.decimal, | ||
| thousand: window.dokanCurrency.thousand, | ||
| precision: window.dokanCurrency.precision, | ||
| format: window.dokanCurrency.format, | ||
| } ); | ||
| }; | ||
|
|
||
| const getRecivableFormated = () => { | ||
| if ( ! withdrawAmount ) { | ||
| return formatMoney( '' ); | ||
| } | ||
|
|
||
| return formatMoney( data?.receivable ?? '' ); | ||
| }; | ||
| const getChargeFormated = () => { | ||
| let chargeText = ''; | ||
| if ( ! withdrawAmount ) { | ||
| return formatMoney( '' ); | ||
| } | ||
|
|
||
| const fixed = data?.charge_data?.fixed | ||
| ? Number( data?.charge_data?.fixed ) | ||
| : ''; | ||
| const percentage = data?.charge_data?.percentage | ||
| ? Number( data?.charge_data?.percentage ) | ||
| : ''; | ||
|
|
||
| if ( fixed ) { | ||
| chargeText += formatMoney( fixed ); | ||
| } | ||
|
|
||
| if ( percentage ) { | ||
| chargeText += chargeText ? ' + ' : ''; | ||
| chargeText += `${ percentage }%`; | ||
| chargeText += ` = ${ formatMoney( data?.charge ) }`; | ||
| } | ||
|
|
||
| if ( ! chargeText ) { | ||
| chargeText = formatMoney( data?.charge ); | ||
| } | ||
|
|
||
| return chargeText; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider centralizing currency formatting logic and improving type safety.
- Currency formatting logic is scattered and uses multiple type assertions
- Utility functions could benefit from memoization
Create a centralized currency utility:
// src/utils/currency.ts
interface CurrencyConfig {
symbol: string;
decimal: string;
thousand: string;
precision: number;
format: string;
}
declare global {
interface Window {
dokanCurrency: CurrencyConfig;
accounting: {
formatMoney: (amount: number, options: Partial<CurrencyConfig>) => string;
formatNumber: (amount: number, precision: number, thousand: string, decimal: string) => string;
unformat: (amount: string, decimal: string) => number;
};
}
}
export const getCurrencyConfig = (): CurrencyConfig => window.dokanCurrency;
export const formatMoney = (amount: number | string): string => {
const config = getCurrencyConfig();
return window.accounting.formatMoney(Number(amount), {
symbol: '',
...config
});
};| const handleCreateWithdraw = () => { | ||
| const payload = { | ||
| method: withdrawMethod, | ||
| amount: unformatNumber( withdrawAmount ), | ||
| }; | ||
|
|
||
| // Call the createWithdraw function here | ||
| withdrawHook | ||
| .createWithdraw( payload ) | ||
| .then( () => { | ||
| setIsOpen( false ); | ||
| toast( { | ||
| title: __( 'Withdraw request created.', 'dokan' ), | ||
| type: 'success', | ||
| } ); | ||
|
|
||
| withdrawRequests.refresh(); | ||
| } ) | ||
| .catch( ( err ) => { | ||
| toast( { | ||
| title: | ||
| err?.message ?? | ||
| __( 'Failed to create withdraw.', 'dokan' ), | ||
| type: 'error', | ||
| } ); | ||
| console.error( 'Error creating withdraw:', err ); | ||
| } ); | ||
| }; | ||
|
|
||
| function handleWithdrawAmount( value ) { | ||
| if ( ! value ) { | ||
| value = 0; | ||
| } | ||
| setWithdrawAmount( value ); | ||
| calculateWithdrawCharge( withdrawMethod, unformatNumber( value ) ); | ||
| } | ||
|
|
||
| const debouncedWithdrawAmount = useDebounceCallback( | ||
| handleWithdrawAmount, | ||
| 500 | ||
| ); | ||
|
|
||
| useEffect( () => { | ||
| if ( settings?.data?.payment_methods.length > 0 ) { | ||
| setWithdrawMethod( settings?.data?.payment_methods[ 0 ].value ); | ||
| } | ||
| }, [ settings ] ); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and prevent potential race conditions.
- Error handling could be more specific
- Initial method selection might race with data loading
useEffect( () => {
- if ( settings?.data?.payment_methods.length > 0 ) {
+ if ( settings?.data?.payment_methods?.length > 0 && !withdrawMethod ) {
setWithdrawMethod( settings?.data?.payment_methods[ 0 ].value );
}
}, [ settings ] );
const handleCreateWithdraw = () => {
+ if (!withdrawMethod || !withdrawAmount) {
+ toast({
+ title: __('Please fill in all required fields', 'dokan'),
+ type: 'error'
+ });
+ return;
+ }
+
const payload = {
method: withdrawMethod,
amount: unformatNumber( withdrawAmount ),
};
withdrawHook
.createWithdraw( payload )
.then( () => {
setIsOpen( false );
+ setWithdrawAmount('');
toast( {
title: __( 'Withdraw request created.', 'dokan' ),
type: 'success',
} );
withdrawRequests.refresh();
} )
- .catch( ( err ) => {
+ .catch( ( err: Error ) => {
toast( {
title:
- err?.message ??
+ (err instanceof Error ? err.message :
__( 'Failed to create withdraw.', 'dokan' )),
type: 'error',
} );
- console.error( 'Error creating withdraw:', err );
+ console.error( '[Withdraw Request Error]:', err );
} );
};📝 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.
| const handleCreateWithdraw = () => { | |
| const payload = { | |
| method: withdrawMethod, | |
| amount: unformatNumber( withdrawAmount ), | |
| }; | |
| // Call the createWithdraw function here | |
| withdrawHook | |
| .createWithdraw( payload ) | |
| .then( () => { | |
| setIsOpen( false ); | |
| toast( { | |
| title: __( 'Withdraw request created.', 'dokan' ), | |
| type: 'success', | |
| } ); | |
| withdrawRequests.refresh(); | |
| } ) | |
| .catch( ( err ) => { | |
| toast( { | |
| title: | |
| err?.message ?? | |
| __( 'Failed to create withdraw.', 'dokan' ), | |
| type: 'error', | |
| } ); | |
| console.error( 'Error creating withdraw:', err ); | |
| } ); | |
| }; | |
| function handleWithdrawAmount( value ) { | |
| if ( ! value ) { | |
| value = 0; | |
| } | |
| setWithdrawAmount( value ); | |
| calculateWithdrawCharge( withdrawMethod, unformatNumber( value ) ); | |
| } | |
| const debouncedWithdrawAmount = useDebounceCallback( | |
| handleWithdrawAmount, | |
| 500 | |
| ); | |
| useEffect( () => { | |
| if ( settings?.data?.payment_methods.length > 0 ) { | |
| setWithdrawMethod( settings?.data?.payment_methods[ 0 ].value ); | |
| } | |
| }, [ settings ] ); | |
| const handleCreateWithdraw = () => { | |
| if (!withdrawMethod || !withdrawAmount) { | |
| toast({ | |
| title: __('Please fill in all required fields', 'dokan'), | |
| type: 'error' | |
| }); | |
| return; | |
| } | |
| const payload = { | |
| method: withdrawMethod, | |
| amount: unformatNumber( withdrawAmount ), | |
| }; | |
| // Call the createWithdraw function here | |
| withdrawHook | |
| .createWithdraw( payload ) | |
| .then( () => { | |
| setIsOpen( false ); | |
| setWithdrawAmount(''); | |
| toast( { | |
| title: __( 'Withdraw request created.', 'dokan' ), | |
| type: 'success', | |
| } ); | |
| withdrawRequests.refresh(); | |
| } ) | |
| .catch( ( err: Error ) => { | |
| toast( { | |
| title: | |
| (err instanceof Error ? err.message : | |
| __( 'Failed to create withdraw.', 'dokan' )), | |
| type: 'error', | |
| } ); | |
| console.error( '[Withdraw Request Error]:', err ); | |
| } ); | |
| }; | |
| function handleWithdrawAmount( value ) { | |
| if ( ! value ) { | |
| value = 0; | |
| } | |
| setWithdrawAmount( value ); | |
| calculateWithdrawCharge( withdrawMethod, unformatNumber( value ) ); | |
| } | |
| const debouncedWithdrawAmount = useDebounceCallback( | |
| handleWithdrawAmount, | |
| 500 | |
| ); | |
| useEffect( () => { | |
| if ( settings?.data?.payment_methods?.length > 0 && !withdrawMethod ) { | |
| setWithdrawMethod( settings?.data?.payment_methods[ 0 ].value ); | |
| } | |
| }, [ settings ] ); |
Aunshon
changed the base branch from
develop
to
update/vendor-dashboard-structure-router-context-support
Aunshon
changed the base branch from
update/vendor-dashboard-structure-router-context-support
to
update/vendor-dashboard-structure
Aunshon
changed the base branch from
update/vendor-dashboard-structure
to
enhance/introduce-dataviews-from-dokan-free
Aunshon
added
Needs: Testing
…ashboard-structure-convert-withdraw # Conflicts: # docs/frontend/components.md # includes/Assets.php # package.json # src/components/dataviews/DataViewTable.tsx # src/components/index.tsx # src/routing/index.tsx # webpack-entries.js # webpack.config.js
…ashboard-structure-convert-withdraw
refactor: use hook from @/hooks
refactor: removed unnecessery types
…ashboard-structure-convert-withdraw
…ashboard-structure-convert-withdraw # Conflicts: # src/components/index.tsx
mrabbani
added
👍 Dev Review Done
and removed
Needs: Dev Review
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
Title
Detailed Description of the pull request. What was previous behaviour
and what will be changed in this PR.
Before Changes
Describe the issue before changes with screenshots(s).
After Changes
Describe the issue after changes with screenshot(s).
Feature Video (optional)
Link of detailed video if this PR is for a feature.
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit
Based on the comprehensive summary, here are the concise release notes:
Release Notes
New Features
Improvements
Performance
Bug Fixes