Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Re-Enginrring] withdraw page #2506

Open
wants to merge 40 commits into
base: update/vendor-dashboard-structure
Choose a base branch
from

Conversation

Aunshon
Copy link
Collaborator

@Aunshon Aunshon commented Jan 9, 2025

All Submissions:

  • My code follow the WordPress' coding standards
  • My code satisfies feature requirements
  • My code is tested
  • My code passes the PHPCS tests
  • My code has proper inline documentation
  • I've included related pull request(s) (optional)
  • I've included developer documentation (optional)
  • I've added proper labels to this pull request

Changes proposed in this Pull Request:

Related Pull Request(s)

  • Full PR Link

Closes

  • Closes #

How to test the changes in this Pull Request:

  • Steps or issue link

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:

  • Code is not following code style guidelines
  • Bad naming: make sure you would understand your code if you read it a few months from now.
  • KISS: Keep it simple, Sweetie (not stupid!).
  • DRY: Don't Repeat Yourself.
  • Code that is not readable: too many nested 'if's are a bad sign.
  • Performance issues
  • Complicated constructions that need refactoring or comments: code should almost always be self-explanatory.
  • Grammar errors.

FOR PR REVIEWER ONLY:

As a reviewer, your feedback should be focused on the idea, not the person. Seek to understand, be respectful, and focus on constructive dialog.

As a contributor, your responsibility is to learn from suggestions and iterate your pull request should it be needed based on feedback. Seek to collaborate and produce the best possible contribution to the greater whole.

  • Correct — Does the change do what it’s supposed to? ie: code 100% fulfilling the requirements?
  • Secure — Would a nefarious party find some way to exploit this change? ie: everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities?
  • Readable — Will your future self be able to understand this change months down the road?
  • Elegant — Does the change fit aesthetically within the overall style and architecture?

Summary by CodeRabbit

Based on the comprehensive summary, here are the concise release notes:

Release Notes

  • New Features

    • Added a new withdrawal dashboard with enhanced user interface
    • Implemented React-based routing for dashboard navigation
    • Introduced DataView Table for improved data management
    • Added comprehensive status page with dynamic content rendering
  • Improvements

    • Enhanced REST API endpoints for customers, continents, and countries
    • Improved withdrawal request management
    • Added TypeScript type definitions for better type safety
    • Implemented custom hooks for state management
  • Performance

    • Optimized loading states and data fetching
    • Integrated WordPress hooks for extensibility
    • Added skeleton loading for better user experience
  • Bug Fixes

    • Resolved issues with navigation and routing
    • Improved error handling in API requests

Aunshon and others added 28 commits November 28, 2024 10:52
…ure-convert-withdraw

# Conflicts:
#	composer.lock
…andle responsive preview from base components.
…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
@Aunshon Aunshon self-assigned this Jan 9, 2025
Copy link
Contributor

coderabbitai bot commented Jan 9, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This 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

File/Group Change Summary
base-tailwind.config.js New Tailwind CSS configuration with scoped preflight styles and custom theming
docs/ Added documentation for feature overrides, components, data views, and utilities
includes/REST/ New REST API controllers for customers, continents, countries, and orders
src/components/ New reusable React components like DataTable, DateTimeHtml, DokanTooltip
src/dashboard/Withdraw/ Comprehensive withdrawal management components and hooks
src/definitions/ TypeScript interfaces for various data models like Products, Orders, Customers
src/layout/ New layout components for dashboard interface
src/Status/ New status management system with React components

Sequence Diagram

sequenceDiagram
    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
Loading

Possibly related PRs

Suggested Labels

needs-testing, frontend-refactor, react-migration

Suggested Reviewers

Poem

🐰 Hop into the code, a rabbit's delight,
Dokan's dashboard now shines so bright!
React components dance with grace,
Tailwind styles set a new pace.
A vendor's journey, smooth and clean,
Our plugin's magic can now be seen! 🌈


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 37

🔭 Outside diff range comments (1)
includes/Assets.php (1)

Line range hint 392-605: Check Compatibility with WooCommerce Versions

The use of WCAdminAssets::get_instance() may cause compatibility issues with older versions of WooCommerce that do not have this class. To prevent potential fatal errors, consider adding a check to ensure that the class exists before calling its methods.

Apply this diff to prevent compatibility issues:

+        if ( class_exists( 'Automattic\WooCommerce\Internal\Admin\WCAdminAssets' ) ) {
             $wc_instance = WCAdminAssets::get_instance();
             $wc_instance->register_scripts();
+        }

Also, consider specifying the minimum WooCommerce version required for your plugin.

🧹 Nitpick comments (81)
src/Definitions/CountryState.ts (4)

1-5: API documentation needs improvement.

The API endpoint example uses a placeholder domain (you-site.test) which should be replaced with proper documentation.

-* Api is: http://you-site.test/wp-json/dokan/v1/data/countries
+* API endpoint: /wp-json/dokan/v1/data/countries
+* 
+* @since 1.0.0
+* @see DokanDataCountriesController

7-14: Add JSDoc comments for better documentation.

The Link and Links interfaces would benefit from JSDoc comments explaining their purpose and relationship to the HAL specification.

+/**
+ * Represents a HAL-compliant hyperlink.
+ * @see https://tools.ietf.org/html/draft-kelly-json-hal
+ */
 export interface Link {
     href: string;
 }

+/**
+ * Collection of HAL-compliant hyperlinks for API navigation.
+ */
 export interface Links {
     self: Link[];
     collection: Link[];
 }

16-26: Consider adding optional fields to State interface.

The State interface could be more comprehensive by including optional fields that might be useful for address formatting or validation.

 export interface State {
     code: string;
     name: string;
+    country_code?: string;
+    is_active?: boolean;
 }

30-34: Consider using a specific error type.

Instead of using a generic string for errors, consider creating a specific error type to better handle different error cases.

+export type CountryStateError = {
+    message: string;
+    code?: string;
+    details?: unknown;
+}

 export interface CountryStateState {
     countries: Record<string, Country>;
     isLoading: Record<string, boolean>;
-    errors: Record<string, string | null>;
+    errors: Record<string, CountryStateError | null>;
 }
src/Definitions/Continents.ts (1)

4-15: Enhance Country interface with validation constraints.

The Country interface could benefit from more specific type constraints and documentation.

+/**
+ * Represents a country with its currency and measurement settings.
+ */
 export interface Country {
     code: string;
+    /** ISO 3166-1 alpha-2 country code */
     name: string;
+    /** ISO 4217 currency code */
     currency_code: string;
     currency_pos: 'left' | 'right' | 'left_space' | 'right_space';
     decimal_sep: string;
     dimension_unit: 'cm' | 'foot';
-    num_decimals: number;
+    num_decimals: number & { __brand: 'NonNegativeInteger' };
     thousand_sep: string;
     weight_unit: 'kg' | 'oz';
     states: State[];
 }
includes/REST/DokanDataCountriesController.php (3)

7-8: Remove Unused Imports

The imported classes WP_Error, WP_REST_Request, and WP_REST_Response are not used in the code. Removing unused imports improves code clarity.

Apply this diff to remove unused imports:

-use WP_Error;
-use WP_REST_Request;
-use WP_REST_Response;

80-83: Replace Placeholder Constants in DocBlocks

The @since DOKAN_SINCE annotation contains a placeholder. Replace it with the actual version number for accurate documentation.

Apply this diff:

- * @since DOKAN_SINCE
+ * @since 3.5.0

91-98: Inconsistent Error Codes and Text Domains

The error code 'dokan_pro_permission_failure' and text domain 'dokan-lite' might not reflect the correct plugin context. Ensure consistency in error handling.

Apply this diff:

-                'dokan_pro_permission_failure',
-                __( 'You are not allowed to do this action.', 'dokan-lite' ),
+                'dokan_permission_failure',
+                __( 'You are not allowed to perform this action.', 'dokan' ),
includes/REST/DokanDataContinentsController.php (4)

7-8: Remove Unused Imports

The imported classes WP_Error, WP_REST_Request, and WP_REST_Response are not utilized. Cleaning them up enhances code readability.

Apply this diff:

-use WP_Error;
-use WP_REST_Request;
-use WP_REST_Response;

80-83: Replace Placeholder Constants in DocBlocks

Update @since DOKAN_SINCE with the actual version number for accurate documentation.

Apply this diff:

- * @since DOKAN_SINCE
+ * @since 3.5.0

91-98: Inconsistent Error Codes and Text Domains

The error code 'dokan_pro_permission_failure' and text domain 'dokan-lite' may not be consistent with the plugin's context. Ensure they accurately reflect the usage.

Apply this diff:

-                'dokan_pro_permission_failure',
-                __( 'You are not allowed to do this action.', 'dokan-lite' ),
+                'dokan_permission_failure',
+                __( 'You are not allowed to perform this action.', 'dokan' ),

48-54: Incorrect Method Documentation

The get_item method documentation mentions returning states for a country, but in the context of continents, this should be corrected.

Apply this diff:

- * Return the list of states for a given country.
+ * Return the data for a given continent.
includes/VendorNavMenuChecker.php (3)

13-13: Typographical Error in DocBlock

There is a missing asterisk in the DocBlock for the $template_dependencies property.

Apply this diff:

- [ 'route' => [ ['slug' => 'template-slug', 'name' => 'template-name' (Optional), 'args' = [] (Optional)  ] ] ]
+  * [ 'route' => [ ['slug' => 'template-slug', 'name' => 'template-name' (Optional), 'args' => [] (Optional) ] ] ]

70-77: Simplify Boolean Logic

The logic in is_dependency_cleared can be simplified to improve readability.

Apply this diff:

-        $clear        = true;
-        $dependencies = $this->get_template_dependencies_resolutions();
-
-        if ( ! empty( $dependencies[ trim( $route, '/' ) ] ) ) {
-            $clear = false;
-        }
+        $dependencies = $this->get_template_dependencies_resolutions();
+        $clear = empty( $dependencies[ trim( $route, '/' ) ] );

201-204: Correct Internationalization Function Usage

The translation function should properly wrap the entire message, including the placeholders.

Apply this diff:

-            $notice = sprintf(
-                /* translators: %s: overridden templates */
-                __( 'The following templates are overridden:<br><code>%s</code>', 'dokan-lite' ),
-                implode( ',<br>', $overridden_templates )
-            );
+            /* translators: %s: overridden templates */
+            $notice = sprintf(
+                __( 'The following templates are overridden:<br><code>%s</code>', 'dokan-lite' ),
+                implode( ',<br>', $overridden_templates )
+            );
includes/REST/CustomersController.php (1)

232-242: Replace Placeholder @since DOKAN_SINCE with Actual Version

In the docblock for the dokan_json_search_found_customers filter, the @since tag uses DOKAN_SINCE as a placeholder. Please replace DOKAN_SINCE with the actual version number when this code is released to maintain accurate documentation.

includes/Assets.php (2)

19-23: Ensure Proper Escaping of Output in wp_footer Action

In the wp_footer action, raw HTML is echoed directly. While the content is static, it's good practice to use proper escaping functions to enhance security and maintainability.

Apply this diff to escape the output:

 add_action(
     'wp_footer', function () {
-        echo '<div class="dokan-layout" id="headlessui-portal-root"><div></div></div>';
+        echo '<div class="dokan-layout" id="headlessui-portal-root"><div></div></div>';
     }
 );

Alternatively, since the HTML is static and controlled, you might consider leaving it as is but be cautious with any dynamic content.


918-918: Avoid Global Namespace Pollution in wp_localize_script

When localizing the script dokan-react-frontend, the variable name dokanCurrency is added to the global scope. To prevent potential conflicts, consider namespacing it within an existing global object like dokan.

Modify the code as follows:

- wp_localize_script( 'dokan-react-frontend', 'dokanCurrency', $this->get_localized_price() );
+ wp_localize_script( 'dokan-react-frontend', 'dokan', array_merge( isset( $dokan ) ? $dokan : [], [ 'currency' => $this->get_localized_price() ] ) );

Then, in your JavaScript, access it using dokan.currency.

src/layout/404.tsx (2)

1-9: Add Type Annotation to Functional Component

To improve type safety and maintainability in TypeScript, it's recommended to add type annotations to your components. Consider specifying the type for the NotFound component.

Apply this diff to add the type annotation:

 const NotFound =  () => {
+// or
+const NotFound: React.FC = () => {

1-9: Enhance the 404 Page User Experience

The current NotFound component displays a simple message. Consider enhancing the user experience by adding navigation options, such as a link to the homepage or a search bar, to help users find what they're looking for.

Example:

 return (
     <div>
-        <h1>404 - Not Found!</h1>
+        <h1>404 - Page Not Found</h1>
+        <p>The page you are looking for does not exist.</p>
+        <a href="/">Go to Homepage</a>
     </div>
 );
src/Definitions/RouterProps.ts (2)

13-13: Consider replacing any with a more specific type.

Using any reduces type safety. Consider defining an interface for the location state or use unknown if the state type is truly unpredictable.

-    location: Location< any >;
+    location: Location< unknown >;

16-16: Consider adding more specific types for UIMatch generics.

The unknown type parameters could be replaced with more specific types based on your route configuration.

-    matches: UIMatch< unknown, unknown >[];
+    matches: UIMatch< object, string >[];  // Adjust based on your actual route params and data
src/layout/ContentArea.tsx (2)

4-15: Add TypeScript type definitions to improve maintainability.

The component could benefit from TypeScript types for better maintainability and developer experience.

-const ContentArea = ( { children } ) => {
+interface ContentAreaProps {
+    children: React.ReactNode;
+}
+
+const ContentArea: React.FC<ContentAreaProps> = ({ children }) => {

8-12: Consider extracting slot names as constants.

The slot names could be extracted as constants to prevent typos and enable reuse.

+const CONTENT_AREA_SLOTS = {
+    BEFORE: 'dokan-layout-content-area-before',
+    AFTER: 'dokan-layout-content-area-after',
+} as const;
+
 <div className="dokan-layout-content-area">
-    <Slot name="dokan-layout-content-area-before" />
+    <Slot name={CONTENT_AREA_SLOTS.BEFORE} />
     { children }
-    <Slot name="dokan-layout-content-area-after" />
+    <Slot name={CONTENT_AREA_SLOTS.AFTER} />
 </div>
src/Components/DokanTooltip.tsx (1)

14-14: Consider extracting Tailwind classes to a constant or using a CSS module.

Hardcoded CSS classes could be made more maintainable by extracting them to a constant or using a CSS module.

+const TOOLTIP_CLASSES = 'block text-sm text-gray-500 mb-1' as const;
+
 <div
-    className="block text-sm text-gray-500 mb-1"
+    className={TOOLTIP_CLASSES}
src/layout/Header.tsx (2)

4-4: Add TypeScript type definition for props

Consider adding explicit type definition for the component props to improve type safety.

-const Header = ( { title = '' } ) => {
+type HeaderProps = {
+    title?: string;
+};
+
+const Header = ( { title = '' }: HeaderProps ) => {

11-11: Consider accessibility improvements for the heading

The h1 heading level might not be appropriate in all contexts where this component is used. Consider making the heading level configurable.

-{ title && (<h1 className="mb-4 text-3xl font-semibold text-gray-800 dark:text-white md:text-4xl lg:text-4xl">{title}</h1>)}
+{ title && (
+    <Heading
+        level={headingLevel}
+        className="mb-4 text-3xl font-semibold text-gray-800 dark:text-white md:text-4xl lg:text-4xl"
+    >
+        {title}
+    </Heading>
+)}
src/Definitions/window-types.ts (2)

18-18: Replace any type with more specific type definition

Using any type reduces TypeScript's type safety benefits. Consider using a more specific return type for the moment function.

-moment: ( date: string ) => any;
+moment: ( date: string ) => moment.Moment;

30-31: Remove unnecessary empty export

The empty export statement is flagged as unnecessary by the static analysis tool since there are already imports present.

-// This empty export is necessary to make this a module
-export {};
🧰 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/dashboard/index.tsx (1)

11-27: Consider memoizing mapped routes

The routes mapping could be memoized to prevent unnecessary re-renders.

+import { useMemo } from 'react';

 const App = () => {
     const routes = getRoutes();
 
-    const mapedRoutes = routes.map((route) => {
+    const mappedRoutes = useMemo(() => routes.map((route) => {
         const WithRouterComponent = withRouter(route.element);
         return {
             path: route.path,
             element: (
                 <Layout
                     headerComponent={route?.header}
                     footerComponent={route?.footer}
                     route={route}
                     title={route?.title}
                 >
                     <WithRouterComponent />
                 </Layout>
             ),
         };
-    });
+    }), [routes]);
src/Components/PriceHtml.tsx (1)

15-22: Simplify default value assignments

The multiple if statements for default values can be simplified using object destructuring with default values.

-const PriceHtml = ( {
-    price = 0,
-    currencySymbol = '',
-    precision = null,
-    thousand = '',
-    decimal = '',
-    format = '',
-}: PriceHtmlProps ) => {
-    if ( ! currencySymbol ) {
-        currencySymbol = window.dokanCurrency.symbol;
-    }
-    if ( ! precision ) {
-        precision = window.dokanCurrency.precision;
-    }
-    if ( ! thousand ) {
-        thousand = window.dokanCurrency.thousand;
-    }
-    if ( ! decimal ) {
-        decimal = window.dokanCurrency.decimal;
-    }
-    if ( ! format ) {
-        format = window.dokanCurrency.format;
-    }
+const PriceHtml = ({
+    price = 0,
+    currencySymbol = window.dokanCurrency.symbol,
+    precision = window.dokanCurrency.precision,
+    thousand = window.dokanCurrency.thousand,
+    decimal = window.dokanCurrency.decimal,
+    format = window.dokanCurrency.format,
+}: PriceHtmlProps) => {
src/hooks/ViewportDimensions.ts (2)

11-11: Update version placeholder

The @since tag contains a placeholder DOKAN_PRO_SINCE. This should be replaced with the actual version number.


16-19: Consider memoizing viewport dimensions object

The viewport dimensions object is recreated on every render. Consider memoizing it with useMemo for better performance.

- const getViewportDimensions = useCallback((): ViewportDimensions => ({
+ const getViewportDimensions = useCallback((): ViewportDimensions => {
+   if (typeof window === 'undefined') {
+     return { width: null, height: null };
+   }
+   return {
    width: typeof window !== 'undefined' ? window.innerWidth : null,
    height: typeof window !== 'undefined' ? window.innerHeight : null,
-  }), []);
+  };
+ }, []);
src/Components/DateTimeHtml.tsx (1)

26-65: Reduce code duplication in static methods

The Date and Time static methods contain similar logic. Consider extracting the common functionality into a shared utility function.

const formatDateTime = (
  value: string,
  format: string,
  defaultValue: string | JSX.Element | null = '-'
) => {
  if (!value) {
    return defaultValue;
  }
  return (
    <RawHTML>
      {dateI18n(format, value, getSettings().timezone.string)}
    </RawHTML>
  );
};
src/Dashboard/Withdraw/Hooks/useCharge.ts (1)

27-51: Consider implementing debouncing

The fetchCharge function could benefit from debouncing to prevent excessive API calls when amount changes frequently.

Consider using the lodash/debounce utility:

import { debounce } from 'lodash';

const debouncedFetch = useCallback(
  debounce(async (method: string, amount: string) => {
    // existing fetch logic
  }, 300),
  []
);
includes/REST/OrderControllerV3.php (1)

12-12: Replace placeholder with actual version number

The @since DOKAN_SINCE placeholder should be replaced with the actual version number.

src/Dashboard/Withdraw/Hooks/useWithdrawSettings.ts (1)

36-57: Add retry mechanism for failed requests

Consider implementing a retry mechanism for failed API requests, especially for network-related errors.

 const fetchSettings = useCallback( async () => {
+    const MAX_RETRIES = 3;
+    let retries = 0;
+    
+    const attemptFetch = async (): Promise<WithdrawSettings> => {
         try {
-            setIsLoading( true );
-            setError( null );
-
-            const response = await apiFetch< WithdrawSettings >( {
+            return await apiFetch< WithdrawSettings >( {
                 path: '/dokan/v2/withdraw/settings',
                 method: 'GET',
             } );
-
-            setData( response );
         } catch ( err ) {
+            if (retries < MAX_RETRIES && err instanceof Error && err.message.includes('network')) {
+                retries++;
+                await new Promise(resolve => setTimeout(resolve, 1000 * retries));
+                return attemptFetch();
+            }
+            throw err;
+        }
+    };
+
+    try {
+        setIsLoading(true);
+        setError(null);
+        const response = await attemptFetch();
+        setData(response);
+    } catch (err) {
             setError(
                 err instanceof Error
                     ? err
                     : new Error( 'Failed to fetch withdraw settings' )
             );
             console.error( 'Error fetching withdraw settings:', err );
-        } finally {
-            setIsLoading( false );
-        }
+    } finally {
+        setIsLoading(false);
+    }
 }, [] );
src/hooks/useCurrentUser.ts (2)

17-17: Improve type safety for meta field

Using any[] for meta reduces type safety. Consider defining a proper interface for the meta structure.

-    meta: any[];
+    meta: {
+        key: string;
+        value: string | number | boolean;
+    }[];

62-64: Remove or document commented code

The commented useEffect suggests automatic data fetching was considered but disabled. Either remove it or document why it's kept as a reference.

-    // useEffect( () => {
-    //     fetchCurrentUser();
-    // }, [ fetchCurrentUser ] );
src/layout/index.tsx (2)

11-12: Consider adding type safety to ThemeContext

The context is created with null as initial value without type definitions. This could lead to runtime type errors.

-const ThemeContext = createContext( null );
+type ThemeContextType = {
+    theme: string;
+    setTheme: (theme: string) => void;
+};
+const ThemeContext = createContext<ThemeContextType | null>(null);

46-71: Consider adding error boundary and loading states

The Layout component looks good but could benefit from error handling and loading states for better user experience.

Consider wrapping the content with an error boundary:

+import { ErrorBoundary } from '@wordpress/element';
+
 const Layout = ({ children, ...props }: LayoutProps) => {
     return (
         <ThemeProvider>
             <SlotFillProvider>
+                <ErrorBoundary fallback={<div>Something went wrong.</div>}>
                 <div className="dokan-layout">
                     {/* ... existing code ... */}
                 </div>
+                </ErrorBoundary>
                 <PluginArea scope={route.id} />
                 <DokanToaster />
             </SlotFillProvider>
         </ThemeProvider>
     );
 };
src/Dashboard/Withdraw/index.tsx (1)

33-65: Consider memoizing child components to prevent unnecessary re-renders

The child components receive new object references on every render which could cause performance issues.

+const memoizedBalance = useMemo(() => (
     <Balance
         masterLoading={currentUser.isLoading || useWithdrawRequestHook.isLoading}
         bodyData={balance}
         settings={withdrawSettings}
         withdrawRequests={useWithdrawRequestHook}
     />
+), [currentUser.isLoading, useWithdrawRequestHook.isLoading, balance, withdrawSettings]);
src/Dashboard/Withdraw/Hooks/useWithdraw.ts (1)

31-35: Add request cancellation handling

The API requests should be cancellable when the component unmounts.

+import { addQueryArgs } from '@wordpress/url';

 const createWithdraw = useCallback(
     async (payload: WithdrawPayload): Promise<void> => {
+        const abortController = new AbortController();
         try {
             setIsLoading(true);
             setError(null);
             await apiFetch({
                 path: '/dokan/v1/withdraw',
                 method: 'POST',
                 data: payload,
+                signal: abortController.signal,
             });
+            return () => abortController.abort();
         } catch (err) {
+            if (err.name === 'AbortError') {
+                return;
+            }
             // ... rest of error handling
         }
     },
     []
 );
src/Routing/index.tsx (1)

46-77: Refactor route definitions for better maintainability.

The current implementation adds routes individually with repeated code. Consider refactoring to use a route configuration array.

Here's a suggested refactor:

const getRoutes = () => {
-    let routes : Array<DokanRoute> = [];
+    const routeConfigs: Array<DokanRoute> = [
+        {
+            id: 'dokan-base',
+            title: __( 'Dashboard', 'dokan-lite' ),
+            element: <h1>Dashboard body</h1>,
+            path: '/',
+            exact: true,
+            order: 10,
+        },
+        {
+            id: 'dokan-withdraw',
+            title: __( 'Withdraw', 'dokan-lite' ),
+            element: <Withdraw/>,
+            path: '/withdraw',
+            exact: true,
+            order: 10,
+        },
+        {
+            id: 'dokan-withdraw-requests',
+            title: __( 'Withdraw', 'dokan-lite' ),
+            element: <WithdrawRequests/>,
+            path: '/withdraw-requests',
+            exact: true,
+            order: 10,
+        },
+    ];
+    
+    let routes = [...routeConfigs];
src/Components/DataTable.tsx (2)

35-37: Improve click handler with optional chaining.

The click handler can be simplified using optional chaining for better readability and safety.

Apply this diff:

-                                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)


9-61: Consider adding loading and empty states.

The table handles data well but doesn't provide feedback for loading or empty states. This could impact user experience.

Consider adding:

  • A loading spinner or skeleton for loading state
  • A meaningful message when there's no data
🧰 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/woocommerce-accounting.d.ts (2)

81-86: Improve type safety of helper methods.

The helper methods use the any type, which reduces type safety. Consider using more specific types.

Apply this diff:

-        isNumber(value: any): boolean;
-        isArray(value: any): boolean;
-        isObject(value: any): boolean;
-        isString(value: any): boolean;
-        isFunction(value: any): boolean;
-        isDefined(value: any): boolean;
+        isNumber(value: unknown): value is number;
+        isArray(value: unknown): value is Array<unknown>;
+        isObject(value: unknown): value is Record<string, unknown>;
+        isString(value: unknown): value is string;
+        isFunction(value: unknown): value is Function;
+        isDefined<T>(value: T | undefined | null): value is T;

2-17: Add JSDoc comments for better documentation.

The interfaces would benefit from JSDoc comments explaining their purpose and usage.

Consider adding JSDoc comments like this:

/** Settings interface for WooCommerce accounting configuration */
interface Settings {
    /** Currency-specific settings */
    currency: {
        // ... (existing properties with comments)
    };
    /** Number formatting settings */
    number: {
        // ... (existing properties with comments)
    };
}

Also applies to: 19-30, 32-110

src/Definitions/Order.ts (2)

37-37: Consider using more specific types instead of any[]

Replace any[] with more specific types to improve type safety. This applies to:

  • taxes arrays in LineItem and ShippingLine
  • tax_lines, fee_lines, and refunds arrays in Order

Example for tax lines:

interface TaxLine {
  id: number;
  rate_code: string;
  rate_id: number;
  label: string;
  compound: boolean;
  tax_total: string;
  shipping_tax_total: string;
}

Also applies to: 51-51, 124-127


76-85: Consider removing the catch-all string from OrderStatus

The OrderStatus type includes a catch-all string which defeats the purpose of having a specific union type. Consider removing it if these are all the valid statuses.

export type OrderStatus =
    | 'pending'
    | 'processing'
    | 'on-hold'
    | 'completed'
    | 'cancelled'
    | 'refunded'
    | 'failed'
-    | 'trash'
-    | string;
+    | 'trash';
src/Dashboard/Withdraw/Balance.tsx (2)

50-51: Remove unnecessary fragment

The fragment wrapper is unnecessary since there's only one child element.

-   return (
-       <>
            <Card className="dokan-withdraw-style-reset dokan-layout">
-       </>
-   );
+   return (
+       <Card className="dokan-withdraw-style-reset dokan-layout">
+   );

Also applies to: 96-97


65-68: Simplify optional chaining

The optional chaining with nullish coalescing can be simplified.

-price={
-    bodyData?.data?.current_balance ??
-    ''
-}
+price={bodyData?.data?.current_balance ?? ''}

-price={
-    bodyData?.data?.withdraw_limit ?? ''
-}
+price={bodyData?.data?.withdraw_limit ?? ''}

Also applies to: 82-84

src/Dashboard/Withdraw/Hooks/useWithdrawRequests.ts (1)

94-101: Define a custom error type for better error handling

Create a custom error type to handle specific withdraw request errors.

interface WithdrawRequestError extends Error {
  code?: string;
  details?: string;
}

// Usage in catch block:
} catch (err) {
    const error: WithdrawRequestError = err instanceof Error
        ? err
        : new Error('Failed to fetch withdraw requests');
    setError(error);
    console.error('Error fetching withdraw requests:', error);
}
src/Components/Pagination.tsx (1)

5-12: Props interface looks good, but could benefit from JSDoc documentation.

The Props interface is well-structured with appropriate types. Consider adding JSDoc comments to document the purpose of each prop for better maintainability.

+/**
+ * Props for the Pagination component
+ * @template T - The type of data being paginated
+ */
 type Props<T> = {
+    /** The table instance */
     table: Table<T>;
+    /** Optional class name for the container */
     className?: string;
+    /** Loading state of the pagination */
     isLoading?: boolean;
+    /** Custom classes for pagination buttons */
     btnClasses?: string;
+    /** Custom classes for text elements */
     textClasses?: string;
+    /** Custom classes for the page input */
     inputClasses?: string;
 };
src/Definitions/Product.ts (1)

82-82: Use string literals for enumerable values.

Several properties that have a fixed set of possible values should use string literals for better type safety.

 interface Product {
     // ...
-    type: string;
-    status: string;
-    catalog_visibility: string;
-    tax_status: string;
-    tax_class: string;
+    type: 'simple' | 'variable' | 'grouped' | 'external' | 'subscription';
+    status: 'draft' | 'pending' | 'private' | 'publish';
+    catalog_visibility: 'visible' | 'catalog' | 'search' | 'hidden';
+    tax_status: 'taxable' | 'shipping' | 'none';
+    tax_class: 'standard' | 'reduced-rate' | 'zero-rate';
     // ...
 }

Also applies to: 83-83, 85-85, 86-86, 107-107, 108-108

base-tailwind.config.js (1)

18-19: Consider adding opacity variants for background colors.

The background colors could benefit from opacity variants for more flexible styling options.

 backgroundColor: {
     dokan: {
         sidebar: {
-            DEFAULT: 'var(--dokan-sidebar-background-color, #F05025)',
-            hover: 'var(--dokan-sidebar-hover-background-color, #F05025)',
+            DEFAULT: 'rgb(var(--dokan-sidebar-background-color-rgb, 240 80 37) / <alpha-value>)',
+            hover: 'rgb(var(--dokan-sidebar-hover-background-color-rgb, 240 80 37) / <alpha-value>)',
         },
         btn: {
-            DEFAULT: 'var(--dokan-button-background-color, #F05025)',
-            hover: 'var(--dokan-button-hover-background-color, #F05025)',
+            DEFAULT: 'rgb(var(--dokan-button-background-color-rgb, 240 80 37) / <alpha-value>)',
+            hover: 'rgb(var(--dokan-button-hover-background-color-rgb, 240 80 37) / <alpha-value>)',
         },
     },
 },

Also applies to: 23-24

src/Dashboard/Withdraw/TableSkeleton.tsx (1)

34-61: Make the number of skeleton rows configurable.

The number of skeleton rows is hardcoded to 10. Consider making this configurable through props for better reusability.

+interface TableSkeletonProps {
+    rowCount?: number;
+}

-const TableSkeleton = () => {
+const TableSkeleton = ({ rowCount = 10 }: TableSkeletonProps) => {
     // ...
-    { [ ...Array( 10 ) ].map( ( _, index ) => (
+    { [ ...Array( rowCount ) ].map( ( _, index ) => (
includes/REST/WithdrawControllerV2.php (2)

76-107: Consider caching the mapped arrays for better performance.

The method performs array mapping operations twice. Consider caching the results to avoid redundant operations.

 public function get_withdraw_settings() {
     $active_methods          = dokan_withdraw_get_withdrawable_active_methods();
     $payment_methods         = array_intersect( dokan_get_seller_active_withdraw_methods(), dokan_withdraw_get_active_methods() );
     $default_withdraw_method = dokan_withdraw_get_default_method( dokan_get_current_user_id() );
     $setup_url              = dokan_get_navigation_url( 'settings/payment' );
 
+    $mapped_payment_methods = array_map(
+        function ( $payment_method ) {
+            return [
+                'label' => dokan_withdraw_get_method_title( $payment_method ),
+                'value' => $payment_method,
+            ];
+        }, $payment_methods
+    );
+
+    $mapped_active_methods = array_map(
+        function ( $active_method ) {
+            return [
+                'label' => dokan_withdraw_get_method_title( $active_method ),
+                'value' => $active_method,
+                'icon'  => dokan_withdraw_get_method_icon( $active_method ),
+                'info'  => dokan_withdraw_get_method_additional_info( $active_method ),
+                'has_information'  => in_array( $active_method, dokan_get_seller_active_withdraw_methods(), true ),
+            ];
+        }, $active_methods
+    );
 
     return rest_ensure_response(
         [
             'withdraw_method' => $default_withdraw_method,
-            'payment_methods' => $payment_methods,
-            'active_methods'  => $active_methods,
+            'payment_methods' => $mapped_payment_methods,
+            'active_methods'  => $mapped_active_methods,
             'setup_url'       => $setup_url,
         ]
     );

124-148: Add success status code constant.

The success status code should use the WP_REST_Response status code constant for consistency.

-        return new WP_REST_Response( __( 'Default method update successful.', 'dokan-lite' ), 200 );
+        return new WP_REST_Response( 
+            __( 'Default method update successful.', 'dokan-lite' ),
+            WP_REST_Response::HTTP_OK
+        );
src/Dashboard/Withdraw/WithdrawRequests.tsx (2)

53-53: Consider adding error boundary.

The component handles loading states but lacks error boundaries for error handling.

+import { ErrorBoundary } from '@getdokan/dokan-ui';
+
 function WithdrawRequests() {
     // ... existing code ...
     return (
+        <ErrorBoundary>
             <SlotFillProvider>
                 {/* existing code */}
             </SlotFillProvider>
+        </ErrorBoundary>
     );
 }

41-55: Add debounce to prevent excessive API calls.

The effect hook might trigger multiple API calls when location.search or currentUser.data changes rapidly.

+import { useDebounce } from '@getdokan/dokan-ui';
+
 useEffect(() => {
+    const debouncedFetch = useDebounce(() => {
         useWithdrawRequestHook.fetchWithdrawRequests({
             per_page: useWithdrawRequestHook?.view?.perPage,
             page,
             status,
             user_id: 1,
         });
+    }, 300);
+
+    debouncedFetch();
+
+    return () => {
+        debouncedFetch.cancel();
+    };
 }, [location.search, currentUser?.data]);
src/Dashboard/Withdraw/PaymentDetails.tsx (1)

125-144: Add empty state handling.

The component should display a message when there are no pending requests.

     { !withdrawRequests?.isLoading &&
         withdrawRequests?.data &&
         Array.isArray(withdrawRequests?.data) &&
-        withdrawRequests?.data.length > 0 && (
+        (withdrawRequests?.data.length > 0 ? (
             <div className="flex flex-col border-t pt-4">
                 <h4 className="font-medium text-gray-900 mb-2">
                     {__('Pending Requests', 'dokan')}
                 </h4>
                 <RequestList
                     withdrawRequests={withdrawRequests}
                     status="pending"
                     loading={masterLoading || withdrawRequests.isLoading || settings.isLoading}
                 />
             </div>
+        ) : (
+            <p className="text-gray-600">
+                {__('No pending withdrawal requests.', 'dokan')}
+            </p>
+        ))
     }
src/Dashboard/Withdraw/PaymentMethods.tsx (1)

56-108: Consider simplifying conditional logic and using router navigation.

The actionButton function could be improved in two ways:

  1. The nested conditionals could be flattened using early returns for better readability
  2. Direct window.location.href usage could be replaced with React Router for better SPA navigation

Consider refactoring to:

 const actionButton = (activemethod: WithdrawMethod) => {
-  if (activemethod.has_information && activemethod?.value === bodyData?.data?.withdraw_method) {
+  if (!activemethod.has_information) {
+    return (
+      <Button
+        type="button"
+        color="secondary"
+        className="bg-dokan-btn hover:bg-dokan-btn-hover text-white"
+        onClick={() => navigate(bodyData?.data?.setup_url)}
+        label={__('Setup', 'dokan-lite')}
+      />
+    );
+  }
+
+  if (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 (
     // ... rest of the code
-  }
-  return (
-    // ... setup button code
-  );
 };
includes/REST/Manager.php (1)

204-207: Clean addition of new REST controllers!

The new controllers are properly mapped following the existing pattern. However, consider adding version numbers to the controller classes for better API versioning control, similar to how OrderControllerV3 is named.

Consider renaming the new controllers to include version numbers:

-DOKAN_DIR . '/includes/REST/CustomersController.php'              => '\WeDevs\Dokan\REST\CustomersController',
+DOKAN_DIR . '/includes/REST/CustomersControllerV1.php'           => '\WeDevs\Dokan\REST\CustomersControllerV1',
-DOKAN_DIR . '/includes/REST/DokanDataCountriesController.php'    => '\WeDevs\Dokan\REST\DokanDataCountriesController',
+DOKAN_DIR . '/includes/REST/DokanDataCountriesControllerV1.php' => '\WeDevs\Dokan\REST\DokanDataCountriesControllerV1',
src/Dashboard/Withdraw/RequestList.tsx (3)

33-38: Consider memoizing the hasWithdrawRequests calculation.

The hasWithdrawRequests check could be memoized using useMemo to avoid unnecessary recalculations on every render.

+import { useMemo } from '@wordpress/element';

-const hasWithdrawRequests =
-    withdrawRequests?.data &&
-    Array.isArray( withdrawRequests?.data ) &&
-    withdrawRequests?.data?.length > 0;
+const hasWithdrawRequests = useMemo(() => 
+    withdrawRequests?.data &&
+    Array.isArray( withdrawRequests?.data ) &&
+    withdrawRequests?.data?.length > 0,
+    [withdrawRequests?.data]
+);

185-206: Enhance error handling in cancellation flow.

While the basic error handling is in place, consider adding more specific error messages and recovery options.

 .catch(() => {
     toast({
         type: 'error',
-        title: __('Failed to cancel request', 'dokan'),
+        title: __('Failed to cancel request. Please try again later.', 'dokan'),
+        description: __('If the problem persists, please contact support.', 'dokan'),
+        duration: 5000,
     });
 })

248-286: Consider extracting the confirmation modal into a separate component.

The modal logic could be extracted into a reusable ConfirmationModal component to improve code organization and reusability.

Consider creating a new component:

interface ConfirmationModalProps {
    isOpen: boolean;
    onClose: () => void;
    onConfirm: () => void;
    isLoading: boolean;
    title: string;
    message: string;
}

const ConfirmationModal: React.FC<ConfirmationModalProps> = ({
    isOpen,
    onClose,
    onConfirm,
    isLoading,
    title,
    message,
}) => {
    // Modal implementation
};
src/Dashboard/Withdraw/RequestWithdrawBtn.tsx (2)

38-74: Consider using a dedicated number formatting library.

The current implementation relies on the global window.accounting object. Consider using a dedicated number formatting library like Intl.NumberFormat or currency.js for better maintainability and type safety.

const formatNumber = (value: string | number): string => {
    if (value === '') {
        return value as string;
    }
    return new Intl.NumberFormat(locale, {
        minimumFractionDigits: window.dokanCurrency.precision,
        maximumFractionDigits: window.dokanCurrency.precision,
    }).format(Number(value));
};

112-139: Improve error handling in withdrawal creation.

The error handling could be more specific and provide better feedback to users.

 .catch((err) => {
+    const errorMessage = err?.response?.data?.message 
+        ?? err?.message 
+        ?? __('Failed to create withdraw request. Please try again.', 'dokan');
     toast({
-        title: err?.message ?? __('Failed to create withdraw.', 'dokan'),
+        title: errorMessage,
         type: 'error',
+        duration: 5000,
     });
-    console.error('Error creating withdraw:', err);
+    console.error('[Withdraw Request Error]:', { error: err, payload });
 });
src/Definitions/Customers.ts (1)

1-13: Consider adding validation patterns for contact information.

The CustomerAddress interface could benefit from more specific types or validation patterns for:

  • Phone numbers (format validation)
  • Postcodes (format validation)
  • Email addresses (format validation)
+type PhoneNumber = string; // Consider regex validation
+type PostCode = string;    // Consider regex validation
+type Email = string;       // Consider regex validation

 export interface CustomerAddress {
     first_name: string;
     last_name: string;
     company: string;
     address_1: string;
     address_2: string;
     city: string;
-    postcode: string;
+    postcode: PostCode;
     country: string;
     state: string;
     email?: Email;
-    phone: string;
+    phone: PhoneNumber;
 }
src/Dashboard/Withdraw/tailwind.scss (2)

1-1: Fix typo in mixin name.

The mixin name windraw-reset appears to be a typo. Should it be withdraw-reset?

-@mixin windraw-reset {
+@mixin withdraw-reset {

15-17: Consider using CSS custom properties for fallback colors.

The hardcoded fallback color #F05025 should be defined as a CSS custom property for better maintainability.

-        background-color: var(--dokan-button-hover-color, #F05025);
-        border-color: var(--dokan-button-hover-background-color, #F05025);
+        background-color: var(--dokan-button-hover-color, var(--dokan-brand-color));
+        border-color: var(--dokan-button-hover-background-color, var(--dokan-brand-color));
docs/frontend/components.md (3)

15-16: Replace placeholder with actual path example.

The script assets path is using a placeholder. Consider providing a realistic example path to help developers understand the expected format.

-$script_assets = 'add_your_script_assets_path_here';
+$script_assets = 'build/vendor/assets.php';

58-62: Add webpack configuration example for external plugins.

The webpack configuration example could be enhanced with more details about external dependencies.

 externals: {
     '@dokan/components': 'dokan.components',
+    // Add other Dokan dependencies
+    '@wordpress/components': 'wp.components',
+    '@wordpress/element': 'wp.element',
     ...
 },

68-82: Add language specification to the file structure code block.

The file structure code block should specify a language for better syntax highlighting.

-```
+```text
 |____ src/
         |___ components/
🧰 Tools
🪛 Markdownlint (0.37.0)

68-68: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

docs/frontend/utilities.md (2)

15-25: Add language specification to the file structure code block.

The file structure code block should specify a language for better syntax highlighting.

-```
+```text
 |____ src/
         |___ utilities/
🧰 Tools
🪛 Markdownlint (0.37.0)

15-15: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


Line range hint 628-639: Add error handling to the filter application.

The filter application code should include error handling for undefined hooks or namespaces.

 const applyFiltersToTableElements = (namespace: string, elementName: string, element) => {
+    if (!namespace || !elementName) {
+        console.warn('Invalid namespace or element name provided to filter');
+        return element;
+    }
     return wp.hooks.applyFilters( `${namespace}.${elementName}`, element );
 };
🧰 Tools
🪛 LanguageTool

[style] ~42-~42: Consider using “incompatible” to avoid wordiness.
Context: ...est versions of change-case (v5+) are not compatible with WordPress. In Dokan utilities,...

(NOT_ABLE_PREMIUM)

🪛 Markdownlint (0.37.0)

15-15: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

docs/feature-override/readme.md (2)

104-108: Fix code indentation in template dependencies array.

The code uses tabs instead of spaces for indentation. Maintain consistent indentation throughout the codebase.

         [
-				'slug' => 'products/products',
-				'name' => 'listing',
-			],
+            'slug' => 'products/products',
+            'name' => 'listing',
+        ],
🧰 Tools
🪛 Markdownlint (0.37.0)

105-105: Column: 1
Hard tabs

(MD010, no-hard-tabs)


106-106: Column: 1
Hard tabs

(MD010, no-hard-tabs)


107-107: Column: 1
Hard tabs

(MD010, no-hard-tabs)


156-162: Add type hints and return type to filter callback.

The filter callback should include parameter and return type hints for better type safety.

-add_filter( 'dokan_is_dashboard_nav_dependency_cleared', function ( $is_cleared, $route ) {
+add_filter( 'dokan_is_dashboard_nav_dependency_cleared', function ( bool $is_cleared, string $route ): bool {
     if ( 'products' === $route ) {
         return true;
     }
docs/frontend/dataviews.md (2)

28-28: Fix grammar in component description.

The sentence has a subject-verb agreement issue.

-Also, an example component are designed to interact with WordPress REST APIs.
+Also, an example component is designed to interact with WordPress REST APIs.
🧰 Tools
🪛 LanguageTool

[grammar] ~28-~28: “component” is a singular noun. It appears that the verb form is incorrect.
Context: ...r dashboard. Also, an example component are designed to interact with WordPress RES...

(PCT_SINGULAR_NOUN_PLURAL_VERB_AGREEMENT)


208-225: Enhance error handling in delete action.

The delete action's error handling could be improved with more specific error messages and user feedback.

 onClick={ async () => {
     try {
         const response = await fetch(`/wp-json/wp/v2/posts/${item.id}`, {
             method: 'DELETE',
             headers: {
                 'X-WP-Nonce': wpApiSettings.nonce,
                 'Content-Type': 'application/json'
             }
         });

-        if (!response.ok) throw new Error('Failed to delete post');
+        if (!response.ok) {
+            const error = await response.json();
+            throw new Error(error.message || 'Failed to delete post');
+        }

         fetchPosts();
         closeModal();
     } catch (error) {
-        console.error('Error deleting post:', error);
+        // Handle error appropriately
+        console.error('Error deleting post:', error.message);
+        // Show error message to user
+        wp.data.dispatch('core/notices').createErrorNotice(
+            error.message || 'Failed to delete post. Please try again.'
+        );
     }
 } }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8217a61 and 9b5f66a.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (67)
  • base-tailwind.config.js (1 hunks)
  • docs/feature-override/readme.md (1 hunks)
  • docs/frontend/components.md (1 hunks)
  • docs/frontend/dataviews.md (1 hunks)
  • docs/frontend/utilities.md (1 hunks)
  • includes/Assets.php (6 hunks)
  • includes/DependencyManagement/Providers/CommonServiceProvider.php (2 hunks)
  • includes/REST/CustomersController.php (1 hunks)
  • includes/REST/DokanDataContinentsController.php (1 hunks)
  • includes/REST/DokanDataCountriesController.php (1 hunks)
  • includes/REST/Manager.php (1 hunks)
  • includes/REST/OrderControllerV3.php (1 hunks)
  • includes/REST/ProductController.php (1 hunks)
  • includes/REST/WithdrawController.php (4 hunks)
  • includes/REST/WithdrawControllerV2.php (4 hunks)
  • includes/VendorNavMenuChecker.php (1 hunks)
  • includes/Withdraw/Withdraws.php (1 hunks)
  • package.json (3 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)
  • src/base-tailwind.scss (1 hunks)
  • src/components/dataviews/DataViewTable.tsx (1 hunks)
  • src/components/dataviews/style.scss (1 hunks)
  • src/components/index.tsx (1 hunks)
  • src/dashboard/index.tsx (1 hunks)
  • src/dashboard/tailwind.scss (1 hunks)
  • src/hooks/ViewportDimensions.ts (1 hunks)
  • src/hooks/useCurrentUser.ts (1 hunks)
  • src/layout/404.tsx (1 hunks)
  • src/layout/ContentArea.tsx (1 hunks)
  • src/layout/Footer.tsx (1 hunks)
  • src/layout/Header.tsx (1 hunks)
  • src/layout/Sidebar.tsx (1 hunks)
  • src/layout/index.tsx (1 hunks)
  • src/utilities/ChangeCase.ts (1 hunks)
  • src/utilities/index.ts (1 hunks)
  • tests/php/src/REST/CustomersControllerTest.php (1 hunks)
  • tests/php/src/VendorNavMenuCheckerTest.php (1 hunks)
  • webpack-entries.js (1 hunks)
  • webpack.config.js (1 hunks)
✅ Files skipped from review due to trivial changes (6)
  • src/components/dataviews/style.scss
  • src/layout/Footer.tsx
  • webpack-entries.js
  • src/layout/Sidebar.tsx
  • src/utilities/index.ts
  • src/utilities/ChangeCase.ts
👮 Files not reviewed due to content moderation or server errors (4)
  • tests/php/src/VendorNavMenuCheckerTest.php
  • tests/php/src/REST/CustomersControllerTest.php
  • includes/REST/WithdrawController.php
  • includes/REST/ProductController.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/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/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/Components/DataTable.tsx

[error] 36-36: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

src/hooks/useCurrentUser.ts

[error] 27-27: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 38-38: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)

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)

webpack.config.js

[error] 2-2: Illegal use of reserved keyword package as an identifier in strict mode

(parse)

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)

🪛 LanguageTool
docs/frontend/components.md

[uncategorized] ~55-~55: You might be missing the article “a” here.
Context: ...include the dokan-react-components as scripts dependency and the @dokan/components ...

(AI_EN_LECTOR_MISSING_DETERMINER_A)

docs/frontend/dataviews.md

[grammar] ~28-~28: “component” is a singular noun. It appears that the verb form is incorrect.
Context: ...r dashboard. Also, an example component are designed to interact with WordPress RES...

(PCT_SINGULAR_NOUN_PLURAL_VERB_AGREEMENT)


[uncategorized] ~534-~534: The preposition ‘to’ seems more likely in this position.
Context: ... need to control paginationInfo props for calculate `total pagination item & page...

(AI_HYDRA_LEO_REPLACE_FOR_TO)


[style] ~534-~534: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...otal pagination item & pages. Also, we need to set perPage&pagein theview` st...

(REP_NEED_TO_VB)


[uncategorized] ~534-~534: The preposition ‘to’ seems more likely in this position.
Context: ... perPage & page in the view state for manage the pagination. #### ~ Set pagi...

(AI_HYDRA_LEO_REPLACE_FOR_TO)


[style] ~569-~569: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...lected items in the DataViewTable. We need to configure action definitions and handle...

(REP_NEED_TO_VB)


[style] ~571-~571: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...nitions and handle bulk operations. We need to enable supportsBulk props in table `a...

(REP_NEED_TO_VB)


[uncategorized] ~571-~571: The preposition ‘to’ seems more likely in this position.
Context: ...supportsBulk props in table actions for handle the bulk actions. Also, we need ...

(AI_HYDRA_LEO_REPLACE_FOR_TO)


[style] ~571-~571: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...for handle the bulk actions. Also, we need to setselection&onChangeSelection` i...

(REP_NEED_TO_VB)


[uncategorized] ~571-~571: The preposition ‘to’ seems more likely in this position.
Context: ...onChangeSelection in the view state for manage the bulk selection. We can set `...

(AI_HYDRA_LEO_REPLACE_FOR_TO)


[misspelling] ~621-~621: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...to DataViews: a function that returns an unique identifier for the record. ### ...

(EN_A_VS_AN)


[uncategorized] ~626-~626: Possible missing article found.
Context: ...t to modify a table from the outside of module or feature, we can use it. const a...

(AI_HYDRA_LEO_MISSING_A)


[typographical] ~650-~650: Consider adding a comma.
Context: ... want to use the default responsive view then don't need to pass the responsive pro...

(IF_THEN_COMMA)


[uncategorized] ~669-~669: The preposition ‘to’ seems more likely in this position.
Context: ... There have another property filterBy for handle the filter condition. **filterB...

(AI_HYDRA_LEO_REPLACE_FOR_TO)


[grammar] ~675-~675: It seems that a pronoun is missing.
Context: ...arameter and should return a ReactNode. If need to handle the field value before r...

(IF_VB)


[uncategorized] ~683-~683: Possible missing article found.
Context: ... in the UI, typically in the tooltip of action button. icon (string) : The icon...

(AI_HYDRA_LEO_MISSING_AN)


[style] ~705-~705: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...r) :** Current active page number. Used in conjunction with perPage for pagination. **search...

(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)


[style] ~727-~727: To form a complete sentence, be sure to include a subject.
Context: ... (array) :** List of visible field IDs. Can be used to control which columns are di...

(MISSING_IT_THERE)

docs/feature-override/readme.md

[style] ~6-~6: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...#declare-react-menu-in-dokan-lite) - [Declare React menu in Dokan Pro or **Exte...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)


[style] ~10-~10: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...in-dokan-pro-or-external-plugin) - [Define the override templates array structure....

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)


[uncategorized] ~18-~18: You might be missing the article “the” here.
Context: ...vailable in React, you need to define route property during the menu registra...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)


[grammar] ~63-~63: “React” is a proper noun and needs to be capitalized.
Context: ...h menu which we are indicating that the react route is available. This will be used ...

(A_GOOGLE)


[grammar] ~64-~64: “React” is a proper noun and needs to be capitalized.
Context: ...etermine if the menu is pointing to the react Application or to the Legacy PHP Route....

(A_GOOGLE)


[uncategorized] ~92-~92: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ritten in PHP to the React application and you want that if some of the PHP templa...

(COMMA_COMPOUND_SENTENCE)


[style] ~112-~112: In contexts where ‘if’ is followed by ‘or’, using ‘whether’ may be more appropriate (and formal).
Context: ...e route. This will be used to determine if the override templates are available fo...

(IF_WHETHER)


[uncategorized] ~149-~149: You might be missing the article “the” here.
Context: ...hich are passed to the template file in dokan_get_template_part() function. (Op...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)


[uncategorized] ~152-~152: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...erride any of the template file directly but you have override functionality by usin...

(COMMA_COMPOUND_SENTENCE)

docs/frontend/utilities.md

[style] ~42-~42: Consider using “incompatible” to avoid wordiness.
Context: ...est versions of change-case (v5+) are not compatible with WordPress. In Dokan utilities,...

(NOT_ABLE_PREMIUM)

🪛 Markdownlint (0.37.0)
docs/frontend/components.md

68-68: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

docs/frontend/dataviews.md

6-6: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


7-7: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


8-8: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


9-9: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


10-10: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


12-12: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


13-13: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


14-14: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


15-15: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


16-16: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


17-17: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


18-18: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


19-19: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


20-20: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


21-21: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


22-22: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


23-23: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


24-24: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


69-69: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


95-95: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


374-374: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


387-387: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


395-395: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


418-418: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


431-431: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


443-443: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


461-461: Punctuation: ':'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


480-480: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


505-505: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


514-514: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


536-536: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


556-556: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


590-590: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


604-604: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


628-628: Expected: fenced; Actual: indented
Code block style

(MD046, code-block-style)


652-652: Expected: fenced; Actual: indented
Code block style

(MD046, code-block-style)


696-696: Expected: fenced; Actual: indented
Code block style

(MD046, code-block-style)


711-711: Expected: fenced; Actual: indented
Code block style

(MD046, code-block-style)


723-723: Expected: fenced; Actual: indented
Code block style

(MD046, code-block-style)


732-732: Expected: fenced; Actual: indented
Code block style

(MD046, code-block-style)


739-739: Expected: fenced; Actual: indented
Code block style

(MD046, code-block-style)


753-753: Expected: fenced; Actual: indented
Code block style

(MD046, code-block-style)


759-759: Expected: fenced; Actual: indented
Code block style

(MD046, code-block-style)


765-765: Expected: fenced; Actual: indented
Code block style

(MD046, code-block-style)


5-5: null
Link fragments should be valid

(MD051, link-fragments)


11-11: null
Link fragments should be valid

(MD051, link-fragments)


18-18: null
Link fragments should be valid

(MD051, link-fragments)


19-19: null
Link fragments should be valid

(MD051, link-fragments)


21-21: null
Link fragments should be valid

(MD051, link-fragments)


22-22: null
Link fragments should be valid

(MD051, link-fragments)


23-23: null
Link fragments should be valid

(MD051, link-fragments)

docs/feature-override/readme.md

8-8: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


9-9: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


10-10: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


105-105: Column: 1
Hard tabs

(MD010, no-hard-tabs)


106-106: Column: 1
Hard tabs

(MD010, no-hard-tabs)


107-107: Column: 1
Hard tabs

(MD010, no-hard-tabs)


1-1: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


17-17: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


20-20: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


72-72: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


90-90: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


93-93: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


114-114: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


130-130: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)

docs/frontend/utilities.md

15-15: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🔇 Additional comments (28)
includes/REST/DokanDataCountriesController.php (2)

17-24: Verify REST API Namespace and Route Base

The namespace is set to 'dokan/v1', and the route base is 'data/countries'. Ensure these values are correctly aligned with the existing API structure and do not conflict with other endpoints.


85-98: ⚠️ Potential issue

Potential Capability Check Issue

The capability 'dokandar' may not be a standard WordPress capability. Ensure that it's correctly defined and that the users have the intended capabilities.

Run the following script to check user capabilities:

This will help verify if 'dokandar' is a valid capability assigned to vendors.

includes/REST/DokanDataContinentsController.php (2)

17-24: Verify REST API Namespace and Route Base

Confirm that the namespace 'dokan/v1' and route base 'data/continents' are appropriate and do not conflict with existing endpoints.


85-98: ⚠️ Potential issue

Potential Capability Check Issue

Verify if 'dokandar' is a valid capability. It might need to be a role or specific capability string.

Run the following script to verify capabilities:

✅ Verification successful

'dokandar' is a Valid Custom Capability

The 'dokandar' capability is properly registered, managed, and used consistently throughout the Dokan plugin. It's added to appropriate roles during installation and follows WordPress capability best practices.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: List capabilities for the 'vendor' role.

wp role get vendor --fields=capabilities

Length of output: 135


Script:

#!/bin/bash
# Search for 'dokandar' usage in the codebase
echo "=== Searching for 'dokandar' usage ==="
rg "dokandar" -B 2 -A 2

echo -e "\n=== Searching for capability registration patterns ==="
rg "add_cap|register_capability|capabilities" -B 2 -A 2

echo -e "\n=== Finding dokan_admin_menu_capability definition ==="
ast-grep --pattern 'function dokan_admin_menu_capability'

Length of output: 109052


Script:

#!/bin/bash
# Search for dokan_admin_menu_capability definition and usage
rg "function dokan_admin_menu_capability" -A 5
rg "dokan_admin_menu_capability\(\)" -B 2 -A 2

Length of output: 1849

includes/VendorNavMenuChecker.php (4)

148-148: Review Template Path Filters

Ensure that the dokan_set_template_path filter and dokan()->template_path() are returning correct paths, and that this aligns with the intended template loading mechanism.


162-164: Ensure Filter Provides Necessary Parameters

Verify that the dokan_get_template_part filter passes all required parameters to function correctly and that any additional expected parameters are included.


139-164: Optimize Template Retrieval Logic

Review the logic in get_overridden_template to ensure it efficiently determines overridden templates and adheres to WordPress standards for template loading.


106-117: 🛠️ Refactor suggestion

Compatibility with PHP Versions

The use of arrow functions (fn) requires PHP 7.4 or higher. If supporting earlier versions, replace arrow functions with anonymous functions.

Apply this diff:

-            $resolved_dependencies = array_map(
-                fn( $dependency_array ): array => array_filter(
-                    array_map(
-                        fn( $dependency ) => $this->get_overridden_template(
-                            $dependency['slug'],
-                            $dependency['name'] ?? '',
-                            $dependency['args'] ?? []
-                        ),
-                        $dependency_array
-                    )
-                ),
-                $dependencies
-            );
+            $resolved_dependencies = array_map(
+                function( $dependency_array ): array {
+                    return array_filter(
+                        array_map(
+                            function( $dependency ) {
+                                return $this->get_overridden_template(
+                                    $dependency['slug'],
+                                    isset( $dependency['name'] ) ? $dependency['name'] : '',
+                                    isset( $dependency['args'] ) ? $dependency['args'] : []
+                                );
+                            },
+                            $dependency_array
+                        )
+                    );
+                },
+                $dependencies
+            );

Verify the minimum PHP version requirement for the plugin.

includes/REST/CustomersController.php (1)

14-363: Overall Code Quality and Structure

The CustomersController class is well-structured, extending WC_REST_Customers_Controller effectively. The use of the perform_vendor_action method to wrap parent methods ensures consistent permission handling. The code is clean, well-documented, and follows best practices.

includes/Assets.php (2)

366-377: Verify Stylesheet Dependencies and Enqueue Order

The stylesheets dokan-react-frontend and dokan-react-components have dependencies on each other and on wp-components. Ensure that these dependencies are correctly registered and that the stylesheets are enqueued in the proper order to prevent styling issues.


366-368: Ensure dokan-tailwind Style Is Enqueued

The dokan-tailwind stylesheet is registered but not enqueued. If it's required for the Tailwind CSS styles to take effect, ensure that it is enqueued in the appropriate place.

src/Components/PriceHtml.tsx (1)

44-53: Consider security implications of using RawHTML

Using RawHTML with formatted money values could potentially lead to XSS vulnerabilities if the accounting library output isn't properly sanitized.

src/Dashboard/Withdraw/Hooks/useMakeDefaultMethod.ts (2)

4-8: Well-structured TypeScript interface!

The interface clearly defines the hook's return type with proper TypeScript types for all properties.


20-24: Consider adding response type validation

The API call lacks type validation for the response. Consider adding a type guard or schema validation to ensure the response matches expected format.

src/hooks/ViewportDimensions.ts (1)

28-33: Good use of requestAnimationFrame!

The implementation correctly uses requestAnimationFrame for performance optimization when handling resize events.

src/Components/DateTimeHtml.tsx (1)

12-14: Add date string validation

Consider adding validation for the date/time string format to prevent potential formatting errors.

Also applies to: 33-35, 53-55

src/Dashboard/Withdraw/Hooks/useWithdrawSettings.ts (1)

41-44: Consider updating API version to v3

The hook is using /dokan/v2/withdraw/settings while new endpoints are being introduced in v3. Consider updating to maintain version consistency across the application.

src/Dashboard/Withdraw/Hooks/useBalance.ts (1)

50-53: Consider updating API version to v3

The hook is using /dokan/v1/withdraw/balance. Consider updating to v3 to maintain consistency with other endpoints.

src/layout/index.tsx (1)

25-36: LGTM! Well-structured type definition

The DokanRoute type is well-defined with clear optional properties and proper JSX element types.

includes/DependencyManagement/Providers/CommonServiceProvider.php (1)

69-71: LGTM! Service registration follows established pattern

The registration of VendorNavMenuChecker follows the same pattern as other services in the container.

webpack.config.js (1)

11-19: LGTM! Well-structured entry points configuration.

The entry points configuration is well-organized, properly spreading existing entry points while adding new ones for components and utilities.

src/components/dataviews/DataViewTable.tsx (1)

82-82: ⚠️ Potential issue

Fix incorrect slot name for after-table slot

The slot name for the after-table slot incorrectly uses "before" instead of "after".

-<Slot name={ `dokan-before-vendor-data-table-${ tableNameSpace }` } fillProps={{ ...filteredProps }} />
+<Slot name={ `dokan-after-vendor-data-table-${ tableNameSpace }` } fillProps={{ ...filteredProps }} />

Likely invalid or redundant comment.

includes/REST/WithdrawControllerV2.php (1)

49-65: LGTM! Well-structured route registration.

The route registration is well-implemented with proper argument validation and enum constraint for the method parameter.

src/Dashboard/Withdraw/PaymentMethods.tsx (2)

1-53: Well-structured component setup with proper TypeScript typing!

The component is well-organized with proper type definitions and necessary imports. The use of TypeScript for prop types enhances code reliability.


109-162: Clean and efficient render logic implementation!

The render logic demonstrates good practices:

  • Proper loading state management
  • Efficient use of conditional rendering
  • Well-structured layout using Card components
🧰 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)

src/base-tailwind.scss (1)

6-21: Well-structured table reset implementation.

The table reset styles are properly scoped within the .dokan-layout class using @layer base, following Tailwind CSS best practices. The comprehensive reset ensures consistent table styling across different browsers.

src/Definitions/Customers.ts (1)

20-36: Ensure consistency in email field requirements.

The email field is optional in CustomerAddress but required in Customer. Consider documenting this design decision or making it consistent across interfaces.

package.json (1)

27-27: Verify compatibility between WordPress packages.

The significant version jump in @wordpress/scripts (^30.7.0) and the addition of various WordPress packages might lead to compatibility issues. Please verify that all WordPress packages work together correctly.

Also applies to: 58-65

✅ Verification successful

WordPress packages compatibility verified successfully

The package dependencies analysis shows compatible version ranges across all WordPress and related packages. The peer dependencies are well-aligned, with no conflicting version requirements detected.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for peer dependency warnings in package-lock.json
jq -r '.packages | .[] | select(.peerDependencies != null) | .name + " requires: " + (.peerDependencies | to_entries | map(.key + "@" + .value) | join(", "))' package-lock.json

Length of output: 12641

export type WeightUnit = Country['weight_unit'];


export type Continents = Continent
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix incorrect type definition.

The Continents type should be an array of Continent objects, not a single Continent.

-export type Continents = Continent
+export type Continents = Continent[]
📝 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
export type Continents = Continent
export type Continents = Continent[]

Comment on lines 29 to 53
export function isContinent(value: any): value is Continent {
return (
typeof value === 'object' &&
value !== null &&
'code' in value &&
'name' in value &&
'countries' in value &&
'_links' in value &&
Array.isArray(value.countries)
);
}

// Type guard to check if a value is a Country
export function isCountry(value: any): value is Country {
return (
typeof value === 'object' &&
value !== null &&
'code' in value &&
'name' in value &&
'currency_code' in value &&
'currency_pos' in value &&
'states' in value &&
Array.isArray(value.states)
);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve type guard implementation.

The type guards could be more robust with additional validation and better typing.

-export function isContinent(value: any): value is Continent {
+export function isContinent(value: unknown): value is Continent {
     return (
         typeof value === 'object' &&
         value !== null &&
         'code' in value &&
+        typeof (value as Continent).code === 'string' &&
         'name' in value &&
+        typeof (value as Continent).name === 'string' &&
         'countries' in value &&
         '_links' in value &&
-        Array.isArray(value.countries)
+        Array.isArray((value as Continent).countries) &&
+        (value as Continent).countries.every(isCountry)
     );
 }

-export function isCountry(value: any): value is Country {
+export function isCountry(value: unknown): value is Country {
     return (
         typeof value === 'object' &&
         value !== null &&
         'code' in value &&
+        typeof (value as Country).code === 'string' &&
         'name' in value &&
         'currency_code' in value &&
         'currency_pos' in value &&
         'states' in value &&
-        Array.isArray(value.states)
+        Array.isArray((value as Country).states) &&
+        (value as Country).states.every(isState)
     );
 }

+export function isState(value: unknown): value is State {
+    return (
+        typeof value === 'object' &&
+        value !== null &&
+        'code' in value &&
+        typeof (value as State).code === 'string' &&
+        'name' in value &&
+        typeof (value as State).name === 'string'
+    );
+}

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines 116 to 122
public function add_subscriptions_read_permission_to_vendors( $permission, $context, $object_id, $obj ) {
if ( 'read' === $context ) {
return true;
}

return $permission;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Method Name Misalignment

The method add_subscriptions_read_permission_to_vendors is misleading, as it modifies permissions for countries data, not subscriptions. Rename the method for clarity.

Apply this diff:

-    public function add_subscriptions_read_permission_to_vendors( $permission, $context, $object_id, $obj ) {
+    public function modify_read_permission_for_vendors( $permission, $context, $object_id, $obj ) {

Also, update the filter hook:

-    add_filter( 'woocommerce_rest_check_permissions', [ $this, 'add_subscriptions_read_permission_to_vendors' ], 10, 4 );
+    add_filter( 'woocommerce_rest_check_permissions', [ $this, 'modify_read_permission_for_vendors' ], 10, 4 );

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines 116 to 122
public function add_subscriptions_read_permission_to_vendors( $permission, $context, $object_id, $obj ) {
if ( 'read' === $context ) {
return true;
}

return $permission;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Method Name Misalignment

The method add_subscriptions_read_permission_to_vendors suggests it's for subscriptions, but it's used for continents data. Rename for clarity.

Apply this diff:

-    public function add_subscriptions_read_permission_to_vendors( $permission, $context, $object_id, $obj ) {
+    public function modify_read_permission_for_vendors( $permission, $context, $object_id, $obj ) {

Update the filter hook accordingly:

-    add_filter( 'woocommerce_rest_check_permissions', [ $this, 'add_subscriptions_read_permission_to_vendors' ], 10, 4 );
+    add_filter( 'woocommerce_rest_check_permissions', [ $this, 'modify_read_permission_for_vendors' ], 10, 4 );

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines 171 to 173
if ( ! current_user_can( 'edit_shop_orders' ) ) {
return new WP_Error( 'dokan_rest_cannot_search', __( 'You do not have permission to search customers.', 'dokan-lite' ), [ 'status' => rest_authorization_required_code() ] );
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Inconsistent Permission Check in search_customers Method

The search_customers method uses current_user_can( 'edit_shop_orders' ) for permission checking, which is inconsistent with other methods that use the check_permission method. For consistency and to centralize permission logic, consider refactoring search_customers to use check_permission.

Apply this diff to align permission checks:

     public function search_customers( $request ) {
-        if ( ! current_user_can( 'edit_shop_orders' ) ) {
-            return new WP_Error( 'dokan_rest_cannot_search', __( 'You do not have permission to search customers.', 'dokan-lite' ), [ 'status' => rest_authorization_required_code() ] );
-        }
+        $permission_check = $this->check_permission( $request, 'search' );
+        if ( is_wp_error( $permission_check ) ) {
+            return $permission_check;
+        }

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines 44 to 51
if (
! bodyData ||
! bodyData.hasOwnProperty( 'isLoading' ) ||
bodyData.isLoading ||
masterLoading
) {
return <Loader />;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Replace hasOwnProperty with Object.hasOwn.

Using Object.prototype.hasOwnProperty directly on objects can be unsafe. Use Object.hasOwn instead.

-    if (
-        !bodyData ||
-        !bodyData.hasOwnProperty('isLoading') ||
-        bodyData.isLoading ||
-        masterLoading
-    ) {
+    if (
+        !bodyData ||
+        !Object.hasOwn(bodyData, 'isLoading') ||
+        bodyData.isLoading ||
+        masterLoading
+    ) {
📝 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
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] 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)

Comment on lines 196 to 229
<MaskedInput
label={ __( 'Withdraw amount', 'dokan' ) }
className="focus:border-none"
addOnLeft={ currencySymbol }
defaultValue={ withdrawAmount }
onChange={ ( e ) => {
debouncedWithdrawAmount(
e.target.value
);
} }
maskRule={ {
numeral: true,
numeralDecimalMark:
window?.dokanCurrency?.decimal ??
'.',
delimiter:
window?.dokanCurrency?.thousand ??
',',
numeralDecimalScale:
window?.dokanCurrency?.precision ??
2,
} }
input={ {
id: 'withdraw-amount',
name: 'withdraw-amount',
type: 'text',
placeholder: __(
'Enter amount',
'dokan'
),
required: true,
disabled: false,
} }
/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add input validation for withdrawal amount.

The withdrawal amount input should include validation against minimum and maximum limits.

 <MaskedInput
     label={__('Withdraw amount', 'dokan')}
     className="focus:border-none"
     addOnLeft={currencySymbol}
     defaultValue={withdrawAmount}
+    error={
+        withdrawAmount && (
+            Number(unformatNumber(withdrawAmount)) < settings?.data?.minimum_withdraw_amount
+                ? __('Amount is below minimum withdrawal limit', 'dokan')
+                : Number(unformatNumber(withdrawAmount)) > settings?.data?.maximum_withdraw_amount
+                    ? __('Amount exceeds maximum withdrawal limit', 'dokan')
+                    : undefined
+        )
+    }
     onChange={(e) => {
         debouncedWithdrawAmount(e.target.value);
     }}

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines 5 to 7
[role-dialog] {
z-index: 9999;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix HTML attribute selector syntax and consider z-index value.

  1. The attribute selector [role-dialog] is incorrect. HTML attributes with values should use quotes: [role="dialog"].
  2. The z-index value of 9999 is extremely high. Consider using a lower value or implementing a z-index management system.
-    [role-dialog] {
+    [role="dialog"] {
-        z-index: 9999;
+        z-index: 100;
    }
📝 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
[role-dialog] {
z-index: 9999;
}
[role="dialog"] {
z-index: 100;
}

Comment on lines 28 to 39
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;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Remove duplicate table reset styles.

These table reset styles are duplicated from src/base-tailwind.scss. Consider importing and reusing the existing styles instead of duplicating them.


### Output:

<a href="https://ibb.co.com/YfyLf6N"><img src="https://i.ibb.co.com/Dw8RwN1/Screenshot-2024-12-26-at-11-25-33-AM.png" alt="Screenshot-2024-12-26-at-11-25-33-AM" border="0"></a>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Fix incorrect ImageBB domain in URLs

The image links are broken due to incorrect domain usage. Remove .com from both URLs:

  • Change https://ibb.co.com/YfyLf6N to https://ibb.co/YfyLf6N
  • Change https://i.ibb.co.com/Dw8RwN1/... to https://i.ibb.co/Dw8RwN1/...
🔗 Analysis chain

Fix broken image link.

The image link appears to be broken or inaccessible. Consider using a more reliable image hosting service or including the image in the repository.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify if the image exists in the repository
fd -t f "Screenshot-2024-12-26-at-11-25-33-AM.png" docs/

Length of output: 56


Script:

#!/bin/bash
# Search for other image links in markdown files
rg -U "https://[^)]*ibb\.co[^)]*" docs/

Length of output: 1173


Script:

#!/bin/bash
# Double check for any other instances of the incorrect domain
rg -l "ibb\.co\.com" .

Length of output: 53

add dataview table id
@Aunshon Aunshon changed the title Update vendor dashboard structure convert withdraw [Re-Enginrring] withdraw page Jan 14, 2025
MdAsifHossainNadim and others added 2 commits January 14, 2025 13:54
…nce/introduce-dataviews-from-dokan-free

# Conflicts:
#	package-lock.json
#	package.json
#	webpack.config.js
…te/vendor-dashboard-structure-convert-withdraw

# Conflicts:
#	package-lock.json
#	package.json
#	webpack.config.js
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 27

🔭 Outside diff range comments (1)
package.json (1)

Line range hint 1-70: Critical: Package lock file is out of sync.

The pipeline failure indicates that package-lock.json is out of sync with package.json. This needs to be fixed before merging.

Please run:

npm install
git add package-lock.json
git commit -m "chore: sync package lock file"
🧰 Tools
🪛 GitHub Actions: E2E_API Tests

[error] Package lock file is out of sync with package.json. Multiple dependencies are missing or have version mismatches. Run 'npm install' to update the lock file.

♻️ Duplicate comments (1)
webpack.config.js (1)

2-2: ⚠️ Potential issue

Fix illegal use of reserved keyword.

The variable name package is a reserved keyword in strict mode JavaScript. This should be renamed to avoid potential issues.

Apply this diff:

-const package = require( './package.json' );
+const packageJson = require( './package.json' );
🧰 Tools
🪛 Biome (1.9.4)

[error] 2-2: Illegal use of reserved keyword package as an identifier in strict mode

(parse)

🧹 Nitpick comments (23)
src/Status/Elements/Paragraph.tsx (1)

4-6: Add JSDoc documentation and prop types interface.

The component lacks documentation explaining its purpose and usage. Additionally, extracting the prop types to a separate interface would improve maintainability.

+interface ParagraphProps {
+  /** The status element containing the paragraph content */
+  element: StatusElement;
+}
+
+/**
+ * Renders a paragraph element with consistent styling for status messages
+ * @param {ParagraphProps} props - The component props
+ * @returns {JSX.Element} A styled paragraph element
+ */
-const Paragraph = ( { element }: { element: StatusElement } ) => {
+const Paragraph = ({ element }: ParagraphProps ) => {
package.json (1)

25-26: Architectural shift to React and Tailwind CSS looks promising.

The addition of Tailwind CSS and React-related dependencies aligns well with modern frontend practices. The use of scoped Tailwind configuration suggests careful consideration of WordPress admin styles integration.

Consider documenting the new architecture patterns and providing migration guidelines for other contributors.

Also applies to: 39-39, 56-70

🧰 Tools
🪛 GitHub Actions: E2E_API Tests

[error] Package lock file is out of sync with package.json. Multiple dependencies are missing or have version mismatches. Run 'npm install' to update the lock file.

includes/VendorNavMenuChecker.php (5)

8-16: Add class-level documentation and update version placeholder.

The class lacks PHPDoc documentation explaining its purpose and responsibilities. Also, the @since tag uses a placeholder DOKAN_SINCE.

Add class-level documentation and replace the version placeholder:

+/**
+ * Class VendorNavMenuChecker
+ *
+ * Manages vendor navigation menu conversion to React format and handles template dependencies.
+ * Provides functionality to check and notify about template overrides that might affect new features.
+ *
+ * @package WeDevs\Dokan
+ */
 class VendorNavMenuChecker {
     /**
-     * @since DOKAN_SINCE
+     * @since 3.8.0
      *
      * @var array $template_dependencies List of template dependencies.

22-26: Add constructor documentation.

The constructor adds important hooks but lacks documentation explaining their purpose.

Add documentation explaining the hooks:

     /**
      * Constructor.
+     *
+     * Sets up the hooks:
+     * - Filters dashboard navigation to convert menu items to React format
+     * - Adds admin notices for overridden templates
+     * - Adds template status section to the status page
      */
     public function __construct() {

28-37: Update method documentation.

The method documentation needs the version placeholder updated and could benefit from more details about the filter.

Enhance the documentation:

     /**
      * Get template dependencies.
      *
-     * @since DOKAN_SINCE
+     * @since 3.8.0
+     *
+     * @see $template_dependencies for array structure
+     *
+     * @filter dokan_get_dashboard_nav_template_dependency Allows modification of template dependencies
      *
      * @return array
      */

49-62: Add type hints to callback function parameters.

The array_map callback function lacks parameter type hints which could lead to type-related issues.

Add type hints to the callback:

     public function convert_to_react_menu( array $menu_items ): array {
         return array_map(
-            function ( $item ) {
+            function ( array $item ): array {
                 if ( ! empty( $item['react_route'] ) && $this->is_dependency_resolved( $item['react_route'] ) ) {

241-242: Make table title translatable.

The table title "General Heading" should be translatable.

Make the string translatable:

         $table = Status::table( 'override_table' )
-                ->set_title( __( 'General Heading', 'dokan-lite' ) )
+                ->set_title( __( 'Overridden Template Details', 'dokan-lite' ) )
includes/Abstracts/StatusElement.php (2)

162-162: Correct the grammar in exception message.

The exception message should use the plural form "child elements" for grammatical correctness.

Apply this diff to correct the message:

- throw new Exception( esc_html__( 'This element does not support child element.', 'dokan-lite' ) );
+ throw new Exception( esc_html__( 'This element does not support child elements.', 'dokan-lite' ) );

210-210: Correct the grammar in exception message.

The exception message should use the plural form "child elements" for grammatical correctness.

Apply this diff to correct the message:

- throw new Exception( esc_html__( 'This element does not support child element.', 'dokan-lite' ) );
+ throw new Exception( esc_html__( 'This element does not support child elements.', 'dokan-lite' ) );
includes/Admin/Status/Heading.php (1)

19-21: Fix inconsistent documentation.

The comment states "No escaping needed for page data" but the implementation uses esc_html(). This inconsistency could mislead other developers. Either:

  1. Remove the comment if escaping is needed, or
  2. Update the comment to explain why escaping is being applied
src/Status/Elements/Link.tsx (1)

6-13: Enhance accessibility and type safety.

Consider these improvements:

  1. Add aria-label for better screen reader support
  2. Use consistent optional chaining (currently used for url/title_text but not title)
  3. Add type checking for required properties
 <a
     href={ element?.url }
     title={ element?.title_text }
+    aria-label={ element?.title_text }
     className="font-medium text-blue-600 dark:text-blue-500 hover:underline"
 >
-    { element.title }
+    { element?.title }
 </a>
includes/Admin/Status/Section.php (1)

7-14: Add comprehensive class documentation.

The class is missing PHPDoc documentation explaining its purpose, responsibilities and usage. Additionally, the $support_children property lacks documentation.

Add the following documentation:

+/**
+ * Class Section
+ *
+ * Represents a section element in the status page that can contain child elements.
+ * This class is responsible for managing and rendering section-level content.
+ */
 class Section extends StatusElement {
     /**
      * @var string
      */
     protected string $type = 'section';
+    /**
+     * @var bool Whether this element can contain child elements
+     */
     protected bool $support_children = true;
includes/Admin/Status/TableColumn.php (1)

1-23: Consider architectural improvements to reduce duplication.

All four classes (Section, TableRow, SubSection, TableColumn) share identical structure and behavior, differing only in their $type value. This suggests an opportunity for architectural improvement.

Consider these alternatives:

  1. Use a single class with type configuration:
class StatusElement {
    public function __construct(string $type) {
        $this->type = $type;
    }
}

// Usage:
$section = new StatusElement('section');
$tableRow = new StatusElement('table-row');
  1. Or use an enum for type safety:
enum StatusElementType {
    case SECTION;
    case TABLE_ROW;
    case SUB_SECTION;
    case TABLE_COLUMN;
}

class StatusElement {
    public function __construct(StatusElementType $type) {
        $this->type = $type->value;
    }
}

This would:

  • Reduce code duplication
  • Centralize documentation and security concerns
  • Make it easier to maintain and modify shared behavior
includes/Admin/Status/Table.php (1)

28-32: Consider validating headers array structure.

The set_headers method accepts any array without validating its structure. Consider adding validation to ensure the headers array meets expected format requirements.

     public function set_headers( array $headers ): Table {
+        foreach ( $headers as $header ) {
+            if ( ! is_array( $header ) || ! isset( $header['label'] ) ) {
+                throw new \InvalidArgumentException( 'Invalid header format' );
+            }
+        }
         $this->headers = $headers;
 
         return $this;
     }
src/Status/Elements/SubSection.tsx (2)

8-10: Add ARIA attributes for better accessibility.

The heading element should have appropriate ARIA attributes for better screen reader support.

-                <h3 className="text-lg font-medium leading-3 text-gray-900">
+                <h3 className="text-lg font-medium leading-3 text-gray-900" aria-label={element.title}>

22-24: Improve key generation for mapped elements.

Using concatenated strings with hook_key and id for keys could potentially lead to collisions. Consider using a more unique identifier.

-                            key={
-                                element.hook_key + '-' + child.id + '-parser'
-                            }
+                            key={`${element.hook_key}-subsection-${child.id}-${child.type}`}
src/Status/Menu.tsx (2)

3-5: Add TypeScript types to classNames function.

The classNames utility function should have proper type definitions.

Apply this diff:

-function classNames( ...classes ) {
+function classNames( ...classes: (string | boolean | undefined)[] ): string {
     return classes.filter( Boolean ).join( ' ' );
 }

7-48: Optimize component performance with React.memo.

The Menu component could benefit from memoization to prevent unnecessary re-renders when parent components update.

Apply this diff:

-const Menu = ( {
+const Menu = React.memo(( {
     pages,
     loading,
     activePage,
     onMenuClick,
 }: {
     pages: StatusElement[];
     loading: boolean;
     activePage: string;
     onMenuClick: ( page: string ) => void;
-} ): JSX.Element => {
+} ): JSX.Element => {
     return (
         // ... existing JSX
     );
-};
+});
includes/DependencyManagement/Providers/AdminServiceProvider.php (1)

15-17: Organize services array for better maintainability.

Consider grouping related services together in the services array and adding comments for each group.

Apply this diff:

 protected $services = [
     self::TAG,
+    // Status Management
     Status::class,
+    // Other service groups can be added here with comments
 ];
src/Status/Elements/Table.tsx (2)

11-28: Consider extracting header rendering logic into a separate component.

The header rendering logic can be extracted into a reusable component to improve code organization and maintainability.

+ const TableHeader = ({ headers, hookKey }: { headers: string[], hookKey: string }) => (
+   <thead className="bg-gray-50">
+     <tr>
+       {headers.map((header: string) => (
+         <th
+           key={`${hookKey}_table_head_${header}`}
+           scope="col"
+           className="px-3 py-3.5 text-left text-sm font-semibold text-gray-900"
+         >
+           {header}
+         </th>
+       ))}
+     </tr>
+   </thead>
+ );

  const Table = ({ element }: { element: StatusElement }) => {
    return (
      <div className="overflow-hidden shadow ring-1 ring-black/5 sm:rounded-lg">
        <table className="min-w-full divide-y divide-gray-300">
-         {element.headers.length > 0 && (
-           <thead className="bg-gray-50">
-             <tr>
-               {element.headers.map((header: string) => (
-                 <th
-                   key={element.hook_key + '_table_head_' + header}
-                   scope="col"
-                   className="px-3 py-3.5 text-left text-sm font-semibold text-gray-900"
-                 >
-                   {header}
-                 </th>
-               ))}
-             </tr>
-           </thead>
-         )}
+         {element.headers.length > 0 && (
+           <TableHeader headers={element.headers} hookKey={element.hook_key} />
+         )}

29-43: Add aria-label to the table for better accessibility.

The table should have an aria-label to provide context for screen readers.

  <table 
    className="min-w-full divide-y divide-gray-300"
+   aria-label={element.title || 'Status information table'}
  >
src/Status/Status.tsx (1)

91-116: Simplify complex useEffect logic.

The useEffect hook handling elements state is overly complex and can be simplified.

Consider extracting the element selection logic into a separate function for better maintainability:

const getElements = () => {
  if (!pages?.length && !tabs?.length) {
    return allSettings;
  }
  if (pages?.length && !tabs?.length && selectedPage) {
    return pages.find((child) => child.id === selectedPage)?.children || [];
  }
  if (tabs?.length && selectedTab) {
    return tabs.find((child) => child.id === selectedTab)?.children || [];
  }
  return [];
};

useEffect(() => {
  if (!loading) {
    setElements(getElements());
  }
}, [allSettings, pages, selectedPage, tabs, selectedTab, loading]);
includes/Admin/Status/Status.php (1)

141-156: Consider adding version fallback for asset file.

The script registration assumes the asset file exists but doesn't handle the case where it might be missing.

Apply this diff to add a version fallback:

 public function register_scripts() {
-    $asset_file = include DOKAN_DIR . '/assets/js/dokan-status.asset.php';
+    $asset_file_path = DOKAN_DIR . '/assets/js/dokan-status.asset.php';
+    $asset_file = file_exists( $asset_file_path )
+        ? include $asset_file_path
+        : [ 'dependencies' => [], 'version' => DOKAN_VERSION ];
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9b5f66a and 30232b8.

📒 Files selected for processing (35)
  • base-tailwind.config.js (1 hunks)
  • includes/Abstracts/StatusElement.php (1 hunks)
  • includes/Admin/Status/Button.php (1 hunks)
  • includes/Admin/Status/Heading.php (1 hunks)
  • includes/Admin/Status/Link.php (1 hunks)
  • includes/Admin/Status/Page.php (1 hunks)
  • includes/Admin/Status/Paragraph.php (1 hunks)
  • includes/Admin/Status/Section.php (1 hunks)
  • includes/Admin/Status/Status.php (1 hunks)
  • includes/Admin/Status/SubSection.php (1 hunks)
  • includes/Admin/Status/Tab.php (1 hunks)
  • includes/Admin/Status/Table.php (1 hunks)
  • includes/Admin/Status/TableColumn.php (1 hunks)
  • includes/Admin/Status/TableRow.php (1 hunks)
  • includes/DependencyManagement/Providers/AdminServiceProvider.php (3 hunks)
  • includes/REST/AdminDashboardController.php (3 hunks)
  • includes/VendorNavMenuChecker.php (1 hunks)
  • package.json (3 hunks)
  • src/Status/Elements/Button.tsx (1 hunks)
  • src/Status/Elements/Heading.tsx (1 hunks)
  • src/Status/Elements/Link.tsx (1 hunks)
  • src/Status/Elements/Paragraph.tsx (1 hunks)
  • src/Status/Elements/Section.tsx (1 hunks)
  • src/Status/Elements/SubSection.tsx (1 hunks)
  • src/Status/Elements/Table.tsx (1 hunks)
  • src/Status/Elements/TableColumn.tsx (1 hunks)
  • src/Status/Elements/TableRow.tsx (1 hunks)
  • src/Status/Menu.tsx (1 hunks)
  • src/Status/SettingsParser.tsx (1 hunks)
  • src/Status/Status.tsx (1 hunks)
  • src/Status/Tab.tsx (1 hunks)
  • src/Status/index.tsx (1 hunks)
  • src/Status/status-tailwind.config.js (1 hunks)
  • src/Status/status.scss (1 hunks)
  • webpack.config.js (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • src/Status/status.scss
🚧 Files skipped from review as they are similar to previous changes (1)
  • base-tailwind.config.js
🧰 Additional context used
🪛 Biome (1.9.4)
src/Status/SettingsParser.tsx

[error] 37-37: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.

(lint/complexity/noUselessFragments)

webpack.config.js

[error] 2-2: Illegal use of reserved keyword package as an identifier in strict mode

(parse)

src/Status/Status.tsx

[error] 9-9: Don't use 'Object' as a type.

Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.

(lint/complexity/noBannedTypes)

🪛 GitHub Actions: E2E_API Tests
package.json

[error] Package lock file is out of sync with package.json. Multiple dependencies are missing or have version mismatches. Run 'npm install' to update the lock file.

🔇 Additional comments (12)
package.json (1)

27-27: Verify compatibility with the major version upgrade of @wordpress/scripts.

The upgrade from ^27.9.0 to ^30.7.0 is a significant jump that might introduce breaking changes.

Run this script to check for breaking changes:

🧰 Tools
🪛 GitHub Actions: E2E_API Tests

[error] Package lock file is out of sync with package.json. Multiple dependencies are missing or have version mismatches. Run 'npm install' to update the lock file.

includes/VendorNavMenuChecker.php (1)

13-15: Verify template dependencies structure.

The template dependencies array structure is crucial for the functionality. Let's verify its usage across the codebase.

✅ Verification successful

Template dependencies structure is correctly documented and implemented.

The array structure follows the documented format:

  • Required: ['slug' => 'template-slug']
  • Optional: 'name' => 'template-name', 'args' => []
  • Properly initialized and consistently used across the codebase
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for usages of template dependencies filter
rg -A 5 "dokan_get_dashboard_nav_template_dependency" --type php

# Search for array structure initialization
ast-grep --pattern 'protected array $template_dependencies = $_'

Length of output: 3094


Script:

#!/bin/bash
# Search for template dependencies property initialization and usage
rg -B 2 -A 5 "\$template_dependencies\s*=\s*\[" --type php

# Search for array access patterns
rg "\\\$template_dependencies\[['\"]\w+['\"]" --type php

Length of output: 2140


Script:

#!/bin/bash
# Search for class constructor and initialization
rg -B 2 -A 10 "class VendorNavMenuChecker" --type php

# Search for complete array structure with all fields
rg -A 3 "(\['slug'|\['name'|\['args')" --type php

Length of output: 23317

includes/Abstracts/StatusElement.php (1)

8-8: Verify PHP version compatibility due to typed properties and return types.

The use of typed properties (e.g., protected bool $support_children = false;) and return types requires PHP 7.4 or higher. Ensure that the project's minimum PHP version requirement is set to 7.4 or higher to maintain compatibility.

Run the following script to check the minimum PHP version specified in your project:

✅ Verification successful

PHP version compatibility verified for typed properties

The codebase requires PHP 7.4 as specified in readme.txt, which fully supports typed properties. The code is compatible with the minimum PHP version requirement.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for minimum PHP version requirement in plugin files.

# Search for 'Requires PHP' tag in readme.txt
if [ -f readme.txt ]; then
  grep 'Requires PHP' readme.txt || echo "'Requires PHP' tag not found in readme.txt"
else
  echo "readme.txt not found."
fi

# Alternatively, check for PHP version in plugin headers
plugin_files=$(fd -e php)
for file in $plugin_files; do
  grep 'Requires PHP' "$file" && echo "Found in $file"
done

Length of output: 65594

src/Status/status-tailwind.config.js (1)

1-9: Tailwind CSS configuration looks good.

The configuration correctly extends the base configuration and specifies the content paths appropriately.

includes/Admin/Status/TableRow.php (1)

7-23: Same issues as Section.php.

This class has the same issues as identified in Section.php:

  1. Missing class-level and property documentation
  2. Potential security concern with unescaped data

Please apply the same fixes as suggested for Section.php.

includes/Admin/Status/SubSection.php (1)

7-23: Same issues as Section.php.

This class has the same issues as identified in Section.php:

  1. Missing class-level and property documentation
  2. Potential security concern with unescaped data

Please apply the same fixes as suggested for Section.php.

includes/Admin/Status/TableColumn.php (2)

7-23: Same issues as Section.php.

This class has the same issues as identified in Section.php:

  1. Missing class-level and property documentation
  2. Potential security concern with unescaped data

Please apply the same fixes as suggested for Section.php.


1-23: Verify data escaping implementation in parent class.

Let's verify if and how data escaping is handled in the parent StatusElement class.

✅ Verification successful

TableColumn's data escaping implementation is appropriate and secure

The current implementation is correct because:

  1. TableColumn is a structural container that delegates data output to child elements
  2. Child elements (Button, Link, Heading, Paragraph) implement appropriate escaping for their specific contexts
  3. The class is used in an admin-only context with proper capability checks
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for escaping implementation in StatusElement class
echo "Searching for StatusElement class definition and prepare_data method..."
rg -A 10 "class StatusElement" 

echo "Searching for escape_data method implementations..."
rg -A 5 "escape_data"

echo "Searching for other escaping functions..."
rg -A 3 "esc_[a-z]+"

Length of output: 102278


Script:

#!/bin/bash
# Search for TableColumn usage and data flow
echo "Searching for TableColumn usage and data handling..."
rg -A 5 "new TableColumn" 

echo "Searching for render() method implementations in Status classes..."
ast-grep --pattern 'class $_ extends StatusElement {
  $$$
  render() {
    $$$
  }
  $$$
}'

Length of output: 731


Script:

#!/bin/bash
# Search for complete Status class implementation and data flow
echo "Searching for Status class implementation..."
rg -l "class Status" | xargs cat

echo "Searching for table column rendering in templates..."
rg -A 10 "table.*column" --type php

echo "Searching for data assignment to TableColumn..."
rg -A 5 "->set_data\(" --type php

Length of output: 43246

includes/REST/AdminDashboardController.php (1)

68-77: LGTM! Route registration looks good.

The route registration follows WordPress REST API conventions and includes appropriate permission checks.

includes/Admin/Status/Status.php (1)

184-226: Uncomment and implement the describe method.

The describe method contains commented-out code that seems to be the intended implementation. This should either be implemented or removed if no longer needed.

Would you like me to help implement the describe method or should it be removed?

webpack.config.js (2)

11-21: LGTM! Entry points configuration looks good.

The entry points are well-organized and properly configured for TypeScript support.


26-30: LGTM! Output configuration is properly structured.

The output configuration correctly sets up the library name and type for window-scoped access.

Comment on lines 1 to 2
import { StatusElement } from '../Status';
import SettingsParser from '../SettingsParser';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Remove unused import SettingsParser.

The SettingsParser import is not utilized within this component and should be removed to maintain clean code.

import { StatusElement } from '../Status';
-import SettingsParser from '../SettingsParser';
📝 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
import { StatusElement } from '../Status';
import SettingsParser from '../SettingsParser';
import { StatusElement } from '../Status';

import SettingsParser from '../SettingsParser';

const Paragraph = ( { element }: { element: StatusElement } ) => {
return <p className="max-w-2xl text-sm text-gray-600">{ element.title }</p>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add null check and ARIA attributes for accessibility.

The component should handle potential null/undefined titles and include appropriate ARIA attributes for better accessibility.

-    return <p className="max-w-2xl text-sm text-gray-600">{ element.title }</p>;
+    return (
+        <p 
+            className="max-w-2xl text-sm text-gray-600"
+            role="status"
+            aria-label={element.title || 'Status message'}
+        >
+            {element.title || ''}
+        </p>
+    );
📝 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
return <p className="max-w-2xl text-sm text-gray-600">{ element.title }</p>;
return (
<p
className="max-w-2xl text-sm text-gray-600"
role="status"
aria-label={element.title || 'Status message'}
>
{element.title || ''}
</p>
);

Comment on lines 137 to 169
protected function get_overridden_template( string $slug, string $name = '', array $args = [] ) {
$defaults = [ 'pro' => false ];
$args = wp_parse_args( $args, $defaults );
$template = '';
$default_template = '';

// Look in yourtheme/dokan/slug-name.php and yourtheme/dokan/slug.php
$template_path = ! empty( $name ) ? "{$slug}-{$name}.php" : "{$slug}.php";
$template = locate_template( [ dokan()->template_path() . $template_path ] );

/**
* Change template directory path filter
*
* @since 2.5.3
*/
$template_path = apply_filters( 'dokan_set_template_path', dokan()->plugin_path() . '/templates', $template, $args );

// Get default slug-name.php
if ( ! $template && $name && file_exists( $template_path . "/{$slug}-{$name}.php" ) ) {
$template = $template_path . "/{$slug}-{$name}.php";
$default_template = $template;
}

if ( ! $template && ! $name && file_exists( $template_path . "/{$slug}.php" ) ) {
$template = $template_path . "/{$slug}.php";
$default_template = $template;
}

// Allow 3rd party plugin filter template file from their plugin
$template = apply_filters( 'dokan_get_template_part', $template, $slug, $name );

return $template && $default_template !== $template ? $template : false;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Sanitize template paths to prevent directory traversal.

The template path handling could be vulnerable to directory traversal if the slug or name parameters contain malicious values.

Add path sanitization:

     protected function get_overridden_template( string $slug, string $name = '', array $args = [] ) {
+        // Sanitize slug and name to prevent directory traversal
+        $slug = sanitize_file_name( $slug );
+        $name = sanitize_file_name( $name );
+
         $defaults         = [ 'pro' => false ];
         $args             = wp_parse_args( $args, $defaults );
📝 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
protected function get_overridden_template( string $slug, string $name = '', array $args = [] ) {
$defaults = [ 'pro' => false ];
$args = wp_parse_args( $args, $defaults );
$template = '';
$default_template = '';
// Look in yourtheme/dokan/slug-name.php and yourtheme/dokan/slug.php
$template_path = ! empty( $name ) ? "{$slug}-{$name}.php" : "{$slug}.php";
$template = locate_template( [ dokan()->template_path() . $template_path ] );
/**
* Change template directory path filter
*
* @since 2.5.3
*/
$template_path = apply_filters( 'dokan_set_template_path', dokan()->plugin_path() . '/templates', $template, $args );
// Get default slug-name.php
if ( ! $template && $name && file_exists( $template_path . "/{$slug}-{$name}.php" ) ) {
$template = $template_path . "/{$slug}-{$name}.php";
$default_template = $template;
}
if ( ! $template && ! $name && file_exists( $template_path . "/{$slug}.php" ) ) {
$template = $template_path . "/{$slug}.php";
$default_template = $template;
}
// Allow 3rd party plugin filter template file from their plugin
$template = apply_filters( 'dokan_get_template_part', $template, $slug, $name );
return $template && $default_template !== $template ? $template : false;
}
protected function get_overridden_template( string $slug, string $name = '', array $args = [] ) {
// Sanitize slug and name to prevent directory traversal
$slug = sanitize_file_name( $slug );
$name = sanitize_file_name( $name );
$defaults = [ 'pro' => false ];
$args = wp_parse_args( $args, $defaults );
$template = '';
$default_template = '';
// Look in yourtheme/dokan/slug-name.php and yourtheme/dokan/slug.php
$template_path = ! empty( $name ) ? "{$slug}-{$name}.php" : "{$slug}.php";
$template = locate_template( [ dokan()->template_path() . $template_path ] );
/**
* Change template directory path filter
*
* @since 2.5.3
*/
$template_path = apply_filters( 'dokan_set_template_path', dokan()->plugin_path() . '/templates', $template, $args );
// Get default slug-name.php
if ( ! $template && $name && file_exists( $template_path . "/{$slug}-{$name}.php" ) ) {
$template = $template_path . "/{$slug}-{$name}.php";
$default_template = $template;
}
if ( ! $template && ! $name && file_exists( $template_path . "/{$slug}.php" ) ) {
$template = $template_path . "/{$slug}.php";
$default_template = $template;
}
// Allow 3rd party plugin filter template file from their plugin
$template = apply_filters( 'dokan_get_template_part', $template, $slug, $name );
return $template && $default_template !== $template ? $template : false;
}

Comment on lines 221 to 224
if ( ! $this->is_support_children() ) {
// translators: %s is Status element type.
throw new Exception( esc_html( sprintf( esc_html__( 'Status %s Does not support adding any children.', 'dokan-lite' ), $this->get_type() ) ) );
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Correct the exception message in remove method to reflect the correct action.

The exception message in the remove method incorrectly mentions "adding any children" instead of "removing children," which can cause confusion.

Apply this diff to correct the message:

- throw new Exception( esc_html( sprintf( esc_html__( 'Status %s Does not support adding any children.', 'dokan-lite' ), $this->get_type() ) ) );
+ throw new Exception( esc_html( sprintf( esc_html__( 'Status %s does not support removing children.', 'dokan-lite' ), $this->get_type() ) ) );
📝 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
if ( ! $this->is_support_children() ) {
// translators: %s is Status element type.
throw new Exception( esc_html( sprintf( esc_html__( 'Status %s Does not support adding any children.', 'dokan-lite' ), $this->get_type() ) ) );
}
if ( ! $this->is_support_children() ) {
// translators: %s is Status element type.
throw new Exception( esc_html( sprintf( esc_html__( 'Status %s does not support removing children.', 'dokan-lite' ), $this->get_type() ) ) );
}

Comment on lines 6 to 13
const statusDomNode = document.getElementById( 'dokan-status' );
const statusRoot = createRoot( statusDomNode! );

domReady( () => {
if ( statusDomNode ) {
statusRoot.render( <Status /> );
}
} );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Ensure DOM elements are accessed after DOM is ready to avoid null references.

Accessing statusDomNode before the DOM is fully loaded may lead to null references. To prevent potential runtime errors, move the DOM element query inside the domReady callback.

Apply this diff to adjust the code:

-import { createRoot } from '@wordpress/element';
-import domReady from '@wordpress/dom-ready';
-import Status from './Status';
-import './status.scss';

-const statusDomNode = document.getElementById( 'dokan-status' );
-const statusRoot = createRoot( statusDomNode! );

 domReady( () => {
+    const statusDomNode = document.getElementById( 'dokan-status' );
+    if ( statusDomNode ) {
+        const statusRoot = createRoot( statusDomNode );
         statusRoot.render( <Status /> );
     }
 } );

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines 28 to 39
<select
id="tabs"
name="tabs"
className="block w-full focus:ring-orange-500 focus:border-orange-500 border-gray-300 rounded-md"
defaultValue={ selectedTab }
>
{ tabs.map( ( tab ) => {
return (
<option key={ tab.id }>{ tab.title }</option>
);
} ) }
</select>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add onChange handler for mobile select.

The mobile select element is missing an onChange handler, making it non-functional.

  <select
    id="tabs"
    name="tabs"
    className="block w-full focus:ring-orange-500 focus:border-orange-500 border-gray-300 rounded-md"
    defaultValue={selectedTab}
+   onChange={(e) => onTabClick(e.target.value)}
  >
📝 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
<select
id="tabs"
name="tabs"
className="block w-full focus:ring-orange-500 focus:border-orange-500 border-gray-300 rounded-md"
defaultValue={ selectedTab }
>
{ tabs.map( ( tab ) => {
return (
<option key={ tab.id }>{ tab.title }</option>
);
} ) }
</select>
<select
id="tabs"
name="tabs"
className="block w-full focus:ring-orange-500 focus:border-orange-500 border-gray-300 rounded-md"
defaultValue={ selectedTab }
onChange={(e) => onTabClick(e.target.value)}
>
{ tabs.map( ( tab ) => {
return (
<option key={ tab.id }>{ tab.title }</option>
);
} ) }
</select>

Comment on lines 47 to 71
{ tabs.map( ( tab ) => {
return (
<a
key={ tab.id }
href={ tab.id }
onClick={ ( e ) => {
e.preventDefault();
onTabClick( tab.id );
} }
className={ classNames(
tab.id === selectedTab
? 'border-orange-500 text-orange-600'
: 'border-transparent text-gray-500 hover:text-gray-700 hover:border-gray-300',
'group inline-flex items-center py-4 px-1 border-b-2 font-medium text-sm'
) }
aria-current={
tab.id === selectedTab
? 'page'
: undefined
}
>
<span>{ tab.title }</span>
</a>
);
} ) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance keyboard navigation for tabs.

The tab navigation should support keyboard interactions for better accessibility.

  <a
    key={tab.id}
    href={tab.id}
+   role="tab"
+   tabIndex={0}
+   onKeyPress={(e) => {
+     if (e.key === 'Enter' || e.key === ' ') {
+       e.preventDefault();
+       onTabClick(tab.id);
+     }
+   }}
    onClick={(e) => {
      e.preventDefault();
      onTabClick(tab.id);
    }}
📝 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
{ tabs.map( ( tab ) => {
return (
<a
key={ tab.id }
href={ tab.id }
onClick={ ( e ) => {
e.preventDefault();
onTabClick( tab.id );
} }
className={ classNames(
tab.id === selectedTab
? 'border-orange-500 text-orange-600'
: 'border-transparent text-gray-500 hover:text-gray-700 hover:border-gray-300',
'group inline-flex items-center py-4 px-1 border-b-2 font-medium text-sm'
) }
aria-current={
tab.id === selectedTab
? 'page'
: undefined
}
>
<span>{ tab.title }</span>
</a>
);
} ) }
{ tabs.map( ( tab ) => {
return (
<a
key={ tab.id }
href={ tab.id }
role="tab"
tabIndex={0}
onKeyPress={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
onTabClick(tab.id);
}
}}
onClick={ ( e ) => {
e.preventDefault();
onTabClick( tab.id );
} }
className={ classNames(
tab.id === selectedTab
? 'border-orange-500 text-orange-600'
: 'border-transparent text-gray-500 hover:text-gray-700 hover:border-gray-300',
'group inline-flex items-center py-4 px-1 border-b-2 font-medium text-sm'
) }
aria-current={
tab.id === selectedTab
? 'page'
: undefined
}
>
<span>{ tab.title }</span>
</a>
);
} ) }

Comment on lines 7 to 22
export type StatusElement = {
headers?: Array< string >;
payload?: Object;
request?: string;
endpoint?: string;
url?: string;
title_text?: string;
id: string;
type: string;
title?: string;
description?: string;
icon?: string;
data?: string;
hook_key?: string;
children?: Array< StatusElement >;
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve type definition for StatusElement.

The Object type is too broad and should be replaced with a more specific type definition.

- payload?: Object;
+ payload?: Record<string, unknown>;
📝 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
export type StatusElement = {
headers?: Array< string >;
payload?: Object;
request?: string;
endpoint?: string;
url?: string;
title_text?: string;
id: string;
type: string;
title?: string;
description?: string;
icon?: string;
data?: string;
hook_key?: string;
children?: Array< StatusElement >;
};
export type StatusElement = {
headers?: Array< string >;
payload?: Record<string, unknown>;
request?: string;
endpoint?: string;
url?: string;
title_text?: string;
id: string;
type: string;
title?: string;
description?: string;
icon?: string;
data?: string;
hook_key?: string;
children?: Array< StatusElement >;
};
🧰 Tools
🪛 Biome (1.9.4)

[error] 9-9: Don't use 'Object' as a type.

Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.

(lint/complexity/noBannedTypes)

Comment on lines 36 to 49
useEffect( () => {
apiFetch< Array< StatusElement > >( {
path: 'dokan/v1/admin/dashboard/status',
} )
.then( ( data ) => {
console.log( data );
setAllSettings( data );
setLoading( false );
} )
.catch( ( error ) => {
console.log( error );
setLoading( false );
} );
}, [] );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance error handling for API requests.

The error handling for API requests should be more robust and user-friendly.

+ const [error, setError] = useState<string | null>(null);

  useEffect(() => {
    apiFetch<Array<StatusElement>>({
      path: 'dokan/v1/admin/dashboard/status',
    })
      .then((data) => {
-       console.log(data);
        setAllSettings(data);
        setLoading(false);
+       setError(null);
      })
      .catch((error) => {
-       console.log(error);
+       setError(error.message || 'Failed to fetch status data');
        setLoading(false);
      });
  }, []);

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines 178 to 186
public function get_status( $request ) {
/**
* @var Status $status
*/
$status = dokan_get_container()->get( Status::class );
$content = $status->render();

return rest_ensure_response( $content );
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding error handling for container resolution.

The get_status method should handle potential exceptions that might occur during container resolution or rendering.

Apply this diff to add error handling:

 public function get_status( $request ) {
     /**
      * @var Status $status
      */
-    $status = dokan_get_container()->get( Status::class );
-    $content = $status->render();
-
-    return rest_ensure_response( $content );
+    try {
+        $status = dokan_get_container()->get( Status::class );
+        $content = $status->render();
+        return rest_ensure_response( $content );
+    } catch ( Exception $e ) {
+        return new WP_Error(
+            'dokan_status_error',
+            $e->getMessage(),
+            [ 'status' => 500 ]
+        );
+    }
 }
📝 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
public function get_status( $request ) {
/**
* @var Status $status
*/
$status = dokan_get_container()->get( Status::class );
$content = $status->render();
return rest_ensure_response( $content );
}
public function get_status( $request ) {
/**
* @var Status $status
*/
try {
$status = dokan_get_container()->get( Status::class );
$content = $status->render();
return rest_ensure_response( $content );
} catch ( Exception $e ) {
return new WP_Error(
'dokan_status_error',
$e->getMessage(),
[ 'status' => 500 ]
);
}
}

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
includes/VendorNavMenuChecker.php (1)

150-182: ⚠️ Potential issue

Sanitize inputs to prevent directory traversal vulnerabilities

The slug and name parameters are used in file path constructions without sanitization in the get_overridden_template method. This could lead to directory traversal attacks if a malicious user inputs harmful values. It's crucial to sanitize these parameters to prevent potential security risks.

Apply this diff to fix the issue:

 protected function get_overridden_template( string $slug, string $name = '', array $args = [] ) {
+    // Sanitize slug and name to prevent directory traversal
+    $slug = sanitize_file_name( $slug );
+    $name = sanitize_file_name( $name );
     $defaults         = [ 'pro' => false ];
     $args             = wp_parse_args( $args, $defaults );
🧹 Nitpick comments (2)
includes/VendorNavMenuChecker.php (2)

11-11: Replace placeholder @since DOKAN_SINCE with the actual version number

The @since annotations in the docblocks currently use the placeholder DOKAN_SINCE. Please update these placeholders with the actual version number to maintain accurate documentation.

Also applies to: 44-44, 55-55, 80-80, 100-100, 115-115, 142-142, 187-187, 204-204, 243-243


245-247: Remove unnecessary @throws Exception annotation

The method add_status_section does not throw any exceptions, yet the docblock includes @throws Exception. Remove this annotation to accurately reflect the method's behavior and maintain clear documentation.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 30232b8 and 4a33fdd.

📒 Files selected for processing (2)
  • includes/VendorNavMenuChecker.php (1 hunks)
  • includes/functions-dashboard-navigation.php (1 hunks)
🔇 Additional comments (1)
includes/functions-dashboard-navigation.php (1)

Line range hint 12-16: Verify PHP version compatibility due to return type declarations

The function dokan_get_dashboard_nav now includes a return type declaration : array. Return type declarations require PHP 7.0 or higher. If the plugin needs to support older PHP versions, this change could cause syntax errors. Please verify the minimum PHP version requirement and ensure compatibility across all supported environments.

Run the following script to identify files using return type declarations and check the PHP version requirement:

Comment on lines 217 to 222
$notice = sprintf(
/* translators: %s: overridden templates */
__( 'The following templates are overridden:<br><code>%s</code>', 'dokan-lite' ),
implode( ',<br>', $overridden_templates )
);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Separate HTML markup from translation strings

Including HTML tags within translation strings can cause issues for translators and hinder localization efforts. To improve internationalization, move HTML tags outside of the translation functions.

Apply this diff to adjust the code:

             $notice = sprintf(
-                /* translators: %s: overridden templates */
-                __( 'The following templates are overridden:<br><code>%s</code>', 'dokan-lite' ),
-                implode( ',<br>', $overridden_templates )
+                /* translators: %s: overridden templates */
+                __( 'The following templates are overridden:', 'dokan-lite' ) . '<br><code>%s</code>',
+                implode( ',<br>', $overridden_templates )
             );
📝 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
$notice = sprintf(
/* translators: %s: overridden templates */
__( 'The following templates are overridden:<br><code>%s</code>', 'dokan-lite' ),
implode( ',<br>', $overridden_templates )
);
$notice = sprintf(
/* translators: %s: overridden templates */
__( 'The following templates are overridden:', 'dokan-lite' ) . '<br><code>%s</code>',
implode( ',<br>', $overridden_templates )
);

…ashboard-structure-convert-withdraw

# Conflicts:
#	includes/Assets.php
…ashboard-structure-convert-withdraw

# Conflicts:
#	includes/Assets.php
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (8)
docs/feature-override/readme.md (2)

1-1: Remove TODO comment from documentation.

The TODO comment at the top of the file should be addressed before merging.


9-11: Fix markdown list indentation.

The unordered list items should have consistent indentation of 2 spaces.

-    - [Define the override templates for a React route in Dokan Lite.](#define-the-override-templates-for-a-react-route-in-dokan-lite)
-    - [Define the override templates for a React route in **Dokan Pro** or **External Plugin**.](#define-the-override-templates-for-a-react-route-in-dokan-pro-or-external-plugin)
-    - [Define the override templates array structure.](#define-the-override-templates-array-structure)
+  - [Define the override templates for a React route in Dokan Lite.](#define-the-override-templates-for-a-react-route-in-dokan-lite)
+  - [Define the override templates for a React route in **Dokan Pro** or **External Plugin**.](#define-the-override-templates-for-a-react-route-in-dokan-pro-or-external-plugin)
+  - [Define the override templates array structure.](#define-the-override-templates-array-structure)
🧰 Tools
🪛 LanguageTool

[style] ~11-~11: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...in-dokan-pro-or-external-plugin) - [Define the override templates array structure....

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)

🪛 Markdownlint (0.37.0)

9-9: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


10-10: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


11-11: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)

src/layout/404.tsx (1)

8-29: Enhance accessibility and style maintainability.

The implementation looks good with proper internationalization and semantic HTML. Consider these improvements:

  1. Add ARIA labels for better screen reader support
  2. Extract common styles to Tailwind components for reusability
-        <div className="text-center">
+        <div className="text-center" role="main" aria-labelledby="error-title">
             <p className="text-lg font-semibold text-indigo-600">
                 { __( '404', 'dokan-lite' ) }
             </p>
-            <h1 className="mt-4 text-balance text-5xl font-semibold tracking-tight text-gray-900 sm:text-7xl">
+            <h1 id="error-title" className="mt-4 text-balance text-5xl font-semibold tracking-tight text-gray-900 sm:text-7xl">
                 { __( 'Page not found', 'dokan-lite' ) }
             </h1>

Consider moving common button styles to a Tailwind component in your config:

// tailwind.config.js
module.exports = {
  theme: {
    extend: {
      components: {
        '.btn-primary': 'rounded-md bg-dokan-btn px-3.5 py-2.5 text-sm font-semibold text-white shadow-sm hover:bg-dokan-btn-hover focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-indigo-600',
      },
    },
  },
}
includes/Dashboard/Templates/NewDashboard.php (2)

12-12: Replace DOKAN_SINCE placeholder with actual version.

The @SInCE tags contain placeholder DOKAN_SINCE which needs to be replaced with the actual version number.

Also applies to: 34-34, 49-49, 68-68


73-76: Consider early return pattern optimization.

The conditional check can be simplified using early return pattern.

-        if ( ! dokan_is_seller_dashboard() || ! isset( $wp->query_vars['new'] ) ) {
-            return;
-        }
+        if ( ! dokan_is_seller_dashboard() ) {
+            return;
+        }
+
+        if ( ! isset( $wp->query_vars['new'] ) ) {
+            return;
+        }
templates/dashboard/new-dashboard.php (3)

72-72: Document the purpose of headlessui-portal-root.

The empty div structure at the bottom needs documentation explaining its purpose for future maintainers.

+<!-- HeadlessUI portal root for React components -->
 <div class="dokan-layout" id="headlessui-portal-root"><div></div></div>

17-23: Enhance hook documentation.

The hook documentation should include:

  1. Return type
  2. Parameters passed to the hook
  3. Example usage

14-68: Consider accessibility improvements.

The dashboard structure could benefit from:

  1. ARIA landmarks
  2. Skip links
  3. Proper heading hierarchy
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4a33fdd and 75f6738.

📒 Files selected for processing (8)
  • docs/feature-override/readme.md (1 hunks)
  • includes/Assets.php (4 hunks)
  • includes/Dashboard/Templates/Manager.php (1 hunks)
  • includes/Dashboard/Templates/NewDashboard.php (1 hunks)
  • includes/VendorNavMenuChecker.php (1 hunks)
  • src/Routing/index.tsx (1 hunks)
  • src/layout/404.tsx (1 hunks)
  • templates/dashboard/new-dashboard.php (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • includes/VendorNavMenuChecker.php
🧰 Additional context used
🪛 LanguageTool
docs/feature-override/readme.md

[style] ~7-~7: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...#declare-react-menu-in-dokan-lite) - [Declare React menu in Dokan Pro or **Exte...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)


[style] ~11-~11: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...in-dokan-pro-or-external-plugin) - [Define the override templates array structure....

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)


[uncategorized] ~19-~19: You might be missing the article “the” here.
Context: ...vailable in React, you need to define route property during the menu registra...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)


[grammar] ~64-~64: “React” is a proper noun and needs to be capitalized.
Context: ...h menu which we are indicating that the react route is available. This will be used ...

(A_GOOGLE)


[grammar] ~65-~65: “React” is a proper noun and needs to be capitalized.
Context: ...etermine if the menu is pointing to the react Application or to the Legacy PHP Route....

(A_GOOGLE)


[uncategorized] ~93-~93: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ritten in PHP to the React application and you want that if some of the PHP templa...

(COMMA_COMPOUND_SENTENCE)


[style] ~113-~113: In contexts where ‘if’ is followed by ‘or’, using ‘whether’ may be more appropriate (and formal).
Context: ...e route. This will be used to determine if the override templates are available fo...

(IF_WHETHER)


[uncategorized] ~150-~150: You might be missing the article “the” here.
Context: ...hich are passed to the template file in dokan_get_template_part() function. (Op...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)


[uncategorized] ~153-~153: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...erride any of the template file directly but you have override functionality by usin...

(COMMA_COMPOUND_SENTENCE)

🪛 Markdownlint (0.37.0)
docs/feature-override/readme.md

9-9: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


10-10: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


11-11: Expected: 2; Actual: 4
Unordered list indentation

(MD007, ul-indent)


106-106: Column: 1
Hard tabs

(MD010, no-hard-tabs)


107-107: Column: 1
Hard tabs

(MD010, no-hard-tabs)


108-108: Column: 1
Hard tabs

(MD010, no-hard-tabs)


2-2: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


18-18: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


21-21: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


73-73: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


91-91: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


94-94: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


115-115: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


131-131: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)

🔇 Additional comments (7)
src/Routing/index.tsx (2)

9-41: LGTM! Well-structured higher-order component.

The withRouter HOC follows React best practices by:

  • Properly handling both element and function components
  • Passing all necessary router props
  • Using proper type checking with isValidElement

43-90: Fix duplicate route IDs.

Multiple routes are using the same ID 'dokan-base'. This could cause issues with route identification and management.

includes/Assets.php (3)

360-370: LGTM! Proper style registration for React components.

The styles are registered following WordPress standards with:

  • Correct dependencies
  • Proper versioning using filemtime
  • Clear naming convention

385-394: LGTM! Well-structured script requirements.

The script requirements are properly handled with:

  • Clear asset file requirements
  • Proper dependency merging
  • Consistent versioning approach

677-678: LGTM! Proper frontend URL localization.

The frontend URLs are correctly added to the localization data.

src/layout/404.tsx (1)

33-33: LGTM!

The export statement follows React conventions.

includes/Dashboard/Templates/Manager.php (1)

34-34: Verify NewDashboard class integration.

The integration follows the existing pattern. Let's verify the class existence and namespace.

✅ Verification successful

NewDashboard class integration verified successfully

The class exists in the correct namespace WeDevs\Dokan\Dashboard\Templates and is properly structured for container usage.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify NewDashboard class existence and namespace
rg -l "class NewDashboard" --type php

Length of output: 85


Script:

#!/bin/bash
# Check namespace and class implementation
rg -A 5 "namespace.*Dashboard.*Templates" includes/Dashboard/Templates/NewDashboard.php

Length of output: 214

Comment on lines 1 to 5
import { __ } from '@wordpress/i18n';

const NotFound = () => {
// @ts-ignore
const dashBoardUrl = window.dokan?.urls?.dashboardUrl ?? '#';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve TypeScript implementation by adding proper type definitions.

Instead of using @ts-ignore, define proper types for the window.dokan object. Also, consider adding a prop interface for future extensibility.

 import { __ } from '@wordpress/i18n';
+
+interface DokanWindow extends Window {
+    dokan?: {
+        urls?: {
+            dashboardUrl?: string;
+        };
+    };
+}
+
+interface NotFoundProps {
+    // Add props here when needed
+}
 
-const NotFound = () => {
-    // @ts-ignore
+const NotFound: React.FC<NotFoundProps> = () => {
     const dashBoardUrl = window.dokan?.urls?.dashboardUrl ?? '#';
📝 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
import { __ } from '@wordpress/i18n';
const NotFound = () => {
// @ts-ignore
const dashBoardUrl = window.dokan?.urls?.dashboardUrl ?? '#';
import { __ } from '@wordpress/i18n';
interface DokanWindow extends Window {
dokan?: {
urls?: {
dashboardUrl?: string;
};
};
}
interface NotFoundProps {
// Add props here when needed
}
const NotFound: React.FC<NotFoundProps> = () => {
const dashBoardUrl = window.dokan?.urls?.dashboardUrl ?? '#';

Comment on lines 57 to 61
if ( ! current_user_can( 'dokan_view_overview_menu' ) ) {
dokan_get_template_part( 'global/no-permission' );
} else {
dokan_get_template_part( 'dashboard/new-dashboard' );
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance security check for dashboard access.

The capability check uses 'dokan_view_overview_menu'. Consider:

  1. Adding nonce verification for the request
  2. Documenting the required capability in the function docblock

Comment on lines 78 to 82
$wc_instance = WCAdminAssets::get_instance();
$wc_instance->register_scripts();

wp_enqueue_script( 'dokan-react-frontend' );
wp_enqueue_style( 'dokan-react-frontend' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Add error handling for WC Admin dependency

The verification confirms that error handling for WCAdminAssets is missing, which could lead to fatal errors if the WooCommerce Admin package is not available. Add checks before using WCAdminAssets:

if ( class_exists( '\Automattic\WooCommerce\Internal\Admin\WCAdminAssets' ) ) {
    $wc_instance = WCAdminAssets::get_instance();
    $wc_instance->register_scripts();
} else {
    // Fallback handling or error logging
}
🔗 Analysis chain

Verify WooCommerce Admin script dependencies.

The code relies on WooCommerce Admin assets. Let's ensure proper dependency handling:

  1. Verify version compatibility
  2. Add error handling for missing WC Admin
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check WC Admin dependency usage
rg "WCAdminAssets" --type php
# Check script registration
rg "dokan-react-frontend.*wp_register_script" --type php

Length of output: 288


Script:

#!/bin/bash
# Find script registration with broader pattern
rg "wp_register_script.*dokan-react" -B 2 -A 2 --type php

# Look for version compatibility checks
rg "WC_VERSION|wc-admin" --type php

# Check for potential error handling around WC Admin
rg "function.*register_scripts|WCAdminAssets" -A 10 -B 2 --type php

Length of output: 6301


Script:

#!/bin/bash
# Search for dokan-react-frontend in asset files and webpack configs
rg "dokan-react-frontend" --type js --type php --type json

# Check for any WC Admin related checks or conditions
rg "wc-admin|wcadmin|woocommerce.*admin" -i --type php

Length of output: 17400

?>

<article class="dashboard-content-area">
<?php esc_html_e( 'Locading...', 'dokan-lite' ); ?>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix typo in loading text.

"Locading" should be "Loading".

-        <?php esc_html_e( 'Locading...', 'dokan-lite' ); ?>
+        <?php esc_html_e( 'Loading...', '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.

Suggested change
<?php esc_html_e( 'Locading...', 'dokan-lite' ); ?>
<?php esc_html_e( 'Loading...', 'dokan-lite' ); ?>

Aunshon and others added 4 commits January 15, 2025 13:49
…nce/introduce-dataviews-from-dokan-free

# Conflicts:
#	includes/Assets.php
…e/vendor-dashboard-structure-convert-withdraw

# Conflicts:
#	package-lock.json
#	package.json
#	src/routing/index.tsx
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 34

🧹 Nitpick comments (42)
src/definitions/RouterProps.ts (1)

13-13: Consider replacing any with a more specific type.

The Location generic parameter is set to any, 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:

  1. Add aria-sort attribute for sortable columns to improve accessibility
  2. 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 value property uses a union with any[] 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 useWithdrawRequestHook is redundant with the 'Hook' suffix since it's already a hook. Consider renaming it to withdrawRequests for 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-color vs hover-background-color might 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:

  1. The empty string default for content might mask missing content. Consider making it required.
  2. 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 for moment.

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, and phone are 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 makeDefaultMethod function 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 ChargeData interface 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 Continents type alias is redundant as it's identical to the Continent type. Consider removing it or documenting why it exists if there's a specific reason.

-export type Continents = Continent
src/dashboard/Withdraw/Hooks/useWithdrawSettings.ts (2)

18-22: Consider adding validation for setup_url.

The setup_url in WithdrawSettings interface 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 for OrderStatus.

The current type allows any string value, which defeats the purpose of using a union type. Consider removing the string type 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 is null or undefined.

-                                            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 allStatus object 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.tsx and 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:

  1. Form validation before submission
  2. Consistent class naming
  3. 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

📥 Commits

Reviewing files that changed from the base of the PR and between 75f6738 and 485d2f8.

📒 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, and Country are well-defined and follow REST API conventions with proper typing.


30-34: LGTM! Redux-style state management structure.

The CountryStateState interface 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 3

Length 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 2

Length 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:

  • dateI18n only processes date format tokens, not HTML
  • Date formats are controlled through WordPress settings
  • RawHTML is 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.json

Length of output: 4086

src/dashboard/Withdraw/Hooks/useWithdrawSettings.ts (1)

41-44: Verify API version consistency.

The hook uses /v2/ API endpoint while useBalance uses /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

Comment on lines +59 to +60
id: 'dokan-base',
title: __( 'Withdraw', 'dokan-lite' ),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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

Comment on lines 79 to 80
// @ts-ignore
routes = wp.hooks.applyFilters('dokan-dashboard-routes', routes) as Array<DokanRoute>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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;
            };
        };
    }
}

Comment on lines +35 to +37
onClick={() => {
onRowClick && onRowClick(row.original);
}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Suggested change
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)

Comment on lines +72 to +143
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;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve type safety and documentation in Product interface.

  1. 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;
}
  1. Consider adding JSDoc documentation for better developer experience.
  2. 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;
 }

Comment on lines +18 to +20
useEffect( () => {
currentUser.fetchCurrentUser();
}, [] );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
useEffect( () => {
currentUser.fetchCurrentUser();
}, [] );
useEffect( () => {
currentUser.fetchCurrentUser();
}, [currentUser.fetchCurrentUser] );

Comment on lines +77 to +90
onClick={ () => {
makeDefaultMethodHook
.makeDefaultMethod( activemethod.value )
.then( () => {
toast( {
type: 'success',
title: __(
'Default method updated',
'dokan-lite'
),
} );
bodyData.refresh();
} );
} }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
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'),
});
});
} }

Comment on lines +109 to +116
if (
! bodyData ||
! bodyData.hasOwnProperty( 'isLoading' ) ||
bodyData.isLoading ||
masterLoading
) {
return <Loader />;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Suggested change
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)

Comment on lines +177 to +206
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 );
} );
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve type safety and error handling in cancellation logic.

  1. The cancelRequestId is stored as a string but used as a number in the API call.
  2. 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.

Suggested change
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 );
} );
};

Comment on lines +38 to +111
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;
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider centralizing currency formatting logic and improving type safety.

  1. Currency formatting logic is scattered and uses multiple type assertions
  2. 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
    });
};

Comment on lines +112 to +159
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 ] );

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve error handling and prevent potential race conditions.

  1. Error handling could be more specific
  2. 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.

Suggested change
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 Aunshon changed the base branch from develop to update/vendor-dashboard-structure-router-context-support January 15, 2025 08:22
@Aunshon Aunshon changed the base branch from update/vendor-dashboard-structure-router-context-support to update/vendor-dashboard-structure January 15, 2025 08:22
@Aunshon Aunshon changed the base branch from update/vendor-dashboard-structure to enhance/introduce-dataviews-from-dokan-free January 15, 2025 08:24
@Aunshon Aunshon added Needs: Testing This requires further testing Needs: Dev Review It requires a developer review and approval labels Jan 16, 2025
Base automatically changed from enhance/introduce-dataviews-from-dokan-free to update/vendor-dashboard-structure January 20, 2025 04:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Dev Review It requires a developer review and approval Needs: Testing This requires further testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants