-
Notifications
You must be signed in to change notification settings - Fork 204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: missing args for several endpoints in the Stores API endpoints #2442
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughThe Changes
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
includes/REST/StoreController.php
(1 hunks)
🧰 Additional context used
🪛 GitHub Check: Run PHPCS inspection
includes/REST/StoreController.php
[failure] 138-138:
Expected 1 spaces after opening parenthesis; 0 found
[failure] 138-138:
Expected 1 spaces before closing parenthesis; 0 found
[failure] 143-143:
Expected 1 spaces after opening parenthesis; 0 found
[failure] 143-143:
Expected 1 spaces before closing parenthesis; 0 found
[failure] 148-148:
Expected 1 spaces after opening parenthesis; 0 found
[failure] 148-148:
Expected 1 spaces before closing parenthesis; 0 found
🔇 Additional comments (1)
includes/REST/StoreController.php (1)
134-151
: New arguments for 'check_store_availability' endpoint are properly defined.
The optional parameters store_slug
, username
, and email
enhance the endpoint's functionality and are correctly implemented in the route registration.
🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[failure] 138-138:
Expected 1 spaces after opening parenthesis; 0 found
[failure] 138-138:
Expected 1 spaces before closing parenthesis; 0 found
[failure] 143-143:
Expected 1 spaces after opening parenthesis; 0 found
[failure] 143-143:
Expected 1 spaces before closing parenthesis; 0 found
[failure] 148-148:
Expected 1 spaces after opening parenthesis; 0 found
[failure] 148-148:
Expected 1 spaces before closing parenthesis; 0 found
includes/REST/StoreController.php
Outdated
'store_slug' => [ | ||
'required' => false, | ||
'type' => 'string', | ||
'description' => __('Slug of the store to check availability.', 'dokan-lite'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix spacing in translation function calls to comply with coding standards.
The __()
function calls should have a space after the opening parenthesis and before the closing parenthesis, as per WordPress coding standards.
Apply this diff to correct the spacing:
- 'description' => __('Slug of the store to check availability.', 'dokan-lite'),
+ 'description' => __( 'Slug of the store to check availability.', 'dokan-lite' ),
- 'description' => __('Username to check availability.', 'dokan-lite'),
+ 'description' => __( 'Username to check availability.', 'dokan-lite' ),
- 'description' => __('Email address to check availability.', 'dokan-lite'),
+ 'description' => __( 'Email address to check availability.', 'dokan-lite' ),
Also applies to: 143-143, 148-148
🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[failure] 138-138:
Expected 1 spaces after opening parenthesis; 0 found
[failure] 138-138:
Expected 1 spaces before closing parenthesis; 0 found
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (2)
includes/REST/StoreController.php (2)
97-101
: Add validation for location coordinatesThe location field should validate geographical coordinates if they are being used.
Add coordinate validation:
'location' => [ 'required' => false, 'type' => 'string', 'description' => __( 'Geographical location of the store.', 'dokan-lite' ), + 'pattern' => '^[-+]?([1-8]?\d(\.\d+)?|90(\.0+)?),\s*[-+]?(180(\.0+)?|((1[0-7]\d)|([1-9]?\d))(\.\d+)?)$' ],
228-243
: Fix inconsistent parameter spacingThe parameter definitions have inconsistent spacing around the array keys and values.
Apply consistent spacing:
'store_slug' => [ - 'required' => false, - 'type' => 'string', + 'required' => false, + 'type' => 'string', 'description' => __( 'Slug of the store to check availability.', 'dokan-lite' ), ], 'username' => [ - 'required' => false, - 'type' => 'string', + 'required' => false, + 'type' => 'string', 'description' => __( 'Username to check availability.', 'dokan-lite' ), ], 'email' => [ - 'required' => false, - 'type' => 'string', + 'required' => false, + 'type' => 'string', 'description' => __( 'Email address to check availability.', 'dokan-lite' ), 'format' => 'email', ],
includes/REST/StoreController.php
Outdated
'show_email' => [ | ||
'required' => false, | ||
'type' => 'string', // Updated from boolean | ||
'description' => __( 'Whether to show the store email publicly.', 'dokan-lite' ), | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix type inconsistency for boolean fields
The following fields are marked as string type but should be boolean since they represent true/false values:
- show_email
- enable_tnc
- show_min_order_discount
Apply this diff to fix the type declarations:
- 'type' => 'string', // Updated from boolean
+ 'type' => 'boolean',
'description' => __( 'Whether to show the store email publicly.', 'dokan-lite' ),
- 'type' => 'string', // Updated from boolean
+ 'type' => 'boolean',
'description' => __( 'Enable Terms and Conditions for the store.', 'dokan-lite' ),
- 'type' => 'string', // Updated from boolean
+ 'type' => 'boolean',
'description' => __( 'Whether to show minimum order discount information.', 'dokan-lite' ),
Also applies to: 117-121, 127-131
'social' => [ | ||
'required' => false, | ||
'type' => 'array', | ||
'description' => __( 'An array of social media details for the store.', 'dokan-lite' ), | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add validation for social and payment array structures
The social and payment arrays lack structure validation. Consider adding properties schema to ensure data consistency.
Add validation by extending the schema:
'social' => [
'required' => false,
'type' => 'array',
'description' => __( 'An array of social media details for the store.', 'dokan-lite' ),
+ 'items' => [
+ 'type' => 'object',
+ 'properties' => [
+ 'network' => [
+ 'type' => 'string',
+ 'description' => __( 'Social network name', 'dokan-lite' ),
+ 'enum' => ['facebook', 'twitter', 'instagram', 'youtube']
+ ],
+ 'url' => [
+ 'type' => 'string',
+ 'format' => 'uri',
+ 'description' => __( 'Social profile URL', 'dokan-lite' )
+ ]
+ ]
+ ]
],
'payment' => [
'required' => false,
'type' => 'array',
'description' => __( 'Payment details for the store. E.g., PayPal or bank details.', 'dokan-lite' ),
+ 'items' => [
+ 'type' => 'object',
+ 'properties' => [
+ 'method' => [
+ 'type' => 'string',
+ 'description' => __( 'Payment method', 'dokan-lite' ),
+ 'enum' => ['paypal', 'bank']
+ ],
+ 'details' => [
+ 'type' => 'object',
+ 'description' => __( 'Payment method details', 'dokan-lite' )
+ ]
+ ]
+ ]
],
Also applies to: 77-81
'phone' => [ | ||
'required' => false, | ||
'type' => 'string', | ||
'description' => __( 'The contact phone number for the store.', 'dokan-lite' ), | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add phone number format validation
The phone field should include format validation to ensure valid phone numbers are provided.
Add format validation:
'phone' => [
'required' => false,
'type' => 'string',
'description' => __( 'The contact phone number for the store.', 'dokan-lite' ),
+ 'pattern' => '^\+?[1-9]\d{1,14}$',
+ 'minLength' => 10,
+ 'maxLength' => 15
],
📝 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.
'phone' => [ | |
'required' => false, | |
'type' => 'string', | |
'description' => __( 'The contact phone number for the store.', 'dokan-lite' ), | |
], | |
'phone' => [ | |
'required' => false, | |
'type' => 'string', | |
'description' => __( 'The contact phone number for the store.', 'dokan-lite' ), | |
'pattern' => '^\+?[1-9]\d{1,14}$', | |
'minLength' => 10, | |
'maxLength' => 15 | |
], |
'store_slug' => [ | ||
'required' => false, | ||
'type' => 'string', | ||
'description' => __( 'Slug of the store to check availability.', 'dokan-lite' ), | ||
], | ||
'username' => [ | ||
'required' => false, | ||
'type' => 'string', | ||
'description' => __( 'Username to check availability.', 'dokan-lite' ), | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add pattern validation for store_slug and username
The store_slug and username fields should include pattern validation to ensure they meet WordPress naming conventions.
Add pattern validation:
'store_slug' => [
'required' => false,
'type' => 'string',
'description' => __( 'Slug of the store to check availability.', 'dokan-lite' ),
+ 'pattern' => '^[a-zA-Z0-9-_]+$',
+ 'minLength' => 3,
+ 'maxLength' => 50
],
'username' => [
'required' => false,
'type' => 'string',
'description' => __( 'Username to check availability.', 'dokan-lite' ),
+ 'pattern' => '^[a-zA-Z0-9-_]+$',
+ 'minLength' => 3,
+ 'maxLength' => 50
],
📝 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.
'store_slug' => [ | |
'required' => false, | |
'type' => 'string', | |
'description' => __( 'Slug of the store to check availability.', 'dokan-lite' ), | |
], | |
'username' => [ | |
'required' => false, | |
'type' => 'string', | |
'description' => __( 'Username to check availability.', 'dokan-lite' ), | |
], | |
'store_slug' => [ | |
'required' => false, | |
'type' => 'string', | |
'description' => __( 'Slug of the store to check availability.', 'dokan-lite' ), | |
'pattern' => '^[a-zA-Z0-9-_]+$', | |
'minLength' => 3, | |
'maxLength' => 50 | |
], | |
'username' => [ | |
'required' => false, | |
'type' => 'string', | |
'description' => __( 'Username to check availability.', 'dokan-lite' ), | |
'pattern' => '^[a-zA-Z0-9-_]+$', | |
'minLength' => 3, | |
'maxLength' => 50 | |
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
includes/REST/StoreController.php (1)
97-101
: Add format validation for location fieldThe location field should validate geographical coordinates if they're being used.
'location' => [ 'required' => false, 'type' => 'string', 'description' => __( 'Geographical location of the store.', 'dokan-lite' ), + 'pattern' => '^-?\d+\.?\d*,\s*-?\d+\.?\d*$', + 'description' => __( 'Geographical location of the store in latitude,longitude format.', 'dokan-lite' ), ],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
includes/REST/StoreController.php
(3 hunks)
🔇 Additional comments (1)
includes/REST/StoreController.php (1)
Line range hint 55-336
: Changes successfully address the PR objectives
The implementation properly adds missing arguments to the REST API schema for multiple endpoints while maintaining WordPress REST API standards. The schema definitions are comprehensive and include proper validation.
'store_name' => [ | ||
'required' => true, | ||
'type' => 'string', | ||
'description' => __( 'The name of the store.', 'dokan-lite' ), | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add validation constraints for store_name field
The store_name field should include length and character constraints to ensure valid store names.
'store_name' => [
'required' => true,
'type' => 'string',
'description' => __( 'The name of the store.', 'dokan-lite' ),
+ 'minLength' => 1,
+ 'maxLength' => 100,
+ 'pattern' => '^[a-zA-Z0-9\s\-\_\.]+$'
],
📝 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.
'store_name' => [ | |
'required' => true, | |
'type' => 'string', | |
'description' => __( 'The name of the store.', 'dokan-lite' ), | |
], | |
'store_name' => [ | |
'required' => true, | |
'type' => 'string', | |
'description' => __( 'The name of the store.', 'dokan-lite' ), | |
'minLength' => 1, | |
'maxLength' => 100, | |
'pattern' => '^[a-zA-Z0-9\s\-\_\.]+$' | |
], |
'args' => [ | ||
'approved' => [ | ||
'type' => 'array', | ||
'required' => false, | ||
'items' => [ | ||
'type' => 'integer', | ||
], | ||
'description' => __( 'List of vendor IDs to approve.', 'dokan-lite' ), | ||
], | ||
'pending' => [ | ||
'type' => 'array', | ||
'required' => false, | ||
'items' => [ | ||
'type' => 'integer', | ||
], | ||
'description' => __( 'List of vendor IDs to set as pending.', 'dokan-lite' ), | ||
], | ||
'delete' => [ | ||
'type' => 'array', | ||
'required' => false, | ||
'items' => [ | ||
'type' => 'integer', | ||
], | ||
'description' => __( 'List of vendor IDs to delete.', 'dokan-lite' ), | ||
], | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add size limits for batch operations
The batch operation arrays should have size limits to prevent performance issues with large batches.
'args' => [
'approved' => [
'type' => 'array',
'required' => false,
'items' => [
'type' => 'integer',
],
'description' => __( 'List of vendor IDs to approve.', 'dokan-lite' ),
+ 'maxItems' => 100,
],
'pending' => [
'type' => 'array',
'required' => false,
'items' => [
'type' => 'integer',
],
'description' => __( 'List of vendor IDs to set as pending.', 'dokan-lite' ),
+ 'maxItems' => 100,
],
'delete' => [
'type' => 'array',
'required' => false,
'items' => [
'type' => 'integer',
],
'description' => __( 'List of vendor IDs to delete.', 'dokan-lite' ),
+ 'maxItems' => 100,
],
],
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
'args' => [ | |
'approved' => [ | |
'type' => 'array', | |
'required' => false, | |
'items' => [ | |
'type' => 'integer', | |
], | |
'description' => __( 'List of vendor IDs to approve.', 'dokan-lite' ), | |
], | |
'pending' => [ | |
'type' => 'array', | |
'required' => false, | |
'items' => [ | |
'type' => 'integer', | |
], | |
'description' => __( 'List of vendor IDs to set as pending.', 'dokan-lite' ), | |
], | |
'delete' => [ | |
'type' => 'array', | |
'required' => false, | |
'items' => [ | |
'type' => 'integer', | |
], | |
'description' => __( 'List of vendor IDs to delete.', 'dokan-lite' ), | |
], | |
], | |
'args' => [ | |
'approved' => [ | |
'type' => 'array', | |
'required' => false, | |
'items' => [ | |
'type' => 'integer', | |
], | |
'description' => __( 'List of vendor IDs to approve.', 'dokan-lite' ), | |
'maxItems' => 100, | |
], | |
'pending' => [ | |
'type' => 'array', | |
'required' => false, | |
'items' => [ | |
'type' => 'integer', | |
], | |
'description' => __( 'List of vendor IDs to set as pending.', 'dokan-lite' ), | |
'maxItems' => 100, | |
], | |
'delete' => [ | |
'type' => 'array', | |
'required' => false, | |
'items' => [ | |
'type' => 'integer', | |
], | |
'description' => __( 'List of vendor IDs to delete.', 'dokan-lite' ), | |
'maxItems' => 100, | |
], | |
], |
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
Missing args for several endpoints in the REST API schema
Fixes missing arguments in the REST API schema for multiple endpoints, enhancing the schema definition and improving data validation.
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
check_store_availability
endpoint with optional parameters for store slugs, usernames, and email checks.create_store
endpoint to accept additional parameters for improved store management, including user login, email, and store details.Bug Fixes
check_store_availability
method, ensuring accurate responses for username and email availability.