Skip to content
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

Merged
merged 12 commits into from
Dec 5, 2024
Merged

2.0 beta 3 enhancements 1 #452

merged 12 commits into from
Dec 5, 2024

Conversation

krugazul
Copy link
Collaborator

@krugazul krugazul commented Dec 4, 2024

Description of the Change

Screenshots

#449
Screenshot 2024-12-04 at 13 23 54

#377
Tour
Screenshot 2024-12-04 at 13 23 49

Accommodation
Screenshot 2024-12-04 at 13 42 23

Screenshot 2024-12-04 at 13 52 55

Meta Query in Action
Screenshot 2024-12-05 at 13 30 16

Issues

Closed
#331
#377
#449

Summary by CodeRabbit


New Features:

  • Added a new CSS class .strike for text decoration in the currency section styling.
  • Introduced an option to disable the 3-letter currency code display.
  • Added a "Filter by Onsale" checkbox in the block editor settings panel.
  • Updated image URLs to use HTTPS and dynamic paths.

Refactor:

  • Improved script enqueuing process for better maintainability.
  • Refactored handling of checkbox states and class names.

Bug Fix:

  • Fixed Google Maps API key debug action call issue.

Documentation:

  • Updated plugin metadata to reflect PHP version requirement change from 7.2 to 8.0.

Style:

  • Changed the title of a block category for consistency in the WordPress editor.

Chore:

  • Removed sections of HTML and PHP code related to various settings in the dashboard.

@krugazul krugazul added the [Type] Enhancement A suggestion for improvement label Dec 4, 2024
@krugazul krugazul added this to the 2.0.0 milestone Dec 4, 2024
@krugazul krugazul self-assigned this Dec 4, 2024
Copy link

coderabbitai bot commented Dec 4, 2024

Warning

CodeRabbit GitHub Action detected

The 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 skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

github-actions bot commented Dec 4, 2024

Image description CodeRabbit

Walkthrough

This 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

Files Summary
assets/css/scss/_icons.scss, assets/css/style.css Added a new CSS class .strike for text decoration in the currency section styling.
includes/classes/blocks/class-bindings.php, includes/classes/legacy/class-accommodation.php, includes/classes/legacy/class-tour.php, includes/constants/settings-fields.php, assets/js/blocks/slider-query.js, includes/classes/blocks/class-registration.php, includes/classes/blocks/general.js, includes/classes/legacy/class-tour-operator.php, assets/js/blocks/general.js Introduced changes to handle the sale price and an option to disable the 3-letter currency code display. Refactored script enqueuing to use a loop for additional scripts. Added a new checkbox for "Filter by Onsale" in the block editor settings panel.
readme.txt, tour-operator.php Updated the stable tag and version. Changed the PHP version requirement from 7.2 to 8.0.
includes/classes/blocks/class-patterns.php Changed the title of a block category for consistency.
includes/pages/config-tour-operator.php Introduced a new callback function that includes a PHP file.
includes/partials/general.php, includes/partials/navigation.php, includes/partials/post-type.php, includes/partials/settings.php Removed sections of HTML and PHP code related to various settings.

"In the land where code rabbits dwell, 🐇💻
Changes are made, stories to tell. 📖✨
Currency display now has a new look, 💰👀
And sale prices have their own nook. 🏷️🎉
With scripts enqueued in a loop, 🔄📜
The code's elegance took a giant swoop. 🕊️🌟
So here's to changes, big and small, 🥂🎈
Celebrating progress, we stand tall!" 🎉🐇


Uplevel your code reviews with CodeRabbit Pro

CodeRabbit Pro

If 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>';
Copy link

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.

Suggested change
$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'] ) ) ) {
Copy link

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.

Suggested change
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 ) {
Copy link

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.

Suggested change
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 ) ) {
Copy link

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.

Suggested change
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'] ) ) ) {
Copy link

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.

Suggested change
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 );
Copy link

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.

Suggested change
$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;
Copy link

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.

Suggested change
return $classes;

@krugazul krugazul changed the base branch from master to 2.0-beta-3 December 4, 2024 12:33
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

Commits Files that changed from the base of the PR and between 80a9811 and 4bb968b commits.
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 Image description 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.

Comment on lines 146 to +171
$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;
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

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 );

Comment on lines +231 to +248

/**
* 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;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

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;

Comment on lines +502 to +515
$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;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

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.

Comment on lines +136 to +152
$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 );
}
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

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' ) {

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

Commits Files that changed from the base of the PR and between 4bb968b and 12e17fb commits.
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 Image description 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.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

Commits Files that changed from the base of the PR and between 12e17fb and f9e7883 commits.
Files selected (1)
  • assets/js/blocks/slider-query.js (1)
Review comments generated (3)
  • Review: 3
  • LGTM: 0

Tips

Chat with Image description 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.

Comment on lines +13 to +20
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;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

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'));

Comment on lines +22 to +29
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;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

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.

Comment on lines +84 to +88
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();
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

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.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

Commits Files that changed from the base of the PR and between f9e7883 and 24d69ed commits.
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 Image description 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.

Comment on lines +142 to +144
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',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

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,

Comment on lines +247 to +249
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',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

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`,

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

Commits Files that changed from the base of the PR and between 24d69ed and b661a36 commits.
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 Image description 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.

Comment on lines +37 to +42
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 );
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

There's a typo in the method name maybe_hide_varitaion. It should be maybe_hide_variation.

- add_filter( 'render_block', array( $this, 'maybe_hide_varitaion' ), 10, 3 );
+ add_filter( 'render_block', array( $this, 'maybe_hide_variation' ), 10, 3 );

Comment on lines +76 to +93
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' )
);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

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.
+ );

Comment on lines +408 to +418
* 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.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

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.

Comment on lines +134 to 139
$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>';
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

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'] ) ) {

Comment on lines +144 to +149
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 );
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

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 ) {

Comment on lines +169 to +172
// 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;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

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'] ) ) ) {

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

Commits Files that changed from the base of the PR and between b661a36 and 69531aa commits.
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 Image description 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.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

Commits Files that changed from the base of the PR and between 69531aa and 151b0f5 commits.
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 Image description 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.

Comment on lines +19 to +21
'callback' => function () {
include( LSX_TO_PATH . 'includes/partials/welcome.php' );
},
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

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.

@krugazul krugazul marked this pull request as ready for review December 5, 2024 12:52
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

Commits Files that changed from the base of the PR and between 151b0f5 and 2a32cc3 commits.
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 Image description 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.

@krugazul krugazul requested a review from ZaredRogers December 5, 2024 13:00
Copy link
Contributor

@ZaredRogers ZaredRogers left a 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

@krugazul krugazul merged commit 513fc33 into 2.0-beta-3 Dec 5, 2024
6 of 8 checks passed
@krugazul krugazul deleted the 2.0-beta-3-enhancements-1 branch December 5, 2024 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants