Add new product form manager #3083
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.
📝 WalkthroughWalkthroughAdds a Product Form Manager: new PHP form system and FormManager, REST controller (dokan/v3) for fields and product create/update, DI/asset registrations, templates, and a React/TypeScript dashboard UI with field editors, variation contexts, layout/validation, and many new components. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as React Dashboard
participant FC as FormContext
participant VC as VariationContext
participant API as REST API (dokan/v3)
participant PHP as PHP Backend (FormManager/ProductControllerV3)
participant DB as Database
User->>UI: Open product edit or new
UI->>API: GET /dokan/v3/products/{id}/fields
API->>PHP: get_form_fields(request)
PHP->>DB: fetch product, variations, earnings
DB-->>PHP: product data
PHP-->>API: respond with sections, fields, vendor_earning
API-->>FC: initialize sections & fields
FC-->>UI: render form layout
User->>UI: Edit fields / manipulate variations
UI->>FC: onChange(fieldId, value)
FC-->>UI: update state / re-evaluate visibility & validation
UI->>VC: addVariation() / saveVariation()
VC->>API: POST/PUT /dokan/v3/products or variation endpoints
API->>PHP: create/update variation
PHP->>DB: persist variation
DB-->>PHP: saved
PHP-->>API: response
API-->>VC: update variations list
User->>UI: Submit form
UI->>FC: validate & prepare payload
UI->>API: POST/PUT /dokan/v3/products
API->>PHP: dokan_save_product_data()
PHP->>DB: persist product + meta
DB-->>PHP: confirmation
PHP-->>API: success/error
API-->>UI: show toast / update UI
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 ) => {
| public function load_product_edit_content() { | ||
| // check for permission | ||
| if ( ! current_user_can( 'dokan_edit_product' ) ) { | ||
| dokan_get_template_part( | ||
| 'global/dokan-error', '', [ | ||
| 'deleted' => false, | ||
| 'message' => __( 'You have no permission to view this page', 'dokan-lite' ), | ||
| ] | ||
| ); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| // check if seller is enabled for selling | ||
| if ( ! dokan_is_seller_enabled( dokan_get_current_user_id() ) ) { | ||
| dokan_seller_not_enabled_notice(); | ||
| return; | ||
| } | ||
| $product_id = isset( $_GET['product_id'] ) ? intval( wp_unslash( $_GET['product_id'] ) ) : 0; //phpcs:ignore | ||
| $new_product = false; | ||
|
|
||
| if ( ! $product_id ) { | ||
| // this is `add new` product page | ||
| $product = new WC_Product_Simple(); | ||
| $product->set_status( 'auto-draft' ); | ||
| $product->set_name( '' ); | ||
| $product->save(); | ||
| $new_product = true; | ||
| $product_id = $product->get_id(); | ||
| } | ||
| $product = wc_get_product( $product_id ); | ||
|
|
||
| if ( ! $product ) { | ||
| dokan_get_template_part( | ||
| 'global/dokan-error', '', [ | ||
| 'deleted' => false, | ||
| 'message' => __( 'Product not found', 'dokan-lite' ), | ||
| ] | ||
| ); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| dokan_get_template_part( | ||
| 'products/form-manager/form-content', | ||
| '', [ | ||
| 'product' => $product, | ||
| ] | ||
| ); | ||
| $vendor_earning = dokan()->commission->get_earning_by_product( $product_id ); | ||
| // load scripts | ||
| wp_enqueue_script( 'dokan-product-form-manager' ); | ||
| wp_enqueue_style( 'dokan-product-form-manager' ); | ||
| wp_localize_script( | ||
| 'dokan-product-form-manager', | ||
| 'dokanFormManager', | ||
| [ | ||
| 'sections' => self::get_form_fields( $product_id ), | ||
| 'form_manager_nonce' => wp_create_nonce( 'form_manager' ), | ||
| 'product_id' => $product_id, | ||
| 'is_new_product' => $new_product, | ||
| 'view_product_url' => get_permalink( $product_id ), | ||
| 'vendor_earning' => $vendor_earning, | ||
| 'variations' => self::get_product_variations( $product_id ), | ||
| ] | ||
| ); | ||
| } |
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.
| 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(), | ||
| ] | ||
| ); | ||
| } | ||
| } |
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.
| public static function get_product_variations( int $product_id ): array { | ||
| $variations = wc_get_products( | ||
| [ | ||
| 'status' => [ 'private', 'publish' ], | ||
| 'type' => 'variation', | ||
| 'parent' => $product_id, | ||
| 'orderby' => [ | ||
| 'menu_order' => 'ASC', | ||
| 'ID' => 'DESC', | ||
| ], | ||
| 'return' => 'objects', | ||
| ] | ||
| ); | ||
|
|
||
| $variations_data = []; | ||
| $parent_product = wc_get_product( $product_id ); | ||
|
|
||
| if ( $variations ) { | ||
| $iteration = 0; | ||
| foreach ( $variations as $variation ) { | ||
| /** @var \WC_Product_Variation $variation */ | ||
| $variation_id = $variation->get_id(); | ||
| $formatted_attrs = []; | ||
| $attribute_values = $variation->get_attributes( 'edit' ); | ||
|
|
||
| foreach ( $parent_product->get_attributes( 'edit' ) as $attribute ) { | ||
| if ( ! $attribute->get_variation() ) { | ||
| continue; | ||
| } | ||
|
|
||
| $selected_val = isset( $attribute_values[ sanitize_title( $attribute->get_name() ) ] ) ? $attribute_values[ sanitize_title( $attribute->get_name() ) ] : ''; | ||
| $options = []; | ||
| $selected = null; | ||
|
|
||
| if ( $attribute->is_taxonomy() ) { | ||
| foreach ( $attribute->get_terms() as $option ) { | ||
| $opt = [ | ||
| 'value' => $option->slug, | ||
| 'label' => apply_filters( 'woocommerce_variation_option_name', $option->name, $option, $attribute->get_name(), $parent_product ), | ||
| ]; | ||
| $options[] = $opt; | ||
|
|
||
| if ( $selected_val === $opt['value'] ) { | ||
| $selected = $opt; | ||
| } | ||
| } | ||
| } else { | ||
| foreach ( $attribute->get_options() as $option ) { | ||
| $opt = [ | ||
| 'value' => $option, | ||
| 'label' => apply_filters( 'woocommerce_variation_option_name', $option, null, $attribute->get_name(), $parent_product ), | ||
| ]; | ||
| $options[] = $opt; | ||
|
|
||
| if ( $selected_val === $opt['value'] ) { | ||
| $selected = $opt; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| $formatted_attrs[] = [ | ||
| 'label' => wc_attribute_label( $attribute->get_name() ), | ||
| 'value' => sanitize_title( $attribute->get_name() ), | ||
| 'selected_value' => $selected, | ||
| 'options' => $options, | ||
| ]; | ||
| } | ||
|
|
||
| $variations_data[] = [ | ||
| 'id' => $variation_id, | ||
| 'parent_id' => $product_id, | ||
| 'menu_order' => $iteration++, | ||
| 'attributes' => $formatted_attrs, | ||
| ]; | ||
| } | ||
| } | ||
| return $variations_data; | ||
| } |
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.
| 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 | ||
| $variations = self::get_product_variations( $product_id ); | ||
|
|
||
| wp_send_json_success( $variations ); |
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.
| 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; | ||
| }, |
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.
| const value = values[ field.id ]; | ||
| if ( ! value || ( Array.isArray( value ) && value.length === 0 ) ) { | ||
| newErrors[ field.id ] = __( | ||
| 'Please fill out this field.', | ||
| 'dokan-lite' | ||
| ); | ||
| } |
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.
| export const injectRemainingFields = ( | ||
| items: any[], | ||
| remainingFieldsBySection: Record< string, string[] > | ||
| ): any[] => { | ||
| return items.map( ( item ) => { | ||
| if ( typeof item === 'string' ) { | ||
| return item; | ||
| } | ||
| const newItem = { ...item }; | ||
|
|
||
| // Append if section ID matches | ||
| if ( newItem.id && remainingFieldsBySection[ newItem.id ] ) { | ||
| newItem.children = [ | ||
| ...( newItem.children || [] ), | ||
| ...remainingFieldsBySection[ newItem.id ], | ||
| ]; | ||
| delete remainingFieldsBySection[ newItem.id ]; | ||
| } | ||
|
|
||
| if ( newItem.children ) { | ||
| newItem.children = injectRemainingFields( | ||
| newItem.children, | ||
| remainingFieldsBySection | ||
| ); | ||
| } | ||
| return newItem; | ||
| } ); | ||
| }; |
There was a problem hiding this comment.
Mutation of remainingFieldsBySection argument.
The delete remainingFieldsBySection[newItem.id] at line 347 mutates the input object, which can cause unexpected behavior if the caller reuses the object after calling this function (as seen in useLayouts.tsx where it's accessed again for newSections).
🐛 Recommended fix: clone or track consumed keys separately
export const injectRemainingFields = (
items: any[],
- remainingFieldsBySection: Record< string, string[] >
+ remainingFieldsBySection: Record< string, string[] >,
+ consumedSections: Set< string > = new Set()
): any[] => {
return items.map( ( item ) => {
if ( typeof item === 'string' ) {
return item;
}
const newItem = { ...item };
// Append if section ID matches
- if ( newItem.id && remainingFieldsBySection[ newItem.id ] ) {
+ if ( newItem.id && remainingFieldsBySection[ newItem.id ] && ! consumedSections.has( newItem.id ) ) {
newItem.children = [
...( newItem.children || [] ),
...remainingFieldsBySection[ newItem.id ],
];
- delete remainingFieldsBySection[ newItem.id ];
+ consumedSections.add( newItem.id );
}
if ( newItem.children ) {
newItem.children = injectRemainingFields(
newItem.children,
- remainingFieldsBySection
+ remainingFieldsBySection,
+ consumedSections
);
}
return newItem;
} );
};Alternatively, if mutation is intentional by design, document this behavior clearly in the JSDoc.
🤖 Prompt for AI Agents
In `@src/dashboard/form-manager/utils.tsx` around lines 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.
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.
| public function prepare_downloads( $file_names, $file_urls, $file_hashes ) { | ||
| $downloads = []; | ||
|
|
||
| if ( ! empty( $file_urls ) ) { | ||
| $file_url_size = count( $file_urls ); | ||
|
|
||
| for ( $i = 0; $i < $file_url_size; $i++ ) { | ||
| if ( ! empty( $file_urls[ $i ] ) ) { | ||
| $downloads[] = [ | ||
| 'name' => wc_clean( $file_names[ $i ] ), | ||
| 'file' => wp_unslash( trim( $file_urls[ $i ] ) ), | ||
| 'download_id' => wc_clean( $file_hashes[ $i ] ), | ||
| ]; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return $downloads; |
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.
| public function create_item( $request ) { | ||
| $product = parent::create_item( $request ); | ||
| $params = $request->get_params(); | ||
| $product_id = is_wp_error( $product ) ? 0 : (int) $product->data['id']; | ||
| do_action( 'dokan_new_product_added', $product_id, $params ); | ||
| return $product; | ||
| } | ||
|
|
||
| /** | ||
| * Create a product item | ||
| * | ||
| * @since DOKAN_SINCE | ||
| * | ||
| * @param WP_REST_Request $request Full details about the request. | ||
| * | ||
| * @return WP_Error|WP_REST_Response | ||
| */ | ||
|
|
||
| public function update_item( $request ) { | ||
| $product = parent::update_item( $request ); | ||
| $product_id = is_wp_error( $product ) ? 0 : (int) $product->data['id']; | ||
| $params = $request->get_params(); | ||
| do_action( 'dokan_product_updated', $product_id, $params ); | ||
| return $product; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine the file in question
find . -name "ProductControllerV3.php" -type fRepository: getdokan/dokan
Length of output: 172
🏁 Script executed:
# Let's also check the directory structure to understand the codebase better
ls -la includes/REST/ 2>/dev/null || find . -type d -name REST | head -5Repository: getdokan/dokan
Length of output: 3055
🏁 Script executed:
# Search for the class definition and parent class to understand inheritance
grep -n "class ProductControllerV3" includes/REST/ProductControllerV3.php 2>/dev/null || rg "class ProductControllerV3" --max-count 1 -A 2Repository: getdokan/dokan
Length of output: 124
🏁 Script executed:
# Read the ProductControllerV3.php file to see the actual code
cat -n includes/REST/ProductControllerV3.phpRepository: getdokan/dokan
Length of output: 5102
🏁 Script executed:
# Search for WP_REST_Response usage patterns in the codebase
rg "WP_REST_Response|get_data\(\)" -t php --max-count 10 -B 2 -A 2Repository: getdokan/dokan
Length of output: 50371
🏁 Script executed:
# Check if there are other similar methods in the codebase that handle responses correctly
rg "is_wp_error.*product" includes/REST/ -t php -A 2 -B 2 | head -50Repository: getdokan/dokan
Length of output: 997
🌐 Web query:
WP_REST_Response get_data method WordPress documentation
💡 Result:
WP_REST_Response::get_data — what it is
- Purpose: returns the response payload (the data/body) stored in a WP_REST_Response (typically an array or scalar). (developer.wordpress.org)
- Typical use: call $data = $response->get_data(); before encoding/embedding or adding _links/_embedded. The REST server uses this internally (e.g., response_to_data()). (developer.wordpress.org)
Reference (official docs): WP_REST_Response class and related REST response handlers on WordPress Developer Resources. (developer.wordpress.org)
Citations:
- 1: https://developer.wordpress.org/reference/classes/wp_rest_response/?utm_source=openai
- 2: https://developer.wordpress.org/reference/classes/wp_rest_server/response_to_data/?utm_source=openai
- 3: https://developer.wordpress.org/reference/classes/wp_rest_response/?utm_source=openai
Use get_data() and return early on error to prevent hooks firing with invalid data.
parent::create_item/update_item() returns WP_REST_Response|WP_Error. The current code accesses $product->data directly (unsafe) and fires hooks even when errors occur—with product_id=0 instead of aborting. Hooks should only fire on success.
🐛 Suggested fix
public function create_item( $request ) {
$product = parent::create_item( $request );
+ if ( is_wp_error( $product ) ) {
+ return $product;
+ }
$params = $request->get_params();
+ $data = $product->get_data();
+ $product_id = isset( $data['id'] ) ? (int) $data['id'] : 0;
- $product_id = is_wp_error( $product ) ? 0 : (int) $product->data['id'];
do_action( 'dokan_new_product_added', $product_id, $params );
return $product;
}
public function update_item( $request ) {
$product = parent::update_item( $request );
+ if ( is_wp_error( $product ) ) {
+ return $product;
+ }
+ $data = $product->get_data();
+ $product_id = isset( $data['id'] ) ? (int) $data['id'] : 0;
- $product_id = is_wp_error( $product ) ? 0 : (int) $product->data['id'];
$params = $request->get_params();
do_action( 'dokan_product_updated', $product_id, $params );
return $product;
}🤖 Prompt for AI Agents
In `@includes/REST/ProductControllerV3.php` around lines 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.
| { 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.
| const defaultData = useMemo( | ||
| () => formDataFactory.create( sections ), | ||
| [ sections ] | ||
| ); | ||
|
|
||
| const [ product, setProduct ] = useState< Record< string, any > >( { | ||
| ...defaultData, | ||
| id: productId, | ||
| vendor_earning: vendorEarning, | ||
| variations: variations, | ||
| } ); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd src/dashboard/form-manager/context && wc -l FormContext.tsxRepository: getdokan/dokan
Length of output: 77
🏁 Script executed:
cat -n src/dashboard/form-manager/context/FormContext.tsxRepository: getdokan/dokan
Length of output: 6209
🏁 Script executed:
rg -n "FormProvider" --type tsx --type ts -B2 -A5Repository: getdokan/dokan
Length of output: 85
🏁 Script executed:
fd -e tsx -e ts | xargs rg -l "FormProvider" | head -10Repository: getdokan/dokan
Length of output: 209
🏁 Script executed:
cat -n src/dashboard/form-manager/App.tsxRepository: getdokan/dokan
Length of output: 8543
🏁 Script executed:
cat -n src/dashboard/form-manager/components/variation/VariationCard.tsxRepository: getdokan/dokan
Length of output: 5148
Sync product state when sections or props change.
In VariationCard.tsx, when sections are fetched and state updates, FormProvider receives new props but product state remains uninitialized with new defaults. Add a useEffect to merge updated defaults and props.
Suggested fix
import {
createContext,
useCallback,
useContext,
+ useEffect,
useMemo,
useState,
} from '@wordpress/element';
...
const [ product, setProduct ] = useState< Record< string, any > >( {
...defaultData,
id: productId,
vendor_earning: vendorEarning,
variations: variations,
} );
+
+ useEffect( () => {
+ setProduct( ( prev ) => ( {
+ ...defaultData,
+ ...prev,
+ id: productId,
+ vendor_earning: vendorEarning,
+ variations,
+ } ) );
+ }, [ defaultData, productId, vendorEarning, variations ] );📝 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 defaultData = useMemo( | |
| () => formDataFactory.create( sections ), | |
| [ sections ] | |
| ); | |
| const [ product, setProduct ] = useState< Record< string, any > >( { | |
| ...defaultData, | |
| id: productId, | |
| vendor_earning: vendorEarning, | |
| variations: variations, | |
| } ); | |
| const defaultData = useMemo( | |
| () => formDataFactory.create( sections ), | |
| [ sections ] | |
| ); | |
| const [ product, setProduct ] = useState< Record< string, any > >( { | |
| ...defaultData, | |
| id: productId, | |
| vendor_earning: vendorEarning, | |
| variations: variations, | |
| } ); | |
| useEffect( () => { | |
| setProduct( ( prev ) => ( { | |
| ...defaultData, | |
| ...prev, | |
| id: productId, | |
| vendor_earning: vendorEarning, | |
| variations, | |
| } ) ); | |
| }, [ defaultData, productId, vendorEarning, variations ] ); |
🤖 Prompt for AI Agents
In `@src/dashboard/form-manager/context/FormContext.tsx` around lines 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.
| /** | ||
| * Handler for select/dropdown fields. | ||
| * Handles normal select options and special cases like product categories. | ||
| * | ||
| * @param {Object} [field] The field configuration. | ||
| * @return {Object} Configuration object including type, elements, and Edit component. | ||
| */ | ||
| export const multiSelectHandler: FieldHandler = ( field ) => { | ||
| const config: any = { | ||
| type: 'array', | ||
| elements: getElementsFromOptions( field ), | ||
| Edit: SelectEdit, | ||
| multiple: true, | ||
| }; | ||
| return config; | ||
| }; |
There was a problem hiding this comment.
JSDoc comment appears to be copy-pasted from selectHandler.
The documentation mentions "special cases like product categories" which doesn't apply to this handler. Consider updating to accurately describe the multi-select functionality.
📝 Suggested JSDoc fix
/**
- * Handler for select/dropdown fields.
- * Handles normal select options and special cases like product categories.
+ * Handler for multi-select dropdown fields.
+ * Returns configuration for selecting multiple values from options.
*
* `@param` {Object} [field] The field configuration.
- * `@return` {Object} Configuration object including type, elements, and Edit component.
+ * `@return` {Object} Configuration object with array type, elements, and multiple selection enabled.
*/📝 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.
| /** | |
| * Handler for select/dropdown fields. | |
| * Handles normal select options and special cases like product categories. | |
| * | |
| * @param {Object} [field] The field configuration. | |
| * @return {Object} Configuration object including type, elements, and Edit component. | |
| */ | |
| export const multiSelectHandler: FieldHandler = ( field ) => { | |
| const config: any = { | |
| type: 'array', | |
| elements: getElementsFromOptions( field ), | |
| Edit: SelectEdit, | |
| multiple: true, | |
| }; | |
| return config; | |
| }; | |
| /** | |
| * Handler for multi-select dropdown fields. | |
| * Returns configuration for selecting multiple values from options. | |
| * | |
| * `@param` {Object} [field] The field configuration. | |
| * `@return` {Object} Configuration object with array type, elements, and multiple selection enabled. | |
| */ | |
| export const multiSelectHandler: FieldHandler = ( field ) => { | |
| const config: any = { | |
| type: 'array', | |
| elements: getElementsFromOptions( field ), | |
| Edit: SelectEdit, | |
| multiple: true, | |
| }; | |
| return config; | |
| }; |
🤖 Prompt for AI Agents
In `@src/dashboard/form-manager/factories/handlers.ts` around lines 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).
| /** | ||
| * 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; - }
| /** | ||
| * Get 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 | ||
| * | ||
| * @since DOKAN_SINCE | ||
| * | ||
| * @return bool | ||
| */ | ||
| public function get_show_in_admin_settings(): bool { | ||
| return wc_string_to_bool( $this->data['show_in_admin_settings'] ); | ||
| } |
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.
| 'sanitize_callback' => function ( $tags ) { | ||
| /** | ||
| * Maximum number of tags a vendor can add. | ||
| * | ||
| * @since 3.3.7 | ||
| * | ||
| * @args int -1 default to unlimited | ||
| */ | ||
| $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 ) ); | ||
| } | ||
|
|
||
| // todo: add support for creating tags | ||
| return array_map( | ||
| function ( $tag ) { | ||
| if ( is_numeric( $tag ) ) { | ||
| return absint( $tag ); | ||
| } else { | ||
| return sanitize_text_field( $tag ); | ||
| } | ||
| }, (array) $tags | ||
| ); | ||
| }, |
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.
| 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 ), | ||
| } } | ||
| /> |
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.
| const DefaultHandler: FieldDataHandler = { | ||
| canHandle: () => true, | ||
| getValue: ( field ) => field.value || '', | ||
| }; |
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 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
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.