Update/dokan core store and api #3053
Conversation
📝 WalkthroughWalkthroughRegisters a new ApiMeta service to expose Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant REST as "WP REST API"
participant ApiMeta as "ApiMeta (dokan_meta handler)"
participant VendorUtil as "VendorUtil"
participant Meta as "User Meta Storage"
Client->>REST: GET /wp/v2/users/{id}
REST->>ApiMeta: resolve `dokan_meta` field (get_callback)
ApiMeta->>VendorUtil: get_vendor_id_for_user(user_id)
VendorUtil->>Meta: read capabilities / `_vendor_id` meta
Meta-->>VendorUtil: vendor_id or 0
VendorUtil-->>ApiMeta: vendor_id
ApiMeta->>ApiMeta: gather other fields via get_user_data_field()
ApiMeta->>ApiMeta: apply `dokan_vendor_api_meta_get_user_data_values` filter
ApiMeta-->>REST: return `dokan_meta` payload
REST-->>Client: user object including `dokan_meta`
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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: 6
🧹 Nitpick comments (3)
src/stores/core/selectors.ts (1)
90-97: Add explicit return type for type safety.The selector should specify its return type for better type safety and developer experience. Based on the optional chaining, the return type should be
number | undefined.🔎 Proposed fix
/** * Get the vendor ID for the current user. * * @param state */ -getVendorId( state: CoreState ) { +getVendorId( state: CoreState ): number | undefined { return state.currentUser?.dokan_meta?.vendor_id; },includes/Vendor/ApiMeta.php (2)
18-30: Update documentation and add REST schema.The docblock references "WooCommerce specific user data" (line 19), but this is Dokan code. Additionally, the
schemaparameter is set tonull(line 27). For REST API best practices, you should define a proper schema describing the structure and types of thedokan_metafield.🔎 Proposed fix
/** - * Registers WooCommerce specific user data to the WordPress user API. + * Registers Dokan-specific user data to the WordPress user API. */ public function register_user_data() { register_rest_field( 'user', 'dokan_meta', array( 'get_callback' => array( $this, 'get_user_data_values' ), - 'schema' => null, + 'schema' => array( + 'description' => 'Dokan vendor meta data', + 'type' => 'object', + 'context' => array( 'view', 'edit' ), + 'properties' => array( + 'vendor_id' => array( + 'description' => 'Vendor/store ID for the user', + 'type' => 'integer', + ), + ), + ), ) ); }
67-80: Fix misleading documentation and add type hints.The docblock states "Migrates old key prefixes as well" (line 70), but the implementation only retrieves user meta without any migration logic. Additionally, the method lacks type hints for better type safety.
🔎 Proposed fix
/** * Helper to retrieve user data fields. * - * Migrates old key prefixes as well. - * * @param int $user_id User ID. * @param string $field Field name. * @return mixed The user field value. */ -public static function get_user_data_field( $user_id, $field ) { +public static function get_user_data_field( int $user_id, string $field ) { $meta_value = get_user_meta( $user_id, $field, true ); return $meta_value; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
includes/DependencyManagement/Providers/CommonServiceProvider.phpincludes/Utilities/VendorUtil.phpincludes/Vendor/ApiMeta.phpsrc/stores/core/resolvers.tssrc/stores/core/selectors.ts
🧰 Additional context used
🧬 Code graph analysis (4)
src/stores/core/selectors.ts (1)
src/definitions/dokan-core.ts (1)
CoreState(3-7)
includes/DependencyManagement/Providers/CommonServiceProvider.php (1)
includes/Vendor/ApiMeta.php (1)
ApiMeta(10-81)
includes/Utilities/VendorUtil.php (1)
includes/functions.php (2)
dokan_get_current_user_id(48-61)dokan_is_user_seller(73-79)
includes/Vendor/ApiMeta.php (1)
includes/Utilities/VendorUtil.php (2)
VendorUtil(4-85)get_vendor_id_for_user(68-84)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: api tests (1, 1)
- GitHub Check: e2e tests (1, 3)
🔇 Additional comments (1)
includes/DependencyManagement/Providers/CommonServiceProvider.php (1)
28-28: LGTM!The ApiMeta service registration follows the existing pattern and is correctly added to the common services list.
| public static function get_vendor_id_for_user( int $user_id = 0 ): int { | ||
| if ( empty( $user_id ) ) { | ||
| $user_id = dokan_get_current_user_id(); | ||
| } | ||
|
|
||
| if ( dokan_is_user_seller( $user_id, true ) ) { | ||
| return (int) $user_id; | ||
| } | ||
|
|
||
| if ( user_can( $user_id, 'vendor_staff' ) ) { | ||
| $vendor_id = (int) get_user_meta( $user_id, '_vendor_id', true ); | ||
|
|
||
| return $vendor_id; | ||
| } | ||
|
|
||
| return 0; | ||
| } |
There was a problem hiding this comment.
Fix logic flaw when resolving vendor ID for current user.
When $user_id is empty (line 69), the code calls dokan_get_current_user_id() (line 70), which already resolves vendor_staff users to their parent vendor ID. Subsequently, lines 77-80 attempt to handle vendor_staff again, but this check becomes unreachable because $user_id now holds the parent vendor ID (not the staff user's ID), and that parent vendor will never have the vendor_staff capability.
This creates inconsistent behavior:
- When
$user_idis explicitly provided, thevendor_stafflogic works correctly. - When
$user_idis empty, thevendor_staffbranch (lines 77-80) is unreachable.
🔎 Recommended fix
Replace dokan_get_current_user_id() with get_current_user_id() to ensure the staff resolution logic executes correctly:
public static function get_vendor_id_for_user( int $user_id = 0 ): int {
if ( empty( $user_id ) ) {
- $user_id = dokan_get_current_user_id();
+ $user_id = get_current_user_id();
}
if ( dokan_is_user_seller( $user_id, true ) ) {
return (int) $user_id;
}
if ( user_can( $user_id, 'vendor_staff' ) ) {
$vendor_id = (int) get_user_meta( $user_id, '_vendor_id', true );
return $vendor_id;
}
return 0;
}🤖 Prompt for AI Agents
In includes/Utilities/VendorUtil.php around lines 68 to 84, the function
incorrectly calls dokan_get_current_user_id() when $user_id is empty, which
returns the parent vendor ID for staff and prevents the subsequent vendor_staff
branch from ever running; replace that call with get_current_user_id() so that
when no $user_id is passed we get the actual current user's ID (allowing the
vendor_staff check to detect staff users and resolve _vendor_id), keep the
existing type casting and return paths intact.
| /** | ||
| * For all the registered user data fields ( Loader::get_user_data_fields ), fetch the data | ||
| * for returning via the REST API. | ||
| * | ||
| * @param WP_User $user Current user. | ||
| */ | ||
| public function get_user_data_values( $user ) { | ||
| $values = [ | ||
| 'vendor_id' => VendorUtil::get_vendor_id_for_user( (int) $user['id'] ), | ||
| ]; | ||
|
|
||
| foreach ( $this->get_user_data_fields() as $field ) { | ||
| $values[ $field ] = self::get_user_data_field( $user['id'], $field ); | ||
| } | ||
|
|
||
| return apply_filters( 'dokan_vendor_api_meta_get_user_data_values', $values, $user ); | ||
| } |
There was a problem hiding this comment.
Fix documentation and add type hints.
Multiple issues with documentation and type safety:
- Line 33: References "Loader::get_user_data_fields" which doesn't exist; should reference
$this->get_user_data_fields(). - Line 36: Parameter type documents
WP_User $user, but the code treats$useras an array (line 40:$user['id']). The actual parameter passed byregister_rest_fieldis an array representation of the user. - Missing return type annotation.
🔎 Proposed fix
/**
- * For all the registered user data fields ( Loader::get_user_data_fields ), fetch the data
+ * For all the registered user data fields, fetch the data
* for returning via the REST API.
*
- * @param WP_User $user Current user.
+ * @param array $user Current user data.
+ *
+ * @return array User meta values.
*/
-public function get_user_data_values( $user ) {
+public function get_user_data_values( array $user ): array {
$values = [
'vendor_id' => VendorUtil::get_vendor_id_for_user( (int) $user['id'] ),
];
foreach ( $this->get_user_data_fields() as $field ) {
$values[ $field ] = self::get_user_data_field( $user['id'], $field );
}
return apply_filters( 'dokan_vendor_api_meta_get_user_data_values', $values, $user );
}🤖 Prompt for AI Agents
In includes/Vendor/ApiMeta.php around lines 32 to 48, the docblock and type
usage are incorrect: update the docblock to reference
$this->get_user_data_fields() instead of Loader::get_user_data_fields, change
the method signature to accept an array (e.g. public function
get_user_data_values(array $user): array) to reflect the REST field payload,
ensure internal uses consistently access $user['id'] (casting to int where
needed, e.g. (int) $user['id'] when calling VendorUtil), and add the return type
declaration (: array) plus update the @param and @return annotations in the
docblock to match.
| * @param state | ||
| */ | ||
| getVendorId( state: CoreState ) { | ||
| return state.currentUser?.dokan_meta?.vendor_id; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for dokan_meta in TypeScript definition files
echo "Searching for dokan_meta type definition..."
rg -n --type=ts -A 3 -B 3 "dokan_meta"Repository: getdokan/dokan
Length of output: 459
🏁 Script executed:
#!/bin/bash
# Search for User type definition and CoreState
echo "=== Searching for User type definition ==="
rg -n --type=ts "interface User|type User" -A 10 | head -50
echo -e "\n=== Searching for CoreState type definition ==="
rg -n --type=ts "interface CoreState|type CoreState" -A 15 | head -100
echo -e "\n=== Searching for type augmentation/declaration files ==="
fd -e "\.d\.ts$" | head -20Repository: getdokan/dokan
Length of output: 451
🏁 Script executed:
#!/bin/bash
# Check the full content of dokan-core.ts
echo "=== Full content of dokan-core.ts ==="
cat -n src/definitions/dokan-core.ts
echo -e "\n=== Search for User type definition ==="
rg -n --type=ts "type User\s*=|interface User\s*" -A 20 | head -100Repository: getdokan/dokan
Length of output: 314
🏁 Script executed:
#!/bin/bash
# Search for type augmentations or declare statements
echo "=== Searching for declare module statements ==="
rg -n --type=ts "declare module" -A 5
echo -e "\n=== Searching for interface User augmentation ==="
rg -n --type=ts "interface User" -A 5
echo -e "\n=== Searching for any User extensions ==="
rg -n --type=ts "extend.*User|User.*extend" -i
echo -e "\n=== Checking for TypeScript config or type roots ==="
cat -n tsconfig.json 2>/dev/null | head -50Repository: getdokan/dokan
Length of output: 6041
🏁 Script executed:
#!/bin/bash
# Check the full context around line 96 in selectors.ts
echo "=== Full context of selectors.ts around line 96 ==="
cat -n src/stores/core/selectors.ts | head -110 | tail -30
echo -e "\n=== Search for ts-ignore or type assertions ==="
rg -n --type=ts "ts-ignore|ts-nocheck|any\s*as|as\s*any" src/stores/core/selectors.ts
echo -e "\n=== Search for all .d.ts files ==="
fd -e "\.d\.ts$" -x cat -n {}
echo -e "\n=== Check if dokan_meta is augmented in @wordpress/core-data ==="
rg -n "wordpress/core-data" --type=ts -A 5 -B 5Repository: getdokan/dokan
Length of output: 2291
🏁 Script executed:
#!/bin/bash
# Find all type definition files
echo "=== Searching for all TypeScript type definitions in src/definitions ==="
fd -e "\.ts$" src/definitions/ -x cat -n {}
echo -e "\n=== Search for dokan_meta in entire codebase ==="
rg -n "dokan_meta" --type=ts
echo -e "\n=== Search for type definitions extending or using User ==="
rg -n "User\s*[&|{]|User\s*&\s*|User\s*\|" --type=ts | head -30Repository: getdokan/dokan
Length of output: 901
🏁 Script executed:
#!/bin/bash
# List all files in src/definitions directory correctly
echo "=== Files in src/definitions ==="
ls -la src/definitions/
echo -e "\n=== Find all .d.ts files in entire project ==="
find . -name "*.d.ts" -type f
echo -e "\n=== Check if there's a types directory with User extensions ==="
ls -la types/ 2>/dev/null || echo "No types directory found"
echo -e "\n=== Search for any module declaration for @wordpress/core-data ==="
rg -n "declare\s+module.*wordpress.*core-data" --type=ts -A 20Repository: getdokan/dokan
Length of output: 1454
🏁 Script executed:
#!/bin/bash
# Check window-types.ts and other relevant type files
echo "=== Content of window-types.ts ==="
cat -n src/definitions/window-types.ts
echo -e "\n=== Content of types/globals.d.ts ==="
cat -n types/globals.d.ts
echo -e "\n=== Check if User type is extended anywhere ==="
rg -n "interface\s+User|type\s+User" --type=ts --type=tsxRepository: getdokan/dokan
Length of output: 2601
🏁 Script executed:
#!/bin/bash
# Check if User type is extended anywhere (just ts)
echo "=== Search for interface/type User ==="
rg -n "interface\s+User|type\s+User" --type=ts
echo -e "\n=== Check dokan-vendor.ts for possible User extensions ==="
cat -n src/definitions/dokan-vendor.tsRepository: getdokan/dokan
Length of output: 4603
Fix TypeScript type error: dokan_meta not defined on User type.
The code accesses dokan_meta?.vendor_id on state.currentUser, but the User type imported from @wordpress/core-data does not include the dokan_meta property. In strict TypeScript mode, this will produce a type error. Define dokan_meta either by extending the User type or creating a type augmentation for @wordpress/core-data:
declare module '@wordpress/core-data' {
interface User {
dokan_meta?: {
vendor_id?: string | number;
[key: string]: unknown;
};
}
}Alternatively, update CoreState to use a custom User type that includes dokan_meta.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/frontend/stores.md (1)
318-361: Consider adding error handling examples for async resolvers.The resolver examples (lines 318-361) show successful async data fetching but lack error handling. Consider adding a code snippet demonstrating how to catch and handle errors when resolvers fail, which will improve developer experience and guide best practices.
For example:
const fetchCurrentUser = async () => { try { await resolveSelect( coreStore ).getCurrentUser(); } catch ( error ) { console.error( 'Failed to fetch user:', error ); } };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/frontend/stores.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (3)
docs/frontend/stores.md (3)
228-289: Excellent comprehensive documentation for the Dokan Core Store.The documentation provides clear, well-organized examples covering selectors, actions, and resolvers. The structure guides developers through common usage patterns and the code examples are practical and easy to follow. The new
getVendorId()documentation aligns well with the existing patterns.
228-289: Verify that thegetVendorId()selector and resolver are correctly implemented in the codebase.The documentation introduces
getVendorId()as a new selector (line 238) and includes usage examples (lines 286-288). Ensure this selector is properly exported and aligns with the implementation insrc/stores/core/selectors.ts. Additionally, verify that the correspondinggetVendorId()resolver is wired correctly insrc/stores/core/resolvers.tsto fetch current user data when needed.#!/bin/bash # Verify getVendorId selector and resolver are properly implemented # Check if getVendorId is exported from selectors rg -A 3 'getVendorId' --type=ts # Verify it's registered in the store index rg -B 5 -A 5 'selectors.*resolvers' --type=ts
272-272: The capability string'dokandar'is a legitimate and standard capability in Dokan. It is the primary vendor capability explicitly registered during plugin installation inincludes/Install/Installer.phpand is used consistently throughout the codebase. The example in the documentation is correct and appropriate.Likely an incorrect or invalid review comment.
mrabbani
force-pushed
the
update/dokan-core-store-and-api
branch
from
50058d7 to
c946c6d
Compare
Purpose of This PRThis PR enhances the Dokan Core Store by adding vendor ID support and improving the overall developer experience with comprehensive documentation updates. Key Changes🔧 Backend Changes (PHP)
🎨 Frontend Changes (TypeScript)
📚 Documentation
Benefits
Technical DetailsThe vendor ID is now accessible via:
The implementation handles edge cases:
All changes follow existing code patterns and maintain backward compatibility. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/stores/core/selectors.ts (1)
91-98: Address the TypeScript type error fordokan_metaproperty.The
Usertype from@wordpress/core-datadoes not include adokan_metaproperty, causing a type error when accessingstate.currentUser?.dokan_meta?.vendor_id. This issue was previously flagged with a solution to extend the type via module augmentation.
🧹 Nitpick comments (1)
src/stores/core/selectors.ts (1)
97-97: Remove unnecessary optional chaining oncurrentUser.The
CoreStatetype definescurrentUseras a non-optionalUserproperty, making the?.operator unnecessary. Use.instead for direct property access.🔎 Proposed fix
- return state.currentUser?.dokan_meta?.vendor_id; + return state.currentUser.dokan_meta?.vendor_id;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
includes/Vendor/ApiMeta.phpsrc/stores/core/selectors.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- includes/Vendor/ApiMeta.php
🧰 Additional context used
🧬 Code graph analysis (1)
src/stores/core/selectors.ts (1)
src/definitions/dokan-core.ts (1)
CoreState(3-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: api tests (1, 1)
- GitHub Check: e2e tests (1, 3)
| getVendorId( state: CoreState ) { | ||
| return state.currentUser?.dokan_meta?.vendor_id; | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add explicit return type annotation and complete the JSDoc.
The selector lacks an explicit return type and the JSDoc is missing the @return tag. Specify the return type based on what vendor_id can be (likely number | undefined or string | number | undefined).
🔎 Proposed fix
/**
* Get the vendor ID for the current user.
*
* @param state
+ *
+ * @return { number | undefined } The vendor ID or undefined if not available.
*/
- getVendorId( state: CoreState ) {
+ getVendorId( state: CoreState ): number | undefined {
return state.currentUser?.dokan_meta?.vendor_id;
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| getVendorId( state: CoreState ) { | |
| return state.currentUser?.dokan_meta?.vendor_id; | |
| }, | |
| /** | |
| * Get the vendor ID for the current user. | |
| * | |
| * @param state | |
| * | |
| * @return { number | undefined } The vendor ID or undefined if not available. | |
| */ | |
| getVendorId( state: CoreState ): number | undefined { | |
| return state.currentUser?.dokan_meta?.vendor_id; | |
| }, |
🤖 Prompt for AI Agents
In src/stores/core/selectors.ts around lines 96 to 98, the getVendorId selector
is missing an explicit return type and its JSDoc lacks a @return tag; update the
function signature to include an explicit return type (e.g., number | string |
undefined or number | undefined depending on how vendor_id is stored) and expand
the JSDoc to include a @return describing the possible return values and when
undefined is returned, keeping the implementation unchanged.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
includes/Utilities/VendorUtil.php (1)
72-88: This issue was previously identified and remains unresolved.Using
dokan_get_current_user_id()on line 74 creates inconsistent behavior. When$user_idis empty and the current user is vendor staff,dokan_get_current_user_id()already resolves to the parent vendor ID (seeincludes/functions.php:47-60). This makes thevendor_staffcapability check on lines 81-85 unreachable for the current user scenario, while it remains functional when an explicit$user_idis provided.Replace
dokan_get_current_user_id()withget_current_user_id()to ensure consistent behavior across both code paths.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
includes/Utilities/VendorUtil.phpincludes/Vendor/ApiMeta.php
🚧 Files skipped from review as they are similar to previous changes (1)
- includes/Vendor/ApiMeta.php
🧰 Additional context used
🧬 Code graph analysis (1)
includes/Utilities/VendorUtil.php (1)
includes/functions.php (2)
dokan_get_current_user_id(48-61)dokan_is_user_seller(73-79)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (1)
includes/Utilities/VendorUtil.php (1)
11-11: LGTM! Docblock correction is accurate.The return type documentation now correctly reflects that the method returns a
string.
| * - Vendor staff: Returns their parent vendor's ID (stored in user meta) | ||
| * - Other users: Returns 0 if not associated with any vendor | ||
| * | ||
| * @since DOKAN_SINCE |
There was a problem hiding this comment.
Replace the version placeholder before release.
The @since DOKAN_SINCE placeholder should be updated with the actual version number (e.g., @since 4.1.0) before this code is released.
🤖 Prompt for AI Agents
In includes/Utilities/VendorUtil.php around line 65, the docblock uses the
placeholder "@since DOKAN_SINCE" which must be replaced with the real release
version; update the annotation to the actual version string (for example "@since
4.1.0" or whichever release applies) so the docblock reflects the correct
version before release.
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
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.