fix: vendor vacation, biography dokan-pro support
#91
Conversation
WalkthroughThe changes in this pull request introduce new methods and hooks to the Changes
Possibly related PRs
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
dokan-wpml.php(2 hunks)
🔇 Additional comments (1)
dokan-wpml.php (1)
172-178: LGTM: Hook declarations follow WordPress conventions.
The new action hooks and filters for vendor vacation messages and biography translations are properly integrated into the existing architecture.
dokan-wpml.php
Outdated
|
|
||
| /** | ||
| * Translate Vendor Biography Text | ||
| * | ||
| * @param $text | ||
| * @param $name | ||
| * | ||
| * @return void | ||
| */ | ||
| public function dokan_vendor_biography_updated($text, $name) { | ||
| $this->register_single_string( | ||
| 'dokan', | ||
| 'Vendor Biography Text: ' . $name, | ||
| $text | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Translated Vendor Biography Text | ||
| * | ||
| * @param string $text | ||
| * @param $name | ||
| * | ||
| * @return string | ||
| */ | ||
| public function get_translated_dokan_vendor_biography_text(string $text , $name) { | ||
| return $this->get_translated_single_string( $text, 'dokan', 'Vendor Biography Text: '.$name ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor biography methods to reduce code duplication.
The vendor biography methods have similar issues as the vacation message methods and share duplicate code patterns.
- First, apply similar improvements as suggested for vacation messages:
/**
* Translate Vendor Biography Text
*
- * @param $text
- * @param $name
+ * @param string $text The biography text to translate
+ * @param string $name The vendor name or identifier
*
* @return void
*/
- public function dokan_vendor_biography_updated($text, $name) {
+ public function dokan_vendor_biography_updated( string $text, string $name ) {
+ if ( empty( $text ) || empty( $name ) ) {
+ return;
+ }
+
$this->register_single_string(
'dokan',
'Vendor Biography Text: ' . $name,
$text
);
}
/**
* Translated Vendor Biography Text
*
* @param string $text
- * @param $name
+ * @param string $name The vendor name or identifier
*
* @return string
*/
- public function get_translated_dokan_vendor_biography_text(string $text , $name) {
- return $this->get_translated_single_string( $text, 'dokan', 'Vendor Biography Text: '.$name );
+ public function get_translated_dokan_vendor_biography_text( string $text, string $name ) {
+ if ( empty( $text ) || empty( $name ) ) {
+ return $text;
+ }
+ return $this->get_translated_single_string( $text, 'dokan', 'Vendor Biography Text: ' . $name );
}- Consider extracting common functionality into private helper methods:
private function register_vendor_text( string $text, string $name, string $type ) {
if ( empty( $text ) || empty( $name ) ) {
return;
}
$this->register_single_string(
'dokan',
"Vendor {$type}: " . $name,
$text
);
}
private function get_translated_vendor_text( string $text, string $name, string $type ) {
if ( empty( $text ) || empty( $name ) ) {
return $text;
}
return $this->get_translated_single_string( $text, 'dokan', "Vendor {$type}: " . $name );
}Then update the public methods to use these helpers:
public function dokan_vendor_biography_updated( string $text, string $name ) {
$this->register_vendor_text( $text, $name, 'Biography Text' );
}
public function get_translated_dokan_vendor_biography_text( string $text, string $name ) {
return $this->get_translated_vendor_text( $text, $name, 'Biography Text' );
}|
|
||
| /** | ||
| * Translate Vendor Vacation Message | ||
| * | ||
| * @param $text | ||
| * @param $name | ||
| * | ||
| * @return void | ||
| */ | ||
| public function dokan_vendor_vacation_message_updated($text, $name) { | ||
| $this->register_single_string( | ||
| 'dokan', | ||
| 'Vendor Vacation Message: ' . $name, | ||
| $text | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Translated Vendor Vacation Message | ||
| * | ||
| * @param string $text | ||
| * @param $name | ||
| * | ||
| * @return string | ||
| */ | ||
| public function get_translated_dokan_vendor_vacation_message(string $text , $name) { | ||
| return $this->get_translated_single_string( $text, 'dokan', 'Vendor Vacation Message: '.$name ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve method documentation and input validation.
The vendor vacation message methods need the following improvements:
- Add complete PHPDoc parameter type hints and descriptions
- Add input validation for parameters
- Fix inconsistent spacing in string concatenation
Apply this diff to improve the implementation:
/**
* Translate Vendor Vacation Message
*
- * @param $text
- * @param $name
+ * @param string $text The vacation message text to translate
+ * @param string $name The vendor name or identifier
*
* @return void
*/
- public function dokan_vendor_vacation_message_updated($text, $name) {
+ public function dokan_vendor_vacation_message_updated( string $text, string $name ) {
+ if ( empty( $text ) || empty( $name ) ) {
+ return;
+ }
+
$this->register_single_string(
'dokan',
'Vendor Vacation Message: ' . $name,
$text
);
}
/**
* Translated Vendor Vacation Message
*
* @param string $text
- * @param $name
+ * @param string $name The vendor name or identifier
*
* @return string
*/
- public function get_translated_dokan_vendor_vacation_message(string $text , $name) {
- return $this->get_translated_single_string( $text, 'dokan', 'Vendor Vacation Message: '.$name );
+ public function get_translated_dokan_vendor_vacation_message( string $text, string $name ) {
+ if ( empty( $text ) || empty( $name ) ) {
+ return $text;
+ }
+ return $this->get_translated_single_string( $text, 'dokan', 'Vendor Vacation Message: ' . $name );
}📝 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.
| /** | |
| * Translate Vendor Vacation Message | |
| * | |
| * @param $text | |
| * @param $name | |
| * | |
| * @return void | |
| */ | |
| public function dokan_vendor_vacation_message_updated($text, $name) { | |
| $this->register_single_string( | |
| 'dokan', | |
| 'Vendor Vacation Message: ' . $name, | |
| $text | |
| ); | |
| } | |
| /** | |
| * Translated Vendor Vacation Message | |
| * | |
| * @param string $text | |
| * @param $name | |
| * | |
| * @return string | |
| */ | |
| public function get_translated_dokan_vendor_vacation_message(string $text , $name) { | |
| return $this->get_translated_single_string( $text, 'dokan', 'Vendor Vacation Message: '.$name ); | |
| } | |
| /** | |
| * Translate Vendor Vacation Message | |
| * | |
| * @param string $text The vacation message text to translate | |
| * @param string $name The vendor name or identifier | |
| * | |
| * @return void | |
| */ | |
| public function dokan_vendor_vacation_message_updated( string $text, string $name ) { | |
| if ( empty( $text ) || empty( $name ) ) { | |
| return; | |
| } | |
| $this->register_single_string( | |
| 'dokan', | |
| 'Vendor Vacation Message: ' . $name, | |
| $text | |
| ); | |
| } | |
| /** | |
| * Translated Vendor Vacation Message | |
| * | |
| * @param string $text | |
| * @param string $name The vendor name or identifier | |
| * | |
| * @return string | |
| */ | |
| public function get_translated_dokan_vendor_vacation_message( string $text, string $name ) { | |
| if ( empty( $text ) || empty( $name ) ) { | |
| return $text; | |
| } | |
| return $this->get_translated_single_string( $text, 'dokan', 'Vendor Vacation Message: ' . $name ); | |
| } |
kzamanbd
requested review from
mrabbani
and removed request for
MdAsifHossainNadim
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
dokan-wpml.php(5 hunks)
🔇 Additional comments (8)
dokan-wpml.php (8)
130-131: No issues found with these filter hooks.
They match the corresponding callback signatures, and the usage appears consistent with the defined methods.
132-132: Looks good.
Hooking 'dokan_vendor_subscription_package_id' to get_product_id_in_base_language is consistent with the method's single-argument signature.
144-145: These filter hooks are well-defined.
Their callback methods accept the correct number of parameters, and no errors are apparent.
1327-1341: Add input validation and type hints.
This method lacks parameter checks and does not guard against empty or invalid values. Additionally, the docblock could specify types (string). This was previously recommended.
1343-1353: Enforce input validation and clarify docblocks.
Just like the vacation message method, please verify $text and $name before usage, and ensure the docblock uses typed parameters.
1356-1370: Check array keys and reduce duplication.
$store_info might not always have vendor_biography. Consider adding a guard and reusing a helper, as previously suggested in older reviews.
1372-1382: Refactor to reduce duplication across biography and vacation methods.
Both sets of methods mirror each other. Centralize shared logic and add input validation.
160-161: Confirm that each action passes the expected method ID.
While register_vendor_verification_method accepts an int $method_id, ensure that both 'dokan_pro_vendor_verification_method_created' and 'dokan_pro_vendor_verification_method_updated' indeed pass that parameter in the first position.
| add_action( 'dokan_vendor_biography_after_update', [ $this, 'dokan_vendor_biography_updated' ], 10, 3 ); | ||
| add_filter( 'dokan_get_vendor_biography_text', [ $this, 'get_translated_dokan_vendor_biography_text' ], 10, 2 ); |
There was a problem hiding this comment.
Mismatch in action parameters.
Similarly, 'dokan_vendor_biography_after_update' is hooked with 10, 3 arguments, but dokan_vendor_biography_updated($store_info, $name) only accepts 2. Adjust the hook or function signature to avoid passing extra parameters.
-add_action( 'dokan_vendor_biography_after_update', [ $this, 'dokan_vendor_biography_updated' ], 10, 3 );
+add_action( 'dokan_vendor_biography_after_update', [ $this, 'dokan_vendor_biography_updated' ], 10, 2 );📝 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.
| add_action( 'dokan_vendor_biography_after_update', [ $this, 'dokan_vendor_biography_updated' ], 10, 3 ); | |
| add_filter( 'dokan_get_vendor_biography_text', [ $this, 'get_translated_dokan_vendor_biography_text' ], 10, 2 ); | |
| add_action( 'dokan_vendor_biography_after_update', [ $this, 'dokan_vendor_biography_updated' ], 10, 2 ); | |
| add_filter( 'dokan_get_vendor_biography_text', [ $this, 'get_translated_dokan_vendor_biography_text' ], 10, 2 ); |
| add_action( 'dokan_vendor_vacation_message_updated', [ $this, 'dokan_vendor_vacation_message_updated' ], 10, 3 ); | ||
| add_action( 'dokan_vendor_vacation_message_schedule_after_update', [ $this, 'dokan_vendor_vacation_message_updated' ], 10, 3 ); | ||
| add_filter( 'dokan_get_vendor_vacation_message', [ $this, 'get_translated_dokan_vendor_vacation_message' ], 10, 2 ); |
There was a problem hiding this comment.
Mismatch in action parameters.
The action 'dokan_vendor_vacation_message_updated' is hooked with 10, 3 arguments, but dokan_vendor_vacation_message_updated($text, $name) only accepts 2 parameters. This may cause inconsistencies or notices in production.
-add_action( 'dokan_vendor_vacation_message_updated', [ $this, 'dokan_vendor_vacation_message_updated' ], 10, 3 );
-add_action( 'dokan_vendor_vacation_message_schedule_after_update', [ $this, 'dokan_vendor_vacation_message_updated' ], 10, 3 );
+add_action( 'dokan_vendor_vacation_message_updated', [ $this, 'dokan_vendor_vacation_message_updated' ], 10, 2 );
+add_action( 'dokan_vendor_vacation_message_schedule_after_update', [ $this, 'dokan_vendor_vacation_message_updated' ], 10, 2 );📝 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.
| add_action( 'dokan_vendor_vacation_message_updated', [ $this, 'dokan_vendor_vacation_message_updated' ], 10, 3 ); | |
| add_action( 'dokan_vendor_vacation_message_schedule_after_update', [ $this, 'dokan_vendor_vacation_message_updated' ], 10, 3 ); | |
| add_filter( 'dokan_get_vendor_vacation_message', [ $this, 'get_translated_dokan_vendor_vacation_message' ], 10, 2 ); | |
| add_action( 'dokan_vendor_vacation_message_updated', [ $this, 'dokan_vendor_vacation_message_updated' ], 10, 2 ); | |
| add_action( 'dokan_vendor_vacation_message_schedule_after_update', [ $this, 'dokan_vendor_vacation_message_updated' ], 10, 2 ); | |
| add_filter( 'dokan_get_vendor_vacation_message', [ $this, 'get_translated_dokan_vendor_vacation_message' ], 10, 2 ); |
Related PR: Link
Summary by CodeRabbit
Summary by CodeRabbit