Beta 2 bugfixes 3 #447
Beta 2 bugfixes 3 #447
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 brings a significant overhaul to the codebase, focusing on improving readability, maintainability, and performance. It includes changes to class names, block settings, image handling, lightbox setup, rating function, and HTML output. Additionally, it removes unused frontend functions, debug actions, redundant lines, and updates initialization of classes. The update also ensures safer data handling and expands options for better user understanding. 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. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files selected (20)
- assets/js/blocks/accommodation.js (2)
- assets/js/src/custom.js (1)
- bin/build.command (1)
- includes/actions.php (1)
- includes/classes/admin/class-setup.php (3)
- includes/classes/blocks/class-bindings.php (3)
- includes/classes/blocks/class-registration.php (2)
- includes/classes/class-tour-operator.php (1)
- includes/classes/legacy/class-accommodation.php (1)
- includes/classes/legacy/class-admin.php (1)
- includes/classes/legacy/class-frontend.php (4)
- includes/classes/legacy/class-tour-operator.php (6)
- includes/classes/legacy/class-tour.php (2)
- includes/classes/legacy/class-unit-query.php (2)
- includes/customizer.php (1)
- includes/functions.php (9)
- includes/layout.php (1)
- includes/metaboxes/config-tour.php (1)
- includes/template-tags/accommodation.php (2)
- tour-operator-bootstrap.php (1)
Files ignored due to filter (5)
- assets/img/rating-star-empty.png
- assets/img/rating-star-full.png
- post-types/accommodation.json
- post-types/destinations.json
- post-types/tours.json
Files not summarized due to errors (3)
- includes/classes/admin/class-setup.php (diff tokens exceeds limit)
- includes/classes/legacy/class-tour-operator.php (diff tokens exceeds limit)
- includes/layout.php (diff tokens exceeds limit)
Files not reviewed due to errors (7)
- assets/js/blocks/accommodation.js (no response)
- includes/classes/legacy/class-accommodation.php (no response)
- includes/classes/legacy/class-frontend.php (no response)
- includes/classes/class-tour-operator.php (no response)
- includes/classes/legacy/class-tour-operator.php (no response)
- includes/template-tags/accommodation.php (no response)
- includes/metaboxes/config-tour.php (no response)
Files skipped from review due to trivial changes (7)
- bin/build.command
- includes/actions.php
- includes/classes/blocks/class-bindings.php
- includes/classes/blocks/class-registration.php
- includes/classes/legacy/class-admin.php
- tour-operator-bootstrap.php
- includes/layout.php (diff too large)
Review comments generated (18)
- Review: 7
- LGTM: 11
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
@coderabbitaiin 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: ignoreanywhere in the PR description to pause further reviews from the bot.
| if ( 0 < $( '.wp-block-gallery.has-nested-images' ).length ) { | ||
| $( '.wp-block-gallery.has-nested-images' ).slickLightbox( { | ||
| caption: function( element, info ) { | ||
| return $( element ).find( 'img' ).attr( 'alt' ); | ||
| } | ||
| } ); | ||
| } |
There was a problem hiding this comment.
The new code checks if there are any elements with the class .wp-block-gallery.has-nested-images and applies slickLightbox to them. This is a good change as it ensures that the lightbox functionality is only applied when necessary, improving performance.
+ if ( 0 < $( '.wp-block-gallery.has-nested-images' ).length ) {
+ $( '.wp-block-gallery.has-nested-images' ).slickLightbox( {
+ caption: function( element, info ) {
+ return $( element ).find( 'img' ).attr( 'alt' );
+ }
+ } );
+ }| if ( 0 < $( '.lsx-units-wrapper .unit-image a' ).length ) { | ||
| let roomImages = $('.lsx-units-wrapper .unit-image a img').map(function() { | ||
| return $(this).attr('src'); | ||
| }).get(); | ||
| console.log(roomImages); | ||
|
|
||
| $( '.lsx-units-wrapper' ).slickLightbox( { | ||
| //images : roomImages, | ||
| itemSelector: '.unit-image a', | ||
| caption: function( element, info ) { | ||
| return $( element ).find( 'img' ).attr( 'alt' ); | ||
| } | ||
| } ); | ||
| } |
There was a problem hiding this comment.
This block of code checks for images within a specific wrapper .lsx-units-wrapper .unit-image a and applies slickLightbox to them. However, there's a console.log statement on line 347 which should be removed as it's not a good practice to leave debugging logs in production code.
+ if ( 0 < $( '.lsx-units-wrapper .unit-image a' ).length ) {
+ let roomImages = $('.lsx-units-wrapper .unit-image a img').map(function() {
+ return $(this).attr('src');
+ }).get();
- console.log(roomImages);
+
+ $( '.lsx-units-wrapper' ).slickLightbox( {
+ //images : roomImages,
+ itemSelector: '.unit-image a',
+ caption: function( element, info ) {
+ return $( element ).find( 'img' ).attr( 'alt' );
+ }
+ } );
+ }| $allowedtags['div']['data-type'] = true; | ||
| $allowedtags['div']['data-cluster-small'] = true; | ||
| $allowedtags['div']['data-cluster-medium'] = true; | ||
| $allowedtags['div']['data-cluster-large'] = true; | ||
| $allowedtags['div']['data-fusion-tables'] = true; | ||
| $allowedtags['div']['data-fusion-tables-colour-border'] = true; | ||
| $allowedtags['div']['data-fusion-tables-width-border'] = true; | ||
| $allowedtags['div']['data-fusion-tables-colour-background'] = true; | ||
| $allowedtags['div']['itemscope'] = true; | ||
| $allowedtags['div']['itemtype'] = true; | ||
| $allowedtags['div']['data-row-height'] = true; | ||
| $allowedtags['div']['data-justified-margins'] = true; | ||
| $allowedtags['div']['data-slick'] = true; | ||
|
|
||
| //Envirta Gallery tags | ||
| // | ||
| $allowedtags['div']['data-envira-id'] = true; | ||
| $allowedtags['div']['data-gallery-config'] = true; | ||
| $allowedtags['div']['data-gallery-images'] = true; | ||
| $allowedtags['div']['data-gallery-theme'] = true; | ||
| $allowedtags['div']['data-envira-columns'] = true; | ||
|
|
||
| if ( ! isset( $allowedtags['img'] ) ) { | ||
| $allowedtags['img'] = array(); | ||
| } | ||
|
|
||
| $allowedtags['img']['data-envira-index'] = true; | ||
| $allowedtags['img']['data-envira-caption'] = true; | ||
| $allowedtags['img']['data-envira-gallery-id'] = true; | ||
| $allowedtags['img']['data-envira-item-id'] = true; | ||
| $allowedtags['img']['data-envira-src'] = true; | ||
| $allowedtags['img']['data-envira-srcset'] = true; | ||
|
|
||
| if ( ! isset( $allowedtags['input'] ) ) { | ||
| $allowedtags['input'] = array(); | ||
| } | ||
|
|
||
| $allowedtags['input']['type'] = true; | ||
| $allowedtags['input']['id'] = true; | ||
| $allowedtags['input']['name'] = true; | ||
| $allowedtags['input']['value'] = true; | ||
| $allowedtags['input']['size'] = true; | ||
| $allowedtags['input']['checked'] = true; | ||
| $allowedtags['input']['onclick'] = true; | ||
| $allowedtags['input']['class'] = true; | ||
| $allowedtags['input']['placeholder'] = true; | ||
| $allowedtags['input']['autocomplete'] = true; | ||
|
|
||
| if ( ! isset( $allowedtags['select'] ) ) { | ||
| $allowedtags['select'] = array(); | ||
| } | ||
|
|
||
| $allowedtags['select']['name'] = true; | ||
| $allowedtags['select']['id'] = true; | ||
| $allowedtags['select']['disabled'] = true; | ||
| $allowedtags['select']['onchange'] = true; | ||
|
|
||
| if ( ! isset( $allowedtags['option'] ) ) { | ||
| $allowedtags['option'] = array(); | ||
| } | ||
|
|
||
| $allowedtags['option']['value'] = true; | ||
| $allowedtags['option']['selected'] = true; | ||
|
|
||
| if ( ! isset( $allowedtags['iframe'] ) ) { | ||
| $allowedtags['iframe'] = array(); | ||
| } | ||
|
|
||
| $allowedtags['iframe']['src'] = true; | ||
| $allowedtags['iframe']['width'] = true; | ||
| $allowedtags['iframe']['height'] = true; | ||
| $allowedtags['iframe']['frameborder'] = true; | ||
| $allowedtags['iframe']['allowfullscreen'] = true; | ||
| $allowedtags['iframe']['style'] = true; | ||
|
|
||
| if ( ! isset( $allowedtags['noscript'] ) ) { | ||
| $allowedtags['noscript'] = array(); | ||
| } | ||
|
|
||
| return $allowedtags; | ||
| } | ||
|
|
||
| /** | ||
| * Allow extra protocols to wp_kses_post() | ||
| */ | ||
| public function kses_allowed_protocols( $allowedprotocols ) { | ||
| $allowedprotocols[] = 'tel'; | ||
|
|
||
| return $allowedprotocols; | ||
| } | ||
|
|
||
| /** | ||
| * Allow extra style attributes to wp_kses_post() | ||
| */ | ||
| public function safe_style_css( $allowedstyles ) { | ||
| $allowedstyles[] = 'display'; | ||
| $allowedstyles[] = 'background-image'; | ||
|
|
||
| return $allowedstyles; | ||
| } |
There was a problem hiding this comment.
The wp_kses_allowed_html function is used to allow extra tags and attributes for the wp_kses_post() function. This is a good practice for security reasons, as it allows you to control what HTML elements and attributes are allowed. However, this list seems quite extensive and could potentially open up for vulnerabilities if not all of these tags and attributes are strictly necessary for your application. I would recommend reviewing this list and removing any tags or attributes that are not needed.
Also, it's important to note that allowing certain attributes like onclick (line 292) can potentially lead to XSS attacks if user input is not properly sanitized before being used in these attributes. Make sure that any data used in these attributes is properly sanitized before use.
- $allowedtags['input']['onclick'] = true;Lastly, there is no newline at the end of the file (line 348). While this won't cause any issues with PHP, it's generally considered good practice to end a file with a newline.
+ | */ | ||
| public $search_fields = false; | ||
|
|
||
| /** | ||
| * Holds the meal plan options | ||
| * | ||
| * @var array | ||
| */ | ||
| public $room_basis = array(); | ||
|
|
||
| /** | ||
| * Holds the meal plan options | ||
| * | ||
| * @var array | ||
| */ | ||
| public $drinks_basis = array(); | ||
|
|
||
| /** | ||
| * Initialize the plugin by setting localization, filters, and | ||
| * administration functions. |
There was a problem hiding this comment.
The removal of $room_basis and $drinks_basis arrays seems to be a significant change. Ensure that these arrays are not used elsewhere in the codebase, or this could lead to undefined variable errors. If they are used elsewhere, consider providing a fallback or handling for their absence.
| @@ -212,7 +212,7 @@ public function item_thumbnail() { | |||
| $thumbnail_src = false; | |||
| $thumbnail_src = apply_filters( 'lsx_to_accommodation_room_thumbnail', $thumbnail_src ); | |||
| if ( $this->have_query && false !== $this->query_item && ! empty( $this->query_item ) ) { | |||
| if ( false !== $this->query_item['gallery'] ) { | |||
| if ( isset( $this->query_item['gallery'] ) && ! empty( $this->query_item['gallery'] ) ) { | |||
There was a problem hiding this comment.
The new code checks if the 'gallery' key is present and not empty in $this->query_item before proceeding, which is a good practice to avoid potential PHP warnings or errors. This change improves error handling.
- if ( false !== $this->query_item['gallery'] ) {
+ if ( isset( $this->query_item['gallery'] ) && ! empty( $this->query_item['gallery'] ) ) {| @@ -234,7 +234,7 @@ | |||
| if ( $this->have_query && false !== $this->query_item ) { | |||
| $images_return = array(); | |||
|
|
|||
| if ( isset( $this->query_item['gallery'] ) && false !== $this->query_item['gallery'] ) { | |||
| if ( isset( $this->query_item['gallery'] ) && ! empty( $this->query_item['gallery'] ) ) { | |||
There was a problem hiding this comment.
Similar to the previous comment, this change also improves error handling by checking for the presence and non-emptiness of the 'gallery' key in $this->query_item.
- if ( isset( $this->query_item['gallery'] ) && false !== $this->query_item['gallery'] ) {
+ if ( isset( $this->query_item['gallery'] ) && ! empty( $this->query_item['gallery'] ) ) {| */ | ||
| function lsx_to_accommodation_reset_units_loop() { | ||
| global $rooms; | ||
|
|
||
| return $rooms->reset_loop(); | ||
| } |
There was a problem hiding this comment.
The removal of the SCP Order Uninstall hook and related functions in this hunk is a significant change. If these functions are no longer needed, this is fine. However, if they are still required, their removal could cause issues. Please ensure that this uninstall functionality is either not needed or implemented elsewhere.
+ function lsx_to_accommodation_reset_units_loop() {
+ global $rooms;
+ return $rooms->reset_loop();
+ }
There was a problem hiding this comment.
Files selected (1)
- includes/classes/legacy/class-accommodation.php (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
@coderabbitaiin 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: ignoreanywhere in the PR description to pause further reviews from the bot.
| $ratings_array[] = '<figure class="wp-block-image size-large is-resized">'; | ||
| $ratings_array[] = '<img src="'; | ||
| if ( (int) $value > 0 ) { | ||
| $ratings_array[] = '<i class="fa fa-star"></i>'; | ||
| $ratings_array[] = LSX_TO_URL . 'assets/img/rating-star-full.png'; | ||
| } else { | ||
| $ratings_array[] = '<i class="fa fa-star-o"></i>'; | ||
| $ratings_array[] = LSX_TO_URL . 'assets/img/rating-star-empty.png'; | ||
| } | ||
| $ratings_array[] = '" alt="" style="width:20px;vertical-align:sub;">'; | ||
| $ratings_array[] = '</figure>'; |
There was a problem hiding this comment.
The new code is using hardcoded image paths for the rating stars. This could be problematic if the images are moved or their names are changed. It would be better to define these paths as constants or configuration settings, so they can be easily updated in one place if necessary.
- $ratings_array[] = LSX_TO_URL . 'assets/img/rating-star-full.png';
+ $ratings_array[] = defined('RATING_STAR_FULL') ? RATING_STAR_FULL : LSX_TO_URL . 'assets/img/rating-star-full.png';
- $ratings_array[] = LSX_TO_URL . 'assets/img/rating-star-empty.png';
+ $ratings_array[] = defined('RATING_STAR_EMPTY') ? RATING_STAR_EMPTY : LSX_TO_URL . 'assets/img/rating-star-empty.png';| $rating_description = ' <small>(' . $rating_type . ')</small>'; | ||
| } | ||
| $html = $before . implode( '', $ratings_array ) . $rating_description . $after; | ||
| do_action( 'qm/debug', [ $html ] ); |
There was a problem hiding this comment.
The debug action do_action( 'qm/debug', [ $html ] ); seems to be left from debugging and should be removed for production code.
- do_action( 'qm/debug', [ $html ] );| $counter --; | ||
| $value --; | ||
| } |
There was a problem hiding this comment.
The decrement of $counter and $value variables is not protected by any condition. If $value becomes negative, it will still continue to decrement until $counter reaches 0. This might lead to unexpected behavior. Consider adding a condition to break the loop when $value is less than or equal to 0.
- $counter --;
- $value --;
+ if ($value > 0) {
+ $counter--;
+ $value--;
+ } else {
+ break;
+ }
There was a problem hiding this comment.
Files selected (2)
- includes/classes/legacy/class-accommodation.php (1)
- includes/template-tags/accommodation.php (2)
Files skipped from review due to trivial changes (1)
- includes/classes/legacy/class-accommodation.php
Review comments generated (2)
- Review: 2
- 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
@coderabbitaiin 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: ignoreanywhere in the PR description to pause further reviews from the bot.
| $args = []; | ||
| $facilities = wp_get_object_terms( get_the_ID(), 'facility' ); | ||
| $main_facilities = []; | ||
| $child_facilities = []; | ||
| $return = ''; |
There was a problem hiding this comment.
The refactoring of the variable initialization is a good improvement. It makes the code cleaner and more readable.
- $facilities = wp_get_object_terms( get_the_ID(), 'facility' );
- $main_facilities = array();
- $child_facilities = array();
- $return = '';
+ $args = [];
+ $facilities = wp_get_object_terms( get_the_ID(), 'facility' );
+ $main_facilities = [];
+ $child_facilities = [];
+ $return = '';| $return .= '<div class="' . $heading->slug . ' wp-block-columns is-layout-flex wp-block-columns-is-layout-flex">'; | ||
| foreach ( $main_facilities as $heading ) { | ||
| if ( isset( $child_facilities[ $heading->term_id ] ) ) { | ||
| $return .= '<div class="' . $heading->slug . ' col-xs-12 col-sm-6"><div class="facilities-content"><h5 class="facilities-title"><a href="' . esc_url( get_term_link( $heading->slug, 'facility' ) ) . '">' . esc_html( $heading->name ) . '</a></h5>'; | ||
| $return .= '<ul class="facilities-list">'; | ||
| $return .= '<div class="wp-block-column is-layout-flow wp-block-column-is-layout-flow"> | ||
| <p class="has-medium-font-size facilities-title wp-block-heading"><a href="' . esc_url( get_term_link( $heading->slug, 'facility' ) ) . '">' . esc_html( $heading->name ) . '</a></h5>'; | ||
| $return .= '<ul class="facilities-list wp-block-list">'; | ||
|
|
||
| foreach ( $child_facilities[ $heading->term_id ] as $child_facility ) { | ||
| $return .= '<li class="facility-item"><a href="' . esc_url( get_term_link( $child_facility->slug, 'facility' ) ) . '">' . esc_html( $child_facility->name ) . '</a></li>'; | ||
| } | ||
|
|
||
| $return .= '</ul>'; | ||
| $return .= '</div></div>'; | ||
| $return .= '</div>'; | ||
| } | ||
| } | ||
| $return .= '</div>'; |
There was a problem hiding this comment.
The changes made to the HTML structure and class names seem to be for styling purposes. However, there's a mismatch in the opening and closing tags on line 138. The opening tag is <p> but the closing tag is </h5>. This should be corrected to maintain proper HTML structure.
- $return .= '<div class="' . $heading->slug . ' col-xs-12 col-sm-6"><div class="facilities-content"><h5 class="facilities-title"><a href="' . esc_url( get_term_link( $heading->slug, 'facility' ) ) . '">' . esc_html( $heading->name ) . '</a></h5>';
+ $return .= '<div class="wp-block-column is-layout-flow wp-block-column-is-layout-flow">
+ <p class="has-medium-font-size facilities-title wp-block-heading"><a href="' . esc_url( get_term_link( $heading->slug, 'facility' ) ) . '">' . esc_html( $heading->name ) . '</a></h5>';
Description of the Change
This PR contains the following Bugherd updates.
Fixing the layout of the facilities BH-68
Fixing the rating image BH-51
Fixing the itinerary Image not showing. BH-45
Fixing the target issues #342
Fixing the gallery images BH-56
Fixing the drinks and room basis fields BH-87
Removing the unused frontend functions
Designs
Summary by CodeRabbit
Refactor:
build_slider_lightboxfunction to dynamically fetch room images and set up the lightbox.ratingfunction by using specific image paths for full and empty stars.lsx_to_accommodation_facilitiesfunction for improved code readability.Bug Fix:
query_itemarray.Chore:
New Feature:
query_args_filterfunction.