Conversation
Introduced a new product form manager with React components and supporting PHP classes. Removed legacy Vue admin scripts and updated related assets, REST controller, and templates to integrate the new form manager.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughA comprehensive product form manager system is introduced, featuring React dashboard components for vendor product editing, PHP backend classes for form schema management, REST API endpoints for data persistence, and admin settings interface for controlling form behavior across product types. Changes
Sequence Diagram(s)sequenceDiagram
actor Admin
participant Settings as Admin Settings
participant API as WordPress REST API
participant DB as Database
Admin->>Settings: Open Form Manager Settings
Settings->>Settings: Load dokan_form_manager section
Admin->>Settings: Toggle field visibility/required
Settings->>API: POST form field config changes
API->>DB: Save field metadata
DB-->>API: Confirm
API-->>Settings: Success toast
Settings-->>Admin: Update UI
Admin->>Settings: Click Reset Fields
Settings->>Settings: Clone & reset field defaults
Settings->>API: POST reset request
API->>DB: Update all fields to defaults
DB-->>API: Confirm
API-->>Settings: Success notification
sequenceDiagram
actor Vendor
participant FormManager as Form Manager UI
participant FormContext as Form Context
participant API as dokan/v3 API
participant DB as Database
Vendor->>FormManager: Open product edit form
FormManager->>FormContext: Initialize with formItems
FormContext->>API: GET /dokan/v3/products/{id}/fields
API->>DB: Fetch product form schema & data
DB-->>API: Return form fields + current values
API-->>FormContext: Populate fields
FormContext-->>FormManager: Render form with values
Vendor->>FormManager: Edit field values
FormManager->>FormContext: onChange with field updates
FormContext->>FormContext: Merge into product state
Vendor->>FormManager: Click Save
FormManager->>FormContext: submitHandler triggered
FormContext->>API: PUT /dokan/v3/products/{id}
API->>API: Validate & transform payload
API->>DB: Update product & meta
DB-->>API: Confirm
API-->>FormContext: Success response
FormContext-->>FormManager: Show success toast
FormManager-->>Vendor: Confirmation
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In `@includes/Product/FormManager.php`:
- Around line 246-323: The get_product_variations function assumes
wc_get_product($product_id) returns a valid variable product; add a guard after
$parent_product = wc_get_product($product_id) that returns an empty array if
$parent_product is false/null or not a variable product (e.g. ! $parent_product
|| ! $parent_product->is_type('variable') or not an instance of
\WC_Product_Variable), so you never call $parent_product->get_attributes() on an
invalid object.
- Around line 141-166: In dokan_save_product_data(), add authorization and
payload sanitation before calling dokan()->product->create: verify the current
user has the proper capability (e.g., current_user_can('edit_post', $product_id)
or a Dokan-specific capability like 'dokan_manage_products'), if an existing
product_id is present check ownership via get_post_field('post_author',
$product_id) and reject if it doesn’t match wp_get_current_user()->ID, and
ensure you wp_unslash($_POST) then validate/sanitize required fields (e.g.,
sanitize_text_field, absint for ids) into a cleaned $data array to pass to
dokan()->product->create; return wp_send_json_error for unauthorized or invalid
payloads.
- Around line 66-132: The load_product_edit_content method currently only checks
current_user_can('dokan_edit_product') which is product-agnostic; after
obtaining $product (or $product_id) and confirming $product exists, add a call
to dokan_is_product_author($product_id) and if it returns false render the same
dokan-error template (or call dokan_get_template_part with the 'You have no
permission to view this page' message) and return; ensure this check occurs
before calling dokan_get_template_part('products/form-manager/form-content',
...) and before enqueuing/localizing scripts so only the product owner can load
the edit form.
- Around line 409-417: The dokan_get_product_variations AJAX handler lacks CSRF
and permission checks; add a nonce verification using the same 'form_manager'
action (verify the nonce from the request with wp_verify_nonce and return
wp_send_json_error on failure) and ensure the current user is the product owner
before returning variations (e.g., compare get_post_field('post_author',
$product_id) to get_current_user_id() or use current_user_can('edit_post',
$product_id)); if ownership check fails, return wp_send_json_error with an
appropriate message/403, otherwise call
self::get_product_variations($product_id) and wp_send_json_success as before.
In `@includes/ProductForm/Field.php`:
- Around line 81-83: The get_name() method in class Field should return the
field id when name is not set or is empty so get_value() lookups work; update
get_name() to check $this->data['name'] (using isset/empty or null coalescing)
and if missing return (string)$this->data['id'] (or an empty string only if id
is also missing) while preserving the string return type so callers like
get_value() receive a valid identifier.
In `@includes/ProductForm/Init.php`:
- Around line 343-346: The return calls a non-existent WC_Product method
get_brand_ids() causing a fatal error; replace it by invoking the Manager's
method with the product ID (e.g., use Manager()->get_brand_ids(
$product->get_id() ) or otherwise obtain brand IDs via the Manager class) so
update the return in the closure to call Manager::get_brand_ids with
$product->get_id() instead of $product->get_brand_ids().
In `@includes/ProductForm/Section.php`:
- Around line 80-86: The add_field method currently ignores errors from
Factory::add_field; change it to capture the result of Factory::add_field($id,
$args), check for is_wp_error(result) (or other falsy failure), and either
return that WP_Error up to the caller or throw an exception so failures aren’t
silent; update the phpdoc/return type for add_field to indicate it may return
WP_Error|void (or throw) and ensure callers of ProductForm::add_field handle the
propagated error.
In `@src/admin/pages/Settings.vue`:
- Around line 710-733: The resetAllFields method can throw when a block lacks
fields or a field lacks a default; update resetAllFields to defensive-check
before dereferencing: inside the settingFields.forEach loop, ensure
sectionValue.fields exists and is an object, ensure
clonedSectionSettingValues[blockId] and its ['fields'][FieldId] exist, and only
then read fieldData.default (or use optional chaining and fallbacks) before
assigning visibility and required; reference the resetAllFields function and the
settingFields/settingValues structures in your changes and skip any field/block
that doesn't have the expected fields/default shape to avoid runtime errors.
In `@src/dashboard/form-manager/components/ImagePreview.tsx`:
- Around line 16-21: In ImagePreview, the items array can contain falsy values
(e.g., when images is null/undefined) causing TypeError when accessing item.url;
update the conversion to normalize and filter falsy entries (e.g., convert
images to an array then apply filter(Boolean)) so items contains only valid
image objects, and keep the existing early-return that checks items.length and
children; references: images, items, children, and item.url in the ImagePreview
component.
In `@src/dashboard/form-manager/components/variation/VariationCard.tsx`:
- Around line 19-38: The current fetchedVariationData toggles setIsExpanded
immediately and only logs errors, leaving the UI stuck on "Loading..."; update
fetchedVariationData to manage loading and error state: introduce local states
(e.g., isLoading and fetchError), set isLoading = true before the apiFetch, move
setIsExpanded( true ) to run after a successful fetch (setSections and
setVendorEarning), and in the catch set fetchError with the error and isLoading
= false (and ensure isExpanded is not left toggled); update the render logic to
show an error message and a retry button that calls fetchedVariationData when
fetchError is present and to use isLoading to drive the "Loading..." indicator.
In `@src/dashboard/form-manager/context/VariationContext.tsx`:
- Around line 203-269: generateVariations, addVariation and removeVariation
currently always show a success toast (and some only log errors) even when the
server returns a failure; update each function to 1) wrap the ajaxRequest call
in try/catch (add try/catch to generateVariations), 2) after awaiting
ajaxRequest check response.success and if true call fetchVariations and show the
success toast using response.data.message, otherwise show an error toast with
response.data.message (or a fallback message) and do not call fetchVariations,
and 3) in the catch block log the error and show an error toast with the caught
error message; reference the functions generateVariations, addVariation and
removeVariation when making these changes.
In `@src/dashboard/form-manager/utils.tsx`:
- Around line 78-84: The required-field check incorrectly uses "!value" which
treats valid falsy values like 0 or false as empty; update the condition in the
validation logic that reads "const value = values[field.id]" to only consider a
field empty when value === undefined || value === null || (typeof value ===
'string' && value.trim() === '') || (Array.isArray(value) && value.length === 0)
— keep the array-length check for arrays and allow numeric 0 and boolean false;
ensure you update the branch that sets newErrors[field.id] so it only triggers
for these stricter emptiness cases (and consider special-case checkbox handling
elsewhere if needed).
🟡 Minor comments (15)
includes/ProductForm/Component.php-366-377 (1)
366-377: Fix DocBlock: says "Get" but method is a setter.The DocBlock at line 366-374 describes "Get show in admin settings" but the method
set_show_in_admin_settingsis a setter. Additionally, the return typevoidbreaks the fluent interface pattern used by all other setters.🐛 Proposed fix
/** - * Get show in admin settings + * Set show in admin settings * * `@since` DOKAN_SINCE * * `@param` bool $show_in_admin_settings * - * `@return` void + * `@return` $this */ - public function set_show_in_admin_settings( bool $show_in_admin_settings ): void { + public function set_show_in_admin_settings( bool $show_in_admin_settings ): self { $this->data['show_in_admin_settings'] = $show_in_admin_settings; + + return $this; }includes/ProductForm/Field.php-505-510 (1)
505-510: Default sanitization skips arrays.If field values are arrays (e.g., multiselect), they bypass
wc_cleanentirely. Consider sanitizing arrays too.🐛 Suggested adjustment
- if ( is_string( $value ) ) { - return wc_clean( $value ); - } + if ( is_string( $value ) || is_array( $value ) ) { + return wc_clean( $value ); + }includes/ProductForm/Field.php-218-228 (1)
218-228: Options fallback path is effectively unreachable.
$this->data['options']is an array by default, so'' !== $this->data['options']is always true and the fallback to$valuenever executes. This makes the variadic fallback useless.🐛 Suggested adjustment
- if ( '' !== $this->data['options'] || empty( $value ) ) { + if ( ! empty( $this->data['options'] ) || empty( $value ) ) { return $this->data['options']; }templates/products/form-manager/form-wrapper.php-64-69 (1)
64-69: Inconsistent comment: references "order" hook instead of product form manager.Same issue as above—the comment mentions
dokan_order_content_inside_afterbut the actual hook isdokan_product_form_manager_content_inside_after.Suggested fix
/** - * Added dokan_order_content_inside_after hook + * Added dokan_product_form_manager_content_inside_after hook * * `@since` DOKAN_SINCE */ do_action( 'dokan_product_form_manager_content_inside_after' );src/dashboard/form-manager/factories/form-data.ts-8-13 (1)
8-13: Add defensive check forfield.value.idaccess.The
canHandlefunction checks!! field.value(truthy), butgetValueaccessesfield.value.id. Iffield.valueis a primitive (e.g., a number ID directly), this would throw a runtime error.Suggested fix
const ImageIdHandler: FieldDataHandler = { canHandle: ( field ) => { - return field.id === 'image_id' && !! field.value; + return field.id === 'image_id' && !! field.value && typeof field.value === 'object' && 'id' in field.value; }, getValue: ( field ) => field.value.id, };templates/products/form-manager/form-wrapper.php-34-41 (1)
34-41: Inconsistent comment: references "order" hook instead of product form manager.The comment mentions
dokan_order_content_inside_beforebut the actual hook isdokan_product_form_manager_content_inside_before. This could confuse developers extending the template.Suggested fix
/** - * Added dokan_order_content_inside_before hook + * Added dokan_product_form_manager_content_inside_before hook * * `@hooked` show_seller_enable_message * * `@since` DOKAN_SINCE */ do_action( 'dokan_product_form_manager_content_inside_before' );src/dashboard/form-manager/components/FileUploadEdit.tsx-35-42 (1)
35-42: Shallow copy leads to object mutation.The spread operator
[ ...files ]creates a shallow copy of the array, butnewFiles[ index ][ key ] = valuestill mutates the original object reference. This can cause React to miss re-renders or produce stale closures.Suggested fix
const updateRow = ( index: number, key: string, value: any ) => { - const newFiles = [ ...files ]; - newFiles[ index ][ key ] = value; + const newFiles = files.map( ( file: any, i: number ) => + i === index ? { ...file, [ key ]: value } : file + ); setFiles( newFiles ); onChange( { [ field.id ]: newFiles, } ); };src/dashboard/form-manager/components/variation/VariationForm.tsx-36-38 (1)
36-38: Avoid using array index as React key for dynamic lists.Using
indexas the key can cause incorrect component state preservation when variations are added, removed, or reordered. Since variations likely have a unique identifier, use that instead.Suggested fix
- { variations.map( ( variation, index: number ) => ( - <VariationCard key={ index } variation={ variation } /> + { variations.map( ( variation ) => ( + <VariationCard key={ variation.id } variation={ variation } /> ) ) }includes/ProductForm/Init.php-234-237 (1)
234-237: Inconsistent error messages for category selection.The ternary condition appears inverted. When
product_category_selection_is_single()returnstrue, the message says "at least one category" but single selection typically means exactly one category should be selected.🐛 Suggested fix
$category_error_message = ProductCategoryHelper::product_category_selection_is_single() - ? __( 'Please select at least one category!', 'dokan-lite' ) - : __( 'Please select a category', 'dokan-lite' ); + ? __( 'Please select a category', 'dokan-lite' ) + : __( 'Please select at least one category!', 'dokan-lite' );includes/ProductForm/Init.php-165-177 (1)
165-177: Inconsistent return type for checkbox field value callback.The
value_callbackforCREATE_SCHEDULE_FOR_DISCOUNTreturns mixed types: a timestamp (int),false,'on', or'off'. Checkbox fields typically expect boolean or consistent string values. The timestamp return on line 169 seems out of place.🐛 Suggested fix for consistent return values
'value_callback' => function ( $product, $value = '' ) { if ( '' !== $value ) { - $time = dokan_current_datetime()->modify( $value ); - - return $time ? $time->getTimestamp() : false; + return $value === 'on' || $value === true ? 'on' : 'off'; } if ( ! $product instanceof WC_Product ) { - return false; + return 'off'; } return ! empty( $product->get_date_on_sale_to() ?? $product->get_date_on_sale_from() ) ? 'on' : 'off'; },src/dashboard/form-manager/components/AttributeVariationEdit.tsx-85-100 (1)
85-100: Using array index as React key causes issues with list mutations.When attributes are removed or reordered, using
indexas key can cause incorrect component state preservation and re-render issues. Use a stable unique identifier instead.🔧 Proposed fix using attribute id and position
{ attributes.map( ( attr, index ) => ( <AttributeCard - key={ index } + key={ `${ attr.id }-${ attr.position }` } attr={ attr }Alternatively, generate a unique ID when creating new attributes:
const newAttribute: Attribute = { id: 0, name: '', + _key: `custom-${ Date.now() }`, // Add unique key for Reactsrc/dashboard/form-manager/App.tsx-80-87 (1)
80-87: Missing success feedback after product save.The
onSubmithandler returns the response but doesn't show a success toast to the user. While errors are handled in theFormProvider'ssubmitHandler, successful saves should provide user feedback.🔧 Proposed fix to add success feedback
+import { useToast } from '@getdokan/dokan-ui'; const App = () => { + const toast = useToast(); const productId = Number( formData.product_id ); const onSubmit = async ( product: Record< string, any > ) => { const response = await ajaxRequest( { ...product, action: 'dokan_save_product_data', _nonce: formData.form_manager_nonce, } ); + if ( response?.success ) { + toast( { + type: 'success', + title: __( 'Product saved successfully', 'dokan-lite' ), + } ); + } return response; };includes/ProductForm/Section.php-43-50 (1)
43-50: Exception message says “Field” instead of “Section.”
Minor but user‑facing; this message is emitted fromSection.💡 Suggested fix
- 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' ),includes/Product/FormManager.php-377-399 (1)
377-399: Add error check forget_terms()return value.
get_terms()can returnWP_Errorwhen an invalid taxonomy is provided or lower-level functions fail; the current truthy check will pass forWP_Errorobjects and cause issues in the foreach loop. Add anis_wp_error()guard before iteration to safely return an empty array on error.Suggested guard
$data = []; $product_tags = get_terms( $drop_down_tags ); + if ( is_wp_error( $product_tags ) ) { + return []; + } if ( $product_tags ) { foreach ( $product_tags as $term ) { $data[] = [includes/ProductForm/Factory.php-212-228 (1)
212-228: Fix namespaced class resolution in sorting.
class_exists( $class_name )with a bare class name won't resolveField/Sectionin this namespace, so sorting never runs. Create a fully qualified class name before the check and call.🛠️ Suggested fix
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' ) ) { + $fqcn = __NAMESPACE__ . '\\' . ltrim( $class_name, '\\' ); + if ( class_exists( $fqcn ) && method_exists( $fqcn, '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 $fqcn::sort( $a, $b, $sort_by ); } ); }
🧹 Nitpick comments (33)
src/components/Select.tsx (3)
13-17: Consider extractingDefaultOptionto a shared types file.This type is duplicated in
AsyncSelect.tsx. Extracting it to a shared location (e.g.,@src/components/select/types.ts) would reduce duplication and ensure consistency.
41-43: Loose typing withRecord<string, any>reduces type safety.The union with
Record<string, any>allows any object to be passed ascomponents, bypassing TypeScript's checks. If this flexibility is intentional for custom component overrides, consider documenting the expected shape or using a more specific type.
54-77: Type casts are pragmatic but obscure potential issues.The
as anycasts on lines 61 and 63 work around react-select's complex generics, but they suppress type errors that could surface at runtime. The existing@ts-ignoreon line 55 indicates ongoing type compatibility challenges.The functional implementation is sound—props are correctly forwarded and the
IsMultigeneric flows through properly.Consider adding a brief inline comment explaining why these casts are necessary for future maintainers, e.g.:
// Cast needed due to react-select's complex generic constraints MultiValue: MultiValue as any,src/dashboard/form-manager/index.scss (1)
1-103: Consider extracting repeated magic numbers to SCSS variables.The file uses several hard-coded pixel values (e.g.,
232pxfor product image dimensions,80pxfor gallery thumbnails) that appear multiple times. Extracting these to SCSS variables at the top of the file would improve maintainability and make dimension adjustments easier.The
!importantdeclarations are acceptable here since they're overriding WordPress component library defaults.♻️ Suggested variable extraction
+$product-image-height: 232px; +$gallery-thumb-size: 80px; + .dokan-dashboard-content:has(.dokan-product-form-manager) { background-color: `#f0f0f1`; } .dokan-product-form-manager { // ... existing rules ... .dokan-product-image_id { // ... - height: 232px; + height: $product-image-height; // ... img { - height: 232px; + height: $product-image-height; // ... } } .variation-form { .dokan-product-image_id { - height: 232px; - width: 232px; + height: $product-image-height; + width: $product-image-height; } img { - height: 232px; - width: 232px; + height: $product-image-height; + width: $product-image-height; } } .dokan-product-gallery_image_ids { // ... - height: 80px; - width: 80px; + height: $gallery-thumb-size; + width: $gallery-thumb-size; // ... } }src/dashboard/form-manager/components/ImagePreview.tsx (2)
3-8: Strengthen TypeScript types for better safety.The
anytype forimagesweakens type safety. Consider defining a proper interface for image items.♻️ Suggested type definition
+interface ImageItem { + url: string; + alt?: string; +} + interface ImagePreviewProps { - images: any; + images: ImageItem | ImageItem[] | null | undefined; onRemove: ( index: number ) => void; children?: React.ReactNode; itemClassName?: string; }
25-27: Use a stable unique key instead ofitem.url.Using
item.urlas the React key can cause issues if:
- Multiple images have the same URL
- The URL changes while the item identity remains the same
Consider using an
idfield or combining URL with index as a fallback.♻️ Suggested fix
{ items.map( ( item: any, index: number ) => ( <div - key={ item.url } + key={ item.id ?? `${ item.url }-${ index }` } className={ `relative group border border-gray-200 rounded-md overflow-hidden ${ itemClassName }` } >includes/ProductForm/Component.php (2)
53-59: The$idparameter is unused in the constructor.The static analysis correctly flags that
$idis not used. Since child classes callset_id($id)before invoking the parent constructor, you could either:
- Remove the parameter from the base constructor signature
- Call
set_id()in the base constructor if not already setBased on the child class pattern (from Field.php and Section.php), option 1 would be cleaner since children already handle ID setting.
♻️ Option 1: Remove unused parameter
- public function __construct( string $id, array $args = [] ) { + public function __construct( array $args = [] ) { foreach ( $args as $key => $value ) { if ( method_exists( $this, "set_{$key}" ) && null !== $value ) { $this->{"set_{$key}"}( $value ); } } }Note: This would require updating child class calls from
parent::__construct( $id, $args )toparent::__construct( $args ).
345-347: Add return type hint for consistency.The
get_error_messagemethod is missing a return type hint, unlike other getter methods in this class.♻️ Suggested fix
- public function get_error_message() { + public function get_error_message(): string { return $this->data['error_message']; }includes/ProductForm/Field.php (1)
392-395: TODO left in public API path.Please either implement field type validation or convert the TODO into a tracked issue to avoid shipping unfinished behavior.
If you want, I can draft a validation matrix for allowed field types and add unit tests.
src/dashboard/form-manager/components/DateTimePickerEdit.tsx (1)
4-12: Avoidanyfor props to preserve type safety.Using
anyhere makes it easy to pass the wrong shape and silently break field updates.♻️ Suggested typing
-import CustomField from './CustomField'; +import CustomField from './CustomField'; -const DateTimePickerEdit = ( { data, field, onChange }: any ) => ( +type DateTimePickerEditProps = { + data: Record<string, unknown>; + field: { id: string; label: string; placeholder?: string; error?: string }; + onChange: (value: Record<string, string>) => void; +}; + +const DateTimePickerEdit = ( { data, field, onChange }: DateTimePickerEditProps ) => ( <CustomField label={ field.label } error={ field.error }> <DateTimePicker placeholder={ field.placeholder } currentDate={ data[ field.id ] } onChange={ ( value: string ) => { onChange( { [ field.id ]: value } ); } } /> </CustomField> );src/dashboard/form-manager/components/FeatureImage.tsx (1)
8-26: Sync local image state with prop updates to avoid stale preview.
Iffield.valuecan change after mount (e.g., async load or reset), the local state won’t update. Consider syncing it with an effect.♻️ Suggested update
-import { useState } from '@wordpress/element'; +import { useEffect, useState } from '@wordpress/element'; @@ - const [ image, setImage ] = useState( field.value ); + const [ image, setImage ] = useState( field.value ?? null ); + + useEffect( () => { + setImage( field.value ?? null ); + }, [ field.value ] );templates/products/form-manager/form-content.php (1)
15-16: Add a defensive guard for missing$product.
If this template can render without aWC_Productinstance,get_id()will fatal. Consider defaulting to0.🛡️ Suggested guard
+<?php +$product_id = ( isset( $product ) && $product instanceof WC_Product ) ? $product->get_id() : 0; +?> -<input type="hidden" name="dokan_product_id" id="dokan_product_id" value="<?php echo esc_attr( $product->get_id() ); ?>" /> +<input type="hidden" name="dokan_product_id" id="dokan_product_id" value="<?php echo esc_attr( $product_id ); ?>" />src/dashboard/form-manager/components/CustomField.tsx (1)
1-11: Prefer importingReactNodeinstead of relying on a globalReactnamespace.
This avoids potential type errors when the React namespace isn’t globally available in the TS config.♻️ Suggested typing update
+import type { ReactNode } from 'react'; + const CustomField = ( { label, children, error, className = '', }: { - label?: string | React.ReactNode; - children: React.ReactNode; + label?: string | ReactNode; + children: ReactNode; error?: string; className?: string; } ) => {src/dashboard/form-manager/components/FileUploadEdit.tsx (2)
68-70: Avoid using array index as React key.Similar to other list renderings, using
indexas the key can cause issues when rows are added or removed. Consider using a unique identifier or generating one when adding new rows.Suggested approach
Generate a unique ID when adding new rows:
const onAddRow = () => { const newFiles = [ ...files, { - id: '', + id: `temp-${ Date.now() }-${ Math.random() }`, title: '', url: '', }, ];Then use
file.idas the key:- <div - key={ index } + <div + key={ file.id || index }
9-10: Consider adding TypeScript types for props.Using
anyfor props reduces type safety. Consider defining a proper interface.Suggested type definition
interface FileUploadEditProps { field: { id: string; label: string; error?: string; value?: Array<{ id: string; title: string; url: string; name?: string }>; }; onChange: ( data: Record< string, any > ) => void; } const FileUploadEdit = ( { field, onChange }: FileUploadEditProps ) => {src/dashboard/form-manager/factories/form-data.ts (1)
15-20: Add defensive check for array item structure.Similar to the image handler, this assumes each array item has an
idproperty. If the array contains primitives (IDs directly),.map( ( img: any ) => img.id )would returnundefinedvalues.Suggested fix
const GalleryImageIdsHandler: FieldDataHandler = { canHandle: ( field ) => { - return field.id === 'gallery_image_ids' && Array.isArray( field.value ); + return field.id === 'gallery_image_ids' && Array.isArray( field.value ) && field.value.length > 0 && typeof field.value[ 0 ] === 'object'; }, getValue: ( field ) => field.value.map( ( img: any ) => img.id ), };src/dashboard/form-manager/hooks/useVariationLayouts.tsx (1)
9-109: Consider extracting static layouts outside the hook.The
layoutsarray is a static configuration that doesn't depend onfieldsorproduct. Defining it insideuseMemorecreates the array structure on every memoization recalculation. Extracting it as a module-level constant improves clarity and avoids unnecessary object creation.Suggested refactor
const VARIATION_LAYOUTS = [ { id: 'variation-image-sku', layout: { type: 'row', alignment: 'center', /* ... */ }, children: [ /* ... */ ], }, // ... rest of layouts ]; const useVariationLayouts = ( fields: FormField[], product: Record< string, any > ) => { const formLayouts = useMemo( () => { return { fields: layoutBuilder( VARIATION_LAYOUTS, fields, product, 'variation' ), }; }, [ fields, product ] ); return { formLayouts }; };src/dashboard/form-manager/components/GalleryImages.tsx (1)
7-8: Consider adding TypeScript types for props.Using
anyfor props reduces type safety. Consider defining a proper interface consistent with other form field components.Suggested type definition
interface GalleryImagesProps { field: { id: string; label: string; error?: string; value?: Array<{ id: number | string; url: string; alt?: string }>; }; onChange: ( data: Record< string, any > ) => void; } const GalleryImages = ( { field, onChange }: GalleryImagesProps ) => {src/dashboard/form-manager/components/PriceEdit.tsx (3)
9-12: Consider adding TypeScript types instead ofany.Using
anyfor the component props loses type safety benefits. Define a proper interface for the props.♻️ Suggested type definition
interface PriceEditProps { data: { id: number; vendor_earning: number; category_ids?: number[]; [key: string]: any; }; field: { id: string; label: string; placeholder?: string; error?: string; }; onChange: (update: Record<string, any>) => void; } const PriceEdit = ({ data, field, onChange }: PriceEditProps) => {
14-31: Add debouncing to prevent excessive API calls on every keystroke.
vendorEarningHandleris invoked on every price input change. This could trigger many API requests while the user types. Consider debouncing the API call.♻️ Suggested debounce implementation
+import { useMemo } from '@wordpress/element'; +import { debounce } from 'lodash'; + const PriceEdit = ( { data, field, onChange }: any ) => { const [ vendorEarning, setVendorEarning ] = useState( Number( data.vendor_earning ) ); - const vendorEarningHandler = async ( price: number ) => { + const vendorEarningHandler = useMemo( + () => + debounce( async ( price: number ) => { + if ( field.id !== 'regular_price' ) return; + try { + const response = await apiFetch( { + path: addQueryArgs( '/dokan/v1/commission', { + amount: price, + product_id: data.id, + category_ids: data.category_ids || [], + context: 'seller', + } ), + } ); + setVendorEarning( Number( response ) ); + } catch ( error ) { + setVendorEarning( 0 ); + } + }, 300 ), + [ field.id, data.id, data.category_ids ] + );
26-28: Validate API response type before casting to Number.The response is directly cast to
Numberwithout validating its type. If the API returns an unexpected value, this could result inNaN.♻️ Suggested validation
- setVendorEarning( Number( response ) ); + const earning = typeof response === 'number' ? response : parseFloat( response as string ); + setVendorEarning( isNaN( earning ) ? 0 : earning );src/dashboard/form-manager/components/FieldRenderer.tsx (2)
54-57: Avoid@ts-ignore- use proper typing instead.Using
@ts-ignoresuppresses TypeScript errors but hides potential issues. Consider typingmappedFieldto allow optionaldescriptionproperty deletion.♻️ Suggested fix
- if ( ! field.help_content && ! field.description ) { - // `@ts-ignore` - delete mappedField.description; - } + if ( ! field.help_content && ! field.description ) { + ( mappedField as { description?: unknown } ).description = undefined; + }Or define
mappedFieldwith an explicit type that hasdescriptionas optional.
62-64: Clarify purpose of emptyFieldRenderercomponent.The component returns
nulland appears to be a placeholder. If this is intentional (e.g., the export is only for thegetFieldConfigfunction), consider adding a comment or removing the default export if it's not needed.src/dashboard/form-manager/components/SelectEdit.tsx (1)
37-56: Consider stronger typing forflattenOptionsaccumulator.The accumulator is typed as
any[]which loses type safety. UseOption[]for better type checking.♻️ Suggested typing improvement
const flattenOptions = ( options: Option[], level = 0 ): Option[] => { - return options.reduce( ( acc: any[], option ) => { + return options.reduce< Option[] >( ( acc, option ) => { // Map term_id to value if strictly needed, or ensure value exists const value = option.term_id || option.value; const flatOption = { ...option, value, level, };includes/ProductForm/Init.php (1)
279-303: Sanitize callback returns error string instead of WP_Error.When tag limit is exceeded (lines 289-292), the callback returns an error message string. This may cause issues downstream since the return type should consistently be the sanitized array. Consider returning a
WP_Errorobject for consistency with other error handling patterns in this file (see line 567-578).♻️ Suggested fix
'sanitize_callback' => function ( $tags ) { $maximum_tags_select_length = apply_filters( 'dokan_product_tags_select_max_length', -1 ); 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( + /* translators: %s: maximum tag length */ + __( 'You can only select %s tags', 'dokan-lite' ), + number_format_i18n( $maximum_tags_select_length ) + ) + ); }src/dashboard/form-manager/App.tsx (1)
60-64: Button label logic appears inverted.For a new product, "Save Changes" is shown, but "Save Product" or "Create Product" would be more intuitive. Conversely, "Update Product" for existing products is correct, but showing it when
isNewProductis false seems right. However, the current logic shows "Save Changes" for new products which is slightly confusing UX.Consider clarifying the labels:
💡 Suggested label improvement
label={ isNewProduct - ? __( 'Save Changes', 'dokan-lite' ) + ? __( 'Create Product', 'dokan-lite' ) : __( 'Update Product', 'dokan-lite' ) }src/dashboard/form-manager/components/variation/AttributeCard.tsx (3)
87-93: Event propagation should be handled within the component.The Remove button's
onClickhandler doesn't calle.stopPropagation(), relying on the parent to handle it. This creates a fragile coupling where forgetting to stop propagation in the parent will cause the accordion to toggle when removing.🔧 Proposed fix
<span role="button" - onClick={ onRemove } + onClick={ ( e ) => { + e.stopPropagation(); + onRemove( e ); + } } className="text-red-500 hover:text-red-700 text-xs font-medium" >
178-199: Non-taxonomy attribute values stored inconsistently.When editing non-taxonomy attributes, the raw string value (e.g.,
"Red | Blue") is stored directly. However, the display logic on lines 181-183 expects either an array or string. This inconsistency may cause issues when the data is used elsewhere expecting an array format.Consider parsing the pipe-delimited string into an array on change, or ensuring consumers handle both formats consistently.
🔧 Proposed fix to normalize to array
<SimpleInput value={ Array.isArray( attr.options ) ? attr.options.join( ' | ' ) : attr.options } onChange={ ( e ) => handleAttributeChange( 'options', - e.target.value + e.target.value + .split( '|' ) + .map( ( v: string ) => v.trim() ) + .filter( Boolean ) ) }
76-116: Accordion header lacks keyboard accessibility.The accordion header has
role="button"but lacks keyboard support (onKeyDownfor Enter/Space) andtabIndexfor focus. This impacts accessibility for keyboard-only users.♿ Proposed accessibility improvement
<div role="button" + tabIndex={ 0 } className="flex justify-between items-center p-3 bg-gray-50 border-b cursor-pointer select-none" onClick={ toggleAccordion } + onKeyDown={ ( e ) => { + if ( e.key === 'Enter' || e.key === ' ' ) { + e.preventDefault(); + toggleAccordion(); + } + } } + aria-expanded={ isExpanded } >src/dashboard/form-manager/components/variation/VariationCard.tsx (1)
46-49: Same keyboard accessibility concern as AttributeCard.The clickable header lacks
tabIndexandonKeyDownhandlers for keyboard navigation.Consider extracting a shared accordion header component to ensure consistent accessibility across
AttributeCardandVariationCard.src/dashboard/form-manager/context/FormContext.tsx (2)
83-98: Only first changed field's error is cleared on multi-field updates.If
newDatacontains multiple fields (which could happen with complex form interactions), only the first key's error is cleared. Other fields retain their error state.🔧 Proposed fix to clear all changed fields' errors
const onChange = useCallback( ( newData: Record< string, any > ) => { setProduct( ( prev ) => ( { ...prev, ...newData } ) ); // Clear error for the field being edited - const changedFieldId = Object.keys( newData )[ 0 ]; - if ( changedFieldId ) { - setErrors( ( prev: any ) => { - if ( ! prev[ changedFieldId ] ) { - return prev; - } - const newErrs = { ...prev }; - delete newErrs[ changedFieldId ]; - return newErrs; - } ); - } + const changedFieldIds = Object.keys( newData ); + if ( changedFieldIds.length > 0 ) { + setErrors( ( prev ) => { + const hasErrorsToRemove = changedFieldIds.some( ( id ) => prev[ id ] ); + if ( ! hasErrorsToRemove ) { + return prev; + } + const newErrs = { ...prev }; + changedFieldIds.forEach( ( id ) => delete newErrs[ id ] ); + return newErrs; + } ); + } }, [] );
115-121: DOM query for error field is timing-sensitive.The
document.querySelector('.is-invalid')runs immediately aftersetErrors, but React's state update is asynchronous. The DOM may not yet reflect the new error classes when this query runs.🔧 Proposed fix using requestAnimationFrame or useEffect
if ( Object.keys( newErrors ).length > 0 ) { setErrors( newErrors ); toast( { type: 'error', title: __( 'Please fill out all required fields.', 'dokan-lite' ), } ); - const firstErrorField = document.querySelector( '.is-invalid' ); - if ( firstErrorField ) { - firstErrorField.scrollIntoView( { - behavior: 'smooth', - block: 'center', - } ); - } + // Wait for DOM to update with error classes + requestAnimationFrame( () => { + const firstErrorField = document.querySelector( '.is-invalid' ); + if ( firstErrorField ) { + firstErrorField.scrollIntoView( { + behavior: 'smooth', + block: 'center', + } ); + } + } ); return false; }src/dashboard/form-manager/components/AttributeVariationEdit.tsx (1)
11-14: Props typed asanyloses type safety.The component props
{ data, field, onChange }are typed asany, which defeats TypeScript's benefits. Consider defining a proper interface.💡 Proposed type definition
+interface AttributeVariationEditorProps { + data: Record< string, any > & { + product_type?: string; + id?: number; + variations?: VariationType[]; + }; + field: { + id: string; + label: string; + error?: string; + elements?: Array< { label: string; value: string | number; terms?: any[] } >; + }; + onChange: ( updates: Record< string, any > ) => void; +} -const AttributeVariationEditor = ( { data, field, onChange }: any ) => { +const AttributeVariationEditor = ( { data, field, onChange }: AttributeVariationEditorProps ) => {
includes/Product/FormManager.php
Outdated
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify how dokan_edit_product is mapped and whether a product-scoped check exists.
rg -nP '\bdokan_edit_product\b' -g '!**/vendor/**'Repository: getdokan/dokan
Length of output: 2422
🏁 Script executed:
#!/bin/bash
# Search for map_meta_cap or dokan_is_product_author
rg -nP '\bmap_meta_cap\b|\bdokan_is_product_author\b' -g '!**/vendor/**'Repository: getdokan/dokan
Length of output: 1409
🏁 Script executed:
#!/bin/bash
# Search for capability mapping or filter hooks related to product editing
rg -nP 'add_filter.*map_meta_cap|apply_filters.*user_has_cap' -g '!**/vendor/**'Repository: getdokan/dokan
Length of output: 170
🏁 Script executed:
#!/bin/bash
# Check the current_user_can calls in the file to understand the pattern
cat -n includes/Product/FormManager.php | head -150Repository: getdokan/dokan
Length of output: 5765
🏁 Script executed:
#!/bin/bash
# Check the edit-product-single template to see how it handles ownership
head -100 templates/products/edit-product-single.phpRepository: getdokan/dokan
Length of output: 3476
🏁 Script executed:
#!/bin/bash
# Search for map_meta_cap related to dokan_edit_product or product capabilities
rg -B3 -A10 'map_meta_cap' includes/ --type php | head -60Repository: getdokan/dokan
Length of output: 2262
🏁 Script executed:
#!/bin/bash
# Check if there's a Permissions file specifically for Products
fd -e php '.*Permissions.*' includes/ | xargs ls -laRepository: getdokan/dokan
Length of output: 143
🏁 Script executed:
#!/bin/bash
# Specifically look for how current_user_can('dokan_edit_product', $product_id) is handled
rg "dokan_edit_product.*\\\$" includes/ --type phpRepository: getdokan/dokan
Length of output: 1323
🏁 Script executed:
#!/bin/bash
# Check if WordPress/Dokan supports product-scoped dokan_edit_product capability
rg -B5 -A10 "current_user_can.*dokan_edit_product.*\\\$" includes/ templates/ --type phpRepository: getdokan/dokan
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Check the dokan_is_product_author function implementation
sed -n '144,155p' includes/functions.phpRepository: getdokan/dokan
Length of output: 401
🏁 Script executed:
#!/bin/bash
# Check how dokan_is_product_author is used - is it checking post_author or vendor ID?
sed -n '144,160p' includes/functions.phpRepository: getdokan/dokan
Length of output: 458
Add ownership verification for existing products before rendering the edit form.
The initial current_user_can( 'dokan_edit_product' ) check is product-agnostic. An unauthorized user with this capability could potentially access any product's edit form by guessing the product ID. Add dokan_is_product_author( $product_id ) after loading the product (before rendering) to verify ownership, matching the pattern used in templates/products/edit-product-single.php (line 79).
🔐 Suggested ownership check
$product = wc_get_product( $product_id );
if ( ! $product ) {
dokan_get_template_part(
'global/dokan-error', '', [
'deleted' => false,
'message' => __( 'Product not found', 'dokan-lite' ),
]
);
return;
}
+ if ( ! dokan_is_product_author( $product_id ) ) {
+ dokan_get_template_part(
+ 'global/dokan-error', '', [
+ 'deleted' => false,
+ 'message' => __( 'You have no permission to edit this product', 'dokan-lite' ),
+ ]
+ );
+ return;
+ }🤖 Prompt for AI Agents
In `@includes/Product/FormManager.php` around lines 66 - 132, The
load_product_edit_content method currently only checks
current_user_can('dokan_edit_product') which is product-agnostic; after
obtaining $product (or $product_id) and confirming $product exists, add a call
to dokan_is_product_author($product_id) and if it returns false render the same
dokan-error template (or call dokan_get_template_part with the 'You have no
permission to view this page' message) and return; ensure this check occurs
before calling dokan_get_template_part('products/form-manager/form-content',
...) and before enqueuing/localizing scripts so only the product owner can load
the edit form.
includes/Product/FormManager.php
Outdated
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "FormManager.php" -type fRepository: getdokan/dokan
Length of output: 172
🏁 Script executed:
rg -l "dokan_edit_product" --type phpRepository: getdokan/dokan
Length of output: 752
🏁 Script executed:
rg -l "current_user_can" includes/Product/ --type php | head -10Repository: getdokan/dokan
Length of output: 148
🏁 Script executed:
rg "function dokan_save_product_data" -A 30 --type phpRepository: getdokan/dokan
Length of output: 1972
🏁 Script executed:
rg "dokan_edit_product" -B 2 -A 2 --type php | head -50Repository: getdokan/dokan
Length of output: 4372
🏁 Script executed:
cat -n includes/Product/FormManager.php | head -170 | tail -40Repository: getdokan/dokan
Length of output: 1323
🏁 Script executed:
rg "function create" includes/Product/ --type php -A 5Repository: getdokan/dokan
Length of output: 582
🏁 Script executed:
rg "register_cap\|add_cap\|map_meta_cap" --type php | grep -i "product\|dokan" | head -20Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
rg "wp_unslash.*\$_POST" --type php | head -10Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
cat -n includes/Product/FormManager.php | head -300 | tail -150Repository: getdokan/dokan
Length of output: 6775
🏁 Script executed:
rg "current_user_can" includes/Product/FormManager.php -B 3 -A 3Repository: getdokan/dokan
Length of output: 328
🏁 Script executed:
rg "wp_send_json_error" includes/Product/FormManager.php -B 5 -A 2Repository: getdokan/dokan
Length of output: 843
🏁 Script executed:
rg "class FormManager" -A 30 --type php | head -50Repository: getdokan/dokan
Length of output: 2150
🏁 Script executed:
cat -n includes/Product/FormManager.php | sed -n '110,130p'Repository: getdokan/dokan
Length of output: 1108
🏁 Script executed:
rg "wp_ajax_dokan_get_product_variations" -A 20 --type phpRepository: getdokan/dokan
Length of output: 1405
🏁 Script executed:
grep -n "current_user_can.*dokan_edit_product" includes/Product/FormManager.php -B 5 -A 5Repository: getdokan/dokan
Length of output: 503
🏁 Script executed:
cat -n includes/Product/Manager.php | sed -n '1,100p'Repository: getdokan/dokan
Length of output: 3532
🏁 Script executed:
rg "wp_filter_post_kses" --type php -B 2 -A 2 | head -30Repository: getdokan/dokan
Length of output: 2020
🏁 Script executed:
rg "dokan_get_product_variations" -A 30 --type php | head -50Repository: getdokan/dokan
Length of output: 2657
🏁 Script executed:
rg "dokan\(\)->product->create" --type php -B 3 -A 3Repository: getdokan/dokan
Length of output: 1005
🏁 Script executed:
grep -r "wp_unslash.*_POST\|wp_unslash.*_REQUEST" includes/Product/ --include="*.php"Repository: getdokan/dokan
Length of output: 603
Add authorization and payload security checks to the save endpoint.
This AJAX handler lacks both user capability checks and authorization for product ownership. It also passes raw $_POST to the product creator without unslashing. A user with only the nonce could potentially create or modify products they shouldn't have access to.
🔐 Suggested fix
public function dokan_save_product_data() {
if ( ! isset( $_POST['_nonce'] ) || ! wp_verify_nonce( sanitize_key( $_POST['_nonce'] ), 'form_manager' ) ) {
wp_send_json_error(
[
'type' => 'nonce',
'message' => __( 'Are you cheating?', 'dokan-lite' ),
]
);
}
+ if ( ! current_user_can( 'dokan_edit_product' ) ) {
+ wp_send_json_error(
+ [
+ 'type' => 'permission',
+ 'message' => __( 'You have no permission to edit products', 'dokan-lite' ),
+ ],
+ 403
+ );
+ }
+ $payload = wp_unslash( $_POST );
+ $product_id = isset( $payload['product_id'] ) ? absint( $payload['product_id'] ) : 0;
+ if ( $product_id && ! current_user_can( 'dokan_edit_product', $product_id ) ) {
+ wp_send_json_error(
+ [
+ 'type' => 'permission',
+ 'message' => __( 'You have no permission to edit this product', 'dokan-lite' ),
+ ],
+ 403
+ );
+ }
try {
- $product = dokan()->product->create( $_POST );
+ $product = dokan()->product->create( $payload );
wp_send_json_success(
[
'product' => $product->get_data(),
'message' => __( 'Product saved successfully', 'dokan-lite' ),
]
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function dokan_save_product_data() { | |
| if ( ! isset( $_POST['_nonce'] ) || ! wp_verify_nonce( sanitize_key( $_POST['_nonce'] ), 'form_manager' ) ) { | |
| wp_send_json_error( | |
| [ | |
| 'type' => 'nonce', | |
| 'message' => __( 'Are you cheating?', 'dokan-lite' ), | |
| ] | |
| ); | |
| } | |
| try { | |
| $product = dokan()->product->create( $_POST ); | |
| wp_send_json_success( | |
| [ | |
| 'product' => $product->get_data(), | |
| 'message' => __( 'Product saved successfully', 'dokan-lite' ), | |
| ] | |
| ); | |
| } catch ( Exception $e ) { | |
| wp_send_json_error( | |
| [ | |
| 'status' => false, | |
| 'message' => $e->getMessage(), | |
| ] | |
| ); | |
| } | |
| } | |
| public function dokan_save_product_data() { | |
| if ( ! isset( $_POST['_nonce'] ) || ! wp_verify_nonce( sanitize_key( $_POST['_nonce'] ), 'form_manager' ) ) { | |
| wp_send_json_error( | |
| [ | |
| 'type' => 'nonce', | |
| 'message' => __( 'Are you cheating?', 'dokan-lite' ), | |
| ] | |
| ); | |
| } | |
| if ( ! current_user_can( 'dokan_edit_product' ) ) { | |
| wp_send_json_error( | |
| [ | |
| 'type' => 'permission', | |
| 'message' => __( 'You have no permission to edit products', 'dokan-lite' ), | |
| ], | |
| 403 | |
| ); | |
| } | |
| $payload = wp_unslash( $_POST ); | |
| $product_id = isset( $payload['product_id'] ) ? absint( $payload['product_id'] ) : 0; | |
| if ( $product_id && ! current_user_can( 'dokan_edit_product', $product_id ) ) { | |
| wp_send_json_error( | |
| [ | |
| 'type' => 'permission', | |
| 'message' => __( 'You have no permission to edit this product', 'dokan-lite' ), | |
| ], | |
| 403 | |
| ); | |
| } | |
| try { | |
| $product = dokan()->product->create( $payload ); | |
| wp_send_json_success( | |
| [ | |
| 'product' => $product->get_data(), | |
| 'message' => __( 'Product saved successfully', 'dokan-lite' ), | |
| ] | |
| ); | |
| } catch ( Exception $e ) { | |
| wp_send_json_error( | |
| [ | |
| 'status' => false, | |
| 'message' => $e->getMessage(), | |
| ] | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@includes/Product/FormManager.php` around lines 141 - 166, In
dokan_save_product_data(), add authorization and payload sanitation before
calling dokan()->product->create: verify the current user has the proper
capability (e.g., current_user_can('edit_post', $product_id) or a Dokan-specific
capability like 'dokan_manage_products'), if an existing product_id is present
check ownership via get_post_field('post_author', $product_id) and reject if it
doesn’t match wp_get_current_user()->ID, and ensure you wp_unslash($_POST) then
validate/sanitize required fields (e.g., sanitize_text_field, absint for ids)
into a cleaned $data array to pass to dokan()->product->create; return
wp_send_json_error for unauthorized or invalid payloads.
includes/Product/FormManager.php
Outdated
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for wc_get_product usage in the codebase to understand typical patterns
rg "wc_get_product\(" -A 3 -B 1 | head -50Repository: getdokan/dokan
Length of output: 172
🏁 Script executed:
# Search specifically for null/false checks on wc_get_product results
rg "wc_get_product\(" | rg -A 2 "if \(" | head -30Repository: getdokan/dokan
Length of output: 172
🏁 Script executed:
# Check if there are other similar functions handling product retrieval
fd "\.php$" -x rg "get_product_variations|get_attributes" {} + | head -20Repository: getdokan/dokan
Length of output: 29864
🌐 Web query:
WooCommerce wc_get_product function return value false invalid product ID
💡 Result:
Common causes why wc_get_product() returns false — and how to fix them:
-
You passed an invalid ID / non‑product (or the product is trashed/deleted). Verify the ID exists and is a product. Use get_post( $id ) first. [1][3]
-
You call wc_get_product() too early (before WooCommerce registers post types/taxonomies). Call it after the WooCommerce init hooks (e.g. on/after 'init' or 'woocommerce_init'), not on plugin load. The function itself returns false if those actions haven’t run. [2][1]
-
Database/upgrade or SKU lookup issues (e.g. wc_get_product_id_by_sku problems) can make IDs unresolved. Ensure the WC DB is upgraded and the SKU → ID lookup returns a valid ID. [4]
-
Plugin/theme code passes the false return downstream (causing fatal errors). Always guard the result before calling methods:
$product = wc_get_product( $id );
if ( ! $product instanceof WC_Product ) {
// handle missing product (skip / log / show fallback)
} else {
// safe to use $product->get_price(), etc.
}
Quick debugging steps:
- Confirm post exists and post_type = 'product'. [3]
- Call wc_get_product() after 'init' (or wrap in add_action('init', ...)). [2][1]
- Check for DB upgrade notices and test wc_get_product_id_by_sku() if using SKUs. [4]
- Add instanceof or truthy checks to avoid fatal errors. [1][3]
Sources: WooCommerce docs/function source and notes; wc_get_product behavior & hook requirement; StackOverflow debugging; WC GitHub/issue about SKU/ID lookups. [1][2][3][4]
Add guard against missing or invalid parent products.
wc_get_product() can return false for invalid/deleted products. Calling get_attributes() on it will cause a fatal error, particularly when the AJAX handler receives a bad product_id. Return early if the parent product doesn't exist or isn't a variable product.
Suggested guard
$variations_data = [];
$parent_product = wc_get_product( $product_id );
+ if ( ! $parent_product || ! $parent_product->is_type( 'variable' ) ) {
+ return [];
+ }
if ( $variations ) {🤖 Prompt for AI Agents
In `@includes/Product/FormManager.php` around lines 246 - 323, The
get_product_variations function assumes wc_get_product($product_id) returns a
valid variable product; add a guard after $parent_product =
wc_get_product($product_id) that returns an empty array if $parent_product is
false/null or not a variable product (e.g. ! $parent_product || !
$parent_product->is_type('variable') or not an instance of
\WC_Product_Variable), so you never call $parent_product->get_attributes() on an
invalid object.
includes/Product/FormManager.php
Outdated
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, verify the file exists and contains the function
find . -path "./includes/Product/FormManager.php" -type fRepository: getdokan/dokan
Length of output: 92
🏁 Script executed:
# Check the file size and view the specified lines
wc -l ./includes/Product/FormManager.php && sed -n '400,425p' ./includes/Product/FormManager.phpRepository: getdokan/dokan
Length of output: 645
🏁 Script executed:
# Search for other AJAX endpoints in the same file to understand security patterns
rg -A 8 'public function.*\(' ./includes/Product/FormManager.php | head -100Repository: getdokan/dokan
Length of output: 2341
🏁 Script executed:
# Check if there are nonce-related patterns elsewhere in the codebase
rg 'wp_verify_nonce|check_ajax_referer' includes/Product/FormManager.phpRepository: getdokan/dokan
Length of output: 175
🏁 Script executed:
# Look for similar security patterns in other files
rg -l 'dokan_edit_product' --type php | head -10Repository: getdokan/dokan
Length of output: 420
🏁 Script executed:
# Search for how dokan_edit_product capability is used with product_id
rg 'dokan_edit_product.*product' --type php | head -20Repository: getdokan/dokan
Length of output: 2092
🏁 Script executed:
# Check for capability checking patterns with product context
rg 'current_user_can.*dokan_edit_product' --type php -A 2 | head -40Repository: getdokan/dokan
Length of output: 2412
🏁 Script executed:
# Look for get_product_variations method to understand what it returns
rg -A 10 'function get_product_variations' ./includes/Product/FormManager.phpRepository: getdokan/dokan
Length of output: 504
🏁 Script executed:
# Check how nonces are generated in the frontend/templates
rg 'wp_nonce_field|wp_create_nonce' includes/Product/FormManager.phpRepository: getdokan/dokan
Length of output: 132
🏁 Script executed:
# Verify wc_get_product usage in the codebase
rg 'wc_get_product\(' --type php | head -10Repository: getdokan/dokan
Length of output: 1105
🏁 Script executed:
# Search for how Dokan handles product-scoped capability checks
rg 'current_user_can.*dokan_edit_product.*\$' --type php -B 2 -A 2Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
# Search for product ownership checks or vendor verification
rg 'get_post.*post_author|vendor_id|get_author' includes/Product/ --type php -A 2 | head -40Repository: getdokan/dokan
Length of output: 1634
🏁 Script executed:
# Look for how dokan_save_product_data verifies product ownership
rg -A 30 'function dokan_save_product_data' ./includes/Product/FormManager.phpRepository: getdokan/dokan
Length of output: 949
🏁 Script executed:
# Check if there's a capability map or filter for dokan_edit_product
rg 'dokan_edit_product' includes/Capabilities/ --type phpRepository: getdokan/dokan
Length of output: 120
🏁 Script executed:
# Search for capability definitions/registration
rg 'register_cap|add_cap|map_meta_cap' --type php | grep -i dokanRepository: getdokan/dokan
Length of output: 352
🏁 Script executed:
# Look for how capabilities are filtered or mapped
rg 'dokan_edit_product' includes/ --type php | grep -E 'apply_filters|do_action' -B 2 -A 2Repository: getdokan/dokan
Length of output: 797
🏁 Script executed:
# Check the REST controller to see how product-scoped checks are done
rg -A 50 'class ProductController' ./includes/REST/ProductController.php | head -70Repository: getdokan/dokan
Length of output: 1188
🏁 Script executed:
# Search for vendor/author checks in other AJAX handlers
rg 'wp_ajax.*dokan' includes/ --type php -A 10 | grep -E 'wp_ajax|vendor|author|post_author' -A 3 | head -50Repository: getdokan/dokan
Length of output: 3947
🏁 Script executed:
# Look specifically at the Ajax.php file for patterns
head -100 includes/Ajax.phpRepository: getdokan/dokan
Length of output: 4038
🏁 Script executed:
# Search for how ownership is verified - check if there's a function for this
rg 'get_current_user_id|wp_get_current_user' includes/Product/FormManager.php -A 3Repository: getdokan/dokan
Length of output: 208
🏁 Script executed:
# Check how dokan_save_product_data verifies it can modify a product
rg -B 5 -A 20 'dokan\(\)->product->create' ./includes/Product/FormManager.phpRepository: getdokan/dokan
Length of output: 691
🏁 Script executed:
# Look for how vendor access is checked in GET requests
rg 'get_post_field.*post_author' includes/ --type php -B 3 -A 3 | head -50Repository: getdokan/dokan
Length of output: 2990
🏁 Script executed:
# Find the complete dokan_is_product_author function
rg -B 5 -A 10 'function dokan_is_product_author' includes/functions.phpRepository: getdokan/dokan
Length of output: 457
🏁 Script executed:
# Check if there are any existing permission checks in the variations endpoint context
rg -B 10 -A 5 'get_product_variations' includes/Product/FormManager.phpRepository: getdokan/dokan
Length of output: 2323
Add nonce validation and vendor ownership check to AJAX variations endpoint.
This endpoint lacks CSRF protection and permission verification. The sibling function dokan_save_product_data() uses nonce validation with the 'form_manager' action (already available in templates). Additionally, any authenticated user can fetch variations for any product; add a vendor ownership check to prevent information disclosure across vendors.
🔐 Suggested hardening
public function dokan_get_product_variations() {
+ if ( ! isset( $_REQUEST['_nonce'] ) || ! wp_verify_nonce( sanitize_key( $_REQUEST['_nonce'] ), 'form_manager' ) ) { // phpcs:ignore
+ wp_send_json_error( __( 'Invalid nonce', 'dokan-lite' ), 403 );
+ }
if ( ! isset( $_REQUEST['product_id'] ) ) {// phpcs:ignore
wp_send_json_error( __( 'Product ID is required', 'dokan-lite' ) );
}
- $product_id = intval( wp_unslash( $_REQUEST['product_id'] ) ); // phpcs:ignore
+ $product_id = absint( wp_unslash( $_REQUEST['product_id'] ) ); // phpcs:ignore
+ if ( ! dokan_is_product_author( $product_id ) ) {
+ wp_send_json_error( __( 'You have no permission to edit this product', 'dokan-lite' ), 403 );
+ }
+ if ( ! wc_get_product( $product_id ) ) {
+ wp_send_json_error( __( 'Product not found', 'dokan-lite' ), 404 );
+ }
$variations = self::get_product_variations( $product_id );🤖 Prompt for AI Agents
In `@includes/Product/FormManager.php` around lines 409 - 417, The
dokan_get_product_variations AJAX handler lacks CSRF and permission checks; add
a nonce verification using the same 'form_manager' action (verify the nonce from
the request with wp_verify_nonce and return wp_send_json_error on failure) and
ensure the current user is the product owner before returning variations (e.g.,
compare get_post_field('post_author', $product_id) to get_current_user_id() or
use current_user_can('edit_post', $product_id)); if ownership check fails,
return wp_send_json_error with an appropriate message/403, otherwise call
self::get_product_variations($product_id) and wp_send_json_success as before.
src/admin/pages/Settings.vue
Outdated
There was a problem hiding this comment.
Guard resetAllFields against missing fields/default to avoid runtime errors.
If any block in dokan_form_manager lacks fields or a default object, this action will throw and break the UI. Add defensive checks before dereferencing.
🔧 Suggested hardening
resetAllFields( sectionId ) {
- let settingFields = Object.entries( this.settingFields[sectionId] );
- let clonedSectionSettingValues = JSON.parse( JSON.stringify( this.settingValues[sectionId] ) );
+ const sectionFields = this.settingFields?.[ sectionId ];
+ const sectionValues = this.settingValues?.[ sectionId ];
+ if ( ! sectionFields || ! sectionValues ) {
+ return;
+ }
+ let settingFields = Object.entries( sectionFields );
+ let clonedSectionSettingValues = JSON.parse( JSON.stringify( sectionValues ) );
- 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( ( key ) => {
+ const blockId = key[0];
+ const sectionValue = key[1];
+
+ if ( ! blockId || ! sectionValue?.fields ) {
+ return;
+ }
+ Object.entries( sectionValue.fields ).forEach( ( [ fieldId, fieldData ] ) => {
+ const defaults = fieldData?.default;
+ if ( ! defaults ) {
+ return;
+ }
+ if ( clonedSectionSettingValues?.[ blockId ]?.fields?.[ fieldId ] ) {
+ clonedSectionSettingValues[blockId].fields[fieldId].visibility = defaults.visibility;
+ clonedSectionSettingValues[blockId].fields[fieldId].required = defaults.required;
+ }
+ } );
+ } );
this.settingValues[sectionId] = clonedSectionSettingValues;
},📝 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.
| handleAction( sectionId ) { | |
| if ( 'dokan_form_manager' === sectionId ) { | |
| this.resetAllFields( sectionId ); | |
| } | |
| }, | |
| 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; | |
| }, | |
| resetAllFields( sectionId ) { | |
| const sectionFields = this.settingFields?.[ sectionId ]; | |
| const sectionValues = this.settingValues?.[ sectionId ]; | |
| if ( ! sectionFields || ! sectionValues ) { | |
| return; | |
| } | |
| let settingFields = Object.entries( sectionFields ); | |
| let clonedSectionSettingValues = JSON.parse( JSON.stringify( sectionValues ) ); | |
| settingFields.forEach( ( key ) => { | |
| const blockId = key[0]; | |
| const sectionValue = key[1]; | |
| if ( ! blockId || ! sectionValue?.fields ) { | |
| return; | |
| } | |
| Object.entries( sectionValue.fields ).forEach( ( [ fieldId, fieldData ] ) => { | |
| const defaults = fieldData?.default; | |
| if ( ! defaults ) { | |
| return; | |
| } | |
| if ( clonedSectionSettingValues?.[ blockId ]?.fields?.[ fieldId ] ) { | |
| clonedSectionSettingValues[blockId].fields[fieldId].visibility = defaults.visibility; | |
| clonedSectionSettingValues[blockId].fields[fieldId].required = defaults.required; | |
| } | |
| } ); | |
| } ); | |
| this.settingValues[sectionId] = clonedSectionSettingValues; | |
| }, |
🤖 Prompt for AI Agents
In `@src/admin/pages/Settings.vue` around lines 710 - 733, The resetAllFields
method can throw when a block lacks fields or a field lacks a default; update
resetAllFields to defensive-check before dereferencing: inside the
settingFields.forEach loop, ensure sectionValue.fields exists and is an object,
ensure clonedSectionSettingValues[blockId] and its ['fields'][FieldId] exist,
and only then read fieldData.default (or use optional chaining and fallbacks)
before assigning visibility and required; reference the resetAllFields function
and the settingFields/settingValues structures in your changes and skip any
field/block that doesn't have the expected fields/default shape to avoid runtime
errors.
| const items = Array.isArray( images ) ? images : [ images ]; | ||
|
|
||
| // If no images and no children (like uploader button), rendering nothing is safer | ||
| if ( items.length === 0 && ! children ) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Filter out falsy items to prevent runtime errors.
If images is null, undefined, or contains falsy elements, the current logic will create an array like [undefined] that passes the length check but causes a TypeError when accessing item.url in the map.
🐛 Proposed fix
- const items = Array.isArray( images ) ? images : [ images ];
+ const items = ( Array.isArray( images ) ? images : [ images ] ).filter( Boolean );
// If no images and no children (like uploader button), rendering nothing is safer
if ( items.length === 0 && ! children ) {
return null;
}📝 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 items = Array.isArray( images ) ? images : [ images ]; | |
| // If no images and no children (like uploader button), rendering nothing is safer | |
| if ( items.length === 0 && ! children ) { | |
| return null; | |
| } | |
| const items = ( Array.isArray( images ) ? images : [ images ] ).filter( Boolean ); | |
| // If no images and no children (like uploader button), rendering nothing is safer | |
| if ( items.length === 0 && ! children ) { | |
| return null; | |
| } |
🤖 Prompt for AI Agents
In `@src/dashboard/form-manager/components/ImagePreview.tsx` around lines 16 - 21,
In ImagePreview, the items array can contain falsy values (e.g., when images is
null/undefined) causing TypeError when accessing item.url; update the conversion
to normalize and filter falsy entries (e.g., convert images to an array then
apply filter(Boolean)) so items contains only valid image objects, and keep the
existing early-return that checks items.length and children; references: images,
items, children, and item.url in the ImagePreview component.
| const fetchedVariationData = async () => { | ||
| setIsExpanded( ! isExpanded ); | ||
|
|
||
| if ( sections.length > 0 ) { | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const response = await apiFetch< { | ||
| sections: Section[]; | ||
| vendor_earning: number; | ||
| } >( { | ||
| path: `/dokan/v1/products/${ variation.id }/fields`, | ||
| } ); | ||
| setSections( response.sections ); | ||
| setVendorEarning( response.vendor_earning ); | ||
| } catch ( error ) { | ||
| console.error( 'Error fetching variation data:', error ); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Fetch error leaves user stuck on "Loading..." with no feedback.
If the API call fails, the error is only logged to console. The user sees "Loading..." indefinitely with no indication of failure or retry option.
🐛 Proposed fix to handle fetch errors
+ const [ fetchError, setFetchError ] = useState< string | null >( null );
const fetchedVariationData = async () => {
setIsExpanded( ! isExpanded );
if ( sections.length > 0 ) {
return;
}
+ setFetchError( null );
try {
const response = await apiFetch< {
sections: Section[];
vendor_earning: number;
} >( {
path: `/dokan/v1/products/${ variation.id }/fields`,
} );
setSections( response.sections );
setVendorEarning( response.vendor_earning );
} catch ( error ) {
console.error( 'Error fetching variation data:', error );
+ setFetchError( __( 'Failed to load variation data. Click to retry.', 'dokan-lite' ) );
}
};Then update the render logic:
- ) : (
+ ) : fetchError ? (
+ <div
+ role="button"
+ onClick={ () => {
+ setSections( [] );
+ fetchedVariationData();
+ } }
+ className="p-4 text-center text-red-500 cursor-pointer"
+ >
+ { fetchError }
+ </div>
+ ) : (
<div className="p-4 text-center text-gray-400">
{ __( 'Loading...', 'dokan-lite' ) }
</div>🤖 Prompt for AI Agents
In `@src/dashboard/form-manager/components/variation/VariationCard.tsx` around
lines 19 - 38, The current fetchedVariationData toggles setIsExpanded
immediately and only logs errors, leaving the UI stuck on "Loading..."; update
fetchedVariationData to manage loading and error state: introduce local states
(e.g., isLoading and fetchError), set isLoading = true before the apiFetch, move
setIsExpanded( true ) to run after a successful fetch (setSections and
setVendorEarning), and in the catch set fetchError with the error and isLoading
= false (and ensure isExpanded is not left toggled); update the render logic to
show an error message and a retry button that calls fetchedVariationData when
fetchError is present and to use isLoading to drive the "Loading..." indicator.
| const generateVariations = async () => { | ||
| // specific logic to generate variations | ||
| if ( | ||
| confirm( | ||
| __( | ||
| 'Are you sure you want to generate variations? This will overwrite existing variations.', | ||
| 'dokan-lite' | ||
| ) | ||
| ) | ||
| ) { | ||
| const response: any = await ajaxRequest( { | ||
| action: 'dokan_link_all_variations', | ||
| post_id: productId, | ||
| security: dokan.link_variation_nonce, | ||
| } ); | ||
| await fetchVariations(); | ||
| toast( { | ||
| type: 'success', | ||
| title: response.data.message, | ||
| } ); | ||
| } | ||
| }; | ||
|
|
||
| const addVariation = async () => { | ||
| try { | ||
| const response: any = await ajaxRequest( { | ||
| action: 'dokan_add_variation', | ||
| post_id: productId, | ||
| security: dokan.add_variation_nonce, | ||
| loop: 0, | ||
| } ); | ||
| await fetchVariations(); | ||
| toast( { | ||
| type: 'success', | ||
| title: response.data.message, | ||
| } ); | ||
| } catch ( error ) { | ||
| console.error( 'Error adding variation:', error ); | ||
| } | ||
| }; | ||
|
|
||
| const removeVariation = async ( variation: VariationType ) => { | ||
| if ( | ||
| ! confirm( | ||
| __( | ||
| 'Are you sure you want to remove this variation?', | ||
| 'dokan-lite' | ||
| ) | ||
| ) | ||
| ) { | ||
| return; | ||
| } | ||
| try { | ||
| const response: any = await ajaxRequest( { | ||
| action: 'dokan_remove_variation', | ||
| 'variation_ids[]': variation.id, | ||
| security: dokan.delete_variations_nonce, | ||
| } ); | ||
| await fetchVariations(); | ||
| toast( { | ||
| type: 'success', | ||
| title: response.data.message, | ||
| } ); | ||
| } catch ( error ) { | ||
| console.error( 'Error removing variation:', error ); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Add/generate/remove flows need error handling and response.success checks.
These flows always show success toasts (or only log errors) even when the server returns a failure payload. That can mislead users and hide failures.
✅ Suggested fix (pattern)
const generateVariations = async () => {
// specific logic to generate variations
if (
confirm(
__(
'Are you sure you want to generate variations? This will overwrite existing variations.',
'dokan-lite'
)
)
) {
- const response: any = await ajaxRequest( {
- action: 'dokan_link_all_variations',
- post_id: productId,
- security: dokan.link_variation_nonce,
- } );
- await fetchVariations();
- toast( {
- type: 'success',
- title: response.data.message,
- } );
+ try {
+ const response: any = await ajaxRequest( {
+ action: 'dokan_link_all_variations',
+ post_id: productId,
+ security: dokan.link_variation_nonce,
+ } );
+ if ( ! response?.success ) {
+ toast( {
+ type: 'error',
+ title:
+ response?.data?.message ||
+ __( 'Error generating variations', 'dokan-lite' ),
+ } );
+ return;
+ }
+ await fetchVariations();
+ toast( {
+ type: 'success',
+ title: response.data.message,
+ } );
+ } catch ( error ) {
+ console.error( 'Error generating variations:', error );
+ toast( {
+ type: 'error',
+ title: __( 'Error generating variations', 'dokan-lite' ),
+ } );
+ }
}
};Apply the same pattern to addVariation and removeVariation (check response.success and toast on error).
🤖 Prompt for AI Agents
In `@src/dashboard/form-manager/context/VariationContext.tsx` around lines 203 -
269, generateVariations, addVariation and removeVariation currently always show
a success toast (and some only log errors) even when the server returns a
failure; update each function to 1) wrap the ajaxRequest call in try/catch (add
try/catch to generateVariations), 2) after awaiting ajaxRequest check
response.success and if true call fetchVariations and show the success toast
using response.data.message, otherwise show an error toast with
response.data.message (or a fallback message) and do not call fetchVariations,
and 3) in the catch block log the error and show an error toast with the caught
error message; reference the functions generateVariations, addVariation and
removeVariation when making these changes.
src/dashboard/form-manager/utils.tsx
Outdated
There was a problem hiding this comment.
Required validation treats 0/false as empty.
!value will flag valid values like 0 (e.g., stock quantity) as missing. This can block submissions for legitimate values. Consider a stricter emptiness check (and optionally a field‑type specific rule for required checkboxes).
✅ Suggested adjustment
- const value = values[ field.id ];
- if ( ! value || ( Array.isArray( value ) && value.length === 0 ) ) {
+ const value = values[ field.id ];
+ const isEmpty =
+ value === undefined ||
+ value === null ||
+ ( typeof value === 'string' && value.trim() === '' ) ||
+ ( Array.isArray( value ) && value.length === 0 );
+ if ( isEmpty ) {
newErrors[ field.id ] = __(
'Please fill out this field.',
'dokan-lite'
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const value = values[ field.id ]; | |
| if ( ! value || ( Array.isArray( value ) && value.length === 0 ) ) { | |
| newErrors[ field.id ] = __( | |
| 'Please fill out this field.', | |
| 'dokan-lite' | |
| ); | |
| } | |
| const value = values[ field.id ]; | |
| const isEmpty = | |
| value === undefined || | |
| value === null || | |
| ( typeof value === 'string' && value.trim() === '' ) || | |
| ( Array.isArray( value ) && value.length === 0 ); | |
| if ( isEmpty ) { | |
| newErrors[ field.id ] = __( | |
| 'Please fill out this field.', | |
| 'dokan-lite' | |
| ); | |
| } |
🤖 Prompt for AI Agents
In `@src/dashboard/form-manager/utils.tsx` around lines 78 - 84, The
required-field check incorrectly uses "!value" which treats valid falsy values
like 0 or false as empty; update the condition in the validation logic that
reads "const value = values[field.id]" to only consider a field empty when value
=== undefined || value === null || (typeof value === 'string' && value.trim()
=== '') || (Array.isArray(value) && value.length === 0) — keep the array-length
check for arrays and allow numeric 0 and boolean false; ensure you update the
branch that sets newErrors[field.id] so it only triggers for these stricter
emptiness cases (and consider special-case checkbox handling elsewhere if
needed).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/dashboard/form-manager/utils.tsx`:
- Around line 331-358: The function injectRemainingFields mutates its
remainingFieldsBySection argument via delete
remainingFieldsBySection[newItem.id]; avoid mutating the caller's object by
creating a local copy or tracking consumed keys instead: e.g., at start of
injectRemainingFields make a shallow copy of remainingFieldsBySection (or
accept/derive a consumedKeys Set and mark keys as consumed there) and use that
copy/Set when appending children so the original remainingFieldsBySection passed
in (as used by useLayouts.tsx) is not modified; update references to delete to
operate on the local copy/Set and keep recursive calls passing the local
copy/Set.
♻️ Duplicate comments (1)
src/dashboard/form-manager/utils.tsx (1)
78-84: Required validation treats0/falseas empty.The
!valuecheck incorrectly flags valid falsy values (e.g.,0for stock quantity,falsefor checkboxes) as missing, potentially blocking valid form submissions.✅ Recommended fix
const value = values[ field.id ]; - if ( ! value || ( Array.isArray( value ) && value.length === 0 ) ) { + const isEmpty = + value === undefined || + value === null || + ( typeof value === 'string' && value.trim() === '' ) || + ( Array.isArray( value ) && value.length === 0 ); + if ( isEmpty ) { newErrors[ field.id ] = __( 'Please fill out this field.', 'dokan-lite' ); }
🧹 Nitpick comments (6)
src/dashboard/form-manager/components/variation/VariationInternalForm.tsx (1)
29-52: Prefer stable key over array index.Using array index as a React key can cause rendering issues if attributes are reordered or filtered. Since
attr.valueappears to be a unique identifier per attribute, use it as the key instead.Also, consider typing the
onChangevalue parameter more strictly to match theselected_valuestructure.♻️ Suggested improvement
- { variation.attributes.map( ( attr, idx: number ) => { + { variation.attributes.map( ( attr ) => { return ( - <div key={ idx }> + <div key={ attr.value }> <Select options={ attr.options } placeholder={ attr.label } value={ attr.selected_value } - onChange={ ( value: any ) => { - const newAttributes = [ - ...variation.attributes, - ]; - newAttributes[ idx ] = { - ...newAttributes[ idx ], + onChange={ ( value: { label: string; value: string } ) => { + const newAttributes = variation.attributes.map( + ( a ) => + a.value === attr.value + ? { ...a, selected_value: value } + : a + ); - selected_value: value, - }; updateVariation( { ...variation, attributes: newAttributes, } ); } } /> </div> ); } ) }src/dashboard/form-manager/utils.tsx (2)
8-30: Consider adding timeout and error handling for AJAX requests.The
ajaxRequestwrapper lacks a timeout configuration, which could leave requests hanging indefinitely. Also, the rejected error object from jQuery may not be serializable or useful.♻️ Suggested improvement
export const ajaxRequest = ( data: Record< string, any > | FormData, - method: string = 'POST' + method: string = 'POST', + timeout: number = 30000 ) => { return new Promise( ( resolve, reject ) => { const options: any = { data, url: dokan.ajaxurl, type: method, + timeout, success( response: any ) { resolve( response ); }, - error( error: any ) { - reject( error ); + error( _jqXHR: any, textStatus: string, errorThrown: string ) { + reject( new Error( textStatus || errorThrown || 'Request failed' ) ); }, };
109-127: Potential type coercion issue with truthy/falsy value mapping.The current logic only maps string values (
'on','yes','off','no') but doesn't normalizedepValuefromdata. IfdepValueis a booleantruebuttargetValuebecomestruefrom string'on', comparison works. However, ifdepValueis string'on'andvalueis booleantrue, comparison fails.♻️ Suggested normalization
let targetValue = value; if ( value === 'on' || value === 'yes' ) { targetValue = true; } if ( value === 'off' || value === 'no' ) { targetValue = false; } + let normalizedDepValue = depValue; + if ( depValue === 'on' || depValue === 'yes' ) { + normalizedDepValue = true; + } + if ( depValue === 'off' || depValue === 'no' ) { + normalizedDepValue = false; + } + if ( operator === 'equal' ) { - return depValue === targetValue; + return normalizedDepValue === targetValue; } if ( operator === 'not_equal' ) { - return depValue !== targetValue; + return normalizedDepValue !== targetValue; }src/dashboard/form-manager/hooks/useLayouts.tsx (3)
37-49:getFieldHeadingreturns partial object inconsistently.When field is not found, an empty object
{}is returned. This is then spread into layout items (e.g., line 140:...getFieldHeading('inventory')). Consider returning explicitundefinedvalues or handling the not-found case where it's used.♻️ Suggested improvement for type safety
const getFieldHeading = useCallback( ( fieldId: string ) => { const field = getField( sections, fieldId ); if ( ! field ) { - return {}; + return { label: undefined, description: undefined }; } return { label: field.title, description: field.description, }; }, [ sections ] );
52-66: Magic number for breakpoint.The
768breakpoint is hardcoded. Consider extracting this to a constant for maintainability and consistency across the codebase.♻️ Suggested improvement
+const RESPONSIVE_BREAKPOINT = 768; + // Define root layout based on window width const rootLayout = useMemo( () => { - if ( width && width > 768 ) { + if ( width && width > RESPONSIVE_BREAKPOINT ) { return {
68-278: Large inline layout definition reduces readability.The static layout structure spanning ~170 lines makes the hook harder to maintain. Consider extracting the base layout definition to a separate constant or configuration file.
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 Fix all issues with AI agents
In `@includes/Product/Hooks.php`:
- Around line 625-629: The early-return treats an empty product_brand as "no
submission" and prevents dokan()->product->save_brands($product_id, $brand_ids)
from removing all brands; change the check to detect presence of the key in
$product_data (e.g., using array_key_exists('product_brand', $product_data) or
isset($product_data['product_brand'])) and only skip when the key is truly
absent, then always call save_brands when the key exists (even if it's an empty
array) so existing brands can be cleared; also replace the tab at the current
indentation with spaces to match the file's spacing style.
In `@includes/Product/Manager.php`:
- Around line 829-846: prepare_downloads currently assumes $file_names and
$file_hashes have the same indexes as $file_urls; guard against mismatched
arrays by checking isset($file_names[$i]) and isset($file_hashes[$i]) (or using
a fallback empty string) before accessing them, and skip or default entries when
those indexes are missing so you don't index out of bounds or create corrupt
download entries; update the loop in prepare_downloads to validate each index
for $file_names and $file_hashes (and sanitize fallback values with wc_clean)
before building the $downloads[] array.
In `@includes/ProductCategory/Helper.php`:
- Line 410: Docblock contains the placeholder "@since DOKAN_SINCE" and must be
updated to the actual release version; locate the docblock for the Helper in the
ProductCategory\Helper code (the docblock above the Helper class or the related
method where `@since` DOKAN_SINCE appears) and replace DOKAN_SINCE with the
concrete version string for this release (or with the canonical version constant
used in the project, e.g., DOKAN_VERSION or similar) so the `@since` tag reflects
the real version.
In `@includes/ProductForm/Init.php`:
- Around line 182-223: The value_callback closures for
Elements::DATE_ON_SALE_FROM and Elements::DATE_ON_SALE_TO are clearing
user-supplied input by returning an empty string when $value !== '' instead of
preserving it; update both value_callback functions to return $value when it is
non-empty, keep the WC_Product checks and the fallback to
product->get_date_on_sale_from('edit')->date('Y-m-d') / get_date_on_sale_to(...)
unchanged so existing behavior remains for empty $value, and ensure the
callbacks return the supplied $value to avoid discarding form resubmissions or
validated input.
In `@includes/REST/ProductControllerV3.php`:
- Around line 90-114: The create_item and update_item methods call
parent::create_item/update_item and then use $product->data directly and fire
hooks even when a WP_Error occurred; change both methods to check
is_wp_error($product) immediately and return the error early, and when
successful call $product->get_data() (not ->data) to obtain the id, then cast it
to int and call do_action('dokan_new_product_added', $product_id, $params) or
do_action('dokan_product_updated', $product_id, $params') respectively so hooks
only fire on successful responses.
In `@src/dashboard/form-manager/App.tsx`:
- Around line 132-152: The attributes mapping in App.tsx may call .map on
attr.options when options is null/undefined; update the product.attributes
mapping (the callback passed to product.attributes.map) to guard attr.options by
using a safe default (e.g., treat attr.options as an empty array when falsy)
before mapping, so options are iterated only when present and the options map
uses (attr.terms?.find(...)? ... ) as before; ensure this change only affects
the options mapping inside the attribute transformation to avoid runtime .map
errors.
- Around line 61-65: The button label is inverted for new products: update the
JSX that sets the label prop (the conditional using isNewProduct in App.tsx) so
when isNewProduct is true it uses a "Create Product" (or "Add Product")
translated string via __( 'Create Product', 'dokan-lite' ) and when false it
uses __( 'Update Product', 'dokan-lite' ); locate the label assignment around
the label={ isNewProduct ? ... : ... } expression and replace the first branch
text accordingly while keeping the translation domain intact.
- Around line 86-110: The mappings for product.category_ids,
product.product_tag, product.product_brand and product.gallery_image_ids in
App.tsx can throw if those fields are null/undefined; update the code that sets
product.categories, product.tags, product.brands and the images concat to
defensively handle missing arrays (e.g., use default empty arrays or optional
chaining before .map and ensure images is an array when spreading) so
product.category_ids?.map or (product.category_ids || []) is used for
product.categories, likewise for product.product_tag and product.product_brand,
and ensure gallery_image_ids is treated as [] when absent before creating or
concatenating into product.images.
In `@src/dashboard/form-manager/components/AttributeVariationEdit.tsx`:
- Around line 85-100: The AttributeCard list is using the array index as key
which can break component identity when attributes are reordered/removed; update
the map to use a stable unique id (e.g., attr.id or a generated attr.tempId) as
the key for each AttributeCard and ensure new attributes get a persistent tempId
when created (generate a UUID or incremental id in the attribute creation
codepath). Locate the attributes mapping where AttributeCard is rendered and
replace key={index} with key={attr.id || attr.tempId}, and add logic in the
attribute creation function to assign a unique tempId to new attribute objects
so handleUpdateAttribute and handleRemoveAttribute continue to operate against
stable identifiers.
- Around line 138-147: The variable product UI renders VariationProvider with
productId={data.id} even when data.id is NaN for new products, causing
fetchVariations/generateVariations/addVariation to call APIs with an invalid id;
update the render guard in AttributeVariationEdit to only mount
VariationProvider (and VariationForm) when productType === 'variable' AND
data.id is a valid numeric id (e.g., check Number.isFinite(data.id) or data.id >
0) so VariationProvider’s internal methods (fetchVariations, generateVariations,
addVariation) never run with an undefined/NaN productId, or alternatively delay
rendering VariationProvider until after the product is saved.
In `@src/dashboard/form-manager/components/FileUploadEdit.tsx`:
- Around line 35-42: updateRow is mutating a nested object because newFiles is a
shallow copy and newFiles[index][key] = value changes the original files item;
instead, create an immutable update by cloning the target row before modifying
it (e.g., replace newFiles[index] with a shallow copy of that row merged with
{[key]: value}), then call setFiles(newFiles) and onChange({ [field.id]:
newFiles }) so no original state object is mutated.
- Around line 68-70: The file FileUploadEdit.tsx uses files.map in the render
and currently assigns key={index}, which can break React reconciliation when
rows are added/removed; update the files data structure to include a stable
unique id (e.g., add a tempId or id field when creating a new file row in the
add/new-row handler) and change the render to use that field as the key instead
of index (reference the files variable and the map callback where key is set).
Ensure the unique id is generated when a row is created so existing items keep
stable keys across inserts/deletes.
In `@src/dashboard/form-manager/context/FormContext.tsx`:
- Around line 68-79: product state isn't updated when sections or incoming props
change; add a useEffect in FormContext that watches defaultData and the incoming
props (productId, vendorEarning, variations) and calls setProduct to merge the
new defaults with the current prop values. Specifically, in the component
containing defaultData and the [product, setProduct] state, implement a
useEffect that computes a merged object like { ...defaultData, id: productId,
vendor_earning: vendorEarning, variations } and calls setProduct(merged), with
dependencies [defaultData, productId, vendorEarning, variations] so product
always reflects updated sections and props.
In `@src/dashboard/form-manager/factories/handlers.ts`:
- Around line 178-194: JSDoc for defaultHandler is inaccurate (mentions
TextWithAddon which isn't used); update the comment above the defaultHandler
function to remove the TextWithAddon reference and instead describe the actual
behavior: it returns a FieldConfig with type 'text' by default, switches to type
'number' and sets Edit = PriceEdit when field.name includes 'price' (keep
references to FieldHandler, FieldConfig, and PriceEdit in the doc). Ensure the
param and return annotations remain correct and reflect the conditional
PriceEdit assignment.
- Around line 119-134: The JSDoc for multiSelectHandler is inaccurate and
mentions "special cases like product categories" copied from selectHandler;
update the comment above multiSelectHandler to clearly describe that this
handler builds a configuration for multi-select fields (returns type 'array',
elements from getElementsFromOptions(field), uses SelectEdit as the Edit
component and sets multiple: true), and remove any irrelevant references to
product categories or single-select behavior so the documentation matches the
function implementation (refer to multiSelectHandler, getElementsFromOptions,
and SelectEdit).
- Around line 98-117: selectHandler currently uses a non-null assertion on the
optional parameter (field!.id) which can throw if called without a field; change
the check to safely handle undefined (e.g., use optional chaining or an early
guard like if (!field) return default config) when comparing against
asyncSelectFields and ensure you reference field.id only after verifying field
exists; also replace the loose any on config with the proper FieldConfig type
(and update the return to match FieldHandler) so types enforce the shape
(symbols to edit: selectHandler, field, config, asyncSelectFields,
AsyncSelectEdit, SelectEdit, FieldHandler, FieldConfig).
♻️ Duplicate comments (3)
includes/ProductForm/Init.php (1)
318-345:WC_Producthas noget_brand_ids()— use the Product Manager helper.Calling
get_brand_ids()onWC_Productwill fatal. Use the manager accessor to resolve brand IDs.🐛 Suggested fix
- return $product->get_brand_ids(); + return dokan()->product->get_brand_ids( $product->get_id() );src/dashboard/form-manager/components/variation/VariationCard.tsx (2)
19-39: Fetch error leaves user stuck on "Loading..." with no feedback.This issue was flagged in a previous review. If the API call fails, the error is only logged to console. The user sees "Loading..." indefinitely with no indication of failure or retry option.
19-24: Expansion toggle before fetch creates inconsistent UX.
setIsExpanded(!isExpanded)is called immediately before the async fetch. If the fetch fails, the card remains expanded showing "Loading..." forever. Consider toggling expansion only after a successful fetch, or managing a separate loading state.🐛 Suggested improvement
const fetchedVariationData = async () => { - setIsExpanded( ! isExpanded ); + // If collapsing, just toggle + if ( isExpanded ) { + setIsExpanded( false ); + return; + } if ( sections.length > 0 ) { + setIsExpanded( true ); return; } try { const response = await apiFetch< { sections: Section[]; vendor_earning: number; } >( { path: `/dokan/v3/products/${ variation.id }/fields`, } ); setSections( response.sections ); setVendorEarning( response.vendor_earning ); + setIsExpanded( true ); } catch ( error ) { // eslint-disable-next-line no-console console.error( 'Error fetching variation data:', error ); + // Consider showing error state to user } };
🧹 Nitpick comments (12)
includes/ProductCategory/Helper.php (1)
432-441: Consider refactoring to eliminate code duplication.The ancestor calculation logic (lines 432-441) is nearly identical to the logic in
set_object_terms_from_chosen_categories(lines 184-194). Consider havingset_object_terms_from_chosen_categoriescall this new method internally to avoid maintaining duplicate logic.♻️ Proposed refactor for set_object_terms_from_chosen_categories
public static function set_object_terms_from_chosen_categories( $post_id, $chosen_categories = [] ) { if ( empty( $chosen_categories ) || ! is_array( $chosen_categories ) ) { return; } - /** - * If enabled any one middle category in dokan product multi-step category selection. - */ - $any_category_selection = self::is_any_category_selection_enabled(); - - $all_ancestors = []; - - // If category middle selection is true, then we will save only the chosen categories or we will save all the ancestors. - if ( $any_category_selection ) { - $all_ancestors = $chosen_categories; - } else { - // we need to assign all ancestor of chosen category to add to the given product - foreach ( $chosen_categories as $term_id ) { - $all_ancestors = array_merge( $all_ancestors, get_ancestors( $term_id, 'product_cat' ), [ $term_id ] ); - } - } + $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' ); + wp_set_object_terms( $post_id, $all_ancestors, 'product_cat' ); }src/dashboard/form-manager/components/PriceEdit.tsx (1)
4-66: Debounce commission lookups and ignore stale responses.Every keystroke fires a request and slower responses can overwrite newer earnings. Debouncing plus a “latest price” guard prevents noisy traffic and stale UI.
♻️ Suggested update
-import { useState } from '@wordpress/element'; +import { useRef, useState } from '@wordpress/element'; ... - const [ vendorEarning, setVendorEarning ] = useState( - Number( data.vendor_earning ) - ); + const [ vendorEarning, setVendorEarning ] = useState( + Number( data.vendor_earning ) || 0 + ); + const lastPriceRef = useRef( '' ); + const debounceRef = useRef< ReturnType< typeof setTimeout > | null >( null ); ... const vendorEarningHandler = async ( price: string ) => { if ( field.id === 'regular_price' ) { + const requestPrice = price; // fetch the vendor earning try { const response = await apiFetch( { path: addQueryArgs( '/dokan/v1/commission', { amount: price, product_id: data.id, category_ids: data.category_ids || [], context: 'seller', } ), } ); - setVendorEarning( Number( response ) ); + if ( lastPriceRef.current === requestPrice ) { + setVendorEarning( Number( response ) ); + } } catch ( error ) { - setVendorEarning( 0 ); + if ( lastPriceRef.current === requestPrice ) { + setVendorEarning( 0 ); + } } } }; ... onChange={ ( _, rawValue ) => { const value = String( rawValue ); onChange( { [ field.id ]: value } ); - void vendorEarningHandler( value ); + if ( field.id === 'regular_price' ) { + lastPriceRef.current = value; + if ( debounceRef.current ) { + clearTimeout( debounceRef.current ); + } + debounceRef.current = setTimeout( () => { + void vendorEarningHandler( value ); + }, 300 ); + } } }src/dashboard/form-manager/hooks/useLayouts.tsx (2)
51-66: Consider extracting the breakpoint as a named constant.The magic number
768represents a tablet/mobile breakpoint. Extracting it improves readability and makes future adjustments easier.♻️ Suggested improvement
+const TABLET_BREAKPOINT = 768; + // Define root layout based on window width const rootLayout = useMemo( () => { - if ( width && width > 768 ) { + if ( width && width > TABLET_BREAKPOINT ) { return { type: 'row',
68-252: Large hardcoded layout structure could benefit from extraction.The layout configuration is extensive (~180 lines). Consider extracting the initial layout structure to a separate constant or configuration file for better maintainability and testability.
That said, the logic for collecting used fields, injecting remaining fields, and appending new sections is well-structured and follows a clear flow.
src/dashboard/form-manager/App.tsx (2)
78-84:preparePayloadmutates keys in-place on a shallow copy.While Line 170 passes
{ ...product }, the shallow copy means nested objects/arrays are still shared references. The direct mutation ofproduct[key] = nullis safe for primitives, but the subsequent operations that reassign array properties could cause issues if the original object is referenced elsewhere.Consider using a deep clone or restructuring to avoid mutation side effects.
♻️ Suggested approach
const preparePayload = ( product: Record< string, any > ) => { + // Create a working copy to avoid mutating the original + const payload: Record< string, any > = {}; + // 1. convert empty strings to null Object.keys( product ).forEach( ( key ) => { - if ( product[ key ] === '' ) { - product[ key ] = null; - } + payload[ key ] = product[ key ] === '' ? null : product[ key ]; } ); + + // Continue building payload without mutating input...
165-185: Error handling doesn't re-throw, preventing FormProvider from knowing about failures.The
onSubmithandler catches errors and shows a toast, but doesn't re-throw. Looking atFormProvider.submitHandler(from relevant snippets), it expects errors to be thrown so it can handle thefinallyblock properly. Currently, theisLoadingstate will be reset correctly, but any error-specific handling in the provider won't trigger.♻️ Suggested fix to re-throw after toast
} catch ( err: any ) { toast( { type: 'error', title: err.message || __( 'Error saving product.', 'dokan-lite' ), } ); // eslint-disable-next-line no-console console.error( 'Error saving product:', err ); + throw err; }src/dashboard/form-manager/components/AttributeVariationEdit.tsx (2)
11-11: Props should be properly typed instead of usingany.The component accepts
data,field, andonChangeprops but usesanytyping, losing type safety benefits.♻️ Suggested typing
-const AttributeVariationEditor = ( { data, field, onChange }: any ) => { +interface AttributeVariationEditorProps { + data: Record< string, any >; + field: { + id: string; + label: string; + error?: string; + elements?: { label: string; value: string | number }[]; + }; + onChange: ( changes: Record< string, any > ) => void; +} + +const AttributeVariationEditor = ( { data, field, onChange }: AttributeVariationEditorProps ) => {
127-133:submitHandlercalled without event argument.Looking at
FormContext.tsx,submitHandlerexpects an optionalReact.FormEventparameter and callse.preventDefault()if present. Calling it without an argument works but is inconsistent with other usages.src/dashboard/form-manager/components/variation/VariationCard.tsx (1)
52-62: Consider using a<button>element for the Remove action.Using
<span role="button">requires additional accessibility handling (keyboard events, focus states). A native<button>provides these for free.♻️ Suggested improvement
- <span - role="button" + <button + type="button" onClick={ ( e ) => { e.stopPropagation(); - e.preventDefault(); removeVariation( variation ); } } className="text-red-500 hover:text-red-700 text-xs font-medium" > { __( 'Remove', 'dokan-lite' ) } - </span> + </button>src/dashboard/form-manager/components/FileUploadEdit.tsx (2)
9-10: Props typed asanyand state won't sync with external changes.
- Props should be properly typed for type safety.
useState(field.value || [])captures the initial value only. Iffield.valuechanges externally (e.g., form reset), the local state won't update.♻️ Suggested improvements
-const FileUploadEdit = ( { field, onChange }: any ) => { - const [ files, setFiles ] = useState( field.value || [] ); +interface FileUploadEditProps { + field: { + id: string; + label: string; + value?: { id: string; title: string; url: string; name?: string }[]; + error?: string; + }; + onChange: ( changes: Record< string, any > ) => void; +} + +const FileUploadEdit = ( { field, onChange }: FileUploadEditProps ) => { + const [ files, setFiles ] = useState( field.value || [] ); + + // Sync with external value changes + useEffect( () => { + setFiles( field.value || [] ); + }, [ field.value ] );Note: Add
useEffectto the imports from@wordpress/element.
12-63: Consider extracting the repeated setFiles + onChange pattern.The pattern of updating local state and calling
onChangeis repeated inonAddRow,onRemoveRow,updateRow, andonSelectFile. A helper function could reduce duplication.♻️ Suggested helper
const updateFiles = ( newFiles: typeof files ) => { setFiles( newFiles ); onChange( { [ field.id ]: newFiles } ); };Then use
updateFiles(newFiles)in each handler instead of the two separate calls.src/dashboard/form-manager/factories/handlers.ts (1)
20-38: Consider adding explicit type annotation for better type safety.The
normalizedOptionsvariable is implicitly typed asany[]. Adding an explicit type would improve type safety and IDE support.♻️ Suggested improvement
export const getElementsFromOptions = ( field?: FormField ) => { const { options } = field || {}; if ( ! options ) { return []; } - let normalizedOptions = []; + let normalizedOptions: { label: string; value: string }[] = []; if ( Array.isArray( options ) ) { normalizedOptions = [ ...options ]; // Clone to prevent mutation of the original array } else { normalizedOptions = Object.entries( options ).map( ( [ value, label ] ) => ( { label, value, } ) ); } return normalizedOptions; };
| $brand_ids = $product_data['product_brand'] ?? array(); | ||
| if ( ! count( $brand_ids ) ) { | ||
| return; | ||
| } | ||
| dokan()->product->save_brands( $product_id, $brand_ids ); |
There was a problem hiding this comment.
Early return may prevent users from removing all brands from a product.
The current logic conflates "brand data not submitted" with "user explicitly cleared all brands". When a user removes all brands and submits the form with an empty product_brand array, this early return skips save_brands(), leaving existing brands intact.
Consider checking whether the key exists rather than checking for an empty value:
🐛 Proposed fix
- $brand_ids = $product_data['product_brand'] ?? array();
- if ( ! count( $brand_ids ) ) {
- return;
- }
+ if ( ! isset( $product_data['product_brand'] ) ) {
+ return;
+ }
+
+ $brand_ids = (array) $product_data['product_brand'];
dokan()->product->save_brands( $product_id, $brand_ids );Additionally, line 628 uses tab indentation while the rest of the file uses spaces—this should be corrected for consistency.
📝 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.
| $brand_ids = $product_data['product_brand'] ?? array(); | |
| if ( ! count( $brand_ids ) ) { | |
| return; | |
| } | |
| dokan()->product->save_brands( $product_id, $brand_ids ); | |
| if ( ! isset( $product_data['product_brand'] ) ) { | |
| return; | |
| } | |
| $brand_ids = (array) $product_data['product_brand']; | |
| dokan()->product->save_brands( $product_id, $brand_ids ); |
🤖 Prompt for AI Agents
In `@includes/Product/Hooks.php` around lines 625 - 629, The early-return treats
an empty product_brand as "no submission" and prevents
dokan()->product->save_brands($product_id, $brand_ids) from removing all brands;
change the check to detect presence of the key in $product_data (e.g., using
array_key_exists('product_brand', $product_data) or
isset($product_data['product_brand'])) and only skip when the key is truly
absent, then always call save_brands when the key exists (even if it's an empty
array) so existing brands can be cleared; also replace the tab at the current
indentation with spaces to match the file's spacing style.
includes/Product/Manager.php
Outdated
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "Manager.php" | grep -i productRepository: getdokan/dokan
Length of output: 156
🏁 Script executed:
cat -n includes/Product/Manager.php | sed -n '820,850p'Repository: getdokan/dokan
Length of output: 1141
🏁 Script executed:
# Check if there are calls to prepare_downloads to understand expected input
rg "prepare_downloads" includes/Product/Manager.php -B 2 -A 5Repository: getdokan/dokan
Length of output: 283
🏁 Script executed:
# Check the broader context - look for how these parameters are created/passed
rg "prepare_downloads" . -t php --max-count 10Repository: getdokan/dokan
Length of output: 306
🏁 Script executed:
rg "prepare_downloads" includes/ProductForm/Init.php -B 10 -A 3Repository: getdokan/dokan
Length of output: 765
🏁 Script executed:
# Get more context about where these arrays come from in the call
cat -n includes/ProductForm/Init.php | grep -A 20 -B 20 "prepare_downloads"Repository: getdokan/dokan
Length of output: 2241
🏁 Script executed:
# Find where 'downloadable' field type is processed
rg "downloadable" includes/ -t php -B 2 -A 2 | head -60Repository: getdokan/dokan
Length of output: 4094
🏁 Script executed:
# Search for sanitize_callback processing and field handling
rg "sanitize_callback" includes/ProductForm/ -t php -B 3 -A 3Repository: getdokan/dokan
Length of output: 10564
🏁 Script executed:
# Look for where the three array parameters are built for sanitize_callback
find includes/ProductForm -name "*.php" -type fRepository: getdokan/dokan
Length of output: 253
🏁 Script executed:
# Find field type rendering for 'downloadable'
find includes -name "*.php" | xargs grep -l "field_type.*downloadable\|downloadable.*field"Repository: getdokan/dokan
Length of output: 87
🏁 Script executed:
# Search for where the downloadable field values are processed/posted
rg "wc_file_urls\|file_names\|file_hashes" includes/ -t php -B 3 -A 3Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
# Find where downloadable field type is rendered/templated
find includes/ -name "*.php" | xargs grep -l "downloadable" | head -10Repository: getdokan/dokan
Length of output: 377
🏁 Script executed:
# Search for JavaScript or templates that handle downloadable field
find . -type f \( -name "*.js" -o -name "*.php" \) | xargs grep -l "downloadable" | grep -v node_modules | head -15Repository: getdokan/dokan
Length of output: 546
🏁 Script executed:
# Look for the actual form rendering in templates
find . -path ./node_modules -prune -o -type f -name "*.php" -print | xargs grep -l "form.*field\|field.*render" | head -10Repository: getdokan/dokan
Length of output: 425
🏁 Script executed:
# Check product-editor.js to see how downloadable field is handled
cat assets/src/js/product-editor.js | head -100Repository: getdokan/dokan
Length of output: 3159
🏁 Script executed:
# Look for where three arrays are collected for downloadable field
rg "_wc_file" assets/src/js/ -B 3 -A 3Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
# Check the REST ProductController to see how downloads are processed
rg "prepare_downloads\|file_names\|file_urls\|file_hashes" includes/REST/ProductController.php -B 5 -A 5Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
# Find where sanitize is called with multiple parameters for downloadable field
rg "sanitize" includes/ProductForm/ -t php -B 5 -A 5 | head -80Repository: getdokan/dokan
Length of output: 4721
🏁 Script executed:
# Look for RestProduct controller and how it handles downloads
cat -n includes/REST/ProductController.php | grep -i "download" -A 5 -B 5 | head -100Repository: getdokan/dokan
Length of output: 4560
🏁 Script executed:
# Search for how file arrays are constructed before being passed
rg "_wc_file_names\|_wc_file_urls\|_download_id" . -t phpRepository: getdokan/dokan
Length of output: 40
🏁 Script executed:
# Find the save_downloadable_files method
rg "save_downloadable_files" includes/REST/ProductController.php -B 2 -A 20Repository: getdokan/dokan
Length of output: 1863
🏁 Script executed:
# Look for how product form data is saved - find the save method
rg "class Component\|class Init\|function save" includes/ProductForm/Component.php -B 2 -A 10Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
# Check if there's processing of downloadable field data before sanitize_callback is called
cat -n includes/ProductForm/Component.php | head -150Repository: getdokan/dokan
Length of output: 4506
🏁 Script executed:
# Find where sanitize method is called on fields - look in Ajax or REST handlers
rg "sanitize\(\|field->sanitize" includes/ -t php -B 3 -A 3 | head -100Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
# Look for where product data is being saved via AJAX/form
rg "dokan.*product.*save\|save_product" includes/Ajax.php -B 3 -A 10Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
# Search for where downloadable field is specifically being saved
rg "downloadable\|downloads" includes/ProductForm/ -t php | grep -v "test\|comment"Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
# Find where field data is saved - look for field processing in REST or AJAX
rg "get_sanitize_callback\|call_user_func.*sanitize" includes/ProductForm/ -t php -B 5 -A 5Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
# Look in REST ProductBlockController which might handle form data differently
cat -n includes/REST/ProductBlockController.php | head -200Repository: getdokan/dokan
Length of output: 6633
🏁 Script executed:
# Search for where three parameters are passed to sanitize
rg "sanitize.*\(" includes/ -t php | grep -i "file\|download"Repository: getdokan/dokan
Length of output: 2719
🏁 Script executed:
# Find where the sanitize method is invoked on Field objects
rg "->sanitize" includes/ProductForm/ -t php -B 5 -A 5Repository: getdokan/dokan
Length of output: 462
🏁 Script executed:
# Look for product form data processing/saving
rg "foreach.*field\|each field\|save.*field" includes/ProductForm/ -t php -B 3 -A 8Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
# Check where field values are collected from request/post data
find includes/ProductForm -name "*.php" -exec grep -l "POST\|REQUEST\|params\|data" {} \;Repository: getdokan/dokan
Length of output: 220
🏁 Script executed:
# Find where field sanitize method is called
rg "sanitize\(" includes/ProductForm/ -t php -B 5 -A 5Repository: getdokan/dokan
Length of output: 710
🏁 Script executed:
# Look at the Init.php file more broadly for field processing
cat -n includes/ProductForm/Init.php | tail -200Repository: getdokan/dokan
Length of output: 9078
🏁 Script executed:
# Search for REST endpoint that might handle product form updates
rg "class ProductBlockController\|register_rest_route" includes/REST/ProductBlockController.php -A 15Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
# Search for REST endpoint that updates products with downloadable field
rg "update_item\|create_item" includes/REST/ProductBlockController.php -B 5 -A 20Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
# Look for where field.sanitize is called with its parameters
find includes -name "*.php" | xargs grep -l "sanitize(" | xargs grep -l "field"Repository: getdokan/dokan
Length of output: 88
🏁 Script executed:
# Check REST ProductController for how it handles downloads parameter
cat -n includes/REST/ProductController.php | sed -n '1180,1210p'Repository: getdokan/dokan
Length of output: 1465
🏁 Script executed:
# Look for where field values are collected from form request and sanitize is called
rg "get_downloads\|file_names\|file_urls\|file_hashes" includes/ -t php -B 5 -A 5 | head -100Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
# Find frontend product form handling
find includes -name "*.php" -path "*/Dashboard/*" | xargs grep -l "product\|form" | head -5Repository: getdokan/dokan
Length of output: 286
🏁 Script executed:
# Check if there's a Product Manager method that handles form data
rg "public function.*prepare_downloads\|public function.*get_downloads" includes/Product/Manager.php -B 2 -A 15Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
# Find where the three arrays are actually constructed for the sanitize callback
# Look for field names _wc_file_names, _wc_file_urls, _wc_file_hashes
rg "_wc_file_names\|_wc_file_urls\|_wc_file_hashes\|_download_id" . -t phpRepository: getdokan/dokan
Length of output: 40
🏁 Script executed:
# Check if there's test data or fixtures showing how these arrays should be structured
find . -path ./node_modules -prune -o -name "*test*" -o -name "*fixture*" -type f -print | xargs grep -l "prepare_downloads\|file_names.*file_urls" 2>/dev/null | head -5Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
# Look for how the downloadable field form data is submitted
rg "field_type.*=.*downloadable" includes/ -t php -B 5 -A 30 | head -150Repository: getdokan/dokan
Length of output: 2726
🏁 Script executed:
# Search for JavaScript or template that renders the downloadable field with name/url/hash inputs
find assets -name "*.js" -o -name "*.jsx" | xargs grep -l "downloadable\|file_urls\|file_names\|file_hashes" 2>/dev/nullRepository: getdokan/dokan
Length of output: 113
🏁 Script executed:
# Check REST/ProductBlockController for how it processes downloads when creating/updating
cat -n includes/REST/ProductBlockController.php | sed -n '30,100p'Repository: getdokan/dokan
Length of output: 4126
🏁 Script executed:
# Search for where form POST data is processed - look for update_product or similar
rg "update_product\|save_product\|product.*update" includes/ -t php | grep -v test | head -10Repository: getdokan/dokan
Length of output: 40
Guard against mismatched download arrays.
Indexing $file_names[$i] and $file_hashes[$i] without bounds checking can trigger notices and corrupt download entries when the three input arrays differ in size or contain gaps.
🐛 Suggested fix
public function prepare_downloads( $file_names, $file_urls, $file_hashes ) {
$downloads = [];
+ $file_urls = (array) $file_urls;
+ $file_names = (array) $file_names;
+ $file_hashes = (array) $file_hashes;
+
if ( ! empty( $file_urls ) ) {
$file_url_size = count( $file_urls );
for ( $i = 0; $i < $file_url_size; $i++ ) {
- if ( ! empty( $file_urls[ $i ] ) ) {
+ $file_url = $file_urls[ $i ] ?? '';
+ if ( ! empty( $file_url ) ) {
+ $file_name = $file_names[ $i ] ?? '';
+ $file_hash = $file_hashes[ $i ] ?? '';
$downloads[] = [
- 'name' => wc_clean( $file_names[ $i ] ),
- 'file' => wp_unslash( trim( $file_urls[ $i ] ) ),
- 'download_id' => wc_clean( $file_hashes[ $i ] ),
+ 'name' => wc_clean( $file_name ?: wc_get_filename_from_url( $file_url ) ),
+ 'file' => wp_unslash( trim( $file_url ) ),
+ 'download_id' => wc_clean( $file_hash ),
];
}
}
}🤖 Prompt for AI Agents
In `@includes/Product/Manager.php` around lines 829 - 846, prepare_downloads
currently assumes $file_names and $file_hashes have the same indexes as
$file_urls; guard against mismatched arrays by checking isset($file_names[$i])
and isset($file_hashes[$i]) (or using a fallback empty string) before accessing
them, and skip or default entries when those indexes are missing so you don't
index out of bounds or create corrupt download entries; update the loop in
prepare_downloads to validate each index for $file_names and $file_hashes (and
sanitize fallback values with wc_clean) before building the $downloads[] array.
| /** | ||
| * Get all ancestors of chosen categories. | ||
| * | ||
| * @since DOKAN_SINCE |
There was a problem hiding this comment.
Replace version placeholder before release.
The DOKAN_SINCE placeholder in the docblock should be replaced with the actual version number.
🤖 Prompt for AI Agents
In `@includes/ProductCategory/Helper.php` at line 410, Docblock contains the
placeholder "@since DOKAN_SINCE" and must be updated to the actual release
version; locate the docblock for the Helper in the ProductCategory\Helper code
(the docblock above the Helper class or the related method where `@since`
DOKAN_SINCE appears) and replace DOKAN_SINCE with the concrete version string
for this release (or with the canonical version constant used in the project,
e.g., DOKAN_VERSION or similar) so the `@since` tag reflects the real version.
| { files.map( ( file: any, index: number ) => ( | ||
| <div | ||
| key={ index } |
There was a problem hiding this comment.
Using array index as key for dynamic list.
When rows are added or removed, using index as key can cause React to incorrectly associate state with elements. Consider using a generated unique ID.
🐛 Suggested fix
When adding a new row, generate a unique temp ID:
const onAddRow = () => {
const newFiles = [
...files,
{
- id: '',
+ id: `temp-${ Date.now() }`,
title: '',
url: '',
},
];Then use it as the key:
{ files.map( ( file: any, index: number ) => (
<div
- key={ index }
+ key={ file.id || index }🤖 Prompt for AI Agents
In `@src/dashboard/form-manager/components/FileUploadEdit.tsx` around lines 68 -
70, The file FileUploadEdit.tsx uses files.map in the render and currently
assigns key={index}, which can break React reconciliation when rows are
added/removed; update the files data structure to include a stable unique id
(e.g., add a tempId or id field when creating a new file row in the add/new-row
handler) and change the render to use that field as the key instead of index
(reference the files variable and the map callback where key is set). Ensure the
unique id is generated when a row is created so existing items keep stable keys
across inserts/deletes.
| /** | ||
| * Default field handler. | ||
| * Applies specific components if icons are present, otherwise defaults to text. | ||
| * | ||
| * @param {Object} [field] The field configuration. | ||
| * @return {Object} Configuration object with type 'text' and optionally TextWithAddon component. | ||
| */ | ||
| export const defaultHandler: FieldHandler = ( field ) => { | ||
| const config: FieldConfig = { | ||
| type: 'text', | ||
| }; | ||
| if ( field?.name?.includes( 'price' ) ) { | ||
| config.Edit = PriceEdit; | ||
| config.type = 'number'; | ||
| } | ||
| return config; | ||
| }; |
There was a problem hiding this comment.
JSDoc mentions TextWithAddon component which is not used in the implementation.
The documentation states "optionally TextWithAddon component" but the code only conditionally uses PriceEdit. Consider updating the JSDoc to accurately reflect the implementation.
📝 Suggested JSDoc fix
/**
* Default field handler.
- * Applies specific components if icons are present, otherwise defaults to text.
+ * Defaults to text type, with special handling for price-related fields.
*
* `@param` {Object} [field] The field configuration.
- * `@return` {Object} Configuration object with type 'text' and optionally TextWithAddon component.
+ * `@return` {Object} Configuration object with type 'text', or 'number' with PriceEdit for price fields.
*/🤖 Prompt for AI Agents
In `@src/dashboard/form-manager/factories/handlers.ts` around lines 178 - 194,
JSDoc for defaultHandler is inaccurate (mentions TextWithAddon which isn't
used); update the comment above the defaultHandler function to remove the
TextWithAddon reference and instead describe the actual behavior: it returns a
FieldConfig with type 'text' by default, switches to type 'number' and sets Edit
= PriceEdit when field.name includes 'price' (keep references to FieldHandler,
FieldConfig, and PriceEdit in the doc). Ensure the param and return annotations
remain correct and reflect the conditional PriceEdit assignment.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@includes/ProductForm/Component.php`:
- Around line 337-359: The docblocks for set_show_in_admin_settings and
get_show_in_admin_settings are swapped; update the PHPDoc for
set_show_in_admin_settings to describe it as a setter (description "Set show in
admin settings", include the `@param` bool $show_in_admin_settings and `@return`
void) and update the PHPDoc for get_show_in_admin_settings to describe it as a
getter (description "Get show in admin settings", include no `@param` and `@return`
bool). Keep the function names set_show_in_admin_settings and
get_show_in_admin_settings as anchors when making these docblock text/annotation
changes.
In `@includes/ProductForm/Init.php`:
- Around line 268-293: The sanitize_callback for product tags returns a
translated string when the tag count exceeds the limit; instead return a
WP_Error so callers can detect the failure. Update the anonymous
sanitize_callback (the closure that reads $maximum_tags_select_length via
apply_filters('dokan_product_tags_select_max_length')) to, on limit violation,
return a new WP_Error (e.g. new WP_Error('too_many_tags', __( 'You can only
select %s tags', 'dokan-lite' ))) rather than a raw string; keep the existing
translated message and use number_format_i18n($maximum_tags_select_length) in
the error message so downstream code that checks is_wp_error() can handle
validation failures properly.
In `@src/dashboard/form-manager/components/FieldRenderer.tsx`:
- Around line 11-31: Create a small reusable component (e.g., SanitizedHtml)
that accepts a single prop (html: string) and returns a span/div with
dangerouslySetInnerHTML using sanitizeHTML(html); move both instances in
FieldRenderer (the label span with className "dokan-form-field-label" and the
description span) to use SanitizedHtml to avoid repetition, add one /*
eslint-disable-next-line biome/noDangerouslySetInnerHtml -- sanitized input via
sanitizeHTML */ immediately above the dangerouslySetInnerHTML in the new
component with a brief justification, and remove the ad-hoc `@ts-ignore` usage in
FieldRenderer (now unnecessary because the prop typing is explicit and
conditional property deletion can be typed safely).
In `@src/dashboard/form-manager/hooks/useLayouts.tsx`:
- Around line 86-93: The label string for the layout with id 'digital-options'
is hardcoded; replace it with a translation call (e.g., __('Digital Product
Options', 'your-text-domain')) so it can be localized. Import the WP i18n helper
if missing (for example: import { __ } from '@wordpress/i18n') and update the
label property inside useLayouts (the object with id 'digital-options') to use
that translation function and proper text domain.
- Around line 243-251: The call to layoutBuilder is passing product.type
directly which can be undefined and unintentionally override the function's
default scope; update the call site that returns fields to pass the resolved
scope (e.g., use the existing product.type if defined, otherwise the default
string 'product') so layoutBuilder receives a concrete scope value; locate the
return that constructs fields with layoutBuilder(updatedLayouts, sections,
fields, product, product.type) and replace the last argument with a resolved
value (product.type ?? 'product' or equivalent) to ensure hidden_scope filtering
behaves correctly.
♻️ Duplicate comments (6)
includes/ProductForm/Init.php (2)
180-190: Return provided sale-date values instead of clearing them.When
$valueis already supplied, these callbacks return an empty string at line 182, which discards user-entered dates. The same issue exists at line 207 forDATE_ON_SALE_TO. Return$valueto preserve input.🐛 Suggested fix
'value_callback' => function ( $product, $value = '' ) { if ( '' !== $value ) { - return ''; + return $value; }
324-337: Fatal error:$product->get_brand_ids()does not exist on WC_Product.The method
get_brand_ids()is not part of theWC_Productclass. This will cause a runtime fatal error when this callback executes. Use the taxonomy API or a helper function instead.🐛 Suggested fix
'value_callback' => function ( $product, $value = '' ) { if ( '' !== $value ) { return $value; } if ( ! $product instanceof WC_Product ) { return []; } - return $product->get_brand_ids(); + return wp_get_post_terms( $product->get_id(), 'product_brand', [ 'fields' => 'ids' ] ); },includes/Product/FormManager.php (4)
66-107: Add ownership verification for existing products before rendering.The
current_user_can( 'dokan_edit_product' )check is product-agnostic. Adddokan_is_product_author( $product_id )after loading the product to verify ownership.🔐 Suggested fix
$product = wc_get_product( $product_id ); if ( ! $product ) { // ... error handling ... return; } + + if ( ! $new_product && ! dokan_is_product_author( $product_id ) ) { + dokan_get_template_part( + 'global/dokan-error', '', [ + 'deleted' => false, + 'message' => __( 'You have no permission to edit this product', 'dokan-lite' ), + ] + ); + return; + }
141-166: Add authorization and payload security checks to the save endpoint.This AJAX handler lacks capability checks and product ownership verification. It also passes raw
$_POSTwithoutwp_unslash().🔐 Suggested fix
public function dokan_save_product_data() { if ( ! isset( $_POST['_nonce'] ) || ! wp_verify_nonce( sanitize_key( $_POST['_nonce'] ), 'form_manager' ) ) { wp_send_json_error( [ 'type' => 'nonce', 'message' => __( 'Are you cheating?', 'dokan-lite' ), ] ); } + + if ( ! current_user_can( 'dokan_edit_product' ) ) { + wp_send_json_error( + [ + 'type' => 'permission', + 'message' => __( 'You have no permission to edit products', 'dokan-lite' ), + ], + 403 + ); + } + + $payload = wp_unslash( $_POST ); + $product_id = isset( $payload['product_id'] ) ? absint( $payload['product_id'] ) : 0; + + if ( $product_id && ! dokan_is_product_author( $product_id ) ) { + wp_send_json_error( + [ + 'type' => 'permission', + 'message' => __( 'You have no permission to edit this product', 'dokan-lite' ), + ], + 403 + ); + } + try { - $product = dokan()->product->create( $_POST ); + $product = dokan()->product->create( $payload );
257-270: Add guard against missing or invalid parent products.
wc_get_product()can returnfalsefor invalid/deleted products. Callingget_attributes()on it (line 269) will cause a fatal error.🐛 Suggested fix
$variations_data = []; $parent_product = wc_get_product( $product_id ); + + if ( ! $parent_product || ! $parent_product->is_type( 'variable' ) ) { + return []; + } if ( $variations ) {
407-416: Add nonce validation and ownership check to AJAX variations endpoint.This endpoint lacks CSRF protection and permission verification. Any authenticated user can fetch variations for any product.
🔐 Suggested fix
public function dokan_get_product_variations() { + if ( ! isset( $_REQUEST['_nonce'] ) || ! wp_verify_nonce( sanitize_key( $_REQUEST['_nonce'] ), 'form_manager' ) ) { + wp_send_json_error( __( 'Invalid nonce', 'dokan-lite' ), 403 ); + } + if ( ! isset( $_REQUEST['product_id'] ) ) {// phpcs:ignore wp_send_json_error( __( 'Product ID is required', 'dokan-lite' ) ); } - $product_id = intval( wp_unslash( $_REQUEST['product_id'] ) ); // phpcs:ignore + $product_id = absint( wp_unslash( $_REQUEST['product_id'] ) ); // phpcs:ignore + + if ( ! dokan_is_product_author( $product_id ) ) { + wp_send_json_error( __( 'You have no permission to view this product', 'dokan-lite' ), 403 ); + } + $variations = self::get_product_variations( $product_id );
🧹 Nitpick comments (3)
src/dashboard/form-manager/hooks/useLayouts.tsx (1)
51-66: Consider extracting the breakpoint to a named constant.The magic number
768is a common responsive breakpoint, but extracting it to a named constant would improve readability and make it easier to maintain consistency across the codebase.♻️ Suggested change
+const TABLET_BREAKPOINT = 768; + // Define root layout based on window width const rootLayout = useMemo( () => { - if ( width && width > 768 ) { + if ( width && width > TABLET_BREAKPOINT ) { return { type: 'row',includes/ProductForm/Component.php (1)
52-58: Unused$idparameter in constructor.The
$idparameter is accepted but never used within the parent constructor. Child classes (Field,Section) callset_id($id)before invokingparent::__construct(), making this parameter redundant here. Consider removing it from the parent signature or documenting why it's retained for interface consistency.♻️ Suggested fix
- public function __construct( string $id, array $args = [] ) { + public function __construct( array $args = [] ) { foreach ( $args as $key => $value ) { if ( method_exists( $this, "set_{$key}" ) && null !== $value ) { $this->{"set_{$key}"}( $value ); } } }Note: This would require updating child class calls from
parent::__construct( $id, $args )toparent::__construct( $args ).src/dashboard/form-manager/components/FieldRenderer.tsx (1)
52-55: Avoid@ts-ignore+deleteby conditionally buildingdescription.This cleanup removes a type escape hatch and keeps the object shape intentional.
♻️ Suggested refactor (conditional spread)
export const getFieldConfig = ( field: FormField ) => { + const description = field.description?.trim(); const mappedField = { ...field, label: ( <div className="flex gap-1 items-center"> <span className="dokan-form-field-label" dangerouslySetInnerHTML={ { __html: sanitizeHTML( field.label ), } } /> { field.tooltip && ( <DokanTooltip content={ field.tooltip }> <Info size={ 16 } /> </DokanTooltip> ) } </div> ), - description: ( - <span - dangerouslySetInnerHTML={ { - __html: sanitizeHTML( field.description ), - } } - /> - ), + ...( description + ? { + description: ( + <span + dangerouslySetInnerHTML={ { + __html: sanitizeHTML( description ), + } } + /> + ), + } + : {} ), placeholder: field.placeholder, required: field.required, type: field.field_type, isValid: { required: field.required, }, }; @@ - if ( ! field.description ) { - // `@ts-ignore` - delete mappedField.description; - }
includes/ProductForm/Component.php
Outdated
There was a problem hiding this comment.
Swapped docblocks between getter and setter.
The docblock for set_show_in_admin_settings() says "Get" and the docblock for get_show_in_admin_settings() says "Set".
📝 Suggested 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 {🤖 Prompt for AI Agents
In `@includes/ProductForm/Component.php` around lines 337 - 359, The docblocks for
set_show_in_admin_settings and get_show_in_admin_settings are swapped; update
the PHPDoc for set_show_in_admin_settings to describe it as a setter
(description "Set show in admin settings", include the `@param` bool
$show_in_admin_settings and `@return` void) and update the PHPDoc for
get_show_in_admin_settings to describe it as a getter (description "Get show in
admin settings", include no `@param` and `@return` bool). Keep the function names
set_show_in_admin_settings and get_show_in_admin_settings as anchors when making
these docblock text/annotation changes.
includes/ProductForm/Init.php
Outdated
There was a problem hiding this comment.
Sanitize callback returns error string instead of WP_Error.
The tags sanitize callback returns a translated string when the limit is exceeded (line 280), but calling code likely expects either sanitized data or a WP_Error. This could cause silent failures or unexpected behavior during product save.
🐛 Suggested fix
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 ) )
+ );
}🤖 Prompt for AI Agents
In `@includes/ProductForm/Init.php` around lines 268 - 293, The sanitize_callback
for product tags returns a translated string when the tag count exceeds the
limit; instead return a WP_Error so callers can detect the failure. Update the
anonymous sanitize_callback (the closure that reads $maximum_tags_select_length
via apply_filters('dokan_product_tags_select_max_length')) to, on limit
violation, return a new WP_Error (e.g. new WP_Error('too_many_tags', __( 'You
can only select %s tags', 'dokan-lite' ))) rather than a raw string; keep the
existing translated message and use
number_format_i18n($maximum_tags_select_length) in the error message so
downstream code that checks is_wp_error() can handle validation failures
properly.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find Biome configuration files
fd -a -g '*biome*' -type f | head -20Repository: getdokan/dokan
Length of output: 228
🏁 Script executed:
#!/bin/bash
# Check the actual FieldRenderer.tsx file
git ls-files | grep -i 'fielrenderer\|form-manager' | head -10Repository: getdokan/dokan
Length of output: 609
🏁 Script executed:
#!/bin/bash
# Search for Biome config files more broadly
find . -maxdepth 3 -name 'biome.json' -o -name '.biomerc' -o -name 'biome.toml' 2>/dev/nullRepository: getdokan/dokan
Length of output: 70
🏁 Script executed:
#!/bin/bash
# Read the Biome configuration
cat biome.jsonRepository: getdokan/dokan
Length of output: 1832
🏁 Script executed:
#!/bin/bash
# Read the FieldRenderer.tsx file
cat -n src/dashboard/form-manager/components/FieldRenderer.tsxRepository: getdokan/dokan
Length of output: 2361
Consolidate dangerouslySetInnerHTML usage into a reusable SanitizedHtml component with centralized lint suppression.
Biome's noDangerouslySetInnerHtml security rule is enforced via the recommended ruleset and will flag lines 15-17 and 28-30. Create a helper component to eliminate duplication and apply a single lint suppression with proper justification:
Suggested refactor
+type SanitizedHtmlProps = {
+ html: string;
+ className?: string;
+};
+
+const SanitizedHtml = ( { html, className }: SanitizedHtmlProps ) => {
+ // biome-ignore lint/security/noDangerouslySetInnerHtml -- content is sanitized via sanitizeHTML
+ return (
+ <span
+ className={ className }
+ dangerouslySetInnerHTML={ { __html: sanitizeHTML( html ) } }
+ />
+ );
+};
+
export const getFieldConfig = ( field: FormField ) => {
const mappedField = {
...field,
label: (
<div className="flex gap-1 items-center">
- <span
- className="dokan-form-field-label"
- dangerouslySetInnerHTML={ {
- __html: sanitizeHTML( field.label ),
- } }
- />
+ <SanitizedHtml
+ className="dokan-form-field-label"
+ html={ field.label }
+ />
{ field.tooltip && (
<DokanTooltip content={ field.tooltip }>
<Info size={ 16 } />
</DokanTooltip>
) }
</div>
),
- description: (
- <span
- dangerouslySetInnerHTML={ {
- __html: sanitizeHTML( field.description ),
- } }
- />
- ),
+ description: <SanitizedHtml html={ field.description } />,
placeholder: field.placeholder,
required: field.required,
type: field.field_type,
isValid: {
required: field.required,
},
};
const specificConfig = getFieldConfigFromFactory( field );
Object.assign( mappedField, specificConfig );
// Handle Visibility/Dependency
mappedField.isVisible = ( data: Record< string, any > ) => {
if ( ! field.visibility ) {
return false;
}
return checkDependency( field.dependency_condition, data );
};
if ( ! field.description ) {
- // `@ts-ignore`
delete mappedField.description;
}
return mappedField;
};This approach also eliminates the @ts-ignore pattern on line 53 since TypeScript can properly infer the type when conditionally deleting a property.
🧰 Tools
🪛 ast-grep (0.40.5)
[warning] 14-14: 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] 27-27: 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)
[error] 15-15: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 28-28: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🤖 Prompt for AI Agents
In `@src/dashboard/form-manager/components/FieldRenderer.tsx` around lines 11 -
31, Create a small reusable component (e.g., SanitizedHtml) that accepts a
single prop (html: string) and returns a span/div with dangerouslySetInnerHTML
using sanitizeHTML(html); move both instances in FieldRenderer (the label span
with className "dokan-form-field-label" and the description span) to use
SanitizedHtml to avoid repetition, add one /* eslint-disable-next-line
biome/noDangerouslySetInnerHtml -- sanitized input via sanitizeHTML */
immediately above the dangerouslySetInnerHTML in the new component with a brief
justification, and remove the ad-hoc `@ts-ignore` usage in FieldRenderer (now
unnecessary because the prop typing is explicit and conditional property
deletion can be typed safely).
| { | ||
| id: 'digital-options', | ||
| layout: { | ||
| type: 'regular', | ||
| }, | ||
| label: 'Digital Product Options', | ||
| children: [ 'downloadable', 'virtual' ], | ||
| }, |
There was a problem hiding this comment.
Hardcoded string should be internationalized.
The label 'Digital Product Options' is hardcoded in English. In a WordPress/Dokan context, this should use the translation function for i18n support.
🌐 Suggested fix
+import { __ } from '@wordpress/i18n';
+
// In the layout definition:
{
id: 'digital-options',
layout: {
type: 'regular',
},
- label: 'Digital Product Options',
+ label: __( 'Digital Product Options', 'dokan-lite' ),
children: [ 'downloadable', 'virtual' ],
},📝 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.
| { | |
| id: 'digital-options', | |
| layout: { | |
| type: 'regular', | |
| }, | |
| label: 'Digital Product Options', | |
| children: [ 'downloadable', 'virtual' ], | |
| }, | |
| { | |
| id: 'digital-options', | |
| layout: { | |
| type: 'regular', | |
| }, | |
| label: __( 'Digital Product Options', 'dokan-lite' ), | |
| children: [ 'downloadable', 'virtual' ], | |
| }, |
🤖 Prompt for AI Agents
In `@src/dashboard/form-manager/hooks/useLayouts.tsx` around lines 86 - 93, The
label string for the layout with id 'digital-options' is hardcoded; replace it
with a translation call (e.g., __('Digital Product Options',
'your-text-domain')) so it can be localized. Import the WP i18n helper if
missing (for example: import { __ } from '@wordpress/i18n') and update the label
property inside useLayouts (the object with id 'digital-options') to use that
translation function and proper text domain.
| return { | ||
| fields: layoutBuilder( | ||
| updatedLayouts, | ||
| sections, | ||
| fields, | ||
| product, | ||
| product.type | ||
| ), | ||
| }; |
There was a problem hiding this comment.
product.type may be undefined, bypassing the default scope.
If product.type is undefined, it will be passed explicitly to layoutBuilder, overriding the default parameter value 'product'. This could cause unexpected behavior in the hidden_scope filtering logic within layoutBuilder.
🔧 Suggested fix
return {
fields: layoutBuilder(
updatedLayouts,
sections,
fields,
product,
- product.type
+ product.type || 'product'
),
};📝 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.
| return { | |
| fields: layoutBuilder( | |
| updatedLayouts, | |
| sections, | |
| fields, | |
| product, | |
| product.type | |
| ), | |
| }; | |
| return { | |
| fields: layoutBuilder( | |
| updatedLayouts, | |
| sections, | |
| fields, | |
| product, | |
| product.type || 'product' | |
| ), | |
| }; |
🤖 Prompt for AI Agents
In `@src/dashboard/form-manager/hooks/useLayouts.tsx` around lines 243 - 251, The
call to layoutBuilder is passing product.type directly which can be undefined
and unintentionally override the function's default scope; update the call site
that returns fields to pass the resolved scope (e.g., use the existing
product.type if defined, otherwise the default string 'product') so
layoutBuilder receives a concrete scope value; locate the return that constructs
fields with layoutBuilder(updatedLayouts, sections, fields, product,
product.type) and replace the last argument with a resolved value (product.type
?? 'product' or equivalent) to ensure hidden_scope filtering behaves correctly.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/dashboard/form-manager/context/VariationContext.tsx`:
- Around line 192-206: fetchVariations currently calls
onUpdateVariations(response.data) without verifying the AJAX result; update
fetchVariations to check response.success first (e.g., if (response &&
response.success) then call onUpdateVariations(response.data || [])), and in the
failure branch (else) log the error and show the toast error (similar to the
catch block) so invalid payloads are not propagated to onUpdateVariations;
reference fetchVariations, onUpdateVariations and response.success when making
the change.
- Around line 116-124: The current guard in useVariationContext will never throw
because VariationContext was created with a concrete default value; change the
context creation to use an undefined default (i.e.
createContext<VariationContextType | undefined>(undefined)) so the runtime check
in useVariationContext can detect missing provider, update types where
VariationContext is defined to allow undefined, and adjust the VariationProvider
(the component that calls VariationContext.Provider) to always supply a
non-undefined value (or use a non-null assertion there) so consumers still get
the correct typed value.
In `@src/dashboard/form-manager/factories/form-data.ts`:
- Around line 35-38: DefaultHandler's getValue currently uses the expression
field.value || '' which converts valid falsy values like 0 or false to an empty
string; change the fallback to use nullish coalescing so that field.value ?? ''
is used instead (update the getValue implementation in DefaultHandler and ensure
it still conforms to FieldDataHandler's return type) so only null or undefined
become '' while 0/false are preserved.
🧹 Nitpick comments (7)
src/dashboard/form-manager/index.scss (4)
6-23: Consider documenting the reason for!importantoverrides.Multiple
!importantdeclarations are used to override third-party component library styles (e.g.,components-card-header,components-input-control__backdrop). While this is sometimes necessary when integrating with libraries like WordPress's@wordpress/components, it can lead to specificity issues if overused.Consider adding a brief comment at the top of the file explaining that these overrides are intentional for component library integration, which helps future maintainers understand the rationale.
66-75: Redundant dimension declarations in.variation-form.The
.variation-form .dokan-product-image_idblock (lines 67-70) setsheight: 232pxandwidth: 232px, but these are identical to the values already defined in lines 55-56. Similarly,imgdimensions on lines 72-73 repeat what's already inherited. These rules have no effect unless the parent values change.If these are intentional placeholders for future variation-specific sizing, consider adding a comment. Otherwise, they can be removed to reduce redundancy.
♻️ Suggested cleanup
- .variation-form { - .dokan-product-image_id { - height: 232px; - width: 232px; - } - img { - height: 232px; - width: 232px; - } - } + // Remove if no variation-specific sizing is needed, or update with different values
1-103: Consider using CSS custom properties for colors and sizing.Hard-coded color values (e.g.,
#f0f0f1,#e9e9e9,#25252d,#828282) and magic numbers for dimensions (e.g.,232px,80px) are scattered throughout. Using CSS custom properties or design tokens would improve maintainability and ensure consistency with the broader Dokan design system.// Example at top of file or imported from a shared variables file :root { --dokan-form-bg: `#f0f0f1`; --dokan-border-color: `#e9e9e9`; --dokan-text-primary: `#25252d`; --dokan-text-secondary: `#828282`; --dokan-product-image-size: 232px; --dokan-gallery-thumb-size: 80px; }This is a recommended improvement for long-term maintainability but not blocking.
34-35: Minor: Units on zero values are unnecessary.
padding-top: 0pxandpadding-bottom: 0pxcan be simplified topadding-top: 0andpadding-bottom: 0. Zero is zero regardless of unit.♻️ Suggested fix
- padding-top: 0px; - padding-bottom: 0px; + padding-top: 0; + padding-bottom: 0;src/dashboard/form-manager/factories/form-data.ts (1)
8-13: Potential runtime error whenfield.valueis truthy but not an object.The check
!! field.valueon line 10 ensures the value is truthy but doesn't guarantee it's an object with anidproperty. Iffield.valueis a primitive (e.g., a number like123), accessingfield.value.idon line 12 will returnundefined.♻️ Suggested fix to add type guard
const ImageIdHandler: FieldDataHandler = { canHandle: ( field ) => { - return field.id === 'image_id' && !! field.value; + return field.id === 'image_id' && typeof field.value === 'object' && field.value !== null && 'id' in field.value; }, getValue: ( field ) => field.value.id, };src/dashboard/form-manager/components/AttributeVariationEdit.tsx (1)
11-14: Consider adding proper TypeScript types for component props.The component props are typed as
any, which reduces type safety and IDE support. Defining an interface for the props would improve maintainability.♻️ Suggested interface
interface AttributeVariationEditorProps { data: Record< string, any >; field: FormField; onChange: ( updates: Record< string, any > ) => void; } const AttributeVariationEditor = ( { data, field, onChange }: AttributeVariationEditorProps ) => {includes/ProductForm/Init.php (1)
237-245: Side effect in sanitize_callback: modifies product meta during sanitization.The
sanitize_callbackadds meta data to the product (line 242), which introduces side effects during data transformation. Sanitize callbacks are typically expected to be pure functions that only transform/validate input. If this callback runs multiple times or in validation-only scenarios, it may cause unexpected behavior.Consider moving the
add_meta_datacall to the product save/update logic instead.
| export const useVariationContext = () => { | ||
| const context = useContext( VariationContext ); | ||
| if ( ! context ) { | ||
| throw new Error( | ||
| 'useVariationContext must be used within a VariationProvider' | ||
| ); | ||
| } | ||
| return context; | ||
| }; |
There was a problem hiding this comment.
The context guard will never trigger because the context has a default value.
The check if ( ! context ) on line 118 will never be true because VariationContext is created with a default value on lines 107-114. This means the error will never be thrown even if the hook is used outside a VariationProvider.
🐛 Suggested fix using undefined default
-export const VariationContext = createContext< VariationContextType >( {
- variations: [],
- saveVariation: () => Promise.resolve(),
- generateVariations: () => {},
- addVariation: () => {},
- updateVariation: () => {},
- removeVariation: () => {},
-} );
+export const VariationContext = createContext< VariationContextType | undefined >( undefined );
export const useVariationContext = () => {
const context = useContext( VariationContext );
if ( ! context ) {
throw new Error(
'useVariationContext must be used within a VariationProvider'
);
}
return context;
};📝 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.
| export const useVariationContext = () => { | |
| const context = useContext( VariationContext ); | |
| if ( ! context ) { | |
| throw new Error( | |
| 'useVariationContext must be used within a VariationProvider' | |
| ); | |
| } | |
| return context; | |
| }; | |
| export const VariationContext = createContext< VariationContextType | undefined >( undefined ); | |
| export const useVariationContext = () => { | |
| const context = useContext( VariationContext ); | |
| if ( ! context ) { | |
| throw new Error( | |
| 'useVariationContext must be used within a VariationProvider' | |
| ); | |
| } | |
| return context; | |
| }; |
🤖 Prompt for AI Agents
In `@src/dashboard/form-manager/context/VariationContext.tsx` around lines 116 -
124, The current guard in useVariationContext will never throw because
VariationContext was created with a concrete default value; change the context
creation to use an undefined default (i.e. createContext<VariationContextType |
undefined>(undefined)) so the runtime check in useVariationContext can detect
missing provider, update types where VariationContext is defined to allow
undefined, and adjust the VariationProvider (the component that calls
VariationContext.Provider) to always supply a non-undefined value (or use a
non-null assertion there) so consumers still get the correct typed value.
| const fetchVariations = async () => { | ||
| try { | ||
| const response: any = await ajaxRequest( { | ||
| action: 'dokan_get_product_variations', | ||
| product_id: productId, | ||
| } ); | ||
| onUpdateVariations( response.data || [] ); | ||
| } catch ( error ) { | ||
| console.error( 'Error fetching variations:', error ); | ||
| toast( { | ||
| type: 'error', | ||
| title: __( 'Error fetching variations', 'dokan-lite' ), | ||
| } ); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Missing response.success check before updating variations.
The fetchVariations function updates variations with response.data without verifying response.success. If the server returns an error payload, invalid data may be propagated to the UI.
🐛 Suggested fix
const fetchVariations = async () => {
try {
const response: any = await ajaxRequest( {
action: 'dokan_get_product_variations',
product_id: productId,
} );
- onUpdateVariations( response.data || [] );
+ if ( response.success ) {
+ onUpdateVariations( response.data || [] );
+ } else {
+ toast( {
+ type: 'error',
+ title: response?.data?.message || __( 'Error fetching variations', 'dokan-lite' ),
+ } );
+ }
} catch ( error ) {
console.error( 'Error fetching variations:', error );
toast( {
type: 'error',
title: __( 'Error fetching variations', 'dokan-lite' ),
} );
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const fetchVariations = async () => { | |
| try { | |
| const response: any = await ajaxRequest( { | |
| action: 'dokan_get_product_variations', | |
| product_id: productId, | |
| } ); | |
| onUpdateVariations( response.data || [] ); | |
| } catch ( error ) { | |
| console.error( 'Error fetching variations:', error ); | |
| toast( { | |
| type: 'error', | |
| title: __( 'Error fetching variations', 'dokan-lite' ), | |
| } ); | |
| } | |
| }; | |
| const fetchVariations = async () => { | |
| try { | |
| const response: any = await ajaxRequest( { | |
| action: 'dokan_get_product_variations', | |
| product_id: productId, | |
| } ); | |
| if ( response.success ) { | |
| onUpdateVariations( response.data || [] ); | |
| } else { | |
| toast( { | |
| type: 'error', | |
| title: response?.data?.message || __( 'Error fetching variations', 'dokan-lite' ), | |
| } ); | |
| } | |
| } catch ( error ) { | |
| console.error( 'Error fetching variations:', error ); | |
| toast( { | |
| type: 'error', | |
| title: __( 'Error fetching variations', 'dokan-lite' ), | |
| } ); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In `@src/dashboard/form-manager/context/VariationContext.tsx` around lines 192 -
206, fetchVariations currently calls onUpdateVariations(response.data) without
verifying the AJAX result; update fetchVariations to check response.success
first (e.g., if (response && response.success) then call
onUpdateVariations(response.data || [])), and in the failure branch (else) log
the error and show the toast error (similar to the catch block) so invalid
payloads are not propagated to onUpdateVariations; reference fetchVariations,
onUpdateVariations and response.success when making the change.
There was a problem hiding this comment.
Falsy values like 0 and false are incorrectly converted to empty string.
The expression field.value || '' on line 37 will coerce valid falsy values (0, false, null) to an empty string. This is problematic for numeric fields where 0 is a valid value.
🐛 Suggested fix using nullish coalescing
const DefaultHandler: FieldDataHandler = {
canHandle: () => true,
- getValue: ( field ) => field.value || '',
+ getValue: ( field ) => field.value ?? '',
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const DefaultHandler: FieldDataHandler = { | |
| canHandle: () => true, | |
| getValue: ( field ) => field.value || '', | |
| }; | |
| const DefaultHandler: FieldDataHandler = { | |
| canHandle: () => true, | |
| getValue: ( field ) => field.value ?? '', | |
| }; |
🤖 Prompt for AI Agents
In `@src/dashboard/form-manager/factories/form-data.ts` around lines 35 - 38,
DefaultHandler's getValue currently uses the expression field.value || '' which
converts valid falsy values like 0 or false to an empty string; change the
fallback to use nullish coalescing so that field.value ?? '' is used instead
(update the getValue implementation in DefaultHandler and ensure it still
conforms to FieldDataHandler's return type) so only null or undefined become ''
while 0/false are preserved.
- Introduced error messages for required fields in the ProductForm. - Updated form validation logic in FormManager to disable submission if the form is invalid. - Enhanced FieldRenderer to include custom validation messages. - Removed unused error state management from FormContext.
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Nitpick comments (6)
includes/REST/ProductControllerV3.php (1)
33-34: Globalrest_pre_dispatchfilter registered insideregister_routes.This filter fires on every REST API request (not just dokan/v3/products). While the route-matching check on Lines 129–135 returns early for non-matching routes, the overhead of running the filter callback and regex match on every REST call is unnecessary.
Consider moving the payload resolution to the route callbacks themselves (before calling
parent::create_item/parent::update_item) rather than using a global filter, or hook it more narrowly (e.g.,rest_request_before_callbacks).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@includes/REST/ProductControllerV3.php` around lines 33 - 34, The global rest_pre_dispatch filter added in register_routes causes resolve_product_payload_before_validation to run on every REST request; remove that add_filter and instead call resolve_product_payload_before_validation directly inside the specific route handlers before invoking parent::create_item and parent::update_item (or, if you prefer a hook, attach it to a narrower filter such as rest_request_before_callbacks for only dokan/v3/products requests). Update register_routes to stop registering rest_pre_dispatch and ensure resolve_product_payload_before_validation is invoked at the start of the create/update route callbacks so the payload is resolved only for the intended routes.src/dashboard/form-manager/types/index.ts (2)
36-40: Index signature[key: string]: anycollapses all type safety onFieldConfig.The explicit fields defined in
Partial<FlatFormItem>andEdit?/elements?become effectively invisible because any property access is widened toanyby the index signature. If this is intentional for extensibility, consider at least narrowing tounknownto force callers to assert types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/form-manager/types/index.ts` around lines 36 - 40, The FieldConfig type currently uses an index signature `[key: string]: any` which erases the explicit types from Partial<FlatFormItem> and the optional Edit/elements props; change that index signature to a safer type (for example `[key: string]: unknown` or a generic Record<string, unknown>) so callers must narrow/assert unknown values, while leaving the names FieldConfig, FlatFormItem, Edit and elements intact; update any callsites that relied on implicit any to add proper type assertions or casts.
13-31: Consider using a string literal union fortypeto enable discriminated-union narrowing.The JSDoc says items are "discriminated by type" (
'section'vs'field'), buttype: stringwon't let TypeScript narrow in conditionals. A union literal would make downstream consumers type-safe.♻️ Proposed change
export type FlatFormItem = { - type: string; + type: 'section' | 'field'; id: string;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/form-manager/types/index.ts` around lines 13 - 31, Replace the broad type: string on FlatFormItem with a string literal union so TypeScript can discriminate (e.g., type: 'section' | 'field' or a named union like FormItemType = 'section' | 'field'); update the FlatFormItem definition (or split into FlatSectionItem / FlatFieldItem that extend a shared base) to use that discriminant so downstream code can narrow on the type field in conditionals and get proper typed properties.includes/ProductForm/ProductForm.php (1)
165-168:ProductCategoryHelper::get_product_categories_tree()called eagerly at definition time.This method (and
get_product_tags(),get_products_brands()) runs database queries every timeget_fields()is invoked, even when the caller doesn't need option lists (e.g., when only resolving values for a saved product). For a high-traffic endpoint, this could be a performance concern.Consider deferring these calls or caching results to avoid repeated DB queries on each
get_fields()invocation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@includes/ProductForm/ProductForm.php` around lines 165 - 168, ProductCategoryHelper::get_product_categories_tree() (and similar calls get_product_tags(), get_products_brands()) are executed eagerly inside ProductForm::get_fields(), causing DB queries on every invocation; change to defer or cache these option list computations by returning a lazy callback or memoized value instead of calling the helper at definition time: update get_fields() to supply a closure or reference that calls ProductCategoryHelper::get_product_categories_tree() only when options are actually requested, or introduce a simple in-memory cache (e.g., static property or a singleton cache in ProductCategoryHelper) to return cached results with a suitable TTL, and apply the same pattern for get_product_tags() and get_products_brands().src/dashboard/form-manager/factories/handlers.ts (1)
99-105:const config: anyinmultiSelectHandler— replace withFieldConfig.Every other handler that uses a typed config object uses
FieldConfig(e.g., Line 156 indefaultHandler).multiSelectHandleris the only outlier.♻️ Proposed fix
- const config: any = { + const config: FieldConfig = { type: 'array', Edit: SelectEdit, multiple: true, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/form-manager/factories/handlers.ts` around lines 99 - 105, The config object in multiSelectHandler is currently typed as any; change it to use the correct FieldConfig type so it matches other handlers (e.g., defaultHandler). Locate the multiSelectHandler function and replace "const config: any = { ... }" with "const config: FieldConfig = { ... }" (or annotate the returned object with FieldConfig), keeping existing properties (type, Edit, multiple) and ensuring imports/typing for FieldConfig are present where needed.src/dashboard/form-manager/hooks/useVariationLayouts.tsx (1)
9-122: Staticlayoutsdefinition insideuseMemo— consider hoisting to module scope.The
layoutsarray at Lines 10–112 is a pure constant — it has no dependency onformItemsorproduct. Defining it insideuseMemomeans the array literal is re-evaluated (then immediately discarded) on every render before memoization kicks in. Moving it to module scope eliminates the unnecessary work entirely.♻️ Proposed refactor
+const VARIATION_LAYOUTS = [ + { + id: 'variation-image-sku', + // ... (full static array as-is) + }, + // ... +]; + const useVariationLayouts = ( formItems: FlatFormItem[], product: Record< string, any > ) => { const formLayouts = useMemo( () => { - const layouts = [ ... ]; return { fields: layoutBuilder( - layouts, + VARIATION_LAYOUTS, formItems, product, 'variation' ), }; }, [ formItems, product ] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dashboard/form-manager/hooks/useVariationLayouts.tsx` around lines 9 - 122, The layouts array defined inside the useMemo in useVariationLayouts is a static constant and should be hoisted to module scope to avoid re-creating the literal each render; move the large layouts definition out of the useMemo (e.g. const VARIATION_LAYOUTS = [...]) and update the useMemo to pass VARIATION_LAYOUTS into layoutBuilder instead of the inline variable, keeping the existing call to layoutBuilder(layouts, formItems, product, 'variation') and preserving the useMemo dependency array ([formItems, product]).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@includes/ProductForm/Hooks.php`:
- Around line 76-83: The current form hook in includes/ProductForm/Hooks.php
creates a new WC_Product_Simple on every load when $product_id is falsy, causing
orphan auto-drafts; change this to either (A) attempt to reuse an existing
auto-draft for the current user before creating a new one (e.g., query posts
with post_type 'product', post_status 'auto-draft', author get_current_user_id()
and optional meta key like '_temp_product_for_user') and assign that ID to
$product_id, or (B) defer creation entirely and only instantiate/save
WC_Product_Simple when the user performs an explicit save action (move the
WC_Product_Simple creation into the save handler). Update any code that expects
$new_product accordingly and set/clear a user-meta or post-meta flag if you
choose reuse to avoid collisions across tabs.
- Around line 169-175: The dokan_get_product_variations AJAX handler lacks nonce
and capability checks allowing any logged-in user to fetch arbitrary product
variations; update dokan_get_product_variations to call check_ajax_referer with
a chosen action (e.g., the same nonce action used by product save or a new
'dokan_product_variations' action) and verify the current user can edit the
target product (use current_user_can( 'edit_post', $product_id )) before calling
dokan()->product_form->get_product_variations; on failed checks return
wp_send_json_error with a clear message.
- Around line 135-160: The dokan_save_product_data function is passing raw
$_POST into dokan()->product->create and lacks an explicit return after nonce
failure; fix by whitelisting and sanitizing only the expected product fields
from $_POST (e.g., title, price, description, sku, stock, categories) before
calling dokan()->product->create, use appropriate sanitizers/escapers for each
field, and pass the cleaned array instead of $_POST; also add an explicit return
after the wp_send_json_error nonce branch (and after
wp_send_json_success/wp_send_json_error responses) so execution cannot continue
after sending a response.
In `@includes/ProductForm/PayloadResolver.php`:
- Around line 172-175: The attributes transformation is missing the cleanup step
used for categories/tags/brands; after calling
self::transform_attributes_for_api( $out[ Elements::ATTRIBUTES ] ) you should
unset the original input key to follow the established pattern—update the block
in PayloadResolver.php so that after assigning $out['attributes'] =
self::transform_attributes_for_api( $out[ Elements::ATTRIBUTES ] ); you call
unset( $out[ Elements::ATTRIBUTES ] ); to remove the original payload entry
(referenced symbols: Elements::ATTRIBUTES and transform_attributes_for_api).
- Around line 155-170: The grouped products array is being normalized but left
under Elements::GROUP_PRODUCTS (which equals 'children') so the WC REST API
ignores it; update the resolver to assign the cleaned ids to the WC API key
'grouped_products' and remove the original Elements::GROUP_PRODUCTS entry —
i.e., after computing $ids and normalizing them with array_map('absint',
array_filter($ids)), set $out['grouped_products'] = ... and unset($out[
Elements::GROUP_PRODUCTS ]) so the API receives the expected key.
In `@includes/ProductForm/ProductForm.php`:
- Around line 590-599: The loop in ProductForm handling Elements::DOWNLOADS
calls attachment_url_to_postid() for each download which issues O(n) DB queries;
change this to perform a single batch lookup and avoid per-download queries:
collect all download['file'] URLs from $product->get_downloads(), determine
which are local attachment paths (skip or mark external URLs to return ''),
resolve local files in one query (e.g., query attachments by
meta_value/_wp_attached_file or by parsing filenames and using get_posts with a
meta_query) and build a map of URL => attachment_id, then iterate the downloads
to populate 'id' from that map (fall back to '' for non-matches) so
Elements::DOWNLOADS no longer calls attachment_url_to_postid() per item and
consider caching results via wp_cache_get/wp_cache_set keyed by the
product/download URLs.
- Around line 611-615: The current check uses empty($value) which treats
legitimate falsy values like 0 or '0' as absent; update the condition after the
apply_filters call (the $value variable returned by
apply_filters('dokan_product_form_field_value', ...)) to only treat truly
missing values as absent — e.g., replace if ( ! empty( $value ) ) with a
stricter check such as if ( $value !== '' && $value !== null ) so that numeric
zero and other valid falsy values are preserved and only empty string/null fall
back to meta/getter.
- Around line 567-568: Add a method_exists guard before calling get_brand_ids on
the $product in the Elements::BRANDS branch: check method_exists($product,
'get_brand_ids') (or is_callable([$product, 'get_brand_ids'])) and only call
$product->get_brand_ids() when that returns true; otherwise return an
appropriate default (e.g., empty array/null) so ProductForm::case
Elements::BRANDS does not call get_brand_ids on products when the brands plugin
is not present.
- Around line 506-514: The usort call in ProductForm.php (sorting $items) is
unstable on PHP 7.4 because many items default to priority 30; make the sort
stable by recording each item's original index and using that index as a
tiebreaker in the comparator: attach an original_index to each element (or build
an indexed array) before calling usort in the same scope, then modify the
comparator closure to compare (int)$a['priority'] and (int)$b['priority'] and
return the original_index comparison when priorities are equal; ensure you
remove or ignore the helper index afterwards if needed so other code (e.g., code
that expects only 'priority' keys) is unaffected.
In `@includes/REST/ProductControllerV3.php`:
- Around line 128-141: The method resolve_product_payload_before_validation
currently calls $request->get_params() which mixes URL/query/body params and
lets the route id leak into the body; change it to fetch only JSON body params
via $request->get_json_params(), pass those to
PayloadResolver::schema_to_wc_api(), and then store the resolved array with
$request->set_body_params($resolved) (remove the wp_json_encode + set_body
usage) so URL/route params like id are not injected into the request body.
In `@src/dashboard/form-manager/App.tsx`:
- Around line 12-21: The module currently assumes window.dokanFormManager exists
and computes productId at module scope, causing a TypeError if it's not
injected; update the initialization of formManager and productId to be
defensive: check for (window as any).dokanFormManager and provide a safe default
shape for the keyed fields used (form_items, product_id, variations, etc.), and
avoid executing Number(formManager.product_id) at module initialization — either
compute productId lazily inside the React component or use optional chaining and
a default (e.g., product_id || "0") when converting; modify the declarations
named formManager and productId in App.tsx accordingly so they never dereference
undefined.
- Line 30: The current conversion using Boolean(formManager.is_new_product) in
App.tsx is wrong because PHP-serialized booleans arrive as strings like "0"/"1"
or "false"/"true"; update the isNewProduct assignment to parse
formManager.is_new_product explicitly by comparing the string value to the
truthy representations (e.g., "1" and "true") and treat "0", "", "false" as
false so components using isNewProduct get a real boolean; locate the assignment
of isNewProduct in the App component and replace the Boolean(...) logic with
explicit string checks on formManager.is_new_product.
In `@src/dashboard/form-manager/components/FieldRenderer.tsx`:
- Around line 55-64: The custom validator in FieldRenderer.tsx incorrectly
treats falsy values as missing by using "! value?.[ field.id ]"; update the
custom function (the "custom" validator that checks field.required, field.id,
and field.error_message) to only treat undefined or null as missing (e.g., check
value?.[field.id] === undefined || value?.[field.id] === null) so that valid
falsy values like 0, false, or "" are accepted and only truly absent values
trigger field.error_message.
In `@src/dashboard/form-manager/components/FileUploadEdit.tsx`:
- Around line 9-20: FileUploadEdit currently reads field.value.length without
guarding for undefined/null; update the initialization of the files state so it
first verifies field?.value is an array (e.g., Array.isArray(field.value)) and
has length > 0 before using it, otherwise fall back to the single empty file
object; reference the FileUploadEdit component, the field.value access, and the
files/setFiles state initializer when making this change and ensure the guard
prevents any access to .length on undefined/null.
- Around line 37-43: onRemoveRow currently removes any row including the last
one, leaving files empty and the UI with no input rows; update onRemoveRow
(referencing files, setFiles, onChange, and field.id) to guard against removing
the final row by either returning early when files.length <= 1 or replacing the
removed item with a blank placeholder (e.g., add a single empty file object) so
that after setFiles and the onChange call there is always at least one entry
rendered.
In `@src/dashboard/form-manager/context/VariationContext.tsx`:
- Line 144: The isLoading state currently only wraps saveVariation, so other
async functions (generateVariations, addVariation, removeVariation,
fetchVariations) don't set loading and callers get incorrect UI state; update
each async function in VariationContext (generateVariations, addVariation,
removeVariation, fetchVariations and saveVariation) to setIsLoading(true) at the
start and setIsLoading(false) in finally (or ensure all code paths clear it) and
handle errors as needed so isLoading accurately reflects any in-flight
operation.
- Around line 72-90: The code is appending non-string values directly to
formData in the loop in VariationContext (the block using
Object.keys(data).forEach), which causes objects/arrays to become "[object
Object]"; before calling formData.append (both in the fieldMap and else
branches), normalize the value: if typeof value === 'object' and value !== null
then JSON.stringify(value), otherwise convert primitives to String(value); use
the same normalized value when calling formData.append (references:
formData.append, fieldMap[key], menuOrder, prefix) so nested variation data is
serialized correctly.
In `@src/dashboard/form-manager/utils.tsx`:
- Around line 180-228: Add the missing hidden_scope property to the FlatFormItem
type and use the scope parameter in layoutBuilder to filter out fields that are
hidden for the current scope: update the FlatFormItem type (in
src/dashboard/form-manager/types/index.ts) to include hidden_scope?: string, and
in layoutBuilder’s mapping logic (within getFlatField usage / mappedLayouts)
skip string field references when flatField.hidden_scope === scope (in addition
to existing dependency checks using resolveDependency) so per-scope visibility
is enforced during the recursive layoutBuilder calls.
---
Duplicate comments:
In `@includes/REST/ProductControllerV3.php`:
- Around line 92-98: In create_item, avoid firing dokan_new_product_added when
parent::create_item($request) returns a WP_Error: check is_wp_error($product)
immediately and return $product early on error; when successful, treat $product
as a WP_REST_Response and obtain the id via $product->get_data()['id'] (cast to
int) instead of accessing $product->data directly, then call
do_action('dokan_new_product_added', $product_id, $params) and return $product.
- Around line 110-116: The update_item method currently mirrors the create_item
bug: it pulls $product_id from $product->data['id'] without checking for
WP_Error and fires dokan_product_updated even when product creation failed; also
the docblock still says "Create a product item". Change update_item to check
is_wp_error($product) first, only extract the ID if $product is not an error and
isset($product->data['id']) (cast to int), and only call
do_action('dokan_product_updated', $product_id, $params) when $product_id > 0;
finally update the method docblock summary to "Update a product item".
In `@src/dashboard/form-manager/App.tsx`:
- Around line 72-76: The button label is inverted: in App.tsx the label prop
currently uses isNewProduct ? __('Save Changes', 'dokan-lite') : __('Update
Product', 'dokan-lite'); update the ternary so new products show a "Save
Product" label and editing existing products show "Save Changes" (or "Update
Product" if you prefer consistency), i.e. swap the strings used for the
true/false branches; keep the same text domain and the label prop and
isNewProduct symbol intact.
In `@src/dashboard/form-manager/components/FieldRenderer.tsx`:
- Around line 33-46: Duplicate use of dangerouslySetInnerHTML with sanitizeHTML
in FieldRenderer should be consolidated into a small reusable component; create
a SanitizedHtml functional component that accepts a single prop (html: string)
and returns a span/div using dangerouslySetInnerHTML={{ __html:
sanitizeHTML(html) }}, then replace the inline spans in FieldRenderer (both the
label span around field.label and the other occurrence at lines ~77-85) with
<SanitizedHtml html={field.label} /> (and corresponding prop for the other use).
Ensure you export/import SanitizedHtml where needed and keep existing className
("dokan-form-field-label") and aria attributes by passing them through or
wrapping as props so the DOM and tooltip structure (DokanTooltip, Info) remain
unchanged.
In `@src/dashboard/form-manager/components/FileUploadEdit.tsx`:
- Around line 45-52: The updateRow function is mutating a nested object by doing
newFiles[index][key] = value; instead create an immutable update: copy the array
and replace the specific item with a shallow-cloned object that has the updated
key, then call setFiles and onChange with that new array (refer to updateRow,
files, setFiles, onChange and field.id to locate the logic). Ensure you do not
mutate the original object—build newFiles = [...files]; updatedItem = {
...newFiles[index], [key]: value }; newFiles[index] = updatedItem;
setFiles(newFiles); onChange({ [field.id]: newFiles });.
- Around line 74-76: The list is using the array index as the React key in
files.map which can break reconciliation; change the key to a stable unique
identifier (e.g., use file.id or file.uuid) instead of index in
FileUploadEdit.tsx (replace key={index} with key={file.id}). If file objects
lack an id, assign a persistent id when rows are created/added (e.g., in the
function that pushes new file rows) so each file has a stable unique property to
use as the key.
In `@src/dashboard/form-manager/context/VariationContext.tsx`:
- Around line 198-212: In fetchVariations, guard the call to onUpdateVariations
by verifying the AJAX response indicates success (check response &&
response.success) before using response.data; if not successful, log the
response and show an error toast (using any server-provided message if
available) instead of updating state. Ensure you update the logic inside the
fetchVariations async function (the block that calls ajaxRequest and currently
calls onUpdateVariations) to perform this response.success check and handle the
error path.
- Around line 214-280: generateVariations, addVariation, and removeVariation
currently assume success and access response.data.message without checking
response.success or handling thrown errors; update each function to wrap the
ajaxRequest call in try/catch, verify response.success before showing a success
toast (and only access response.data.message when success is true), show an
error toast with either response.data?.message or a generic message when success
is false, and in the catch block log the error and show a user-facing error
toast; apply these changes to the functions generateVariations, addVariation,
and removeVariation (and keep existing confirm checks and nonce fields intact).
- Around line 108-126: The guard in useVariationContext is dead because
VariationContext is created with a concrete default object; change the context
to be optionally undefined by creating it as createContext<VariationContextType
| undefined>(undefined) and update the type accordingly, then keep the runtime
check as if (context === undefined) throw new Error(...); reference the
VariationContext and useVariationContext symbols and ensure any providers
(VariationProvider) still pass a fully initialized VariationContextType value so
consumers get a real object at runtime.
In `@src/dashboard/form-manager/factories/handlers.ts`:
- Around line 148-153: The JSDoc for defaultHandler incorrectly mentions a
non-existent TextWithAddon component; update the docstring for the
defaultHandler function to reflect the actual implementation (remove the
TextWithAddon reference or replace it with the real component name used in the
code, e.g., TextWithIcon if that is what's used) and ensure the description and
return annotation accurately describe the returned configuration (type 'text'
and when/how an addon/icon component is applied). Locate the JSDoc directly
above the defaultHandler function and make the wording consistent with the
function's behavior and component identifiers.
- Around line 93-106: The JSDoc for multiSelectHandler incorrectly duplicates
selectHandler's description; update the comment above multiSelectHandler to
accurately describe that it handles multi-select/select-multiple fields (returns
type 'array', uses Edit component SelectEdit with multiple: true), mention
expected value shape (array of selected option values) and any special
behavior/validation differences compared to selectHandler; reference the
multiSelectHandler function and the SelectEdit/multiple config in the updated
comment.
In `@src/dashboard/form-manager/hooks/useLayouts.tsx`:
- Around line 239-245: The call in useLayouts (returning fields) passes
product.type directly to layoutBuilder which can be undefined and will override
layoutBuilder's default 'product' scope; update the call site in useLayouts.tsx
to pass a guaranteed scope by using a fallback (e.g., product.type || 'product'
or product.type ?? 'product') when invoking layoutBuilder with (updatedLayouts,
formItems, product, product.type) so layoutBuilder receives 'product' when
product.type is undefined.
- Around line 85-91: The hardcoded label "Digital Product Options" in the layout
object (id: 'digital-options') must be internationalized; replace the bare
string with a call to the translation helper __('Digital Product Options',
'your-text-domain') and ensure the translation helper is imported (e.g., import
{ __ } from '@wordpress/i18n') at the top of useLayouts.tsx so the label becomes
localized via __('Digital Product Options', 'your-text-domain').
In `@src/dashboard/form-manager/utils.tsx`:
- Around line 89-93: In validateProductForm replace the current falsy check (if
(!value || (Array.isArray(value) && value.length === 0))) with a call to the
existing isEmptyValue helper to correctly treat 0/false as valid values; update
the branch that assigns newErrors[field.id] to use isEmptyValue(value) (and keep
the array-length logic if isEmptyValue doesn't already cover empty arrays) so
validation uses the centralized isEmptyValue behavior instead of raw !value
checks.
- Around line 362-389: injectRemainingFields currently mutates its
remainingFieldsBySection input via delete remainingFieldsBySection[newItem.id];
stop mutating the caller object by working on a shallow copy: at the top of
injectRemainingFields create const remaining = { ...remainingFieldsBySection }
and use remaining for lookups and deletes (i.e., check remaining[newItem.id],
assign newItem.children from remaining[newItem.id], delete
remaining[newItem.id], and pass remaining into recursive calls to
injectRemainingFields) so the original remainingFieldsBySection is never
modified.
---
Nitpick comments:
In `@includes/ProductForm/ProductForm.php`:
- Around line 165-168: ProductCategoryHelper::get_product_categories_tree() (and
similar calls get_product_tags(), get_products_brands()) are executed eagerly
inside ProductForm::get_fields(), causing DB queries on every invocation; change
to defer or cache these option list computations by returning a lazy callback or
memoized value instead of calling the helper at definition time: update
get_fields() to supply a closure or reference that calls
ProductCategoryHelper::get_product_categories_tree() only when options are
actually requested, or introduce a simple in-memory cache (e.g., static property
or a singleton cache in ProductCategoryHelper) to return cached results with a
suitable TTL, and apply the same pattern for get_product_tags() and
get_products_brands().
In `@includes/REST/ProductControllerV3.php`:
- Around line 33-34: The global rest_pre_dispatch filter added in
register_routes causes resolve_product_payload_before_validation to run on every
REST request; remove that add_filter and instead call
resolve_product_payload_before_validation directly inside the specific route
handlers before invoking parent::create_item and parent::update_item (or, if you
prefer a hook, attach it to a narrower filter such as
rest_request_before_callbacks for only dokan/v3/products requests). Update
register_routes to stop registering rest_pre_dispatch and ensure
resolve_product_payload_before_validation is invoked at the start of the
create/update route callbacks so the payload is resolved only for the intended
routes.
In `@src/dashboard/form-manager/factories/handlers.ts`:
- Around line 99-105: The config object in multiSelectHandler is currently typed
as any; change it to use the correct FieldConfig type so it matches other
handlers (e.g., defaultHandler). Locate the multiSelectHandler function and
replace "const config: any = { ... }" with "const config: FieldConfig = { ... }"
(or annotate the returned object with FieldConfig), keeping existing properties
(type, Edit, multiple) and ensuring imports/typing for FieldConfig are present
where needed.
In `@src/dashboard/form-manager/hooks/useVariationLayouts.tsx`:
- Around line 9-122: The layouts array defined inside the useMemo in
useVariationLayouts is a static constant and should be hoisted to module scope
to avoid re-creating the literal each render; move the large layouts definition
out of the useMemo (e.g. const VARIATION_LAYOUTS = [...]) and update the useMemo
to pass VARIATION_LAYOUTS into layoutBuilder instead of the inline variable,
keeping the existing call to layoutBuilder(layouts, formItems, product,
'variation') and preserving the useMemo dependency array ([formItems, product]).
In `@src/dashboard/form-manager/types/index.ts`:
- Around line 36-40: The FieldConfig type currently uses an index signature
`[key: string]: any` which erases the explicit types from Partial<FlatFormItem>
and the optional Edit/elements props; change that index signature to a safer
type (for example `[key: string]: unknown` or a generic Record<string, unknown>)
so callers must narrow/assert unknown values, while leaving the names
FieldConfig, FlatFormItem, Edit and elements intact; update any callsites that
relied on implicit any to add proper type assertions or casts.
- Around line 13-31: Replace the broad type: string on FlatFormItem with a
string literal union so TypeScript can discriminate (e.g., type: 'section' |
'field' or a named union like FormItemType = 'section' | 'field'); update the
FlatFormItem definition (or split into FlatSectionItem / FlatFieldItem that
extend a shared base) to use that discriminant so downstream code can narrow on
the type field in conditionals and get proper typed properties.
| if ( ! $product_id ) { | ||
| $product = new WC_Product_Simple(); | ||
| $product->set_status( 'auto-draft' ); | ||
| $product->set_name( '' ); | ||
| $product->save(); | ||
| $new_product = true; | ||
| $product_id = $product->get_id(); | ||
| } |
There was a problem hiding this comment.
Every form load without product_id creates an orphan auto-draft product.
If a user refreshes the page, navigates back, or opens the form in multiple tabs, each load creates a new WC_Product_Simple with auto-draft status that is never cleaned up. This can accumulate significant database bloat over time.
Consider either reusing an existing auto-draft for the current user, or deferring product creation until the first save.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@includes/ProductForm/Hooks.php` around lines 76 - 83, The current form hook
in includes/ProductForm/Hooks.php creates a new WC_Product_Simple on every load
when $product_id is falsy, causing orphan auto-drafts; change this to either (A)
attempt to reuse an existing auto-draft for the current user before creating a
new one (e.g., query posts with post_type 'product', post_status 'auto-draft',
author get_current_user_id() and optional meta key like
'_temp_product_for_user') and assign that ID to $product_id, or (B) defer
creation entirely and only instantiate/save WC_Product_Simple when the user
performs an explicit save action (move the WC_Product_Simple creation into the
save handler). Update any code that expects $new_product accordingly and
set/clear a user-meta or post-meta flag if you choose reuse to avoid collisions
across tabs.
includes/ProductForm/Hooks.php
Outdated
There was a problem hiding this comment.
Raw $_POST passed directly to dokan()->product->create().
Line 145 passes $_POST without any sanitization or field whitelisting. Even if create() internally sanitizes, this practice is risky — it could allow mass-assignment of unexpected fields. Also, after wp_verify_nonce fails, wp_send_json_error does call wp_die() internally so execution stops, but the lack of an explicit return makes this non-obvious.
🐛 Suggested improvement
if ( ! isset( $_POST['_nonce'] ) || ! wp_verify_nonce( sanitize_key( $_POST['_nonce'] ), 'form_manager' ) ) {
wp_send_json_error(
[
'type' => 'nonce',
'message' => __( 'Are you cheating?', 'dokan-lite' ),
]
);
+ return; // wp_send_json_error calls wp_die, but explicit return for clarity
}
+
+ if ( ! current_user_can( 'dokan_edit_product' ) ) {
+ wp_send_json_error(
+ [
+ 'type' => 'permission',
+ 'message' => __( 'You do not have permission to perform this action.', 'dokan-lite' ),
+ ]
+ );
+ return;
+ }
+
try {
- $product = dokan()->product->create( $_POST );
+ $data = wc_clean( wp_unslash( $_POST ) );
+ $product = dokan()->product->create( $data );📝 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 dokan_save_product_data() { | |
| if ( ! isset( $_POST['_nonce'] ) || ! wp_verify_nonce( sanitize_key( $_POST['_nonce'] ), 'form_manager' ) ) { | |
| wp_send_json_error( | |
| [ | |
| 'type' => 'nonce', | |
| 'message' => __( 'Are you cheating?', 'dokan-lite' ), | |
| ] | |
| ); | |
| } | |
| try { | |
| $product = dokan()->product->create( $_POST ); | |
| wp_send_json_success( | |
| [ | |
| 'product' => $product->get_data(), | |
| 'message' => __( 'Product saved successfully', 'dokan-lite' ), | |
| ] | |
| ); | |
| } catch ( Exception $e ) { | |
| wp_send_json_error( | |
| [ | |
| 'status' => false, | |
| 'message' => $e->getMessage(), | |
| ] | |
| ); | |
| } | |
| } | |
| public function dokan_save_product_data() { | |
| if ( ! isset( $_POST['_nonce'] ) || ! wp_verify_nonce( sanitize_key( $_POST['_nonce'] ), 'form_manager' ) ) { | |
| wp_send_json_error( | |
| [ | |
| 'type' => 'nonce', | |
| 'message' => __( 'Are you cheating?', 'dokan-lite' ), | |
| ] | |
| ); | |
| return; | |
| } | |
| if ( ! current_user_can( 'dokan_edit_product' ) ) { | |
| wp_send_json_error( | |
| [ | |
| 'type' => 'permission', | |
| 'message' => __( 'You do not have permission to perform this action.', 'dokan-lite' ), | |
| ] | |
| ); | |
| return; | |
| } | |
| try { | |
| $data = wc_clean( wp_unslash( $_POST ) ); | |
| $product = dokan()->product->create( $data ); | |
| wp_send_json_success( | |
| [ | |
| 'product' => $product->get_data(), | |
| 'message' => __( 'Product saved successfully', 'dokan-lite' ), | |
| ] | |
| ); | |
| } catch ( Exception $e ) { | |
| wp_send_json_error( | |
| [ | |
| 'status' => false, | |
| 'message' => $e->getMessage(), | |
| ] | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@includes/ProductForm/Hooks.php` around lines 135 - 160, The
dokan_save_product_data function is passing raw $_POST into
dokan()->product->create and lacks an explicit return after nonce failure; fix
by whitelisting and sanitizing only the expected product fields from $_POST
(e.g., title, price, description, sku, stock, categories) before calling
dokan()->product->create, use appropriate sanitizers/escapers for each field,
and pass the cleaned array instead of $_POST; also add an explicit return after
the wp_send_json_error nonce branch (and after
wp_send_json_success/wp_send_json_error responses) so execution cannot continue
after sending a response.
includes/ProductForm/Hooks.php
Outdated
There was a problem hiding this comment.
Missing nonce verification on dokan_get_product_variations AJAX handler.
Unlike dokan_save_product_data (which checks a nonce), this handler has no nonce or capability check beyond WordPress's built-in wp_ajax_ requiring a logged-in user. Any authenticated user (including subscribers) can fetch variation data for any product by supplying an arbitrary product_id.
🐛 Proposed fix
public function dokan_get_product_variations() {
+ if ( ! isset( $_REQUEST['_nonce'] ) || ! wp_verify_nonce( sanitize_key( $_REQUEST['_nonce'] ), 'form_manager' ) ) {
+ wp_send_json_error( __( 'Nonce verification failed.', 'dokan-lite' ) );
+ }
+ if ( ! current_user_can( 'dokan_edit_product' ) ) {
+ wp_send_json_error( __( 'Permission denied.', 'dokan-lite' ) );
+ }
if ( ! isset( $_REQUEST['product_id'] ) ) { // phpcs:ignore
wp_send_json_error( __( 'Product ID is required', 'dokan-lite' ) );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function dokan_get_product_variations() { | |
| if ( ! isset( $_REQUEST['product_id'] ) ) { // phpcs:ignore | |
| wp_send_json_error( __( 'Product ID is required', 'dokan-lite' ) ); | |
| } | |
| $product_id = intval( wp_unslash( $_REQUEST['product_id'] ) ); // phpcs:ignore | |
| wp_send_json_success( dokan()->product_form->get_product_variations( $product_id ) ); | |
| } | |
| public function dokan_get_product_variations() { | |
| if ( ! isset( $_REQUEST['_nonce'] ) || ! wp_verify_nonce( sanitize_key( $_REQUEST['_nonce'] ), 'form_manager' ) ) { | |
| wp_send_json_error( __( 'Nonce verification failed.', 'dokan-lite' ) ); | |
| } | |
| if ( ! current_user_can( 'dokan_edit_product' ) ) { | |
| wp_send_json_error( __( 'Permission denied.', 'dokan-lite' ) ); | |
| } | |
| if ( ! isset( $_REQUEST['product_id'] ) ) { // phpcs:ignore | |
| wp_send_json_error( __( 'Product ID is required', 'dokan-lite' ) ); | |
| } | |
| $product_id = intval( wp_unslash( $_REQUEST['product_id'] ) ); // phpcs:ignore | |
| wp_send_json_success( dokan()->product_form->get_product_variations( $product_id ) ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@includes/ProductForm/Hooks.php` around lines 169 - 175, The
dokan_get_product_variations AJAX handler lacks nonce and capability checks
allowing any logged-in user to fetch arbitrary product variations; update
dokan_get_product_variations to call check_ajax_referer with a chosen action
(e.g., the same nonce action used by product save or a new
'dokan_product_variations' action) and verify the current user can edit the
target product (use current_user_can( 'edit_post', $product_id )) before calling
dokan()->product_form->get_product_variations; on failed checks return
wp_send_json_error with a clear message.
| // grouped products (children): ensure array of ids | ||
| if ( isset( $out[ Elements::GROUP_PRODUCTS ] ) && is_array( $out[ Elements::GROUP_PRODUCTS ] ) ) { | ||
| $ids = array_map( | ||
| static function ( $c ) { | ||
| if ( is_array( $c ) ) { | ||
| return (int) ( $c['value'] ?? $c['id'] ?? 0 ); | ||
| } | ||
| if ( is_object( $c ) && isset( $c->value ) ) { | ||
| return (int) $c->value; | ||
| } | ||
| return (int) $c; | ||
| }, | ||
| $out[ Elements::GROUP_PRODUCTS ] | ||
| ); | ||
| $out[ Elements::GROUP_PRODUCTS ] = array_map( 'absint', array_filter( $ids ) ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "GROUP_PRODUCTS" --type php -C 1Repository: getdokan/dokan
Length of output: 1036
🏁 Script executed:
head -n 200 includes/ProductForm/PayloadResolver.php | cat -nRepository: getdokan/dokan
Length of output: 10314
🏁 Script executed:
sed -n '124,154p' includes/ProductForm/PayloadResolver.php | cat -nRepository: getdokan/dokan
Length of output: 1587
🏁 Script executed:
sed -n '80,90p' includes/ProductForm/Elements.php | cat -nRepository: getdokan/dokan
Length of output: 669
🏁 Script executed:
rg "grouped_products" --type php -nRepository: getdokan/dokan
Length of output: 836
🏁 Script executed:
rg "const ATTRIBUTES" includes/ProductForm/Elements.php -A 1 -B 1Repository: getdokan/dokan
Length of output: 434
🏁 Script executed:
rg "const UPSELL_IDS|const CROSS_SELL_IDS" includes/ProductForm/Elements.php -A 1Repository: getdokan/dokan
Length of output: 178
Grouped products key must be remapped to the WC REST API expected key.
The code resolves Elements::GROUP_PRODUCTS (which equals 'children') in-place without remapping to the WC REST API key grouped_products. WC REST API expects grouped_products for the grouped product children (confirmed at ProductController.php:1220–1221 and Product/Manager.php:355–356). This inconsistency will cause the grouped products field to be ignored by the API.
Remap the key similarly to how categories, tags, brands, and shipping_class are handled:
$out['grouped_products'] = array_map( 'absint', array_filter( $ids ) );
unset( $out[ Elements::GROUP_PRODUCTS ] );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@includes/ProductForm/PayloadResolver.php` around lines 155 - 170, The grouped
products array is being normalized but left under Elements::GROUP_PRODUCTS
(which equals 'children') so the WC REST API ignores it; update the resolver to
assign the cleaned ids to the WC API key 'grouped_products' and remove the
original Elements::GROUP_PRODUCTS entry — i.e., after computing $ids and
normalizing them with array_map('absint', array_filter($ids)), set
$out['grouped_products'] = ... and unset($out[ Elements::GROUP_PRODUCTS ]) so
the API receives the expected key.
| // attributes: form shape -> WC API shape (id, name, position, visible, variation, options) | ||
| if ( isset( $out[ Elements::ATTRIBUTES ] ) && is_array( $out[ Elements::ATTRIBUTES ] ) ) { | ||
| $out['attributes'] = self::transform_attributes_for_api( $out[ Elements::ATTRIBUTES ] ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg "ATTRIBUTES\s*=" --type php -C 2 -g '*/Elements.php'Repository: getdokan/dokan
Length of output: 206
🏁 Script executed:
fd -t f "Elements.php"Repository: getdokan/dokan
Length of output: 91
🏁 Script executed:
rg "ATTRIBUTES\s*=" --type php -C 2 includes/ProductForm/Elements.phpRepository: getdokan/dokan
Length of output: 471
🏁 Script executed:
sed -n '165,185p' includes/ProductForm/PayloadResolver.phpRepository: getdokan/dokan
Length of output: 914
🏁 Script executed:
rg -n "categories|tags|brands" includes/ProductForm/PayloadResolver.php -C 3Repository: getdokan/dokan
Length of output: 1531
🏁 Script executed:
rg "const (CATEGORIES|TAGS|BRANDS)\s*=" --type php includes/ProductForm/Elements.phpRepository: getdokan/dokan
Length of output: 164
Inconsistent cleanup pattern: attributes not unsetting original key like categories/tags/brands do.
While Elements::ATTRIBUTES = 'attributes' (matching the target key), the transformation diverges from the established pattern. Categories, tags, and brands all explicitly unset() their original keys after transformation (unset( $out[ Elements::CATEGORIES ] ), etc.), but attributes does not. This inconsistency could become problematic if the constant is later changed, and it makes the cleanup pattern unpredictable for maintainers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@includes/ProductForm/PayloadResolver.php` around lines 172 - 175, The
attributes transformation is missing the cleanup step used for
categories/tags/brands; after calling self::transform_attributes_for_api( $out[
Elements::ATTRIBUTES ] ) you should unset the original input key to follow the
established pattern—update the block in PayloadResolver.php so that after
assigning $out['attributes'] = self::transform_attributes_for_api( $out[
Elements::ATTRIBUTES ] ); you call unset( $out[ Elements::ATTRIBUTES ] ); to
remove the original payload entry (referenced symbols: Elements::ATTRIBUTES and
transform_attributes_for_api).
There was a problem hiding this comment.
Guard against undefined/null field.value before accessing .length.
If field.value is undefined or null (e.g., a new field with no saved data), field.value.length will throw a TypeError at runtime.
🐛 Proposed fix
const [ files, setFiles ] = useState(
- field.value.length > 0
+ Array.isArray( field.value ) && field.value.length > 0
? field.value
: [
{
id: '',
name: '',
file: '',
},
]
);📝 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 FileUploadEdit = ( { field, onChange }: any ) => { | |
| const [ files, setFiles ] = useState( | |
| field.value.length > 0 | |
| ? field.value | |
| : [ | |
| { | |
| id: '', | |
| name: '', | |
| file: '', | |
| }, | |
| ] | |
| ); | |
| const FileUploadEdit = ( { field, onChange }: any ) => { | |
| const [ files, setFiles ] = useState( | |
| Array.isArray( field.value ) && field.value.length > 0 | |
| ? field.value | |
| : [ | |
| { | |
| id: '', | |
| name: '', | |
| file: '', | |
| }, | |
| ] | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/dashboard/form-manager/components/FileUploadEdit.tsx` around lines 9 -
20, FileUploadEdit currently reads field.value.length without guarding for
undefined/null; update the initialization of the files state so it first
verifies field?.value is an array (e.g., Array.isArray(field.value)) and has
length > 0 before using it, otherwise fall back to the single empty file object;
reference the FileUploadEdit component, the field.value access, and the
files/setFiles state initializer when making this change and ensure the guard
prevents any access to .length on undefined/null.
| const onRemoveRow = ( index: number ) => { | ||
| const newFiles = files.filter( ( _: any, i: number ) => i !== index ); | ||
| setFiles( newFiles ); | ||
| onChange( { | ||
| [ field.id ]: newFiles, | ||
| } ); | ||
| }; |
There was a problem hiding this comment.
No minimum-row guard on onRemoveRow.
If the user removes the last row, files becomes an empty array and the UI renders no input rows. Consider preventing removal when only one row remains, or re-adding a blank row.
🛡️ Proposed fix
const onRemoveRow = ( index: number ) => {
- const newFiles = files.filter( ( _: any, i: number ) => i !== index );
+ let newFiles = files.filter( ( _: any, i: number ) => i !== index );
+ if ( newFiles.length === 0 ) {
+ newFiles = [ { id: '', name: '', file: '' } ];
+ }
setFiles( newFiles );
onChange( {
[ field.id ]: newFiles,
} );
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/dashboard/form-manager/components/FileUploadEdit.tsx` around lines 37 -
43, onRemoveRow currently removes any row including the last one, leaving files
empty and the UI with no input rows; update onRemoveRow (referencing files,
setFiles, onChange, and field.id) to guard against removing the final row by
either returning early when files.length <= 1 or replacing the removed item with
a blank placeholder (e.g., add a single empty file object) so that after
setFiles and the onChange call there is always at least one entry rendered.
| Object.keys( data ).forEach( ( key ) => { | ||
| if ( key === 'attributes' ) { | ||
| return; | ||
| } | ||
| // prefix field names if not in map | ||
| const prefix = key.startsWith( '_' ) ? 'variable' : 'variable_'; | ||
|
|
||
| if ( fieldMap[ key ] ) { | ||
| formData.append( | ||
| `${ fieldMap[ key ] }[${ menuOrder }]`, | ||
| data[ key ] | ||
| ); | ||
| } else { | ||
| formData.append( | ||
| `${ prefix }${ key }[${ menuOrder }]`, | ||
| data[ key ] | ||
| ); | ||
| } | ||
| } ); |
There was a problem hiding this comment.
Non-string values appended to FormData will be coerced to "[object Object]".
FormData.append() converts all values to strings. If data[key] is an object or array (e.g., nested variation data), it becomes the string "[object Object]". Consider serializing complex values before appending.
🐛 Proposed fix
Object.keys( data ).forEach( ( key ) => {
if ( key === 'attributes' ) {
return;
}
// prefix field names if not in map
const prefix = key.startsWith( '_' ) ? 'variable' : 'variable_';
+ const val = typeof data[ key ] === 'object' && data[ key ] !== null
+ ? JSON.stringify( data[ key ] )
+ : String( data[ key ] ?? '' );
if ( fieldMap[ key ] ) {
formData.append(
`${ fieldMap[ key ] }[${ menuOrder }]`,
- data[ key ]
+ val
);
} else {
formData.append(
`${ prefix }${ key }[${ menuOrder }]`,
- data[ key ]
+ val
);
}
} );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/dashboard/form-manager/context/VariationContext.tsx` around lines 72 -
90, The code is appending non-string values directly to formData in the loop in
VariationContext (the block using Object.keys(data).forEach), which causes
objects/arrays to become "[object Object]"; before calling formData.append (both
in the fieldMap and else branches), normalize the value: if typeof value ===
'object' and value !== null then JSON.stringify(value), otherwise convert
primitives to String(value); use the same normalized value when calling
formData.append (references: formData.append, fieldMap[key], menuOrder, prefix)
so nested variation data is serialized correctly.
There was a problem hiding this comment.
isLoading only tracks saveVariation, not other async operations.
setIsLoading is only called in saveVariation. The generateVariations, addVariation, removeVariation, and fetchVariations operations do not set the loading state, so consumers relying on isLoading won't reflect those in-flight requests.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/dashboard/form-manager/context/VariationContext.tsx` at line 144, The
isLoading state currently only wraps saveVariation, so other async functions
(generateVariations, addVariation, removeVariation, fetchVariations) don't set
loading and callers get incorrect UI state; update each async function in
VariationContext (generateVariations, addVariation, removeVariation,
fetchVariations and saveVariation) to setIsLoading(true) at the start and
setIsLoading(false) in finally (or ensure all code paths clear it) and handle
errors as needed so isLoading accurately reflects any in-flight operation.
| export const layoutBuilder = ( | ||
| layouts: any[], | ||
| formItems: FlatFormItem[] = [], | ||
| product: Record< string, any >, | ||
| scope: string = 'product' | ||
| ): any[] => { | ||
| const getFlatField = ( id: string ): FlatFormItem | undefined => | ||
| formItems.find( | ||
| ( i ) => i.type === 'field' && i.id === id | ||
| ); | ||
|
|
||
| const getOrder = ( item: any ) => { | ||
| if ( typeof item === 'string' ) { | ||
| const flat = getFlatField( item ); | ||
| if ( flat && typeof flat.order === 'number' ) { | ||
| return flat.order; | ||
| } | ||
| return 30; | ||
| } | ||
|
|
||
| if ( typeof item.order === 'number' ) { | ||
| return item.order; | ||
| } | ||
|
|
||
| if ( item.id ) { | ||
| const flat = formItems.find( ( i ) => i.id === item.id ); | ||
| if ( flat && 'order' in flat && typeof flat.order === 'number' ) { | ||
| return flat.order; | ||
| } | ||
| } | ||
|
|
||
| return 30; | ||
| }; | ||
|
|
||
| const sortedLayouts = [ ...layouts ].sort( ( a, b ) => { | ||
| return getOrder( a ) - getOrder( b ); | ||
| } ); | ||
|
|
||
| const mappedLayouts = sortedLayouts.map( ( field ) => { | ||
| if ( typeof field === 'string' ) { | ||
| const flatField = getFlatField( field ); | ||
| if ( ! flatField ) { | ||
| return null; | ||
| } | ||
| const condition = flatField.dependencies; | ||
| if ( ! resolveDependency( condition, product ) ) { | ||
| return null; | ||
| } | ||
| return field; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/dashboard/form-manager/utils.tsx | sed -n '170,240p'Repository: getdokan/dokan
Length of output: 3066
🏁 Script executed:
rg -n "hidden_scope" --type ts --type tsx -B2 -A2Repository: getdokan/dokan
Length of output: 85
🏁 Script executed:
rg -n "layoutBuilder" --type ts --type tsx -B5 -A1 | head -50Repository: getdokan/dokan
Length of output: 85
🏁 Script executed:
rg -n "scope" src/dashboard/form-manager/utils.tsx -B2 -A2Repository: getdokan/dokan
Length of output: 804
🏁 Script executed:
cat -n src/dashboard/form-manager/utils.tsx | sed -n '218,270p'Repository: getdokan/dokan
Length of output: 2055
🏁 Script executed:
rg "hidden_scope" --type ts -B3 -A3Repository: getdokan/dokan
Length of output: 739
🏁 Script executed:
rg "layoutBuilder" --type ts -B10 -A3 | head -100Repository: getdokan/dokan
Length of output: 5942
🏁 Script executed:
rg "layoutBuilder\(" --type ts -A3 -B1 | grep -A3 "fields:"Repository: getdokan/dokan
Length of output: 711
🏁 Script executed:
cat -n src/dashboard/form-manager/hooks/useVariationLayouts.tsx | sed -n '1,50p'Repository: getdokan/dokan
Length of output: 2043
🏁 Script executed:
cat -n src/dashboard/form-manager/hooks/useLayouts.tsx | sed -n '1,50p'Repository: getdokan/dokan
Length of output: 1878
🏁 Script executed:
rg "layoutBuilder\(" --type ts -A4 -B1Repository: getdokan/dokan
Length of output: 1428
🏁 Script executed:
grep -n "hidden_scope" src/dashboard/form-manager/types.tsRepository: getdokan/dokan
Length of output: 126
🏁 Script executed:
cat -n src/dashboard/form-manager/types.ts | sed -n '1,100p'Repository: getdokan/dokan
Length of output: 125
🏁 Script executed:
fd "types" src/dashboard/form-manager/ --type fRepository: getdokan/dokan
Length of output: 40
🏁 Script executed:
rg "FlatFormItem" --type ts -B2 -A5 | head -40Repository: getdokan/dokan
Length of output: 2579
🏁 Script executed:
fd types --type f | head -20Repository: getdokan/dokan
Length of output: 325
🏁 Script executed:
rg "interface FlatFormItem\|type FlatFormItem" --type ts -A10Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
rg "export.*FlatFormItem" --type ts -A15Repository: getdokan/dokan
Length of output: 3558
🏁 Script executed:
cat -n src/dashboard/form-manager/types/index.ts | sed -n '1,50p'Repository: getdokan/dokan
Length of output: 1868
🏁 Script executed:
rg "hidden_scope" src/dashboard/form-manager/types/index.ts -B2 -A2Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
rg "hidden_scope" --type ts --type js -iRepository: getdokan/dokan
Length of output: 204
🏁 Script executed:
git log --all --oneline -S "hidden_scope" | head -10Repository: getdokan/dokan
Length of output: 144
scope parameter is not used for field visibility filtering, and hidden_scope type is missing.
The scope parameter is passed through recursive layoutBuilder calls (line 254) but never used to filter fields based on their visibility rules. The JSDoc documents hidden_scope as a property on formItems, but the FlatFormItem type definition (src/dashboard/form-manager/types/index.ts) does not include this property. This prevents the intended per-scope field filtering from working.
Both the type definition and the filtering logic must be implemented:
- Add
hidden_scope?: stringto theFlatFormItemtype - Filter string field references in the mapping function when
flatField.hidden_scope === scope
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/dashboard/form-manager/utils.tsx` around lines 180 - 228, Add the missing
hidden_scope property to the FlatFormItem type and use the scope parameter in
layoutBuilder to filter out fields that are hidden for the current scope: update
the FlatFormItem type (in src/dashboard/form-manager/types/index.ts) to include
hidden_scope?: string, and in layoutBuilder’s mapping logic (within getFlatField
usage / mappedLayouts) skip string field references when flatField.hidden_scope
=== scope (in addition to existing dependency checks using resolveDependency) so
per-scope visibility is enforced during the recursive layoutBuilder calls.
Move and rename the form-manager feature folder to product-form. All files under src/dashboard/form-manager (components, contexts, hooks, factories, types, styles, utils and entry index) were relocated to src/dashboard/product-form with identical contents. Update webpack-entries.js to point the 'form-manager' entry to the new ./src/dashboard/product-form/index.tsx path so the build uses the relocated module.
1 participant
Introduced a new product form manager with React components and supporting PHP classes. Removed legacy Vue admin scripts and updated related assets, REST controller, and templates to integrate the new form manager.
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
Release Notes
New Features
Bug Fixes
Chores