Feat/product form manager #3046
Conversation
… block instead of generic sub section
…nd made Single Step Product Edit page as default refactor: replaced product add/edit page code with OOP fix: added Dokan Pro checks for `product_vendors_can_create_tags` and `product_category_style` admin settings
… dokan_product_edit_meta_data, this will save some executions refactor: removed update_post_meta and get_post_meta references
…Manager.php to prevent direct access to the files
…o module/form_manager
…JS validation for required fields
…selector template
…ory from assets class
# Conflicts: # dokan.php # tests/pw/pages/productsPage.ts # tests/pw/pages/selectors.ts
… service provider
# Conflicts: # includes/ProductCategory/Categories.php # templates/products/edit-product-single.php
…lving merge conflict
# Conflicts: # assets/src/js/product-editor.js # includes/Admin/Notices/Manager.php # includes/Product/Manager.php # templates/products/edit-product-single.php # templates/products/tmpl-add-product-popup.php
…age after resolving merge conflict
📝 WalkthroughWalkthroughAdds a ProductForm system (Component/Field/Section/Factory/Init), migrates product edit UI to section-driven templates and a React form manager, removes popup/AJAX product-creation paths, changes meta handling to product-object/meta filters, and adds Product Manager APIs and related assets/tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant EditPHP as PHP Template
participant Factory as ProductFormFactory
participant Field as Field/Section
participant WC as WC_Product
participant DB as Database
Browser->>EditPHP: GET product edit page
EditPHP->>Factory: request sections & fields
Factory-->>EditPHP: sections + fields
EditPHP->>Browser: render form
Note over Browser,EditPHP: User submits form
Browser->>EditPHP: POST form data
EditPHP->>Factory: map POST → fields
Factory->>Field: per-field sanitize & validate
Field-->>Factory: {props, meta} or errors
alt validation success
Factory->>WC: set_props()/add_meta_data()
WC->>DB: persist product & meta
DB-->>WC: saved
WC-->>EditPHP: success
EditPHP->>Browser: redirect / success
else validation errors
Field-->>EditPHP: error list
EditPHP->>Browser: render form with errors
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/pw/utils/testData.ts (1)
237-387: Duplicatedescriptionkeys mean the new “test long description” values are ignoredFor multiple product variants (simple, downloadable, virtual, variable, external, simpleSubscription, variableSubscription, vendorSubscription) you now have patterns like:
simple: { // ... description: 'test long description', // ... description: 'test description', // ... }In a JS/TS object literal the last
descriptionwins, so:
- All of the newly added
'test long description'values are effectively discarded.- Linters (Biome) correctly flag these as duplicate keys.
You should collapse each pair into a single property, or introduce distinct names if you truly need both a short and long description. For example, if the intent is to use the long description for the new product form tests:
Illustrative diff for one variant (apply similarly to others)
simple: { productType: 'simple', productName: () => `${faker.commerce.productName()}_${faker.string.nanoid(5)} (Simple)`, category: 'Uncategorized', regularPrice: () => faker.finance.amount({ min: 100, max: 200, dec: faker.helpers.arrayElement([1, 2]) }).replace('.', ','), - description: 'test long description', + description: 'test long description', storeName: `${VENDOR}store`, status: 'publish', stockStatus: false, editProduct: '', - description: 'test description', saveSuccessMessage: 'Success! The product has been saved successfully. View Product →', },Please apply the same cleanup to the other affected variants so there is only one
descriptionkey per object and the intended value is used.includes/Product/Manager.php (1)
647-656: Data structure mismatch:save_taxonomy_termsreceives incompatible format from Product/Manager callers.The method expects an array of associative arrays with an
'id'key (as returned byget_taxonomy_terms), but callers at lines 313 and 318 in Product/Manager pass plain integers fromarray_map('absint', ProductCategoryHelper::get_object_terms_from_chosen_categories(...)). This causeswp_list_pluck($terms, 'id')to return an empty array, preventing categories and tags from being saved.Either restructure the data passed from Product/Manager to match the expected format, or modify
save_taxonomy_termsto handle both plain integers and objects with an'id'key.
🟡 Minor comments (13)
includes/Product/functions.php-17-17 (1)
17-17: Replace version placeholder with actual version number.The deprecation annotations use
DOKAN_SINCEas a placeholder. This should be replaced with the actual version number where this function is being deprecated (e.g.,4.0.0or whatever the target version is for this PR).🔎 Proposed fix
Assuming the target version is
4.0.0:- * @deprecated DOKAN_SINCE + * @deprecated 4.0.0And on line 22:
- wc_deprecated_function( __FUNCTION__, 'DOKAN_SINCE', 'dokan()->product->create()' ); + wc_deprecated_function( __FUNCTION__, '4.0.0', 'dokan()->product->create()' );Also applies to: 22-22
includes/wc-functions.php-34-38 (1)
34-38: Remove type casting assumption; the filter could return an unexpected type.The filter
dokan_restrict_product_gallery_images_on_editreceives unsanitized input and its return type is not validated before being passed toexplode()on line 36. If a plugin hooks this filter and returns an array, null, or object instead of a string,explode()will fail or produce unexpected results. Add explicit type validation:$attachment_ids = apply_filters( 'dokan_restrict_product_gallery_images_on_edit', wp_unslash( $data['gallery_image_ids'] ) ); if ( ! is_string( $attachment_ids ) ) { $attachment_ids = ''; } $attachment_ids_arr = array_filter( explode( ',', wc_clean( $attachment_ids ) ) );templates/products/edit/edit-product-single.php-239-241 (1)
239-241: Same issue: potential undefined variable.Consistent with the earlier usage, apply the same fix here.
🔎 Proposed fix
-if ( ! $from_shortcode ) { +if ( empty( $from_shortcode ) ) { get_footer(); }templates/products/edit/edit-product-single.php-56-58 (1)
56-58: Potential undefined variable notice.The variable
$from_shortcodeis checked on line 56, but if the template is loaded directly (not via shortcode), this variable may not be defined, causing a PHP notice. The check on line 17 usesisset(), but line 56 does not.🔎 Proposed fix
-if ( ! $from_shortcode ) { +if ( empty( $from_shortcode ) ) { get_header(); }templates/products/edit/sections/others.php-47-47 (1)
47-47: Useis_required()instead ofget_required()for consistency.Line 47 uses
$product_status->get_required(), which is inconsistent with line 71 and all other template files in the codebase. The standard method throughout the product form templates isis_required()(used 20+ times). Both methods are functionally equivalent—is_required()simply wrapsget_required()—but consistency matters.- <select id="<?php echo esc_attr( $product_status->get_name() ); ?>" class="dokan-form-control" name="<?php echo esc_attr( $product_status->get_name() ); ?>" <?php echo $product_status->get_required() ? 'required' : ''; ?>> + <select id="<?php echo esc_attr( $product_status->get_name() ); ?>" class="dokan-form-control" name="<?php echo esc_attr( $product_status->get_name() ); ?>" <?php echo $product_status->is_required() ? 'required' : ''; ?>>templates/products/edit/sections/general.php-291-291 (1)
291-291: Missing output escaping on tags title.The title is echoed directly without escaping. Use
esc_html()for consistency with other fields and to prevent potential XSS.🔎 Proposed fix
- <?php echo $tags->get_title(); ?> + <?php echo esc_html( $tags->get_title() ); ?>includes/ProductForm/Section.php-46-53 (1)
46-53: Error message references "Field" instead of "Section".The exception message says "Dokan ProductForm Field" but this is the Section class.
🔎 Proposed fix
throw new \Exception( sprintf( /* translators: 1: Missing arguments list. */ - esc_html__( 'You are missing required arguments of Dokan ProductForm Field: %1$s', 'dokan-lite' ), + esc_html__( 'You are missing required arguments of Dokan ProductForm Section: %1$s', 'dokan-lite' ), esc_attr( join( ', ', $missing_arguments ) ) ) );templates/products/edit/sections/general.php-302-306 (1)
302-306: Incorrect error check forget_term_by().
get_term_by()returnsfalseon failure, not aWP_Error. Theis_wp_error()check will never be true here.🔎 Proposed fix
foreach ( $terms as $tax_term ) : $tax_term = get_term_by( 'id', $tax_term, 'product_tag' ); - if ( is_wp_error( $tax_term ) ) { + if ( ! $tax_term || is_wp_error( $tax_term ) ) { continue; }includes/ProductForm/Section.php-189-193 (1)
189-193: Type inconsistency inset_product_category().The method is typed to return
Sectionbut assigns0(integer) when empty, while the property should be a string perget_product_category()return type.🔎 Proposed fix
public function set_product_category( string $product_category ): Section { - $this->data['product_category'] = ! empty( $product_category ) ? $product_category : 0; + $this->data['product_category'] = ! empty( $product_category ) ? $product_category : ''; return $this; }includes/ProductForm/Factory.php-274-282 (1)
274-282: Error message references "WooCommerce Form" instead of "Dokan".The duplicate ID error message mentions "WooCommerce Form" but this is Dokan's ProductForm system.
🔎 Proposed fix
return new WP_Error( 'dokan_product_form_' . $type . '_duplicate_field_id', sprintf( /* translators: 1: Item type 2: Duplicate registered item id. */ - esc_html__( 'You have attempted to register a duplicate form %1$s with WooCommerce Form: %2$s', 'dokan-lite' ), + esc_html__( 'You have attempted to register a duplicate form %1$s with Dokan ProductForm: %2$s', 'dokan-lite' ), $type, '`' . $id . '`' ) );includes/ProductForm/Init.php-264-267 (1)
264-267: Inconsistent error handling in tags sanitize callback.The callback returns a translated string on error while other sanitize callbacks return
WP_Error. This inconsistency could cause issues in consuming code.🔎 Proposed fix for consistency
if ( $maximum_tags_select_length !== -1 && count( $tags ) > $maximum_tags_select_length ) { // translators: %s: maximum tag length - return sprintf( __( 'You can only select %s tags', 'dokan-lite' ), number_format_i18n( $maximum_tags_select_length ) ); + return new \WP_Error( + 'too_many_tags', + sprintf( __( 'You can only select %s tags', 'dokan-lite' ), number_format_i18n( $maximum_tags_select_length ) ) + ); }includes/ProductForm/Component.php-335-357 (1)
335-357: Swapped docblock comments for getter/setter.The docblock for
set_show_in_admin_settingssays "Get" and the one forget_show_in_admin_settingssays "Set".🔎 Proposed fix
/** - * Get show in admin settings + * Set show in admin settings * * @since DOKAN_SINCE * * @param bool $show_in_admin_settings * * @return void */ public function set_show_in_admin_settings( bool $show_in_admin_settings ): void { $this->data['show_in_admin_settings'] = $show_in_admin_settings; } /** - * Set show in admin settings + * Get show in admin settings * * @since DOKAN_SINCE * * @return bool */ public function get_show_in_admin_settings(): bool {includes/ProductForm/Field.php-215-228 (1)
215-228: Review the options fallback logic.Two potential issues:
- Line 223: The condition
'' !== $this->data['options']evaluates totruewhen options is an empty array[], which may bypass the intended fallback logic.- Line 227: Returns
$value(an array from variadic parameters) directly, creating an inconsistent return type similar to the issue inget_value().🔎 Proposed fix
- // if options are set under the field, return that options - if ( '' !== $this->data['options'] || empty( $value ) ) { - return $this->data['options']; - } - - return $value; + // if options are set under the field, return that options + if ( ! empty( $this->data['options'] ) || empty( $value ) ) { + return $this->data['options']; + } + + return [];
🧹 Nitpick comments (23)
includes/ProductCategory/Helper.php (2)
168-204: Good refactor to extract ancestor computation logic.The method correctly handles both
WC_Productobjects and integer IDs, with proper null checks. The logic for computing ancestors is sound.Minor note: The
@since 3.6.2tag suggests this method existed since that version, but this appears to be a newly added method. Consider updating to the current version.
221-231: Redundantabsintmapping.
get_object_terms_from_chosen_categories()already appliesarray_map('absint', ...)at line 203 before returning. The same mapping at line 229 is therefore redundant.🔎 Proposed fix
$all_ancestors = self::get_object_terms_from_chosen_categories( $post_id, $chosen_categories ); // save chosen cat to database update_post_meta( $post_id, 'chosen_product_cat', $chosen_categories ); // add all ancestor and chosen cat as product category - // We have to convert all the categories into integer because if an category is string ex: '23' not int ex: 23 - // wp_set_object_terms will create a new term named 23. we don't want that. - $all_ancestors = array_map( 'absint', $all_ancestors ); - wp_set_object_terms( $post_id, array_unique( $all_ancestors ), 'product_cat' );tests/pw/utils/interfaces.ts (1)
126-262: Avoid duplicatedescriptionproperties in nested product variantsFor the following nested interfaces under
product:
simpledownloadablevariableexternalsimpleSubscriptionvariableSubscriptionvendorSubscriptioneach object now declares
description: string;twice (once just afterregularPrice/subscriptionPrice, and again near the bottom of the object). While TypeScript tolerates this when the types match, it’s misleading and makes it unclear whether there are meant to be two distinct concepts.Consider either:
- Keeping a single
descriptionproperty per object, or- Renaming one of them (e.g.,
shortDescription,expectedDescriptionMessage) if they carry different meanings.This will simplify the test data shape and avoid confusion for future readers.
dokan-class.php (1)
19-21: Dashboard service annotation andDOKAN_TEMPLATE_DIRconstant are consistentThe new
@property WeDevs\Dokan\Dashboard\Manager $dashboarddocblock entry matches how services are exposed via__get, and definingDOKAN_TEMPLATE_DIRalongsideDOKAN_INC_DIR/DOKAN_LIB_DIRgives a clear canonical template base path.As a follow-up (not blocking), you might eventually standardize existing uses of
DOKAN_DIR . '/templates/'to this constant to avoid path drift.Also applies to: 216-223
templates/products/edit/sections/others.php (1)
119-129: Hardcoded label instead of using field's title.The "Enable product reviews" label is hardcoded at line 125, whereas other fields in this template use
$field->get_title(). For consistency and maintainability (e.g., if the title changes or needs translation updates), consider using the field's title.🔎 Proposed fix
dokan_post_input_box( $product->get_id(), $enable_reviews->get_name(), [ 'name' => $enable_reviews->get_name(), 'value' => $enable_reviews->get_value( $product ) ? 'yes' : 'no', - 'label' => __( 'Enable product reviews', 'dokan-lite' ), + 'label' => $enable_reviews->get_title(), 'required' => $enable_reviews->is_required(), ], 'checkbox' );templates/products/edit/sections/downloadable.php (3)
9-9: Incorrect package annotation.The
@packageannotation saysWooCommerce\Adminbut this is a Dokan template.🔎 Proposed fix
- * @package WooCommerce\Admin + * @package WeDevs\Dokan
73-73: Unused variable:$disabled_downloads_count.This variable is initialized but never used. Either remove it or implement the intended functionality.
🔎 Proposed fix
$downloadable_files = $product->get_downloads( 'edit' ); - $disabled_downloads_count = 0; if ( $downloadable_files ) {
134-137: Inconsistent placeholder handling.Line 105 uses
$download_limit->get_placeholder()but line 136 hardcodes__( 'Never', 'dokan-lite' ). For consistency, consider using the field's placeholder if defined.🔎 Proposed fix
[ 'value' => - 1 === $download_expiry->get_value( $product ) ? '' : $download_expiry->get_value( $product ), - 'placeholder' => __( 'Never', 'dokan-lite' ), + 'placeholder' => $download_expiry->get_placeholder() ?: __( 'Never', 'dokan-lite' ), 'min' => $download_expiry->get_additional_properties( 'min' ),includes/Dashboard/Templates/Manager.php (1)
12-18: PHPDoc inconsistencies.
- Line 14:
$productsis documented as "Instance of Dashboard class" but should be "Instance of Products class"- The
new_dashboardproperty added at line 40 is not documented in the@propertyannotations🔎 Proposed fix
/** * Dashboard Template Manager * * @since 3.0.0 * * @property Dashboard $dashboard Instance of Dashboard class * @property Orders $orders Instance of Orders class - * @property Products $products Instance of Dashboard class + * @property Products $products Instance of Products class * @property Settings $settings Instance of Settings class * @property Withdraw $withdraw Instance of Withdraw class * @property MultiStepCategories $product_category Instance of MultiStepCategories class * @property ReverseWithdrawal $reverse_withdrawal Instance of ReverseWithdrawal class + * @property NewDashboard $new_dashboard Instance of NewDashboard class */includes/Product/Manager.php (2)
695-709: Redundantisset()check on$stock_status.The variable
$stock_statusis always defined before this check (lines 220-224), soisset( $stock_status )will always betrue.🔎 Suggested simplification
protected function maybe_update_stock_status( $product, $stock_status ) { - if ( isset( $stock_status ) ) { + if ( $stock_status ) { if ( $product->is_type( 'variable' ) && ! $product->get_manage_stock() ) {
725-743: Good addition ofprepare_downloads()helper.This method cleanly encapsulates download preparation logic. Consider adding array bounds validation to prevent potential index out-of-bounds if arrays have mismatched lengths.
🔎 Safer iteration with bounds check
public function prepare_downloads( $file_names, $file_urls, $file_hashes ) { $downloads = []; if ( ! empty( $file_urls ) ) { $file_url_size = count( $file_urls ); + $file_names = array_pad( (array) $file_names, $file_url_size, '' ); + $file_hashes = array_pad( (array) $file_hashes, $file_url_size, '' ); for ( $i = 0; $i < $file_url_size; $i++ ) {includes/Product/Hooks.php (1)
386-394: Unused$productparameter.The
$productparameter is not used in the method body. While this is valid for filter callbacks that may need future access, consider adding a note or removing it if not needed.The static analysis hint is correct. If the product is not needed for setting email status, you could use an underscore prefix to indicate intentional non-use:
-public function set_new_product_email_status( array $meta_data, WC_Product $product, bool $is_new_product ) { +public function set_new_product_email_status( array $meta_data, WC_Product $_product, bool $is_new_product ) {Alternatively, keep it as-is if future usage is anticipated.
includes/ProductForm/Section.php (1)
80-86: TODO: Handle error fromFactory::add_field().The TODO comment indicates error handling is needed.
Factory::add_field()can returnWP_Error, which is currently ignored.Would you like me to propose error handling logic, such as returning the error or logging it?
includes/ProductForm/Init.php (1)
637-645: Duplicate order value for sections.Both 'downloadable' (line 565) and 'others' (line 643) sections have
'order' => 30. This may cause unpredictable ordering.🔎 Proposed fix
$section = Factory::add_section( 'others', [ 'title' => __( 'Other Options', 'dokan-lite' ), 'description' => __( 'Set your extra product options', 'dokan-lite' ), - 'order' => 30, + 'order' => 40, ] );includes/ProductForm/Component.php (1)
50-56: Unused$idparameter in constructor.The
$idparameter is declared but not used in the constructor body. Child classes (Field, Section) callset_id($id)before callingparent::__construct().This is fine since child classes handle ID assignment, but the parameter could be removed from the parent constructor signature for clarity.
🔎 Alternative: use $id in constructor
public function __construct( string $id, array $args = [] ) { + $this->set_id( $id ); + foreach ( $args as $key => $value ) { if ( method_exists( $this, "set_{$key}" ) && null !== $value ) { $this->{"set_{$key}"}( $value ); } } }Then remove
$this->set_id( $id );from child classes.includes/ProductForm/Factory.php (1)
5-5: Unused import:Singletontrait.The
Singletontrait is imported but never used in this class.🔎 Proposed fix
namespace WeDevs\Dokan\ProductForm; -use WeDevs\Dokan\Traits\Singleton; use WP_Error;includes/ProductForm/Field.php (3)
451-459: Consider merge behavior for additional properties.When
$propertyis an array, Line 455 replaces the entireadditional_propertiesarray rather than merging. This could be surprising if developers expect to preserve existing properties.🔎 Alternative approach using merge
if ( ! empty( $property ) && null !== $value ) { $this->data['additional_properties'][ $property ] = $value; } elseif ( is_array( $property ) ) { - $this->data['additional_properties'] = $property; + $this->data['additional_properties'] = array_merge( $this->data['additional_properties'], $property ); }
389-394: TODO: Add field type validation.The comment indicates missing validation for
$field_type. Currently, any sanitized string is accepted, which could lead to invalid field types being stored.Do you want me to generate validation logic for common HTML field types (text, number, email, select, checkbox, radio, textarea, etc.)?
519-532: Prefer CSS class over inline styles for maintainability.Line 524 uses inline CSS for the required symbol. Using a CSS class would improve maintainability and allow theme/plugin customization.
🔎 Proposed refactor using CSS class
- $symbol = '<span style="color: red; display: inline-block;">*</span>'; + $symbol = '<span class="dokan-required-symbol">*</span>';Ensure the CSS class is defined in your stylesheet:
.dokan-required-symbol { color: red; display: inline-block; }includes/Dashboard/Templates/Products.php (4)
195-220: Unused $post parameter in template loader methods.The
$postparameter is unused in:
load_download_virtual_template(line 195)load_inventory_template(line 232)load_downloadable_template(line 263)load_others_template(line 299)If the parameter is required by the action hook signature but not needed in the implementation, consider prefixing it with an underscore (
$_post) to indicate intentional non-use, or add a docblock comment explaining why it's present.🔎 Proposed fix to indicate intentional non-use
- public static function load_download_virtual_template( $post, $post_id ) { + public static function load_download_virtual_template( $_post, $post_id ) {Apply similarly to the other three methods.
Also applies to: 232-251, 263-287, 299-317
403-407: Clarify product type determination logic.Lines 404-407 contain nested function calls and a ternary operator that determine the product type and class. While functionally correct, the logic could be more readable.
🔎 More readable alternative
- // Process the product type first so that we have the correct class to run setters. - $product_type_field = ProductFormFactory::get_field( ProductFormElements::TYPE ); - $product_type = empty( $_POST[ $product_type_field->get_name() ] ) ? WC_Product_Factory::get_product_type( $product_id ) : sanitize_title( wp_unslash( $_POST[ $product_type_field->get_name() ] ) ); - $classname = WC_Product_Factory::get_product_classname( $product_id, $product_type ? $product_type : 'simple' ); - $product = new $classname( $product_id ); + // Process the product type first so that we have the correct class to run setters. + $product_type_field = ProductFormFactory::get_field( ProductFormElements::TYPE ); + $field_name = $product_type_field->get_name(); + + if ( ! empty( $_POST[ $field_name ] ) ) { + $product_type = sanitize_title( wp_unslash( $_POST[ $field_name ] ) ); + } else { + $product_type = WC_Product_Factory::get_product_type( $product_id ); + } + + $product_type = $product_type ? $product_type : 'simple'; + $classname = WC_Product_Factory::get_product_classname( $product_id, $product_type ); + $product = new $classname( $product_id );
425-434: Remove unused $field_id variable.Line 425: The
$field_idvariable is extracted in the foreach loop but never used. Use only$fieldin the loop.🔎 Proposed fix
- foreach ( ProductFormFactory::get_fields() as $field_id => $field ) { + foreach ( ProductFormFactory::get_fields() as $field ) {
506-510: Consider defense-in-depth for gallery image IDs.Line 507 passes
$_POST['gallery_image_ids']throughapply_filters()before sanitization (line 508). While the final value is sanitized, plugins receiving the filter may process unsanitized input.🔎 Sanitize before and after the filter
- $attachment_ids = isset( $_POST['gallery_image_ids'] ) ? apply_filters( 'dokan_restrict_product_gallery_images_on_edit', wp_unslash( $_POST['gallery_image_ids'] ) ) : ''; - $attachment_ids = array_filter( explode( ',', wc_clean( $attachment_ids ) ) ); + $attachment_ids = isset( $_POST['gallery_image_ids'] ) ? wc_clean( wp_unslash( $_POST['gallery_image_ids'] ) ) : ''; + $attachment_ids = apply_filters( 'dokan_restrict_product_gallery_images_on_edit', $attachment_ids ); + $attachment_ids = array_filter( explode( ',', wc_clean( $attachment_ids ) ) );
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (53)
assets/src/less/dashboard.lessassets/src/less/products.lessdokan-class.phpincludes/Admin/Settings.phpincludes/Ajax.phpincludes/Assets.phpincludes/Blocks/ProductBlock.phpincludes/CatalogMode/Dashboard/ProductBulkEdit.phpincludes/CatalogMode/Dashboard/Products.phpincludes/CatalogMode/Helper.phpincludes/Dashboard/Manager.phpincludes/Dashboard/Templates/Manager.phpincludes/Dashboard/Templates/Products.phpincludes/Emails/NewProduct.phpincludes/Emails/NewProductPending.phpincludes/Product/Hooks.phpincludes/Product/Manager.phpincludes/Product/functions.phpincludes/ProductCategory/Helper.phpincludes/ProductForm/Component.phpincludes/ProductForm/Elements.phpincludes/ProductForm/Factory.phpincludes/ProductForm/Field.phpincludes/ProductForm/Init.phpincludes/ProductForm/Section.phpincludes/REST/Manager.phpincludes/Rewrites.phpincludes/functions.phpincludes/wc-functions.phpincludes/woo-views/html-product-download.phpsrc/admin/components/Fields.vuesrc/admin/components/Switches.vuesrc/admin/pages/Settings.vuetemplates/products/add-new-product-modal.phptemplates/products/dokan-category-header-ui.phptemplates/products/download-virtual.phptemplates/products/downloadable.phptemplates/products/edit/edit-product-single.phptemplates/products/edit/sections/catalog-mode-content.phptemplates/products/edit/sections/download-virtual.phptemplates/products/edit/sections/downloadable.phptemplates/products/edit/sections/general.phptemplates/products/edit/sections/inventory.phptemplates/products/edit/sections/others.phptemplates/products/new-product.phptemplates/products/others.phptemplates/products/tmpl-add-product-popup.phptests/pw/pages/noticeAndPromotionPage.tstests/pw/pages/settingsPage.tstests/pw/utils/dbData.tstests/pw/utils/interfaces.tstests/pw/utils/schemas.tstests/pw/utils/testData.ts
💤 Files with no reviewable changes (10)
- templates/products/add-new-product-modal.php
- tests/pw/pages/settingsPage.ts
- tests/pw/utils/dbData.ts
- includes/Ajax.php
- templates/products/tmpl-add-product-popup.php
- templates/products/others.php
- includes/Admin/Settings.php
- templates/products/downloadable.php
- templates/products/new-product.php
- templates/products/download-virtual.php
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-19T06:19:41.376Z
Learnt from: mrabbani
Repo: getdokan/dokan PR: 2891
File: src/admin/dashboard/pages/vendors-single/InformationTabs/WithdrawTab.tsx:58-59
Timestamp: 2025-09-19T06:19:41.376Z
Learning: The Dokan codebase uses 'dokan-lite' as the text domain for WordPress internationalization (__() function calls).
Applied to files:
dokan-class.phpincludes/Assets.php
🧬 Code graph analysis (20)
templates/products/edit/sections/download-virtual.php (5)
includes/ProductForm/Elements.php (1)
Elements(12-78)includes/ProductForm/Section.php (2)
Section(14-194)get_field(95-97)tests/pw/utils/interfaces.ts (1)
product(45-475)includes/ProductForm/Factory.php (1)
get_field(149-155)includes/functions.php (1)
dokan_post_input_box(604-721)
includes/REST/Manager.php (7)
includes/Admin/Dashboard/Pages/Vendors.php (1)
get_id(6-8)includes/Vendor/Vendor.php (1)
get_id(312-314)includes/Admin/Dashboard/Pages/Withdraw.php (1)
get_id(14-16)includes/Admin/OnboardingSetup/Steps/AbstractStep.php (1)
get_id(58-60)includes/Admin/OnboardingSetup/Steps/StepInterface.php (1)
get_id(19-19)includes/Admin/Dashboard/Pages/SetupGuide.php (1)
get_id(26-28)includes/ProductForm/Component.php (1)
get_id(76-78)
includes/Rewrites.php (1)
includes/functions.php (1)
dokan_locate_template(936-959)
includes/wc-functions.php (1)
assets/src/js/product-editor.js (1)
attachment_ids(1233-1233)
includes/Blocks/ProductBlock.php (3)
dokan.php (1)
dokan(90-92)dokan-class.php (1)
is_pro_exists(398-400)includes/functions.php (1)
dokan_get_option(1062-1072)
templates/products/edit/sections/downloadable.php (5)
includes/ProductForm/Elements.php (1)
Elements(12-78)includes/ProductForm/Section.php (2)
Section(14-194)get_field(95-97)assets/src/js/product-editor.js (1)
key(1887-1887)includes/ProductForm/Factory.php (1)
get_field(149-155)includes/functions.php (1)
dokan_post_input_box(604-721)
templates/products/edit/sections/general.php (5)
includes/ProductCategory/Helper.php (2)
Helper(13-433)get_saved_products_category(57-89)includes/ProductForm/Elements.php (1)
Elements(12-78)includes/ProductForm/Factory.php (3)
Factory(15-294)get_section(194-200)get_field(149-155)includes/ProductForm/Field.php (4)
get_section(280-282)get_name(81-83)print_required_symbol(519-532)get_placeholder(403-405)includes/functions.php (1)
dokan_post_input_box(604-721)
tests/pw/pages/noticeAndPromotionPage.ts (1)
tests/pw/pages/selectors.ts (1)
selector(3-8246)
includes/Product/Manager.php (1)
includes/ProductForm/Elements.php (1)
Elements(12-78)
includes/CatalogMode/Dashboard/Products.php (1)
includes/CatalogMode/Helper.php (2)
Helper(14-185)get_defaults(129-136)
includes/ProductForm/Init.php (4)
includes/ProductForm/Section.php (2)
add_field(80-86)get_type(124-126)includes/ProductForm/Field.php (2)
get_name(81-83)get_type(306-308)includes/ProductForm/Factory.php (3)
Factory(15-294)add_section(84-101)add_field(55-72)includes/ProductForm/Elements.php (1)
Elements(12-78)
templates/products/edit/sections/inventory.php (6)
includes/ProductForm/Elements.php (1)
Elements(12-78)includes/ProductForm/Section.php (2)
Section(14-194)get_field(95-97)tests/pw/utils/interfaces.ts (1)
product(45-475)includes/ProductForm/Factory.php (1)
get_field(149-155)includes/ProductForm/Field.php (3)
get_name(81-83)get_value(135-165)get_options(215-228)includes/functions.php (1)
dokan_post_input_box(604-721)
includes/ProductForm/Factory.php (3)
includes/ProductForm/Section.php (4)
add_field(80-86)get_fields(63-68)get_type(124-126)get_field(95-97)includes/ProductForm/Field.php (2)
get_section(280-282)get_type(306-308)includes/ProductForm/Component.php (1)
sort(379-387)
includes/Dashboard/Templates/Products.php (4)
includes/ProductForm/Elements.php (1)
Elements(12-78)includes/ProductForm/Factory.php (5)
Factory(15-294)get_section(194-200)get_field(149-155)get_fields(108-110)get_fields_by_type(122-140)includes/functions.php (1)
dokan_get_template_part(850-889)includes/ProductForm/Field.php (5)
get_section(280-282)sanitize(496-508)is_prop(317-319)is_meta(328-330)is_custom(339-341)
includes/ProductForm/Field.php (2)
includes/ProductForm/Component.php (4)
Component(16-411)__construct(50-56)set_id(91-95)get_missing_arguments(396-410)includes/ProductForm/Section.php (3)
__construct(28-54)get_type(124-126)set_type(137-141)
includes/ProductForm/Component.php (3)
includes/ProductForm/Init.php (1)
__construct(24-26)includes/ProductForm/Section.php (1)
__construct(28-54)includes/ProductForm/Field.php (1)
__construct(37-72)
templates/products/edit/sections/others.php (6)
includes/ProductForm/Elements.php (1)
Elements(12-78)includes/ProductForm/Section.php (2)
Section(14-194)get_field(95-97)includes/ProductForm/Factory.php (1)
get_field(149-155)includes/ProductForm/Field.php (4)
get_name(81-83)print_required_symbol(519-532)get_value(135-165)get_placeholder(403-405)src/vendor-dashboard/reports/analytics/settings/historical-data/layout.js (1)
status(162-162)includes/functions.php (1)
dokan_post_input_box(604-721)
includes/Assets.php (2)
includes/ProductForm/Factory.php (2)
Factory(15-294)get_fields(108-110)includes/ProductForm/Component.php (1)
toArray(65-67)
includes/Product/functions.php (2)
includes/Product/Manager.php (1)
create(79-417)includes/REST/ProductController.php (2)
get_attributes(1467-1512)get_default_attributes(1412-1434)
includes/ProductCategory/Helper.php (2)
dokan.php (1)
dokan(90-92)dokan-class.php (1)
is_pro_exists(398-400)
🪛 Biome (2.1.2)
tests/pw/utils/testData.ts
[error] 242-242: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 256-256: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 277-278: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 291-292: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 313-314: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 337-338: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 355-356: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 379-380: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
🪛 GitHub Check: Run PHPCS inspection
includes/Dashboard/Templates/Products.php
[failure] 507-507:
Detected usage of a non-sanitized input variable: $_POST['gallery_image_ids']
includes/Assets.php
[warning] 690-690:
json_encode() is discouraged. Use wp_json_encode() instead.
🪛 PHPMD (2.15.0)
includes/Dashboard/Templates/Products.php
195-195: Avoid unused parameters such as '$post'. (undefined)
(UnusedFormalParameter)
232-232: Avoid unused parameters such as '$post'. (undefined)
(UnusedFormalParameter)
263-263: Avoid unused parameters such as '$post'. (undefined)
(UnusedFormalParameter)
299-299: Avoid unused parameters such as '$post'. (undefined)
(UnusedFormalParameter)
425-425: Avoid unused local variables such as '$field_id'. (undefined)
(UnusedLocalVariable)
includes/ProductForm/Component.php
50-50: Avoid unused parameters such as '$id'. (undefined)
(UnusedFormalParameter)
includes/Product/Hooks.php
386-386: Avoid unused parameters such as '$product'. (undefined)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: api tests (1, 1)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (1, 3)
| public function get_product_fields() { | ||
| $temp_fields = []; | ||
| foreach ( ProductFormFactory::get_fields() as $field_id => $field ) { | ||
| $temp_fields[ $field_id ] = $field->toArray(); | ||
| } | ||
|
|
||
| return json_encode( $temp_fields ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use wp_json_encode() instead of json_encode().
Per WordPress coding standards, wp_json_encode() should be used as it handles encoding errors gracefully and ensures proper character encoding.
Additionally, consider adding a return type hint for clarity.
🔎 Proposed fix
- public function get_product_fields() {
+ public function get_product_fields(): string {
$temp_fields = [];
foreach ( ProductFormFactory::get_fields() as $field_id => $field ) {
$temp_fields[ $field_id ] = $field->toArray();
}
- return json_encode( $temp_fields );
+ return wp_json_encode( $temp_fields );
}📝 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.
| public function get_product_fields() { | |
| $temp_fields = []; | |
| foreach ( ProductFormFactory::get_fields() as $field_id => $field ) { | |
| $temp_fields[ $field_id ] = $field->toArray(); | |
| } | |
| return json_encode( $temp_fields ); | |
| } | |
| public function get_product_fields(): string { | |
| $temp_fields = []; | |
| foreach ( ProductFormFactory::get_fields() as $field_id => $field ) { | |
| $temp_fields[ $field_id ] = $field->toArray(); | |
| } | |
| return wp_json_encode( $temp_fields ); | |
| } |
🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[warning] 690-690:
json_encode() is discouraged. Use wp_json_encode() instead.
🤖 Prompt for AI Agents
In includes/Assets.php around lines 684 to 691, replace the use of json_encode
with wp_json_encode and add a return type hint for clarity; change the method
signature to declare the return type (string) and use wp_json_encode(
$temp_fields ) when returning so WordPress-specific encoding and error handling
are used instead of json_encode.
| if ( 'variation' === $product->get_type() ) { | ||
| $product_id = $product->get_parent_id(); | ||
| } else { | ||
| $product_id = $product->get_id(); | ||
| $product = wc_get_product( $product->get_parent_id() ); | ||
| } |
There was a problem hiding this comment.
Add null check after loading parent product for variations.
When the product is a variation, the parent product is loaded at Line 177. However, there's no verification that the parent exists. An orphaned variation (where the parent product has been deleted) would result in $product being null, causing a fatal error when $product->get_meta() is called at Line 181.
🔎 Proposed fix to add null check for parent product
// get product id
if ( 'variation' === $product->get_type() ) {
$product = wc_get_product( $product->get_parent_id() );
+ // return defaults if parent product doesn't exist
+ if ( ! $product ) {
+ return $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.
| if ( 'variation' === $product->get_type() ) { | |
| $product_id = $product->get_parent_id(); | |
| } else { | |
| $product_id = $product->get_id(); | |
| $product = wc_get_product( $product->get_parent_id() ); | |
| } | |
| if ( 'variation' === $product->get_type() ) { | |
| $product = wc_get_product( $product->get_parent_id() ); | |
| // return defaults if parent product doesn't exist | |
| if ( ! $product ) { | |
| return $defaults; | |
| } | |
| } |
🤖 Prompt for AI Agents
In includes/CatalogMode/Helper.php around lines 176 to 178, after loading the
parent product for a variation you must guard against a null parent: check if
wc_get_product returned null and handle it (e.g., return early, skip processing,
or set a safe default) so subsequent calls like $product->get_meta() at line 181
do not cause a fatal error; add the null check and appropriate early exit or
fallback immediately after the wc_get_product(...) call.
| // check if this is a product add or edit page | ||
| $product_id = intval( wp_unslash( $_GET['product_id'] ) ); | ||
| $new_product = false; | ||
| if ( ! $product_id ) { | ||
| // this is a product add page request | ||
| // now create a new product with auto draft status | ||
| $product = new WC_Product_Simple(); | ||
| $product->set_status( 'auto-draft' ); | ||
| $product->save(); | ||
| $new_product = true; | ||
| } else { | ||
| // this is a product edit page request | ||
| $product = wc_get_product( $product_id ); | ||
| } |
There was a problem hiding this comment.
Potential database bloat from repeated auto-draft creation.
Lines 155-158 create a new auto-draft product every time product_id is 0 (e.g., when users navigate to the add-product page). If users repeatedly visit this page without completing the form, multiple orphaned auto-draft products will accumulate in the database.
Consider one of these approaches:
- Check for existing auto-draft products owned by the current user and reuse them
- Implement a cleanup job to remove abandoned auto-drafts older than X hours/days
- Only create the auto-draft on first form interaction (via AJAX)
|
|
||
| // Get attributes | ||
| $attributes = maybe_unserialize( get_post_meta( $post->ID, '_product_attributes', true ) ); | ||
| $product_object = wc_get_product( $post->ID ); |
There was a problem hiding this comment.
Add null check for product object.
If wc_get_product() returns false or null (e.g., when the post ID is invalid or the product doesn't exist), subsequent method calls on $product_object will cause fatal errors.
🔎 Proposed fix
$product_object = wc_get_product( $post->ID );
+
+ if ( ! $product_object ) {
+ return;
+ }📝 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.
| $product_object = wc_get_product( $post->ID ); | |
| $product_object = wc_get_product( $post->ID ); | |
| if ( ! $product_object ) { | |
| return; | |
| } |
🤖 Prompt for AI Agents
In includes/Product/functions.php around line 41, wc_get_product($post->ID) may
return null/false for invalid post IDs; add a null check after this call and
return early or skip subsequent product method calls when $product_object is
falsy to prevent fatal errors — e.g., verify $product_object is truthy before
using it and handle the error case (return, continue, or log) so no methods are
invoked on a null/false value.
| public static function product_category_selection_is_single() { | ||
| if ( ! dokan()->is_pro_exists() ) { | ||
| return 'single'; | ||
| } | ||
|
|
||
| return 'single' === dokan_get_option( 'product_category_style', 'dokan_selling', 'single' ); |
There was a problem hiding this comment.
Return type inconsistency: returns string instead of boolean.
The docblock declares @return boolean, but line 24 returns the string 'single' while line 27 returns an actual boolean. This inconsistency could cause issues with strict comparisons and changes the data type passed to JavaScript at line 279 for non-pro users.
🔎 Proposed fix
public static function product_category_selection_is_single() {
if ( ! dokan()->is_pro_exists() ) {
- return 'single';
+ return true;
}
return 'single' === dokan_get_option( 'product_category_style', 'dokan_selling', 'single' );
}📝 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.
| public static function product_category_selection_is_single() { | |
| if ( ! dokan()->is_pro_exists() ) { | |
| return 'single'; | |
| } | |
| return 'single' === dokan_get_option( 'product_category_style', 'dokan_selling', 'single' ); | |
| public static function product_category_selection_is_single() { | |
| if ( ! dokan()->is_pro_exists() ) { | |
| return true; | |
| } | |
| return 'single' === dokan_get_option( 'product_category_style', 'dokan_selling', 'single' ); | |
| } |
🤖 Prompt for AI Agents
In includes/ProductCategory/Helper.php around lines 22 to 27, the method
product_category_selection_is_single() returns a string ('single') for non-pro
users but returns a boolean for pro users, causing a return type mismatch;
change the non-pro branch to return a boolean (true) instead of the string so
both branches return a boolean consistent with the docblock and the JS
expectation.
| private static function get_items( string $type, string $class_name, string $sort_by = 'asc' ) { | ||
| $item_list = self::get_item_list( $type ); | ||
|
|
||
| if ( class_exists( $class_name ) && method_exists( $class_name, 'sort' ) ) { | ||
| /** | ||
| * @var Component $class_name | ||
| */ | ||
| uasort( | ||
| $item_list, | ||
| function ( $a, $b ) use ( $sort_by, $class_name ) { | ||
| return $class_name::sort( $a, $b, $sort_by ); | ||
| } | ||
| ); | ||
| } | ||
|
|
||
| return $item_list; | ||
| } |
There was a problem hiding this comment.
Class existence check uses unqualified name.
class_exists( $class_name ) on line 214 checks for 'Field' or 'Section' without the namespace, which will always return false. The namespace-qualified name should be used.
🔎 Proposed fix
private static function get_items( string $type, string $class_name, string $sort_by = 'asc' ) {
$item_list = self::get_item_list( $type );
+ $fq_class_name = __NAMESPACE__ . '\\' . $class_name;
- if ( class_exists( $class_name ) && method_exists( $class_name, 'sort' ) ) {
+ if ( class_exists( $fq_class_name ) && method_exists( $fq_class_name, 'sort' ) ) {
/**
* @var Component $class_name
*/
uasort(
$item_list,
- function ( $a, $b ) use ( $sort_by, $class_name ) {
- return $class_name::sort( $a, $b, $sort_by );
+ function ( $a, $b ) use ( $sort_by, $fq_class_name ) {
+ return $fq_class_name::sort( $a, $b, $sort_by );
}
);
}
return $item_list;
}📝 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.
| private static function get_items( string $type, string $class_name, string $sort_by = 'asc' ) { | |
| $item_list = self::get_item_list( $type ); | |
| if ( class_exists( $class_name ) && method_exists( $class_name, 'sort' ) ) { | |
| /** | |
| * @var Component $class_name | |
| */ | |
| uasort( | |
| $item_list, | |
| function ( $a, $b ) use ( $sort_by, $class_name ) { | |
| return $class_name::sort( $a, $b, $sort_by ); | |
| } | |
| ); | |
| } | |
| return $item_list; | |
| } | |
| private static function get_items( string $type, string $class_name, string $sort_by = 'asc' ) { | |
| $item_list = self::get_item_list( $type ); | |
| $fq_class_name = __NAMESPACE__ . '\\' . $class_name; | |
| if ( class_exists( $fq_class_name ) && method_exists( $fq_class_name, 'sort' ) ) { | |
| /** | |
| * @var Component $class_name | |
| */ | |
| uasort( | |
| $item_list, | |
| function ( $a, $b ) use ( $sort_by, $fq_class_name ) { | |
| return $fq_class_name::sort( $a, $b, $sort_by ); | |
| } | |
| ); | |
| } | |
| return $item_list; | |
| } |
🤖 Prompt for AI Agents
In includes/ProductForm/Factory.php around lines 211 to 227, the class_exists
check uses an unqualified class name (e.g., 'Field' or 'Section') which will
fail for namespaced classes; build a fully-qualified class name first (if
$class_name already contains a leading backslash use it as-is, otherwise prefix
with __NAMESPACE__ . '\\' . $class_name), then call class_exists and
method_exists against that FQCN and invoke the sort method using the FQCN (e.g.,
$fqcn::sort). Ensure the phpdoc var comment matches the FQCN usage.
| public function get_value( ...$value ) { | ||
| $callback = $this->get_value_callback(); | ||
| if ( ! empty( $callback ) && is_callable( $callback ) ) { | ||
| $value[] = $this->data['value']; | ||
| return call_user_func( $callback, ...$value ); | ||
| } | ||
|
|
||
| // if value is set under the field, return that value | ||
| if ( '' !== $this->data['value'] || empty( $value ) ) { | ||
| return $this->data['value']; | ||
| } | ||
|
|
||
| $product = current( $value ); | ||
| if ( ! $product instanceof \WC_Product ) { | ||
| return ''; | ||
| } | ||
|
|
||
| $field_name = sanitize_key( $this->get_name() ); | ||
| if ( $this->is_prop() && 0 === strpos( $field_name, '_' ) ) { | ||
| $field_name = preg_replace( '/_/', '', $field_name, 1 ); | ||
| } | ||
|
|
||
| if ( $this->is_prop() ) { | ||
| $method_name = 'get_' . $field_name; | ||
| return $product->{$method_name}(); | ||
| } elseif ( $this->is_meta() || $this->is_custom() ) { | ||
| return $product->get_meta( $field_name ); | ||
| } | ||
|
|
||
| return $value; | ||
| } |
There was a problem hiding this comment.
Fix inconsistent return type in get_value fallback.
Line 164 returns $value (an array from the variadic parameter) when the field type doesn't match prop/meta/custom. This creates an inconsistent return type—the method should return a scalar value or empty string in the fallback case.
🔎 Proposed fix
- return $value;
+ return '';🤖 Prompt for AI Agents
In includes/ProductForm/Field.php around lines 135 to 165, the final fallback
currently returns $value (an array) which causes an inconsistent return type;
change the fallback to return an empty string (or another scalar/nullable
consistent type) instead of $value so the method always returns a scalar or
empty string in error/fallback cases—replace the final return $value; with
return ''; and ensure no other paths return arrays.
| $edit_product_url = dokan_locate_template( 'products/edit/edit-product-single.php' ); | ||
|
|
||
| return apply_filters( 'dokan_get_product_edit_template', $edit_product_url ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine the Rewrites.php file at lines 269-271
find . -name "Rewrites.php" -type f | head -5Repository: getdokan/dokan
Length of output: 81
🏁 Script executed:
# Also search for dokan_locate_template function definition
rg "function dokan_locate_template" -A 10Repository: getdokan/dokan
Length of output: 704
🏁 Script executed:
# Look for references to the old template path
rg "products/edit-product-single.php" -nRepository: getdokan/dokan
Length of output: 3044
🏁 Script executed:
# Look for references to the new template path
rg "products/edit/edit-product-single.php" -nRepository: getdokan/dokan
Length of output: 177
🏁 Script executed:
# Check if there are any backward compatibility checks or fallbacks in template handling
rg "dokan_locate_template|locate_template" -A 5 --type php | grep -A 5 "fallback\|old\|compat\|deprecated"Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
# Check if template files exist
find . -path "*/templates/products/edit*" -type f | head -20Repository: getdokan/dokan
Length of output: 462
🏁 Script executed:
# Check the current Rewrites.php file around line 269
cat -n includes/Rewrites.php | sed -n '260,280p'Repository: getdokan/dokan
Length of output: 724
🏁 Script executed:
# Search for any git history or comments about template reorganization
rg "edit-product-single|edit/edit-product" --type php -B 2 -A 2Repository: getdokan/dokan
Length of output: 1066
🏁 Script executed:
# Check the content of both template files to see if old one is a wrapper
wc -l templates/products/edit-product-single.php templates/products/edit/edit-product-single.phpRepository: getdokan/dokan
Length of output: 172
🏁 Script executed:
# Show first 20 lines of both to understand their relationship
echo "=== OLD FLAT PATH ===" && head -20 templates/products/edit-product-single.php && echo -e "\n=== NEW NESTED PATH ===" && head -20 templates/products/edit/edit-product-single.phpRepository: getdokan/dokan
Length of output: 1207
🏁 Script executed:
# Get full dokan_locate_template implementation
cat -n includes/functions.php | sed -n '/function dokan_locate_template/,/^[0-9]*-}/p' | head -50Repository: getdokan/dokan
Length of output: 1749
🏁 Script executed:
# Check if there's a BC wrapper or fallback mechanism
rg "products/edit-product-single" --type php -B 3 -A 3Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
# Check if old template file includes or redirects to the new one
tail -50 templates/products/edit-product-single.phpRepository: getdokan/dokan
Length of output: 1093
🏁 Script executed:
# Search for any includes or references from old to new template
rg "get_template_part|include|require" templates/products/edit-product-single.php -A 2Repository: getdokan/dokan
Length of output: 344
🏁 Script executed:
# Find all references to old flat template path in PHP code
rg "edit-product-single\.php" --type php -n | grep -v ".pot"Repository: getdokan/dokan
Length of output: 333
🏁 Script executed:
# Check if there's documentation about this change - look for migration guides, changelog
find . -name "CHANGELOG*" -o -name "MIGRATION*" -o -name "UPGRADE*" | head -10Repository: getdokan/dokan
Length of output: 72
🏁 Script executed:
# Check the version in readme or main file to understand if this is a major version change
cat readme.txt | head -30Repository: getdokan/dokan
Length of output: 1993
🏁 Script executed:
# Look for any deprecation notices or migration notes in the codebase
rg "deprecated|migration|BC|backward" -i --type php | head -20Repository: getdokan/dokan
Length of output: 3489
🏁 Script executed:
# Verify if old template is still being loaded anywhere
rg "edit-product-single" --type php -w -B 2 -A 2 | grep -v ".pot" | grep -v whats-newRepository: getdokan/dokan
Length of output: 722
Template path change breaks existing theme overrides without fallback
The template lookup path changed from products/edit-product-single.php to products/edit/edit-product-single.php. This means themes with overrides at dokan/products/edit-product-single.php will no longer be used—they will silently fall back to the plugin default instead of raising any error or warning.
Since both template files exist in the plugin but only the new nested path is being looked up, integrators customizing this template have no way to know their override is broken unless they manually test. Consider adding a fallback in dokan_locate_template() or providing explicit migration documentation for this structural change.
| resetAllFields( sectionId ) { | ||
| let settingFields = Object.entries( this.settingFields[sectionId] ); | ||
| let clonedSectionSettingValues = JSON.parse( JSON.stringify( this.settingValues[sectionId] ) ); | ||
| settingFields.forEach( ( key, value ) => { | ||
| let blockId = key[0]; | ||
| if ( blockId ) { | ||
| let sectionValue = key[1]; | ||
| for (const [FieldId, fieldData] of Object.entries(sectionValue.fields)) { | ||
| clonedSectionSettingValues[blockId]['fields'][FieldId]['visibility'] = fieldData.default.visibility; | ||
| clonedSectionSettingValues[blockId]['fields'][FieldId]['required'] = fieldData.default.required; | ||
| } | ||
| } | ||
| } ); | ||
| this.settingValues[sectionId] = clonedSectionSettingValues; | ||
| }, |
There was a problem hiding this comment.
Add defensive checks to prevent runtime errors.
The resetAllFields method accesses nested properties without null/undefined guards. If sectionValue.fields, fieldData.default, or the nested path doesn't exist, this will throw a runtime error.
Additionally, the forEach callback parameters are misleadingly named—key is actually the entry tuple [blockId, sectionValue], and value is the array index.
🔎 Proposed fix with defensive checks and clearer naming
resetAllFields( sectionId ) {
let settingFields = Object.entries( this.settingFields[sectionId] );
let clonedSectionSettingValues = JSON.parse( JSON.stringify( this.settingValues[sectionId] ) );
- settingFields.forEach( ( key, value ) => {
- let blockId = key[0];
-
- if ( blockId ) {
- let sectionValue = key[1];
- for (const [FieldId, fieldData] of Object.entries(sectionValue.fields)) {
- clonedSectionSettingValues[blockId]['fields'][FieldId]['visibility'] = fieldData.default.visibility;
- clonedSectionSettingValues[blockId]['fields'][FieldId]['required'] = fieldData.default.required;
+ settingFields.forEach( ( [blockId, sectionValue] ) => {
+ if ( blockId && sectionValue?.fields ) {
+ for ( const [fieldId, fieldData] of Object.entries( sectionValue.fields ) ) {
+ if ( fieldData?.default && clonedSectionSettingValues[blockId]?.fields?.[fieldId] ) {
+ clonedSectionSettingValues[blockId]['fields'][fieldId]['visibility'] = fieldData.default.visibility;
+ clonedSectionSettingValues[blockId]['fields'][fieldId]['required'] = fieldData.default.required;
+ }
}
}
} );
this.settingValues[sectionId] = clonedSectionSettingValues;
},🤖 Prompt for AI Agents
In src/admin/pages/Settings.vue around lines 716 to 733, resetAllFields iterates
entries but uses misleading parameter names and accesses nested properties
(sectionValue.fields, fieldData.default,
clonedSectionSettingValues[blockId].fields[FieldId]) without guards which can
throw if any are undefined; rename the forEach parameters to (entry) or
destructure as ([blockId, sectionValue]) for clarity, add defensive checks
before using nested properties (ensure sectionValue && sectionValue.fields;
ensure fieldData && fieldData.default; ensure
clonedSectionSettingValues[blockId] and its .fields exist) and skip or continue
when a required path is missing, then apply the default visibility/required only
when all checks pass so the function no longer throws on missing data.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
includes/Assets.php (1)
699-706: Usewp_json_encode()instead ofjson_encode().Per WordPress coding standards and PHPCS warning,
wp_json_encode()should be used as it handles encoding errors gracefully and ensures proper character encoding. Additionally, consider adding a return type hint and docblock for clarity.🔎 Proposed fix
+ /** + * Get product form fields as JSON string. + * + * @since DOKAN_SINCE + * + * @return string JSON-encoded product form fields. + */ - public function get_product_fields() { + public function get_product_fields(): string { $temp_fields = []; foreach ( ProductFormFactory::get_fields() as $field_id => $field ) { $temp_fields[ $field_id ] = $field->toArray(); } - return json_encode( $temp_fields ); + return wp_json_encode( $temp_fields ); }
🧹 Nitpick comments (1)
includes/Assets.php (1)
1232-1255: LGTM with noted TODO.The variation-related i18n strings and security nonces are properly implemented. The TODO comment on lines 1232-1233 notes that these strings should be moved to Dokan Pro Assets class after the product form manager module release.
Would you like me to open an issue to track moving these variation-related strings to the Dokan Pro Assets class?
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
dokan-class.phpincludes/Assets.php
🚧 Files skipped from review as they are similar to previous changes (1)
- dokan-class.php
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-19T06:19:41.376Z
Learnt from: mrabbani
Repo: getdokan/dokan PR: 2891
File: src/admin/dashboard/pages/vendors-single/InformationTabs/WithdrawTab.tsx:58-59
Timestamp: 2025-09-19T06:19:41.376Z
Learning: The Dokan codebase uses 'dokan-lite' as the text domain for WordPress internationalization (__() function calls).
Applied to files:
includes/Assets.php
🧬 Code graph analysis (1)
includes/Assets.php (3)
includes/ProductForm/Factory.php (2)
Factory(15-294)get_fields(108-110)includes/ProductForm/Section.php (1)
get_fields(63-68)includes/ProductForm/Component.php (1)
toArray(65-67)
🪛 GitHub Check: Run PHPCS inspection
includes/Assets.php
[warning] 705-705:
json_encode() is discouraged. Use wp_json_encode() instead.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (5)
includes/Assets.php (5)
10-10: LGTM!The import alias for
ProductFormFactoryfollows the existing naming conventions in the file and is correctly referenced in the newget_product_fields()method.
964-968: LGTM!The sweetalert localization strings are properly internationalized using the correct
'dokan-lite'text domain. Based on learnings, this is the standard text domain for the Dokan codebase.
1170-1184: LGTM!The i18n strings for product form UI are properly internationalized using the correct text domain and escaping where appropriate.
1401-1401: LGTM!The
product_typeskey is now consistently exposed in both admin and front-end localized scripts, allowing the admin Vue app to access the same filtered product types as the front-end.
787-787: Verify: JSON string passed towp_localize_script()may cause double-encoding.The
get_product_fields()method returns a JSON-encoded string (line 705), which is then passed towp_localize_script()(line 787). Sincewp_localize_script()internally JSON-encodes all values, the front-end will receive a double-encoded string rather than a JavaScript object. This would require explicitJSON.parse()on the front-end to use the value as an object.However, no usage of
product_form_fieldswas found in the source JavaScript files. The impact depends on whether the front-end code is designed to handle this pattern (e.g., through compiled Vue components or dynamic code not visible in the source repository).
| 'dokan-util-helper' => [ | ||
| 'src' => $asset_url . '/js/helper.js', | ||
| 'deps' => [ 'jquery', 'dokan-sweetalert2', 'moment' ], | ||
| 'deps' => [ 'jquery', 'dokan-sweetalert2', 'moment', 'jquery-tiptip' ], | ||
| 'version' => filemtime( $asset_path . 'js/helper.js' ), | ||
| 'in_footer' => false, | ||
| ], |
There was a problem hiding this comment.
Potential issue: Hardcoded jquery-tiptip dependency may break on WC 10.3.0+.
The dokan-util-helper script uses a hardcoded 'jquery-tiptip' dependency, but WooCommerce 10.3.0+ renamed this handle to 'wc-jquery-tiptip'. Other scripts in this file (e.g., dokan-script on line 512) correctly use the dynamic $jquery_tiptip variable obtained from self::get_wc_handler().
Consider using the dynamic handler for consistency:
🔎 Proposed fix
'dokan-util-helper' => [
'src' => $asset_url . '/js/helper.js',
- 'deps' => [ 'jquery', 'dokan-sweetalert2', 'moment', 'jquery-tiptip' ],
+ 'deps' => [ 'jquery', 'dokan-sweetalert2', 'moment', $jquery_tiptip ],
'version' => filemtime( $asset_path . 'js/helper.js' ),
'in_footer' => false,
],📝 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.
| 'dokan-util-helper' => [ | |
| 'src' => $asset_url . '/js/helper.js', | |
| 'deps' => [ 'jquery', 'dokan-sweetalert2', 'moment' ], | |
| 'deps' => [ 'jquery', 'dokan-sweetalert2', 'moment', 'jquery-tiptip' ], | |
| 'version' => filemtime( $asset_path . 'js/helper.js' ), | |
| 'in_footer' => false, | |
| ], | |
| 'dokan-util-helper' => [ | |
| 'src' => $asset_url . '/js/helper.js', | |
| 'deps' => [ 'jquery', 'dokan-sweetalert2', 'moment', $jquery_tiptip ], | |
| 'version' => filemtime( $asset_path . 'js/helper.js' ), | |
| 'in_footer' => false, | |
| ], |
🤖 Prompt for AI Agents
In includes/Assets.php around lines 545 to 550, the 'dokan-util-helper'
registration hardcodes the 'jquery-tiptip' dependency which breaks on WC
10.3.0+; replace that hardcoded string with the dynamic $jquery_tiptip handler
obtained from self::get_wc_handler() (like other registrations do), ensuring
$jquery_tiptip is retrieved earlier in the method and then used in the 'deps'
array for 'dokan-util-helper'.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
includes/DependencyManagement/Providers/ServiceProvider.php
🧰 Additional context used
🧬 Code graph analysis (1)
includes/DependencyManagement/Providers/ServiceProvider.php (1)
includes/ProductForm/Init.php (1)
Init(15-740)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: api tests (1, 1)
| 'widgets' => \WeDevs\Dokan\Widgets\Manager::class, | ||
| 'admin_notices' => \WeDevs\Dokan\Admin\Notices\Manager::class, | ||
| 'tracker' => \WeDevs\Dokan\Tracker::class, | ||
| 'form_manager' => \WeDevs\Dokan\ProductForm\Init::class, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for form_manager service retrieval/instantiation
echo "=== Searching for form_manager service usage ==="
rg -n --type=php "(get|make)\s*\(\s*['\"]form_manager['\"]" --max-count=20
echo ""
echo "=== Searching for ProductForm\Init references ==="
rg -n --type=php "ProductForm" --max-count=30
echo ""
echo "=== Searching for service container initialization patterns ==="
rg -n --type=php "\-\>get\(|->make\(" | grep -i "form\|init\|manager" | head -20Repository: getdokan/dokan
Length of output: 5545
🏁 Script executed:
#!/bin/bash
# Check the Init class and its hook registration
echo "=== Init class constructor and init action hook ==="
rg -n --type=php -A 20 "class Init" includes/ProductForm/Init.php
echo ""
echo "=== Check for any bootstrap or initialization of services ==="
fd -e php -type f | xargs rg -n "form_manager|ProductForm" includes/ | grep -E "(->make|->get|container|bootstrap|Initialize|init)" | head -20
echo ""
echo "=== Check if ProductFormFactory depends on Init being instantiated ==="
rg -n --type=php -B 5 -A 15 "class Factory" includes/ProductForm/Factory.php | head -40Repository: getdokan/dokan
Length of output: 1605
🏁 Script executed:
#!/bin/bash
# Search for all references to init_form_fields
echo "=== Searching for init_form_fields method calls ==="
rg -n "init_form_fields" --type=php
echo ""
echo "=== Check entire Init class ==="
cat -n includes/ProductForm/Init.phpRepository: getdokan/dokan
Length of output: 36076
🏁 Script executed:
#!/bin/bash
# Search for bootstrap or initialization patterns that might instantiate services
echo "=== Checking main plugin files ==="
find . -maxdepth 2 -name "*.php" | xargs grep -l "dokan()" | head -5
echo ""
echo "=== Look for service container initialization ==="
rg -n "container|register_services|boot|initialize" --type=php includes/ | grep -i "provider\|service\|container" | head -20
echo ""
echo "=== Check for any $container->get calls ==="
rg -n "\->get\(" --type=php includes/ | head -10Repository: getdokan/dokan
Length of output: 3553
🏁 Script executed:
#!/bin/bash
# Check main plugin file for service provider initialization
echo "=== Check dokan.php for service provider setup ==="
grep -n -A 10 -B 2 "ServiceProvider\|register" dokan.php | head -40
echo ""
echo "=== Search for any place where form_manager might be instantiated ==="
rg -n "form_manager|form-manager" --type=php
echo ""
echo "=== Check ServiceProvider register method ==="
grep -n -A 30 "public function register" includes/DependencyManagement/Providers/ServiceProvider.phpRepository: getdokan/dokan
Length of output: 923
Remove the unused form_manager service registration or ensure it's instantiated early in the application lifecycle.
The form_manager service is registered in the container with addShared() but is never requested anywhere in the codebase. Since it uses lazy-loading, the Init class constructor—which hooks into the WordPress 'init' action to register form fields—never fires. This means the form field initialization code never executes.
Either remove this service registration if it's not needed, or add code to the application bootstrap that explicitly requests the service early (e.g., dokan_get_container()->get('form_manager')) to ensure the 'init' hook fires during the WordPress initialization sequence.
🤖 Prompt for AI Agents
In includes/DependencyManagement/Providers/ServiceProvider.php around line 52,
the 'form_manager' service is registered but never instantiated, so its Init
class never hooks into WordPress 'init' and form fields are not registered;
either remove this unused addShared('form_manager') registration, or ensure it
is requested during bootstrap by calling the container to resolve it early (for
example, in the plugin bootstrap or an existing initialization routine invoke
dokan_get_container()->get('form_manager') so the Init constructor runs and
registers the 'init' hook).
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
includes/Assets.php (1)
554-554: Hardcodedjquery-tiptipdependency (previously flagged).This was flagged in a previous review - use the dynamic
$jquery_tiptipvariable for WC 10.3.0+ compatibility.
🧹 Nitpick comments (4)
includes/Product/Hooks.php (1)
391-398: Unused$productparameter.The
$productparameter is declared but not used. If this is intentional for filter signature consistency, consider prefixing with underscore ($_product) to indicate it's intentionally unused. Otherwise, verify if it should be used in the logic.🔎 Proposed fix (if intentionally unused)
- public function set_new_product_email_status( array $meta_data, WC_Product $product, bool $is_new_product ) { + public function set_new_product_email_status( array $meta_data, WC_Product $_product, bool $is_new_product ) {includes/Assets.php (1)
725-726: Unused loop variable$key.The
$keyvariable in the foreach loop is never used.🔎 Proposed fix
- foreach ( $sections as $key => $section ) { + foreach ( $sections as $section ) {src/dashboard/form-manager/components/FieldRenderer.tsx (2)
148-152: EmptyFieldRenderercomponent appears unused.The
FieldRenderercomponent returnsnulland doesn't appear to be used anywhere. If it's a placeholder for future implementation, consider adding a TODO comment. Otherwise, consider removing it and only exportinggetFieldConfigandCustomField.
31-60: Consider adding a remove/clear button for uploaded images.The
ImageEditcomponent allows uploading but doesn't provide a way to remove an uploaded image. Users may need to clear the selection.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.gitignoreincludes/Assets.phpincludes/Product/Hooks.phpsrc/dashboard/form-manager/ProductForm.tsxsrc/dashboard/form-manager/components/FieldRenderer.tsxsrc/dashboard/form-manager/index.csssrc/dashboard/form-manager/index.tsxsrc/dashboard/form-manager/types.tssrc/vendor-dashboard/reports/index.jswebpack-entries.js
💤 Files with no reviewable changes (1)
- src/vendor-dashboard/reports/index.js
✅ Files skipped from review due to trivial changes (2)
- src/dashboard/form-manager/index.css
- .gitignore
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-19T06:19:41.376Z
Learnt from: mrabbani
Repo: getdokan/dokan PR: 2891
File: src/admin/dashboard/pages/vendors-single/InformationTabs/WithdrawTab.tsx:58-59
Timestamp: 2025-09-19T06:19:41.376Z
Learning: The Dokan codebase uses 'dokan-lite' as the text domain for WordPress internationalization (__() function calls).
Applied to files:
includes/Assets.php
🧬 Code graph analysis (3)
src/dashboard/form-manager/ProductForm.tsx (3)
src/dashboard/form-manager/types.ts (1)
Section(3-9)src/dashboard/form-manager/components/FieldRenderer.tsx (1)
getFieldConfig(62-146)src/components/index.tsx (1)
DataForm(6-6)
src/dashboard/form-manager/components/FieldRenderer.tsx (1)
src/dashboard/form-manager/types.ts (2)
FormField(11-26)Section(3-9)
src/dashboard/form-manager/index.tsx (1)
src/vendor-dashboard/reports/index.js (1)
root(31-31)
🪛 ast-grep (0.40.3)
src/dashboard/form-manager/components/FieldRenderer.tsx
[warning] 64-64: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
[warning] 67-67: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
src/dashboard/form-manager/components/FieldRenderer.tsx
[error] 98-108: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 65-65: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 68-68: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🪛 PHPMD (2.15.0)
includes/Product/Hooks.php
391-391: Avoid unused parameters such as '$product'. (undefined)
(UnusedFormalParameter)
includes/Assets.php
725-725: Avoid unused local variables such as '$key'. (undefined)
(UnusedLocalVariable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (12)
src/dashboard/form-manager/index.tsx (1)
1-9: LGTM!Clean entry point implementation. The null check before mounting is good defensive coding, and the pattern matches the existing codebase conventions (similar to
src/vendor-dashboard/reports/index.js).webpack-entries.js (1)
89-89: LGTM!The new
form-managerentry point follows the existing naming conventions and path patterns used by other dashboard entries.src/dashboard/form-manager/ProductForm.tsx (1)
8-69: Component structure looks reasonable.The use of
useMemofor computed values anduseCallbackfor the stableonChangehandler follows React best practices. The empty dependency arrays are intentional sincesectionsis a module-level constant.includes/Product/Hooks.php (2)
59-61: LGTM!Simple container element for the React mount point. The static HTML output is appropriate here.
36-36: This backward compatibility concern is unfounded.The hook registration at line 36 is new—there is no prior action hook with the same callback. Search results show no previous
add_actionforset_new_product_email_statusand the old action hookdokan_new_product_addedremains intact and unchanged elsewhere in the codebase. The filterdokan_product_edit_meta_dataand its callback are introduced together without breaking external dependencies.Likely an incorrect or invalid review comment.
includes/Assets.php (4)
780-787: Usewp_json_encode()instead ofjson_encode()(previously flagged).This was addressed in a previous review comment. The code now uses
wp_json_encode()at line 786, which is correct.
384-388: LGTM!Style registration follows existing conventions with proper versioning using
filemtime().
703-711: LGTM!Script registration follows existing patterns, including merging asset file dependencies with additional component dependencies.
716-778: Robust implementation with proper error handling.Good defensive coding with
is_wp_error()checks and try-catch for value retrieval. The method properly handles missing products and invisible fields.src/dashboard/form-manager/components/FieldRenderer.tsx (2)
4-17: LGTM!Clean layout wrapper component with proper TypeScript prop types.
121-143: Visibility logic implementation looks correct.The
isVisiblefunction properly handles thedependency_conditionwithequalandnot_equaloperators. Defaults totruewhen no conditions apply.src/dashboard/form-manager/types.ts (1)
1-26: FormField type definition doesn't naturally align with Field and requires transformation.While
FormFieldextendsField<any>, the properties don't directly correspond:FormFieldusesfield_type(nottype),title(notlabel), and lacks several Field-specific properties likegetValue,render, orEdit. ThegetFieldConfigfunction must actively map these properties (field_type→type,title→label,help_content→description) to achieve compatibility.Consider either:
- Removing the
& Field<any>intersection and explicitly defining the actual data structure, or- Creating a helper type that clarifies the transformation needed from FormField to Field
Using
Field<any>without proper property alignment reduces type safety without providing the intended benefit.
includes/Assets.php
Outdated
| // load the form manager | ||
| wp_enqueue_script( 'dokan-product-form-manager' ); | ||
| wp_enqueue_style( 'dokan-product-form-manager' ); | ||
| wp_localize_script( | ||
| 'dokan-product-form-manager', | ||
| 'dokanFormManager', | ||
| [ | ||
| 'sections' => $this->get_form_fields(), | ||
| ] | ||
| ); |
There was a problem hiding this comment.
Form manager scripts are loaded unconditionally on all frontend pages.
The form manager scripts are enqueued in enqueue_front_scripts() without any conditional check. This adds unnecessary payload on pages where the product form isn't displayed.
🔎 Proposed fix
+ // Load form manager only on product edit pages
+ global $wp;
+ if ( isset( $wp->query_vars['products'] ) && isset( $_GET['product_id'] ) ) {
// load the form manager
wp_enqueue_script( 'dokan-product-form-manager' );
wp_enqueue_style( 'dokan-product-form-manager' );
wp_localize_script(
'dokan-product-form-manager',
'dokanFormManager',
[
'sections' => $this->get_form_fields(),
]
);
+ }🤖 Prompt for AI Agents
In includes/Assets.php around lines 982–991, the product form manager scripts
are enqueued unconditionally causing extra payload on pages that don't show the
form; wrap the wp_enqueue_script/wp_enqueue_style/wp_localize_script calls in a
conditional that detects when the product form will be rendered (for example use
an existing Dokan helper like dokan_is_product_edit_page() or check
is_singular('product')/a specific page template/shortcode presence or a
configurable filter), so only enqueue and localize the script when that
detection returns true.
| import { getFieldConfig } from './components/FieldRenderer'; | ||
| import { Section } from './types'; | ||
|
|
||
| const sections = ( window as any ).dokanFormManager.sections as Section[]; |
There was a problem hiding this comment.
Add null safety for global data access.
If window.dokanFormManager is undefined (e.g., script loaded on an unexpected page or before localization), this will throw a runtime error.
🔎 Proposed fix
-const sections = ( window as any ).dokanFormManager.sections as Section[];
+const sections = ( ( window as any ).dokanFormManager?.sections ?? [] ) as Section[];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const sections = ( window as any ).dokanFormManager.sections as Section[]; | |
| const sections = ( ( window as any ).dokanFormManager?.sections ?? [] ) as Section[]; |
🤖 Prompt for AI Agents
In src/dashboard/form-manager/ProductForm.tsx around line 6, accessing (window
as any).dokanFormManager.sections without null checks can throw if
dokanFormManager is undefined; update the code to safely access this global by
using optional chaining or a guard (e.g., const sections = ((window as
any).dokanFormManager?.sections ?? []) as Section[]), or check for
window.dokanFormManager before reading sections and provide a sensible default
(empty array) and/or log a warning so the component handles missing global data
safely.
| // eslint-disable-next-line no-console | ||
| console.log( { 'Form Layout:': formLayout, fields: fields } ); |
There was a problem hiding this comment.
Remove debug console.log before merging.
This logging statement will clutter the browser console in production.
🔎 Proposed fix
- // eslint-disable-next-line no-console
- console.log( { 'Form Layout:': formLayout, fields: fields } );
-
return (📝 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.
| // eslint-disable-next-line no-console | |
| console.log( { 'Form Layout:': formLayout, fields: fields } ); | |
| return ( |
🤖 Prompt for AI Agents
In src/dashboard/form-manager/ProductForm.tsx around lines 56 to 57, remove the
debug console.log statement that prints formLayout and fields so it doesn't
clutter the browser console in production; if runtime debug info is still
needed, replace it with a proper logger.debug or wrap the logging behind a
NODE_ENV check (only log when process.env.NODE_ENV !== 'production').
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
includes/Assets.php (2)
552-557: Hardcodedjquery-tiptipdependency may break on WC 10.3.0+.The
$jquery_tiptipvariable (defined at line 401) should be used instead of the hardcoded string to maintain compatibility with WooCommerce 10.3.0+ which renamed the handle towc-jquery-tiptip.🔎 Proposed fix
'dokan-util-helper' => [ 'src' => $asset_url . '/js/helper.js', - 'deps' => [ 'jquery', 'dokan-sweetalert2', 'moment', 'jquery-tiptip' ], + 'deps' => [ 'jquery', 'dokan-sweetalert2', 'moment', $jquery_tiptip ], 'version' => filemtime( $asset_path . 'js/helper.js' ), 'in_footer' => false, ],
981-990: Form manager scripts loaded unconditionally on all frontend pages.The form manager scripts and styles are enqueued without checking if the user is on a product edit page. This adds unnecessary JavaScript/CSS payload on all frontend pages. Consider wrapping these enqueues in a conditional check similar to the category UI loading at lines 926-929.
🔎 Proposed fix
+ // Load form manager only on product edit pages + if ( isset( $wp->query_vars['products'] ) && isset( $_GET['product_id'] ) ) { // phpcs:ignore // load the form manager wp_enqueue_script( 'dokan-product-form-manager' ); wp_enqueue_style( 'dokan-product-form-manager' ); wp_localize_script( 'dokan-product-form-manager', 'dokanFormManager', [ 'sections' => $this->get_form_fields(), ] ); + }
🧹 Nitpick comments (1)
includes/Assets.php (1)
779-786: Add return type hint for consistency.The
wp_json_encode()fix from the previous review has been applied. Consider adding the return type hint for consistency with PHP best practices.🔎 Proposed fix
- public function get_product_fields() { + public function get_product_fields(): string { $temp_fields = []; foreach ( ProductFormFactory::get_fields() as $field_id => $field ) { $temp_fields[ $field_id ] = $field->toArray(); } return wp_json_encode( $temp_fields ); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
includes/Assets.php
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-19T06:19:41.376Z
Learnt from: mrabbani
Repo: getdokan/dokan PR: 2891
File: src/admin/dashboard/pages/vendors-single/InformationTabs/WithdrawTab.tsx:58-59
Timestamp: 2025-09-19T06:19:41.376Z
Learning: The Dokan codebase uses 'dokan-lite' as the text domain for WordPress internationalization (__() function calls).
Applied to files:
includes/Assets.php
🧬 Code graph analysis (1)
includes/Assets.php (3)
includes/ProductForm/Field.php (6)
Field(12-533)get_value(135-165)get_name(81-83)get_placeholder(403-405)get_field_type(376-378)get_options(215-228)includes/ProductForm/Factory.php (3)
Factory(15-294)get_sections(164-166)get_fields(108-110)includes/ProductForm/Component.php (1)
is_visible(199-201)
🪛 GitHub Check: Run PHPCS inspection
includes/Assets.php
[failure] 721-721:
Functions must not contain multiple empty lines in a row; found 2 empty lines
🪛 PHPMD (2.15.0)
includes/Assets.php
725-725: Avoid unused local variables such as '$key'. (undefined)
(UnusedLocalVariable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (5)
includes/Assets.php (5)
8-12: LGTM!The new imports for
Field,Section, andProductFormFactoryare correctly added and appropriately used in the newget_form_fields()andget_product_fields()methods.
384-388: LGTM!The form manager style registration is correctly configured. The CSS file path in the
/js/directory is typical for bundled assets from a JS build process.
703-711: LGTM!The form manager script registration follows the established pattern for dynamic asset loading, correctly using the asset manifest file for versioning and dependencies.
1323-1346: LGTM!The variation-related i18n strings and nonces are properly implemented:
- All strings are correctly escaped with
esc_js()- Nonces are properly created for each variation operation
- Text domain
'dokan-lite'is correctly used (consistent with codebase conventions)
1492-1492: LGTM!The
product_typesaddition to admin localized scripts maintains consistency with the frontend localization at line 866, ensuring both admin and frontend contexts have access to the same product type configuration.
includes/Assets.php
Outdated
| public function get_form_fields() { | ||
| $product_id = isset( $_GET['product_id'] ) ? intval( $_GET['product_id'] ) : 0; // phpcs:ignore | ||
| $product = wc_get_product( $product_id ); | ||
|
|
||
| $sections_data = []; | ||
|
|
||
|
|
||
| $sections = ProductFormFactory::get_sections(); | ||
|
|
||
| foreach ( $sections as $key => $section ) { | ||
| /** @var Section $section */ | ||
| if ( is_wp_error( $section ) ) { | ||
| continue; | ||
| } | ||
|
|
||
| $fields = []; | ||
|
|
||
| foreach ( $section->get_fields() as $field ) { | ||
| /** @var Field $field */ | ||
| if ( is_wp_error( $field ) || ! $field->is_visible() ) { | ||
| continue; | ||
| } | ||
|
|
||
| $value = ''; | ||
| if ( $product ) { | ||
| try { | ||
| $value = $field->get_value( $product ); | ||
| } catch ( \Throwable $e ) { | ||
| $value = ''; | ||
| } | ||
| } | ||
|
|
||
| $field_data = [ | ||
| 'id' => $field->get_id(), | ||
| 'name' => $field->get_name(), | ||
| 'title' => $field->get_title(), | ||
| 'help' => $field->get_help_content(), | ||
| 'placeholder' => $field->get_placeholder(), | ||
| 'help_content' => $field->get_help_content(), | ||
| 'description' => $field->get_description(), | ||
| 'required' => $field->is_required(), | ||
| 'value' => $value, | ||
| 'field_type' => $field->get_field_type(), | ||
| 'options' => method_exists( $field, 'get_options' ) ? $field->get_options( $product ) : [], | ||
| 'errors' => $field->get_error_message(), | ||
| 'visibility' => $field->is_visible(), | ||
| 'dependency_condition' => $field->get_dependency_condition(), | ||
| ]; | ||
|
|
||
| $fields[] = $field_data; | ||
| } | ||
|
|
||
| $sections_data[] = [ | ||
| 'id' => $section->get_id(), | ||
| 'title' => $section->get_title(), | ||
| 'order' => $section->get_order(), | ||
| 'fields' => $fields, | ||
| 'description' => $section->get_description(), | ||
| ]; | ||
| } | ||
| return $sections_data; | ||
| } |
There was a problem hiding this comment.
Address code quality issues in get_form_fields().
Several issues identified by static analysis and code review:
- Line 721-722: Multiple empty lines violate PHPCS rules.
- Line 725: Unused variable
$key- use$sectiondirectly or prefix with underscore. - Lines 741-745: Silently catching all
Throwableexceptions may hide bugs. Consider logging or limiting to specific exceptions. - Missing return type hint: Add
: arrayreturn type for clarity.
🔎 Proposed fix
- public function get_form_fields() {
+ public function get_form_fields(): array {
$product_id = isset( $_GET['product_id'] ) ? intval( $_GET['product_id'] ) : 0; // phpcs:ignore
$product = wc_get_product( $product_id );
$sections_data = [];
-
-
$sections = ProductFormFactory::get_sections();
- foreach ( $sections as $key => $section ) {
+ foreach ( $sections as $section ) {
/** @var Section $section */
if ( is_wp_error( $section ) ) {
continue;
}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[failure] 721-721:
Functions must not contain multiple empty lines in a row; found 2 empty lines
🪛 PHPMD (2.15.0)
725-725: Avoid unused local variables such as '$key'. (undefined)
(UnusedLocalVariable)
🤖 Prompt for AI Agents
In includes/Assets.php around lines 716 to 777, tidy up get_form_fields():
remove the extra blank lines at ~721-722, stop declaring the unused $key in the
foreach (use foreach ( $sections as $section ) or rename to $_key), add an
explicit return type ": array" to the method signature, and replace the broad
catch (\Throwable $e) with a narrower exception type if possible or at minimum
log the caught exception (e.g. process/product logger) instead of silently
swallowing it so errors are visible during debugging.
kzamanbd
force-pushed
the
feat/product-form-manager
branch
from
cfc9e8f to
35c15d7
Compare
Previous Pull Request #2098
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
Title
Detailed Description of the pull request. What was previous behaviour
and what will be changed in this PR.
Before Changes
Describe the issue before changes with screenshots(s).
After Changes
Describe the issue after changes with screenshot(s).
Feature Video (optional)
Link of detailed video if this PR is for a feature.
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit
New Features
Improvements
Removed
✏️ Tip: You can customize this high-level summary in your review settings.