-
Notifications
You must be signed in to change notification settings - Fork 0
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
2.0 beta 3 enhancements 1 #452
Conversation
Warning CodeRabbit GitHub Action detectedThe repository is using both CodeRabbit Pro and CodeRabbit Open Source (via GitHub Actions), which is not recommended as it may lead to duplicate comments and extra noise. Please remove the CodeRabbit GitHub Action. Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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
Documentation and Community
|
WalkthroughThis update introduces enhancements to the currency display and sale price handling in the LSX Tour Operator plugin. It also includes a new option to disable the 3-letter currency code, adds a "Filter by Onsale" checkbox in the block editor settings panel, and refactors script enqueuing for improved maintainability. The PHP version requirement is updated from 7.2 to 8.0. Some sections of HTML and PHP code related to various settings are removed. Changes
Uplevel your code reviews with CodeRabbit ProCodeRabbit ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects. |
$currency = $tour_operator->options['currency']; | ||
$currency = '<span class="currency-icon ' . mb_strtolower( $currency ) . '">' . $currency . '</span>'; | ||
$letter_code = $tour_operator->options['currency']; | ||
$currency = '<span class="currency-icon ' . mb_strtolower( $letter_code ) . '"></span>'; |
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.
Equals sign not aligned with surrounding assignments; expected 4 spaces but found 1 space.
$currency = '<span class="currency-icon ' . mb_strtolower( $letter_code ) . '"></span>'; | |
$currency = '<span class="currency-icon ' . mb_strtolower( $letter_code ) . '"></span>'; |
} | ||
|
||
$value = $currency . $value; | ||
|
||
// Get the currency settings | ||
if ( is_object( $tour_operator ) && ( isset( $tour_operator->options['country_code_disabled'] ) && 0 === intval( $tour_operator->options['country_code_disabled'] ) || ! isset( $tour_operator->options['country_code_disabled'] ) ) ) { |
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.
Expected 1 space after "&&"; 2 found.
if ( is_object( $tour_operator ) && ( isset( $tour_operator->options['country_code_disabled'] ) && 0 === intval( $tour_operator->options['country_code_disabled'] ) || ! isset( $tour_operator->options['country_code_disabled'] ) ) ) { | |
if (is_object( $tour_operator ) && ( isset( $tour_operator->options['country_code_disabled'] ) && 0 === intval( $tour_operator->options['country_code_disabled'] ) || ! isset( $tour_operator->options['country_code_disabled'] ) )) { |
} | ||
|
||
$value = $currency . $value; | ||
|
||
// Get the Sale Price | ||
if ( 'price' === $meta_key ) { |
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.
Expected 0 spaces before closing bracket; 1 found.
if ( 'price' === $meta_key ) { | |
if ('price' === $meta_key) { |
// Get the Sale Price | ||
if ( 'price' === $meta_key ) { | ||
$sale_price = get_post_meta( get_the_ID(), 'sale_price', true ); | ||
if ( false !== $sale_price && ! empty( $sale_price ) && 0 !== intval( $sale_price ) ) { |
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.
Expected 0 spaces after opening bracket; 1 found.
if ( false !== $sale_price && ! empty( $sale_price ) && 0 !== intval( $sale_price ) ) { | |
if (false !== $sale_price && ! empty( $sale_price ) && 0 !== intval( $sale_price )) { |
} | ||
|
||
// Get the currency settings | ||
if ( is_object( $tour_operator ) && ( isset( $tour_operator->options['country_code_disabled'] ) && 0 === intval( $tour_operator->options['country_code_disabled'] ) || ! isset( $tour_operator->options['country_code_disabled'] ) ) ) { |
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.
Expected 1 space after "&&"; 2 found.
if ( is_object( $tour_operator ) && ( isset( $tour_operator->options['country_code_disabled'] ) && 0 === intval( $tour_operator->options['country_code_disabled'] ) || ! isset( $tour_operator->options['country_code_disabled'] ) ) ) { | |
if (is_object( $tour_operator ) && ( isset( $tour_operator->options['country_code_disabled'] ) && 0 === intval( $tour_operator->options['country_code_disabled'] ) || ! isset( $tour_operator->options['country_code_disabled'] ) )) { |
return $classes; | ||
} | ||
|
||
$sale_price = get_post_meta( get_the_ID(), 'sale_price', true ); |
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.
Expected 0 spaces before closing bracket; 1 found.
$sale_price = get_post_meta( get_the_ID(), 'sale_price', true ); | |
if (false !== $sale_price && ! empty( $sale_price ) && 0 !== intval( $sale_price )) { |
if ( false !== $sale_price && ! empty( $sale_price ) && 0 !== intval( $sale_price ) ) { | ||
$classes[] = 'on-sale'; | ||
} | ||
return $classes; |
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.
Expected 1 blank line before closing function brace; 0 found.
return $classes; |
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.
Files selected (8)
- assets/css/scss/_icons.scss (1)
- assets/css/style.css (1)
- includes/classes/blocks/class-bindings.php (1)
- includes/classes/legacy/class-accommodation.php (2)
- includes/classes/legacy/class-tour.php (3)
- includes/constants/settings-fields.php (1)
- readme.txt (1)
- tour-operator.php (1)
Files ignored due to filter (2)
- vendor/content-models/post-types/accommodation.json
- vendor/content-models/post-types/tour.json
Files skipped from review due to trivial changes (4)
- assets/css/scss/_icons.scss
- assets/css/style.css
- readme.txt
- tour-operator.php
Review comments generated (7)
- Review: 5
- LGTM: 2
Tips
Chat with CodeRabbit Bot (@coderabbitai
)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitai
in a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignore
anywhere in the PR description to pause further reviews from the bot.
$tour_operator = tour_operator(); | ||
$currency = ''; | ||
$letter_code = ''; | ||
$value = number_format( (int) $value, 2 ); | ||
|
||
// Get the currency settings | ||
if ( is_object( $tour_operator ) && isset( $tour_operator->options['currency'] ) && ! empty( $tour_operator->options['currency'] ) ) { | ||
$currency = $tour_operator->options['currency']; | ||
$currency = '<span class="currency-icon ' . strtolower( $currency ) . '">' . $currency . '</span>'; | ||
$letter_code = $tour_operator->options['currency']; | ||
$currency = '<span class="currency-icon ' . strtolower( $letter_code ) . '"></span>'; | ||
} | ||
|
||
$value = $currency . $value; | ||
|
||
// Get the Sale Price | ||
if ( 'price' === $meta_key ) { | ||
$sale_price = get_post_meta( get_the_ID(), 'sale_price', true ); | ||
if ( false !== $sale_price && ! empty( $sale_price ) && 0 !== intval( $sale_price ) ) { | ||
$value = '<span class="strike">' . $value . '</span>' . ' ' . $currency . number_format( intval( $sale_price ) , 2 ); | ||
} | ||
} | ||
|
||
// Get the currency settings | ||
if ( is_object( $tour_operator ) && ( isset( $tour_operator->options['country_code_disabled'] ) && 0 === intval( $tour_operator->options['country_code_disabled'] ) || ! isset( $tour_operator->options['country_code_disabled'] ) ) ) { | ||
$value = $letter_code . $value; | ||
} | ||
|
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.
There's a potential issue with the way you're handling currency formatting. You're using number_format
to format the price, but this function uses a comma as the thousands separator by default. This might not be suitable for all locales. Consider using number_format_i18n
instead, which will use the correct thousands separator based on the site's locale.
- $value = number_format( (int) $value, 2 );
+ $value = number_format_i18n( (int) $value, 2 );
...
- $value = '<span class="strike">' . $value . '</span>' . ' ' . $currency . number_format( intval( $sale_price ) , 2 );
+ $value = '<span class="strike">' . $value . '</span>' . ' ' . $currency . number_format_i18n( intval( $sale_price ) , 2 );
|
||
/** | ||
* Adds in the onsale classes. | ||
* | ||
* @param array $classes | ||
* @return array | ||
*/ | ||
public function tour_classes( $classes ) { | ||
if ( ! is_singular( 'tour' ) ) { | ||
return $classes; | ||
} | ||
|
||
$sale_price = get_post_meta( get_the_ID(), 'sale_price', true ); | ||
if ( false !== $sale_price && ! empty( $sale_price ) && 0 !== intval( $sale_price ) ) { | ||
$classes[] = 'on-sale'; | ||
} | ||
return $classes; | ||
} |
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.
The tour_classes
method is checking if the post type is 'tour' and then adding an 'on-sale' class if the tour has a sale price. However, it doesn't check if the current post actually has the 'tour' post type. This could lead to unexpected behavior if this method is called in a context where the global post isn't a tour. Consider adding a check for the post type:
+ global $post;
+ if (!$post || 'tour' !== $post->post_type) {
+ return $classes;
+ }
$sale_price = get_post_meta( get_the_ID(), 'sale_price', true );
if ( false !== $sale_price && ! empty( $sale_price ) && 0 !== intval( $sale_price ) ) {
$classes[] = 'on-sale';
}
return $classes;
$value = $rooms->item_price( '', '', false ); | ||
$letter_code = ''; | ||
|
||
if ( is_object( $tour_operator ) && isset( $tour_operator->options['currency'] ) && ! empty( $tour_operator->options['currency'] ) ) { | ||
$currency = $tour_operator->options['currency']; | ||
$currency = '<span class="currency-icon ' . mb_strtolower( $currency ) . '">' . $currency . '</span>'; | ||
$letter_code = $tour_operator->options['currency']; | ||
$currency = '<span class="currency-icon ' . mb_strtolower( $letter_code ) . '"></span>'; | ||
} | ||
|
||
$value = $currency . $value; | ||
|
||
// Get the currency settings | ||
if ( is_object( $tour_operator ) && ( isset( $tour_operator->options['country_code_disabled'] ) && 0 === intval( $tour_operator->options['country_code_disabled'] ) || ! isset( $tour_operator->options['country_code_disabled'] ) ) ) { | ||
$value = $letter_code . $value; | ||
} |
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.
The logic for handling the currency code seems to be a bit convoluted. It would be more readable and maintainable if you refactor it into a separate function. This function could take in the tour_operator
object as an argument and return the appropriate currency string based on the settings.
- $value = $rooms->item_price( '', '', false );
- $letter_code = '';
-
- if ( is_object( $tour_operator ) && isset( $tour_operator->options['currency'] ) && ! empty( $tour_operator->options['currency'] ) ) {
- $letter_code = $tour_operator->options['currency'];
- $currency = '<span class="currency-icon ' . mb_strtolower( $letter_code ) . '"></span>';
- }
-
- $value = $currency . $value;
-
- // Get the currency settings
- if ( is_object( $tour_operator ) && ( isset( $tour_operator->options['country_code_disabled'] ) && 0 === intval( $tour_operator->options['country_code_disabled'] ) || ! isset( $tour_operator->options['country_code_disabled'] ) ) ) {
- $value = $letter_code . $value;
- }
+ $value = $rooms->item_price( '', '', false );
+ $value = $this->getCurrencyString($tour_operator) . $value;
...
+ private function getCurrencyString($tour_operator) {
+ $letter_code = '';
+
+ if (is_object($tour_operator) && isset($tour_operator->options['currency']) && !empty($tour_operator->options['currency'])) {
+ $letter_code = $tour_operator->options['currency'];
+ $currency = '<span class="currency-icon ' . mb_strtolower($letter_code) . '"></span>';
+ }
+
+ // Get the currency settings
+ if (is_object($tour_operator) && (isset($tour_operator->options['country_code_disabled']) && 0 === intval($tour_operator->options['country_code_disabled']) || !isset($tour_operator->options['country_code_disabled']))) {
+ $currency = $letter_code . $currency;
+ }
+
+ return $currency;
+ }
This refactoring makes the code more modular and easier to understand. It also improves maintainability by isolating the logic for generating the currency string into a separate function, which can be modified independently of the rest of the code.
$letter_code = ''; | ||
|
||
if ( is_object( $tour_operator ) && isset( $tour_operator->options['currency'] ) && ! empty( $tour_operator->options['currency'] ) ) { | ||
$currency = $tour_operator->options['currency']; | ||
$currency = '<span class="currency-icon ' . mb_strtolower( $currency ) . '">' . $currency . '</span>'; | ||
$letter_code = $tour_operator->options['currency']; | ||
$currency = '<span class="currency-icon ' . mb_strtolower( $letter_code ) . '"></span>'; | ||
} | ||
|
||
$value = apply_filters( 'lsx_to_accommodation_price', $value, $price_type, $currency ); | ||
|
||
// Get the Sale Price | ||
if ( 'price' === $meta_key ) { | ||
$sale_price = get_post_meta( get_the_ID(), 'sale_price', true ); | ||
if ( false !== $sale_price && ! empty( $sale_price ) && 0 !== intval( $sale_price ) ) { | ||
$value = number_format( intval( $sale_price ) , 2 ); | ||
} | ||
} | ||
|
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.
The logic for getting the sale price seems to be only applicable when the $meta_key
is 'price'. This might not cover all scenarios where a sale price should be applied. Consider making this logic more generic or ensure that 'price' is the only key that would require a sale price.
- if ( 'price' === $meta_key ) {
+ if ( 'price' === $meta_key || 'other_condition' ) {
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.
Files selected (2)
- assets/js/blocks/slider-query.js (1)
- includes/classes/blocks/class-registration.php (2)
Files skipped from review due to trivial changes (1)
- includes/classes/blocks/class-registration.php
Review comments generated (1)
- Review: 0
- LGTM: 1
Tips
Chat with CodeRabbit Bot (@coderabbitai
)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitai
in a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignore
anywhere in the PR description to pause further reviews from the bot.
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.
Files selected (1)
- assets/js/blocks/slider-query.js (1)
Review comments generated (3)
- Review: 3
- LGTM: 0
Tips
Chat with CodeRabbit Bot (@coderabbitai
)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitai
in a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignore
anywhere in the PR description to pause further reviews from the bot.
var hasCustomClass = props.attributes.hasCustomClass || false; | ||
if ( undefined === props.attributes.hasCustomClass ) { | ||
if ( props.attributes.className && props.attributes.className.includes( 'lsx-to-slider' ) ) { | ||
hasCustomClass = true; | ||
} | ||
} else { | ||
hasCustomClass = props.attributes.hasCustomClass; | ||
} |
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.
The logic for setting hasCustomClass
is a bit redundant. You're checking if props.attributes.hasCustomClass
is undefined and then setting hasCustomClass
based on the condition, but then you're doing it again in the else block. This can be simplified.
- var hasCustomClass = props.attributes.hasCustomClass || false;
- if ( undefined === props.attributes.hasCustomClass ) {
- if ( props.attributes.className && props.attributes.className.includes( 'lsx-to-slider' ) ) {
- hasCustomClass = true;
- }
- } else {
- hasCustomClass = props.attributes.hasCustomClass;
- }
+ var hasCustomClass = props.attributes.hasCustomClass !== undefined ? props.attributes.hasCustomClass :
+ (props.attributes.className && props.attributes.className.includes('lsx-to-slider'));
var filterByOnsale = props.attributes.filterByOnsale || false; | ||
if ( undefined === props.attributes.filterByOnsale ) { | ||
if ( props.attributes.className && props.attributes.className.includes( 'on-sale' ) ) { | ||
filterByOnsale = true; | ||
} | ||
} else { | ||
filterByOnsale = props.attributes.filterByOnsale; | ||
} |
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.
Similar to the previous comment, the logic for setting filterByOnsale
is also redundant and can be simplified.
- var filterByOnsale = props.attributes.filterByOnsale || false;
- if ( undefined === props.attributes.filterByOnsale ) {
- if ( props.attributes.className && props.attributes.className.includes( 'on-sale' ) ) {
- filterByOnsale = true;
- }
- } else {
- filterByOnsale = props.attributes.filterByOnsale;
- }
+ var filterByOnsale = props.attributes.filterByOnsale !== undefined ? props.attributes.filterByOnsale :
+ (props.attributes.className && props.attributes.className.includes('on-sale'));
41, 51, 74, 78, 81:
Console logs should not be present in production code as they can expose sensitive information and clutter the console output. Please remove these debug statements.
if ( true === attributes.filterByOnsale ) { | ||
extraProps.className = (extraProps.className || '') + ' on-sale'; | ||
} else if ( false === attributes.filterByOnsale && extraProps.className ) { | ||
extraProps.className = extraProps.className.replace(/\bon-sale\b\s*/g, '').trim(); | ||
} |
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.
These blocks of code are very similar and can be abstracted into a function to follow the DRY (Don't Repeat Yourself) principle.
- if ( true === attributes.hasCustomClass ) {
- extraProps.className = (extraProps.className || '') + ' lsx-to-slider';
- console.log('adding');
- } else if ( false === attributes.hasCustomClass && extraProps.className ) {
- extraProps.className = extraProps.className.replace(/\blsx-to-slider\b\s*/g, '').trim();
- console.log('removing');
- }
-
- if ( true === attributes.filterByOnsale ) {
- extraProps.className = (extraProps.className || '') + ' on-sale';
- } else if ( false === attributes.filterByOnsale && extraProps.className ) {
- extraProps.className = extraProps.className.replace(/\bon-sale\b\s*/g, '').trim();
- }
+ function updateClassName(attribute, className, classToAddOrRemove) {
+ if (true === attribute) {
+ return (className || '') + ` ${classToAddOrRemove}`;
+ } else if (false === attribute && className) {
+ return className.replace(new RegExp(`\\b${classToAddOrRemove}\\b\\s*`, 'g'), '').trim();
+ }
+ return className;
+ }
+
+ extraProps.className = updateClassName(attributes.hasCustomClass, extraProps.className, 'lsx-to-slider');
+ extraProps.className = updateClassName(attributes.filterByOnsale, extraProps.className, 'on-sale');
This new function updateClassName
takes in the attribute, current className, and the class to add or remove. It then returns the updated className based on the attribute value. This reduces code duplication and improves maintainability.
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.
Files selected (2)
- assets/js/blocks/general.js (2)
- includes/classes/blocks/class-registration.php (3)
Review comments generated (5)
- Review: 4
- LGTM: 1
Tips
Chat with CodeRabbit Bot (@coderabbitai
)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitai
in a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignore
anywhere in the PR description to pause further reviews from the bot.
width: '20px', | ||
sizeSlug: 'large', | ||
url: 'http://localhost:8883/wp-content/themes/lsx-tour-operator/assets/images/Typetype-icon.png', | ||
url: 'https://tour-operator.lsx.design/wp-content/uploads/2024/06/image-1.png', |
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.
The width property of the image is set as a string '20px' instead of an integer. This could potentially cause issues if the code expects an integer value for the width. Also, it's better to avoid hardcoding URLs in your code. Consider using a configuration file or environment variable to store these values.
- width: '20px',
+ width: 20,
- url: 'https://tour-operator.lsx.design/wp-content/uploads/2024/06/image-1.png',
+ url: config.IMAGE_URL || process.env.IMAGE_URL,
width: '20px', | ||
sizeSlug: 'large', | ||
url: 'http://localhost:8883/wp-content/themes/lsx-tour-operator/assets/images/best-months-to-travel-TO-icon-black-20px-1-1.png', | ||
url: lsxToEditor.homeUrl + 'wp-content/uploads/2024/11/booking-validity-icon-black-52px-1.svg', |
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.
Similar to the previous comment, the width property should be an integer and not a string. The URL concatenation could be improved by using a template literal for readability and maintainability.
- width: '20px',
+ width: 20,
- url: lsxToEditor.homeUrl + 'wp-content/uploads/2024/11/booking-validity-icon-black-52px-1.svg',
+ url: `${lsxToEditor.homeUrl}/wp-content/uploads/2024/11/booking-validity-icon-black-52px-1.svg`,
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.
Files selected (2)
- includes/classes/blocks/class-registration.php (8)
- includes/classes/legacy/class-accommodation.php (3)
Review comments generated (12)
- Review: 6
- LGTM: 6
Tips
Chat with CodeRabbit Bot (@coderabbitai
)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitai
in a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignore
anywhere in the PR description to pause further reviews from the bot.
add_filter( 'query_loop_block_query_vars', array( $this, 'query_args_filter' ), 1, 2 ); | ||
add_filter( 'render_block', array( $this, 'maybe_hide_varitaion' ), 10, 3 ); | ||
|
||
add_filter( 'posts_pre_query', array( $this, 'posts_pre_query' ), 10, 2 ); | ||
|
||
add_filter( 'render_block_data', array( $this, 'save_onsale_queries' ), 10, 1 ); |
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.
if ( 'general' === $slug ) { | ||
$param_array = array( | ||
'homeUrl' => trailingslashit( home_url() ), | ||
'assetsUrl' => LSX_TO_URL . 'assets/img/' | ||
); | ||
$param_array = apply_filters( 'lsx_to_editor_params', $param_array ); | ||
wp_localize_script( 'lsx-to-block-' . $slug . '-variations', 'lsxToEditor', $param_array ); | ||
} | ||
} | ||
|
||
// Enqueue linked-cover.js | ||
wp_enqueue_script( | ||
'lsx-to-linked-cover', | ||
LSX_TO_URL . 'assets/js/blocks/linked-cover.js', | ||
array( 'wp-blocks', 'wp-element', 'wp-block-editor', 'wp-components', 'wp-compose', 'wp-data', 'wp-hooks' ), | ||
filemtime( LSX_TO_PATH . 'assets/js/blocks/linked-cover.js' ) | ||
); | ||
|
||
// Enqueue linked-cover.js | ||
wp_enqueue_script( | ||
'lsx-to-slider-query', | ||
LSX_TO_URL . 'assets/js/blocks/slider-query.js', | ||
array( 'wp-blocks', 'wp-element', 'wp-editor', 'wp-components' ), | ||
filemtime( LSX_TO_PATH . 'assets/js/blocks/slider-query.js' ) | ||
); | ||
foreach ( $additional_scripts as $slug => $dependancies ) { | ||
wp_enqueue_script( | ||
'lsx-to-' . $slug, | ||
LSX_TO_URL . 'assets/js/blocks/' . $slug . '.js', | ||
$dependancies, | ||
filemtime( LSX_TO_PATH . 'assets/js/blocks/' . $slug . '.js' ) | ||
); | ||
} |
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.
The refactoring of script enqueuing is good for maintainability and reducing duplication. However, it seems that the versioning with file modification time and enqueueing in the footer are missing for the additional scripts. This could lead to caching issues and unexpected behavior if the scripts rely on other scripts that are loaded in the footer.
- wp_enqueue_script(
- 'lsx-to-' . $slug,
- LSX_TO_URL . 'assets/js/blocks/' . $slug . '.js',
- $dependancies,
- filemtime( LSX_TO_PATH . 'assets/js/blocks/' . $slug . '.js' )
- );
+ wp_enqueue_script(
+ 'lsx-to-' . $slug,
+ LSX_TO_URL . 'assets/js/blocks/' . $slug . '.js',
+ $dependancies,
+ filemtime( LSX_TO_PATH . 'assets/js/blocks/' . $slug . '.js' ),
+ true // Enqueue in the footer.
+ );
* Determines if a post exists based on the ID. | ||
* | ||
* | ||
* @global wpdb $wpdb WordPress database abstraction object. | ||
* | ||
* @param string $title Post title. | ||
* @param string $content Optional. Post content. | ||
* @param string $date Optional. Post date. | ||
* @param string $type Optional. Post type. | ||
* @param string $status Optional. Post status. | ||
* @return int Post ID if post exists, 0 otherwise. |
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.
The function comment does not match the function implementation. The comment suggests that the function checks if a post exists based on title, content, date, type, and status, but the function actually checks if posts exist based on their IDs. Please update the comment to reflect the actual functionality.
- * @param string $title Post title.
- * @param string $content Optional. Post content.
- * @param string $date Optional. Post date.
- * @param string $type Optional. Post type.
- * @param string $status Optional. Post status.
+ * @param array $ids Array of post IDs.
$letter_code = ''; | ||
|
||
if ( is_object( $tour_operator ) && isset( $tour_operator->options['currency'] ) && ! empty( $tour_operator->options['currency'] ) ) { | ||
$currency = $tour_operator->options['currency']; | ||
$currency = '<span class="currency-icon ' . mb_strtolower( $currency ) . '">' . $currency . '</span>'; | ||
$letter_code = $tour_operator->options['currency']; | ||
$currency = '<span class="currency-icon ' . mb_strtolower( $letter_code ) . '"></span>'; | ||
} |
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.
The code checks if the tour_operator
object exists and if it has a non-empty currency
option. However, it doesn't handle the case where the options
property itself might not exist or be null. This could potentially lead to a PHP notice or warning. To avoid this, you should also check if the options
property is set and is an array.
- if ( is_object( $tour_operator ) && isset( $tour_operator->options['currency'] ) && ! empty( $tour_operator->options['currency'] ) ) {
+ if ( is_object( $tour_operator ) && isset( $tour_operator->options ) && is_array( $tour_operator->options ) && !empty( $tour_operator->options['currency'] ) ) {
if ( 'price' === $meta_key ) { | ||
$sale_price = get_post_meta( get_the_ID(), 'sale_price', true ); | ||
if ( false !== $sale_price && ! empty( $sale_price ) && 0 !== intval( $sale_price ) ) { | ||
$value = number_format( intval( $sale_price ) , 2 ); | ||
} | ||
} |
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.
The sale price is being fetched from post meta and used if it's not false, not empty, and not zero. However, there's no validation for negative values. If a negative value is entered as the sale price, it would pass these conditions and be used. You should add a condition to check that the sale price is greater than zero.
- if ( false !== $sale_price && ! empty( $sale_price ) && 0 !== intval( $sale_price ) ) {
+ if ( false !== $sale_price && ! empty( $sale_price ) && 0 !== intval( $sale_price ) && intval( $sale_price ) > 0 ) {
// Get the currency settings | ||
if ( is_object( $tour_operator ) && ( isset( $tour_operator->options['country_code_disabled'] ) && 0 === intval( $tour_operator->options['country_code_disabled'] ) || ! isset( $tour_operator->options['country_code_disabled'] ) ) ) { | ||
$value = $letter_code . $value; | ||
} |
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.
Similar to the previous comment, the code doesn't handle the case where the options
property itself might not exist or be null. This could potentially lead to a PHP notice or warning. To avoid this, you should also check if the options
property is set and is an array.
- if ( is_object( $tour_operator ) && ( isset( $tour_operator->options['country_code_disabled'] ) && 0 === intval( $tour_operator->options['country_code_disabled'] ) || ! isset( $tour_operator->options['country_code_disabled'] ) ) ) {
+ if ( is_object( $tour_operator ) && isset( $tour_operator->options ) && is_array( $tour_operator->options ) && ( isset( $tour_operator->options['country_code_disabled'] ) && 0 === intval( $tour_operator->options['country_code_disabled'] ) || ! isset( $tour_operator->options['country_code_disabled'] ) ) ) {
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.
Files selected (2)
- includes/classes/legacy/class-tour-operator.php (1)
- readme.txt (1)
Files skipped from review due to trivial changes (2)
- includes/classes/legacy/class-tour-operator.php
- readme.txt
Review comments generated (0)
- Review: 0
- LGTM: 0
Tips
Chat with CodeRabbit Bot (@coderabbitai
)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitai
in a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignore
anywhere in the PR description to pause further reviews from the bot.
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.
Files selected (6)
- includes/classes/blocks/class-patterns.php (2)
- includes/pages/config-tour-operator.php (1)
- includes/partials/general.php (1)
- includes/partials/navigation.php (1)
- includes/partials/post-type.php (1)
- includes/partials/settings.php (1)
Files skipped from review due to trivial changes (5)
- includes/classes/blocks/class-patterns.php
- includes/partials/general.php
- includes/partials/navigation.php
- includes/partials/post-type.php
- includes/partials/settings.php
Review comments generated (1)
- Review: 1
- LGTM: 0
Tips
Chat with CodeRabbit Bot (@coderabbitai
)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitai
in a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignore
anywhere in the PR description to pause further reviews from the bot.
'callback' => function () { | ||
include( LSX_TO_PATH . 'includes/partials/welcome.php' ); | ||
}, |
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.
The callback function includes a PHP file directly. This could potentially lead to issues if the file is not found or if there are errors within the included file that prevent it from being loaded correctly. It would be better to use require_once
instead of include
, as require_once
will produce a fatal error if the file cannot be included, which makes debugging easier.
- 'callback' => function () {
- include( LSX_TO_PATH . 'includes/partials/welcome.php' );
- },
+ 'callback' => function () {
+ require_once( LSX_TO_PATH . 'includes/partials/welcome.php' );
+ },
This way, if the file is not found or cannot be loaded for some reason, the script will stop execution and throw a fatal error, making it easier to identify and fix the problem.
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.
Files selected (1)
- assets/js/blocks/general.js (3)
Files skipped from review due to trivial changes (1)
- assets/js/blocks/general.js
Review comments generated (0)
- Review: 0
- LGTM: 0
Tips
Chat with CodeRabbit Bot (@coderabbitai
)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitai
in a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignore
anywhere in the PR description to pause further reviews from the bot.
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.
Looks good. Thanks Warwick
Description of the Change
Screenshots
#449
#377
Tour
Accommodation
Meta Query in Action
Issues
Closed
#331
#377
#449
Summary by CodeRabbit
New Features:
.strike
for text decoration in the currency section styling.Refactor:
Bug Fix:
Documentation:
Style:
Chore: