-
Notifications
You must be signed in to change notification settings - Fork 11
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/Few string does not translate regarding the Dokan RFQ module. #220 #88
base: develop
Are you sure you want to change the base?
FIx/Few string does not translate regarding the Dokan RFQ module. #220 #88
Conversation
WalkthroughThe changes in this pull request involve enhancements 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 (
|
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: 0
🧹 Outside diff range and nitpick comments (1)
dokan-wpml.php (1)
1340-1357
: Consider adding type hints for the args parameter.The method implementation is correct, but could benefit from more specific type hints for better code maintainability and IDE support.
- public function dokan_request_quote_button_text_registration(bool $action, array $args, int $rule_id) + /** + * @param bool $action + * @param array{button_text: string} $args + * @param int $rule_id + */ + public function dokan_request_quote_button_text_registration(bool $action, array $args, int $rule_id): void
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
dokan-wpml.php
(2 hunks)
🔇 Additional comments (3)
dokan-wpml.php (3)
169-171
: LGTM: Action hooks are properly implemented.
The hooks for quote rule creation and update are correctly implemented and follow WordPress conventions.
173-175
: LGTM: Filter is properly implemented.
The filter for request quote button text translation is correctly implemented and consistently placed with other translation filters.
1327-1339
: LGTM: Translation getter method is well implemented.
The method properly handles translation retrieval using the existing infrastructure and includes appropriate type hints.
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
🧹 Outside diff range and nitpick comments (2)
dokan-wpml.php (2)
1329-1341
: Clarify the type of the$rule
parameter in the docblockTo enhance code understanding and static analysis, specify the exact class or interface of the
$rule
parameter in the docblock forget_translated_dokan_request_quote_button_text
.Apply this diff to update the docblock:
/** * Get the translated text for the Dokan request quote button. * * @param string $text The original button text. - * @param object $rule The rule object containing the ID. + * @param \Dokan_Rule $rule The rule object containing the ID. * * @return string The translated button text. */Replace
\Dokan_Rule
with the actual class name of the$rule
object.
1343-1346
: Add a docblock forget_translated_dokan_request_quote_price_hide
methodThe method
get_translated_dokan_request_quote_price_hide
lacks a docblock. Adding one will improve code readability and maintainability.Consider adding the following docblock:
/** * Get the translated text for hiding the price in the Dokan request quote. * * @param string $text The original hide price text. * @param \Dokan_Rule $rule The rule object containing the ID. * * @return string The translated hide price text. */Ensure that
\Dokan_Rule
is replaced with the correct class name of the$rule
object.
📜 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)
169-178
: Verify that action hook parameters match the method signature
The method dokan_request_quote_button_text_registration
is hooked to dokan_quote_rule_created
and dokan_quote_rule_updated
with 3 parameters. Ensure that these actions pass parameters matching the method's signature (bool $action, array $args, int $rule_id)
to prevent potential errors.
Run the following script to verify the action hook parameters:
dokan-wpml.php
Outdated
/** | ||
* Registers the request quote button text for translation. | ||
* | ||
* @param bool $action The action status. | ||
* @param array $args The arguments containing the button text. | ||
* @param int $rule_id The ID of the rule. | ||
* | ||
* @return void | ||
*/ | ||
public function dokan_request_quote_button_text_registration(bool $action, array $args, int $rule_id) | ||
{ | ||
if (! $action) { | ||
return; | ||
} | ||
$button_text = $args['button_text']; | ||
$this->register_single_string( 'dokan', 'Dokan Request Quote Button Text: ' . $rule_id, $button_text ); | ||
|
||
$hide_price_text = $args['hide_price_text']; | ||
|
||
$this->register_single_string( 'dokan', 'Dokan Request Quote Hide Price Text: ' . $rule_id, $hide_price_text ); | ||
|
||
} |
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.
Prevent undefined index errors by checking array keys
In the method dokan_request_quote_button_text_registration
, the keys 'button_text'
and 'hide_price_text'
are accessed directly from $args
without checking if they exist. This could lead to undefined index notices if these keys are missing.
Apply this diff to add checks for the array keys:
public function dokan_request_quote_button_text_registration(bool $action, array $args, int $rule_id)
{
if (! $action) {
return;
}
- $button_text = $args['button_text'];
+ $button_text = isset( $args['button_text'] ) ? $args['button_text'] : '';
$this->register_single_string( 'dokan', 'Dokan Request Quote Button Text: ' . $rule_id, $button_text );
- $hide_price_text = $args['hide_price_text'];
+ $hide_price_text = isset( $args['hide_price_text'] ) ? $args['hide_price_text'] : '';
$this->register_single_string( 'dokan', 'Dokan Request Quote Hide Price Text: ' . $rule_id, $hide_price_text );
}
Alternatively, if you're using PHP 7 or higher, you can use the null coalescing operator:
- $button_text = $args['button_text'];
+ $button_text = $args['button_text'] ?? '';
...
- $hide_price_text = $args['hide_price_text'];
+ $hide_price_text = $args['hide_price_text'] ?? '';
Ensure that using a default value aligns with the desired behavior of your application.
📝 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.
/** | |
* Registers the request quote button text for translation. | |
* | |
* @param bool $action The action status. | |
* @param array $args The arguments containing the button text. | |
* @param int $rule_id The ID of the rule. | |
* | |
* @return void | |
*/ | |
public function dokan_request_quote_button_text_registration(bool $action, array $args, int $rule_id) | |
{ | |
if (! $action) { | |
return; | |
} | |
$button_text = $args['button_text']; | |
$this->register_single_string( 'dokan', 'Dokan Request Quote Button Text: ' . $rule_id, $button_text ); | |
$hide_price_text = $args['hide_price_text']; | |
$this->register_single_string( 'dokan', 'Dokan Request Quote Hide Price Text: ' . $rule_id, $hide_price_text ); | |
} | |
/** | |
* Registers the request quote button text for translation. | |
* | |
* @param bool $action The action status. | |
* @param array $args The arguments containing the button text. | |
* @param int $rule_id The ID of the rule. | |
* | |
* @return void | |
*/ | |
public function dokan_request_quote_button_text_registration(bool $action, array $args, int $rule_id) | |
{ | |
if (! $action) { | |
return; | |
} | |
$button_text = isset( $args['button_text'] ) ? $args['button_text'] : ''; | |
$this->register_single_string( 'dokan', 'Dokan Request Quote Button Text: ' . $rule_id, $button_text ); | |
$hide_price_text = isset( $args['hide_price_text'] ) ? $args['hide_price_text'] : ''; | |
$this->register_single_string( 'dokan', 'Dokan Request Quote Hide Price Text: ' . $rule_id, $hide_price_text ); | |
} |
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
📜 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)
1357-1369
: Confirmed: Handling of undefined array keys is now safe
The use of the null coalescing operator ensures that 'button_text'
and 'hide_price_text'
are retrieved safely from $args
, preventing undefined index notices if these keys are missing.
public function get_translated_dokan_request_quote_button_text(string $text, $rule): string | ||
{ | ||
return $this->get_translated_single_string( $text, 'dokan', 'Dokan Request Quote Button Text: ' . $rule->id ); | ||
} |
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.
Add null checks for $rule
to prevent potential errors
In the method get_translated_dokan_request_quote_button_text
, you're accessing $rule->id
without verifying that $rule
is an object and that the id
property exists. This could lead to errors if $rule
is null or doesn't have an id
.
Apply this diff to add checks before accessing $rule->id
:
public function get_translated_dokan_request_quote_button_text(string $text, $rule): string
{
+ if ( ! is_object( $rule ) || ! isset( $rule->id ) ) {
+ return $text;
+ }
return $this->get_translated_single_string( $text, 'dokan', 'Dokan Request Quote Button Text: ' . $rule->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.
public function get_translated_dokan_request_quote_button_text(string $text, $rule): string | |
{ | |
return $this->get_translated_single_string( $text, 'dokan', 'Dokan Request Quote Button Text: ' . $rule->id ); | |
} | |
public function get_translated_dokan_request_quote_button_text(string $text, $rule): string | |
{ | |
if ( ! is_object( $rule ) || ! isset( $rule->id ) ) { | |
return $text; | |
} | |
return $this->get_translated_single_string( $text, 'dokan', 'Dokan Request Quote Button Text: ' . $rule->id ); | |
} |
public function get_translated_dokan_request_quote_price_hide(string $text, $rule): string | ||
{ | ||
return $this->get_translated_single_string( $text, 'dokan', 'Dokan Request Quote Hide Price Text: ' . $rule->id ); | ||
} |
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.
Add null checks for $rule
to prevent potential errors
In the method get_translated_dokan_request_quote_price_hide
, you're accessing $rule->id
without verifying that $rule
is an object and that the id
property exists. This may cause errors if $rule
is null or lacks the id
property.
Apply this diff to add checks before accessing $rule->id
:
public function get_translated_dokan_request_quote_price_hide(string $text, $rule): string
{
+ if ( ! is_object( $rule ) || ! isset( $rule->id ) ) {
+ return $text;
+ }
return $this->get_translated_single_string( $text, 'dokan', 'Dokan Request Quote Hide Price Text: ' . $rule->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.
public function get_translated_dokan_request_quote_price_hide(string $text, $rule): string | |
{ | |
return $this->get_translated_single_string( $text, 'dokan', 'Dokan Request Quote Hide Price Text: ' . $rule->id ); | |
} | |
public function get_translated_dokan_request_quote_price_hide(string $text, $rule): string | |
{ | |
if ( ! is_object( $rule ) || ! isset( $rule->id ) ) { | |
return $text; | |
} | |
return $this->get_translated_single_string( $text, 'dokan', 'Dokan Request Quote Hide Price Text: ' . $rule->id ); | |
} |
Summary by CodeRabbit
New Features
Enhancements