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

Plugin header updates #181

Merged
merged 4 commits into from
Dec 19, 2024
Merged

Plugin header updates #181

merged 4 commits into from
Dec 19, 2024

Conversation

ZaredRogers
Copy link
Contributor

@ZaredRogers ZaredRogers commented Dec 17, 2024

Description

Updated the plugin header to include the following:

  • Added plugin requirements for Tour Operator
  • Added various fields to the header with their values
  • Modified the tags to match TO tags

This PR solves #171 and #172

Summary by CodeRabbit

  • New Features

    • Updated CSS structure for the LSX Importer, enhancing styling and responsiveness.
    • Improved error handling during the import process for a more reliable user experience.
    • Added new methods to enhance functionality related to video data and team member associations.
  • Bug Fixes

    • Enhanced error handling for AJAX responses to prevent issues during import.
  • Documentation

    • Updated versioning information in the changelog, readme, and metadata files.
  • Chores

    • Removed unused class related to accommodation connections.

Added Tour Operator as a required plugin to the Importer header. This solves #171 

Signed-off-by: Zared Rogers <[email protected]>
Updated the Wetu Importer plugin header to include all necessary fields. This solves #171 and #172 

Signed-off-by: Zared Rogers <[email protected]>
Updated the tags in readme.txt

Signed-off-by: Zared Rogers <[email protected]>
@ZaredRogers ZaredRogers added this to the 2.0.0 milestone Dec 17, 2024
@ZaredRogers ZaredRogers requested a review from krugazul December 17, 2024 14:07
@ZaredRogers ZaredRogers self-assigned this Dec 17, 2024
Copy link

coderabbitai bot commented Dec 17, 2024

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.

Walkthrough

The pull request introduces significant updates to the LSX WETU Importer plugin, focusing on refactoring the codebase, updating metadata, and improving the plugin's structure. Changes span multiple files, including CSS, JavaScript, PHP classes, and configuration files. The modifications primarily involve updating class selectors, enhancing error handling, modifying URL parameters, and preparing for version 1.5.0 release. The changes aim to improve the plugin's functionality, maintainability, and compatibility with newer WordPress versions.

Changes

File Change Summary
.mergify.yml Removed head branch deletion rule
assets/css/lsx-wetu-importer.css Updated CSS selectors from body.tour-operator_page_lsx-wetu-importer to body .to-wrapper
assets/css/lsx-wetu-importer.scss Similar CSS selector modification
assets/js/lsx-wetu-importer.js Updated initialization and import logic
changelog.md Added version 1.5.0 entry
classes/* Multiple class modifications, including method signature updates and new methods
lsx-importer-for-wetu.php Updated plugin metadata and version
package.json Version bump to 1.5.0
readme.txt Updated tags and stable version

Poem

🐰 A Rabbit's Ode to Code Renewal

Selectors shift, like whiskers in the breeze,
Version numbers dance with elegant ease,
Importing data, smooth as carrot cake,
Each line of code, a journey we shall make!
Hop, hop, hurray for version one-point-five! 🚀


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 generate docstrings to generate docstrings for this PR. (Beta)
  • @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.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

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

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 14

🔭 Outside diff range comments (5)
classes/class-lsx-wetu-importer-settings.php (1)

Line range hint 507-507: Fix typo in accommodation sync description

The word "accodring" should be "according".

-                <p><?php esc_html_e( 'Update the accommodation images accodring to the schedule above.', 'lsx-wetu-importer' ); ?></p>
+                <p><?php esc_html_e( 'Update the accommodation images according to the schedule above.', 'lsx-wetu-importer' ); ?></p>
classes/class-lsx-wetu-importer-accommodation.php (1)

Line range hint 722-744: Review Use of sanitize_title Function

When saving custom fields for spoken_languages, suggested_visitor_types, and special_interests, using sanitize_title may alter the data (e.g., replacing spaces with hyphens). Consider using sanitize_text_field instead to preserve the original text.

Apply this diff to modify the sanitization function:

- $this->save_custom_field( sanitize_title( $spoken_language ), 'spoken_languages', $id, false, false );
+ $this->save_custom_field( sanitize_text_field( $spoken_language ), 'spoken_languages', $id, false, false );

And similarly for suggested_visitor_types and special_interests.

.mergify.yml (1)

Line range hint 1-19: Consider restoring the head branch deletion rule

Ag, I notice you've removed the rule for deleting head branches after merging. This might lead to cluttered branches in your repository, hey? The automatic branch cleanup is quite useful for maintaining a tidy repository.

Here's a suggestion to add back:

  - name: automatic merge for ImgBot pull requests
    conditions:
      - author=imgbot[bot]
    actions:
      merge:
        method: merge
+ - name: delete head branch on merged pull requests
+   conditions:
+     - merged
+   actions:
+     delete_head_branch: {}
🧰 Tools
🪛 yamllint (1.35.1)

[error] 19-19: no new line character at the end of file

(new-line-at-end-of-file)

classes/class-lsx-wetu-importer-banner-integration.php (1)

Line range hint 249-257: Improve error handling for image operations

The image handling code should include better error handling and logging for failed operations.

Consider adding try-catch blocks and logging:

+try {
   $thumbnail = wp_get_attachment_image_src( $banner_image, 'thumbnail' );
   if (false === $thumbnail) {
     throw new Exception('Failed to get thumbnail for image ID: ' . $banner_image);
   }
+} catch (Exception $e) {
+  error_log('LSX WETU Importer: ' . $e->getMessage());
+  continue;
+}
assets/js/lsx-wetu-importer.js (1)

Line range hint 229-238: Remove console.log and enhance error feedback

Sharp! The error handling could be improved:

  1. Remove the console.log statement as it's not suitable for production
  2. Add user-friendly error messages to inform users about import failures
 .fail( function( reason ) {
-    console.log($row.find('.check-column'));
     $row.find('.post-title').css('color','red');
     $row.find('.check-column input').attr('checked','');
     $row.find('.check-column input').removeClass('importing');
     $row.find('.check-column input').removeClass('queued');
     $row.find('.check-column input').show();
     $row.find('.check-column img').remove();
+    // Display error message to user
+    $row.find('.post-title').append('<span class="error-message">Import failed. Please try again.</span>');
 } );
📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between af357c7 and 93d0c5f.

⛔ Files ignored due to path filters (2)
  • .DS_Store is excluded by !**/.DS_Store
  • assets/css/lsx-wetu-importer.css.map is excluded by !**/*.map
📒 Files selected for processing (16)
  • .mergify.yml (1 hunks)
  • assets/css/lsx-wetu-importer.css (1 hunks)
  • assets/css/lsx-wetu-importer.scss (1 hunks)
  • assets/js/lsx-wetu-importer.js (1 hunks)
  • changelog.md (1 hunks)
  • classes/class-lsx-wetu-importer-accommodation.php (12 hunks)
  • classes/class-lsx-wetu-importer-banner-integration.php (1 hunks)
  • classes/class-lsx-wetu-importer-connect-accommodation.php (0 hunks)
  • classes/class-lsx-wetu-importer-destination.php (7 hunks)
  • classes/class-lsx-wetu-importer-settings.php (7 hunks)
  • classes/class-lsx-wetu-importer-tours.php (8 hunks)
  • classes/class-lsx-wetu-importer-welcome.php (4 hunks)
  • classes/class-lsx-wetu-importer.php (14 hunks)
  • lsx-importer-for-wetu.php (1 hunks)
  • package.json (1 hunks)
  • readme.txt (1 hunks)
💤 Files with no reviewable changes (1)
  • classes/class-lsx-wetu-importer-connect-accommodation.php
🧰 Additional context used
🪛 yamllint (1.35.1)
.mergify.yml

[error] 19-19: no new line character at the end of file

(new-line-at-end-of-file)

🪛 Markdownlint (0.37.0)
changelog.md

4-4: Expected: 0 or 2; Actual: 1
Trailing spaces

(MD009, no-trailing-spaces)


3-3: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


4-4: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

🪛 Biome (1.9.4)
assets/css/lsx-wetu-importer.css

[error] 56-56: Generic font family missing.

Consider adding a generic font family as a fallback.
For examples and more information, see the MDN Web Docs

(lint/a11y/useGenericFontNames)

🔇 Additional comments (36)
classes/class-lsx-wetu-importer-settings.php (3)

98-98: Added Tour Operator specific wrapper class

The addition of the 'to-wrapper' class aligns with Tour Operator styling requirements, maintaining consistency across the importer interface.


352-355: Improved form input formatting

The formatting changes enhance code readability whilst maintaining functionality:

  • Consistent spacing around PHP tags
  • Better alignment of conditional blocks

Also applies to: 364-368, 391-395, 403-407


Line range hint 50-65: Default settings optimised for Tour Operator compatibility

The changes to default values appear to be well-considered optimisations:

  • Increased image dimensions (1200x800) for better quality
  • Reduced image limit to 12 for improved performance
  • Changed scaling to 'raw' for original quality images

Let's verify these changes don't affect existing tours:

✅ Verification successful

Let me analyze the findings from the shell script results:

  1. The image dimensions and scaling settings are used throughout the codebase:
    • class-lsx-wetu-importer.php uses 800x600 for some images and 1920x600 for banners
    • class-lsx-wetu-importer-tours.php uses 1920x600 for tour banners
    • class-lsx-wetu-importer-banner-integration.php uses 1920x800 for banner images

Let me verify if these changes are backwards compatible by checking for any version-specific code:


Image dimension changes are safe to implement

The changes to default image dimensions and scaling settings are well-considered and backwards compatible:

  • The codebase has version checks but they're only for PHP compatibility
  • Image dimensions are handled dynamically through settings, not hardcoded
  • The WETU API integration uses dynamic URLs that adapt to configured dimensions
  • Existing banners use larger dimensions (1920x600/800) so the new 1200x800 default won't affect quality
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any hardcoded image dimensions or limits in the codebase
rg -g '!vendor' -g '*.php' '800|600|15|12|scaling' -C 2

Length of output: 13318


Script:

#!/bin/bash
# Check for version checks or upgrade routines
rg -g '!vendor' -g '*.php' 'upgrade|version|update.*options' -C 2

Length of output: 5071

classes/class-lsx-wetu-importer-destination.php (5)

93-93: Consistent Styling: Added 'to-wrapper' class

The addition of the 'to-wrapper' class to the <div> ensures consistent styling across the importer interface.


280-280: New Instruction Message is Clear

The added message instructing users to check the "draft" list for any countries is clear and helpful.


332-332: Consistent Data Processing

The call to prepare_row_attributes with $cs_key and $ccs_id ensures correct data handling.


481-481: Improved Error Handling for $jdata

The added check for is_wp_error( $jdata ) enhances the robustness of the code by ensuring that any WordPress error is properly handled before proceeding.


522-523: Consistent Method Signature Update

The method call to check_for_parent now includes the $id parameter, aligning with the updated method signature.

classes/class-lsx-wetu-importer-accommodation.php (9)

101-101: Consistent Styling: Added 'to-wrapper' class

The addition of the 'to-wrapper' class to the <div> ensures consistent styling across the importer interface.


213-213: Updated Completion Message with Correct Link

The updated message provides a link to import destinations next, improving user guidance.


265-265: Consistent Data Processing

The call to prepare_row_attributes with $cs_key and $ccs_id ensures correct data handling.


346-346: Set Default $row_key to Integer Zero

The default value for $row_key is now set to 0, ensuring it's an integer and preventing potential type errors.


361-361: Casting $row_key to Integer

Casting $row_key to an integer before incrementing ensures accurate calculation of the row number.


429-429: Improved Error Handling for $jdata

Adding the is_wp_error( $jdata ) check enhances error handling robustness.


610-610: Verify Parameters in set_destination Method Call

Confirm that passing 0 as the third parameter in set_destination( $data[0]['position'], $id, 0 ) is appropriate and aligns with the method's expected parameters.

Would you like to verify if the method set_destination correctly handles 0 as the third parameter?


659-673: Limit Room Images to Two

The implementation correctly limits the number of images per room to a maximum of two.


682-682: Store All Room Data in a Single Meta Entry

Saving all room data in a single meta entry under units improves data organization.

classes/class-lsx-wetu-importer-tours.php (3)

115-115: Consistent Styling: Added 'to-wrapper' class

The addition of the 'to-wrapper' class to the <div> enhances styling consistency across the importer interface.


223-223: Updated Completion Message with Correct Link

The updated message provides a direct link to import accommodation next, improving user navigation.


Line range hint 612-814: Optimised Itinerary Processing

Collecting itinerary days into the $days array and calling set_itinerary_day once after processing all legs improves performance and code clarity.

classes/class-lsx-wetu-importer.php (9)

87-93: Added $attachment_urls Property

The new property $attachment_urls stores previously attached images, enhancing image handling.


219-219: Increased Priority of admin_menu Action Hook

Changing the priority to 200 ensures that register_importer_page is called after other menu items are registered.


458-458: Updated Condition to Check import Parameter

Changing the condition to check $_GET['import'] enhances correct detection of the importer's context.


484-484: Consistent Styling: Added 'to-wrapper' class

Adding 'to-wrapper' class to the <div> ensures consistent styling across the importer interface.


606-612: Updated Navigation Links to Use import Parameter

Modifying navigation links to use the import parameter instead of page for consistency.


772-792: Enhanced save_custom_field Method

Adding default parameter values and handling of merged fields improves flexibility and functionality.


794-806: New save_merged_field Method for Merging Meta Fields

Introduction of the save_merged_field method allows for proper merging of meta fields when dealing with post connections.


Line range hint 1156-1181: Improved Image Attachment Logic

The revised attach_image method efficiently checks for existing attachments before adding new ones, preventing duplicates.


1325-1355: Implemented set_video_data Method

The new set_video_data method enhances video handling by saving YouTube video details.

package.json (1)

3-3: Verify version consistency across files

Ja, I see you've bumped the version to 1.5.0. Let's just make sure this matches everywhere, hey?

✅ Verification successful

Lekker, boet! The version numbers are all sorted

The version 1.5.0 is consistently used in the main package files (package.json and composer.lock). The 1.5.1 versions found in npm-shrinkwrap.json are for different dependencies (domutils and pumpify) and not related to your package version.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check version consistency across files
echo "Checking version numbers in various files..."

# Search for version strings
rg -i "version.*1\.5\.0?" --type php --type json

Length of output: 414


Script:

#!/bin/bash
# Let's check the specific context of those 1.5.1 versions in npm-shrinkwrap.json
echo "Checking npm-shrinkwrap.json for version context..."
rg -A 2 -B 2 "version.*1\.5\.1" npm-shrinkwrap.json

Length of output: 660

lsx-importer-for-wetu.php (1)

4-18: LGTM: Header updates look proper

Sharp! The plugin header updates look lekker. The new URIs, license details, and plugin requirements are all properly specified. Nice work on adding the Tour Operator requirement - that's going to help users understand the dependencies better.

readme.txt (1)

4-4: Tag updates and version bump look good!

The updated tags better reflect the tourism focus of the plugin, and the version increment to 1.5.0 is consistent with the changelog.

Also applies to: 8-8

assets/css/lsx-wetu-importer.scss (1)

2-2: Selector change improves reusability

The change from a page-specific selector to .to-wrapper makes the styles more modular and reusable across different pages.

Let's verify the usage of the new wrapper class:

✅ Verification successful

Selector change is correctly implemented and used consistently

The change from a page-specific selector to body .to-wrapper is well-implemented across the codebase. The wrapper class is consistently used in:

  • All PHP template files for page rendering
  • JavaScript for DOM manipulation
  • SCSS/CSS for styling the admin interface components

The new selector maintains proper specificity while making the styles more modular.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for the new wrapper class implementation
rg -l "to-wrapper"

Length of output: 392


Script:

#!/bin/bash
# Check the actual usage context of to-wrapper class
rg "to-wrapper" -B2 -A2

Length of output: 15836

classes/class-lsx-wetu-importer-welcome.php (1)

70-79: Verify URL parameter changes across the application

The change from page=lsx-wetu-importer to import=lsx-wetu-importer needs verification to ensure all parts of the application handle this change correctly.

✅ Verification successful

Let me gather more comprehensive information about URL parameter usage in the codebase.


URL parameter change is consistently implemented

The search results show that the new import=lsx-wetu-importer parameter is used consistently throughout the codebase, with no instances of the old page=lsx-wetu-importer parameter remaining. All URL constructions in the main importer classes and welcome pages are using the new parameter format correctly.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining instances of the old URL parameter
echo "Checking for old URL parameter usage:"
rg -l "page=lsx-wetu-importer"

Length of output: 115


Script:

#!/bin/bash
# Search for any URL construction with admin.php and parameters
echo "Checking URL parameter patterns in PHP files:"
rg -A 2 "admin_url.*\?.*=" --type php

# Search for form actions or redirects that might use these parameters
echo -e "\nChecking form and redirect patterns:"
rg -A 2 "(action|redirect).*=(.*import|.*page).*lsx-wetu" --type php

# Search specifically for the new parameter format
echo -e "\nChecking new URL parameter usage:"
rg -A 2 "import=lsx-wetu-importer" --type php

Length of output: 17080

classes/class-lsx-wetu-importer-banner-integration.php (1)

38-38: Verify CSS class addition across templates

The addition of to-wrapper class should be consistent across all admin templates.

✅ Verification successful

CSS class to-wrapper is consistently used across admin templates

The verification shows that the to-wrapper class is consistently used in all relevant admin template files within the classes directory, including:

  • accommodation importer
  • banner integration
  • destination importer
  • settings
  • tours importer
  • main WETU importer
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for consistent usage of to-wrapper class
echo "Checking for to-wrapper class usage:"
rg -l "class=\"wrap to-wrapper\""

Length of output: 392

assets/js/lsx-wetu-importer.js (1)

7-7: Verify the existence of the .to-wrapper class

Howzit! The condition change from checking the body class to finding elements with '.wrap.to-wrapper' needs verification to ensure the wrapper class is consistently implemented across all relevant templates.

✅ Verification successful

The .to-wrapper class implementation is consistent and valid

Howzit! The condition change in the JavaScript file is perfectly valid. The search results confirm that the '.wrap.to-wrapper' class combination is consistently implemented across all relevant PHP template files in the WETU importer, including:

  • Settings page
  • Tours page
  • Destinations page
  • Banner integration page
  • Accommodation page
  • Main importer page
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for .to-wrapper class implementation in PHP templates
rg -t php "to-wrapper" 

Length of output: 490

Comment on lines 391 to 395
<input placeholder="1200" type="text" value="<?php
if ( isset( $options['width'] ) && '' !== $options['width'] ) {
echo esc_attr( $options['width'] );
}
?>
"
name="width" />
?>" name="width" />
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add numeric validation for dimension inputs

The width and height inputs should validate numeric values to prevent invalid dimensions.

-                <input placeholder="1200" type="text" value="<?php
+                <input placeholder="1200" type="number" min="1" max="2400" value="<?php
-                <input placeholder="800" type="text" value="<?php
+                <input placeholder="800" type="number" min="1" max="1600" value="<?php

Also applies to: 403-407


/**
* Run through the accommodation grabbed from the DB.
* Run through the accommodation grab d from the DB.
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Typo in Comment: Correct 'grab d' to 'grabbed'

There's a typographical error in the comment. The word 'grab d' contains an unintended tab character.

Apply this diff to fix the typo:

- * Run through the accommodation grab	d from the DB.
+ * Run through the accommodation grabbed from the DB.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
* Run through the accommodation grab d from the DB.
* Run through the accommodation grabbed from the DB.

Comment on lines 698 to 704
public function check_for_parent( $data = array(), $id ) {
$country_id = 0;

if ( $data[0]['position']['country_content_entity_id'] !== $data[0]['position']['destination_content_entity_id'] ) {
$result = $wpdb->get_var( $wpdb->prepare( "SELECT post_id FROM $wpdb->postmeta WHERE meta_key = 'lsx_wetu_id' AND meta_value = '%s'", array( $data[0]['position']['country_content_entity_id'] ) ) );
if ( ! empty( $result ) && '' !== $result && false !== $result ) {
return $result;
}
$country_id = $this->set_country( $data[0]['position']['country_content_entity_id'], $id );
}
return false;
return $country_id;
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add Checks for Undefined Array Keys in check_for_parent Method

In the check_for_parent method, it's recommended to verify that country_content_entity_id exists in $data[0]['position'] before accessing it to prevent undefined index notices.

Apply this diff to enhance the code:

 public function check_for_parent( $data = array(), $id ) {
     $country_id = 0;

-    if ( $data[0]['position']['country_content_entity_id'] !== $data[0]['position']['destination_content_entity_id'] ) {
+    if ( isset( $data[0]['position']['country_content_entity_id'] ) && $data[0]['position']['country_content_entity_id'] !== $data[0]['position']['destination_content_entity_id'] ) {
         $country_id = $this->set_country( $data[0]['position']['country_content_entity_id'], $id );
     }
     return $country_id;
 }

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines 254 to 260
value="personal"><?php esc_html_e( 'Personal', 'lsx-wetu-importer' ); ?></option>
<option
<?php
if ( in_array( 'sample', $form_options ) ) {
echo esc_attr( 'selected="selected"' ); }
?>
value="sample"><?php esc_html_e( 'Sample', 'lsx-wetu-importer' ); ?></option>
value="sample"><?php esc_html_e( 'Sample', 'lsx-wetu-importer' ); ?></option>
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Simplify selected Attribute Handling in <option> Tags

The current formatting of the <option> tags with the selected attribute can be improved for better readability and maintainability.

Suggest modifying the code to set the selected attribute directly within the <option> tag.

Apply this diff:

- <option 
- <?php
- if ( in_array( 'personal', $form_options ) ) {
-     echo esc_attr( 'selected="selected"' ); }
- ?>
- value="personal"><?php esc_html_e( 'Personal', 'lsx-wetu-importer' ); ?></option>
+ <option value="personal" <?php selected( in_array( 'personal', $form_options ) ); ?>><?php esc_html_e( 'Personal', 'lsx-wetu-importer' ); ?></option>

Similarly for the 'Sample' option.

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines 436 to 443
//add_submenu_page( 'tools.php', esc_html__( 'WETU Importer', 'tour-operator' ), esc_html__( 'WETU Importer', 'tour-operator' ), 'manage_options', 'lsx-wetu-importer', array( $this, 'display_page' ) );

register_importer(
'lsx-wetu-importer', // A unique slug for your importer.
'TO WETU Importer', // The name of the importer as it appears on the Tools -> Import page.
'Import your tour itineraries from WETU and display them using the Tour Operator plugin.', // A brief description of the importer.
array( $this, 'display_page' ) // The callback function that handles the importing process.
);
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Switch to register_importer for Importer Registration

Replacing add_submenu_page with register_importer aligns with WordPress standards for registering importers.

The use of register_importer makes the importer accessible from the Tools → Import page, improving user accessibility.

changelog.md Outdated
Comment on lines 3 to 5
## [[1.5.0]](https://github.com/lightspeeddevelopment/wetu-importer/releases/tag/1.5.0) - IN Development
-

Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Fix markdown formatting

There are some markdown formatting issues that should be addressed:

  • Add a blank line before the version heading
  • Remove trailing spaces

Apply this diff:

 # Change log
+
 ## [[1.5.0]](https://github.com/lightspeeddevelopment/wetu-importer/releases/tag/1.5.0) - IN Development
-
+### Updated
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
## [[1.5.0]](https://github.com/lightspeeddevelopment/wetu-importer/releases/tag/1.5.0) - IN Development
-
# Change log
## [[1.5.0]](https://github.com/lightspeeddevelopment/wetu-importer/releases/tag/1.5.0) - IN Development
### Updated
🧰 Tools
🪛 Markdownlint (0.37.0)

4-4: Expected: 0 or 2; Actual: 1
Trailing spaces

(MD009, no-trailing-spaces)


3-3: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


4-4: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


🧹 Nitpick (assertive)

Changelog needs content for version 1.5.0

The new version entry is empty. Please add details about the plugin header updates and any other changes included in this release.

Consider this format:

 ## [[1.5.0]](https://github.com/lightspeeddevelopment/wetu-importer/releases/tag/1.5.0) - IN Development
-
+### Updated
+- Updated plugin header with Tour Operator requirements
+- Modified plugin tags to better reflect tourism focus
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
## [[1.5.0]](https://github.com/lightspeeddevelopment/wetu-importer/releases/tag/1.5.0) - IN Development
-
## [[1.5.0]](https://github.com/lightspeeddevelopment/wetu-importer/releases/tag/1.5.0) - IN Development
### Updated
- Updated plugin header with Tour Operator requirements
- Modified plugin tags to better reflect tourism focus
🧰 Tools
🪛 Markdownlint (0.37.0)

4-4: Expected: 0 or 2; Actual: 1
Trailing spaces

(MD009, no-trailing-spaces)


3-3: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


4-4: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

Comment on lines 56 to 57
font-family: dashicons;
text-decoration: inherit;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add fallback font family for better accessibility

The dashicons font family should include a fallback generic font family.

Apply this diff:

-  font-family: dashicons;
+  font-family: dashicons, sans-serif;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
font-family: dashicons;
text-decoration: inherit;
font-family: dashicons, sans-serif;
text-decoration: inherit;
🧰 Tools
🪛 Biome (1.9.4)

[error] 56-56: Generic font family missing.

Consider adding a generic font family as a fallback.
For examples and more information, see the MDN Web Docs

(lint/a11y/useGenericFontNames)

Comment on lines 181 to 195
@media (max-width: 1670px) {
body .to-wrapper .welcome-block.postbox {
width: calc(24% - 26px);
}
}
@media (max-width: 1272px) {
body .to-wrapper .welcome-block.postbox {
width: calc(24% - 27px);
position: relative;
z-index: 2; }
body.tour-operator_page_lsx-wetu-importer .welcome-block-header a:focus {
box-shadow: none; }
body.tour-operator_page_lsx-wetu-importer .welcome-block-header .heading a {
text-decoration: none; }
body.tour-operator_page_lsx-wetu-importer .welcome-block-header .value span {
display: block;
color: white;
background-color: #0073aa;
padding: 10px;
border-radius: 25px 25px 25px 25px;
width: 20px;
height: 20px;
display: inline-block; }
body.tour-operator_page_lsx-wetu-importer .welcome-block-header .value a:hover span {
background-color: #00a0d2; }
body.tour-operator_page_lsx-wetu-importer .spacer {
box-sizing: border-box;
width: 98%;
border-top: 1px solid #979797;
opacity: 0.33;
margin: -43px auto 0 auto;
position: absolute;
bottom: 40px;
left: 1%;
z-index: 1; }
body.tour-operator_page_lsx-wetu-importer .welcome-block.postbox {
width: calc(24% - 25px); }
@media (max-width: 1670px) {
body.tour-operator_page_lsx-wetu-importer .welcome-block.postbox {
width: calc(24% - 26px); } }
@media (max-width: 1272px) {
body.tour-operator_page_lsx-wetu-importer .welcome-block.postbox {
width: calc(24% - 27px); } }
@media (max-width: 690px) {
body.tour-operator_page_lsx-wetu-importer .welcome-block.postbox {
width: calc(48% - 27px); } }
body.tour-operator_page_lsx-wetu-importer .welcome-block.postbox h2.title {
height: 2em;
line-height: 28px;
margin: 0 0 2em 0; }
@media (max-width: 910px) {
body.tour-operator_page_lsx-wetu-importer .welcome-block.postbox h2.title {
height: 3em; } }
body.tour-operator_page_lsx-wetu-importer .welcome-block.postbox .excerpt {
min-height: 100px; }
body.tour-operator_page_lsx-wetu-importer .welcome-block.postbox .button {
width: 100%;
text-align: center; }
body.tour-operator_page_lsx-wetu-importer .welcome-block.postbox .link-list li a {
font-size: 14px; }
body.tour-operator_page_lsx-wetu-importer .wetu-status h3 {
margin: 0px; }
body.tour-operator_page_lsx-wetu-importer .subsubsub {
margin-bottom: 10px; }
body.tour-operator_page_lsx-wetu-importer .documentation {
list-style: none;
margin: 11px 0 0;
padding: 0;
font-size: 13px;
float: left;
text-decoration: none; }
body.tour-operator_page_lsx-wetu-importer .documentation:before {
content: "|";
color: #666;
margin-right: 6px;
margin-left: 6px; }
body.tour-operator_page_lsx-wetu-importer #lsx-wetu-importer-search-form .normal-search {
display: inline-block; }
body.tour-operator_page_lsx-wetu-importer #lsx-wetu-importer-search-form .normal-search input.keyword {
height: 30px; }
body.tour-operator_page_lsx-wetu-importer .advanced-search-toggle {
display: inline-block;
margin-top: 0; }
body.tour-operator_page_lsx-wetu-importer .widefat .check-column {
padding: 6px 0 6px;
vertical-align: middle; }
body.tour-operator_page_lsx-wetu-importer .widefat #the-list .check-column {
vertical-align: top;
padding: 10px 0 6px; }
body.tour-operator_page_lsx-wetu-importer #import-list .widefat .check-column {
vertical-align: top;
padding: 10px 0 6px; }
}
}
@media (max-width: 690px) {
body .to-wrapper .welcome-block.postbox {
width: calc(48% - 27px);
}
}
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Review media query breakpoints for consistency

The breakpoints (1670px, 1272px, 690px) seem arbitrary. Consider standardising these to common device widths.

Consider using standard breakpoints:

-@media (max-width: 1670px)
+@media (max-width: 1600px)

-@media (max-width: 1272px)
+@media (max-width: 1200px)

-@media (max-width: 690px)
+@media (max-width: 768px)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@media (max-width: 1670px) {
body .to-wrapper .welcome-block.postbox {
width: calc(24% - 26px);
}
}
@media (max-width: 1272px) {
body .to-wrapper .welcome-block.postbox {
width: calc(24% - 27px);
position: relative;
z-index: 2; }
body.tour-operator_page_lsx-wetu-importer .welcome-block-header a:focus {
box-shadow: none; }
body.tour-operator_page_lsx-wetu-importer .welcome-block-header .heading a {
text-decoration: none; }
body.tour-operator_page_lsx-wetu-importer .welcome-block-header .value span {
display: block;
color: white;
background-color: #0073aa;
padding: 10px;
border-radius: 25px 25px 25px 25px;
width: 20px;
height: 20px;
display: inline-block; }
body.tour-operator_page_lsx-wetu-importer .welcome-block-header .value a:hover span {
background-color: #00a0d2; }
body.tour-operator_page_lsx-wetu-importer .spacer {
box-sizing: border-box;
width: 98%;
border-top: 1px solid #979797;
opacity: 0.33;
margin: -43px auto 0 auto;
position: absolute;
bottom: 40px;
left: 1%;
z-index: 1; }
body.tour-operator_page_lsx-wetu-importer .welcome-block.postbox {
width: calc(24% - 25px); }
@media (max-width: 1670px) {
body.tour-operator_page_lsx-wetu-importer .welcome-block.postbox {
width: calc(24% - 26px); } }
@media (max-width: 1272px) {
body.tour-operator_page_lsx-wetu-importer .welcome-block.postbox {
width: calc(24% - 27px); } }
@media (max-width: 690px) {
body.tour-operator_page_lsx-wetu-importer .welcome-block.postbox {
width: calc(48% - 27px); } }
body.tour-operator_page_lsx-wetu-importer .welcome-block.postbox h2.title {
height: 2em;
line-height: 28px;
margin: 0 0 2em 0; }
@media (max-width: 910px) {
body.tour-operator_page_lsx-wetu-importer .welcome-block.postbox h2.title {
height: 3em; } }
body.tour-operator_page_lsx-wetu-importer .welcome-block.postbox .excerpt {
min-height: 100px; }
body.tour-operator_page_lsx-wetu-importer .welcome-block.postbox .button {
width: 100%;
text-align: center; }
body.tour-operator_page_lsx-wetu-importer .welcome-block.postbox .link-list li a {
font-size: 14px; }
body.tour-operator_page_lsx-wetu-importer .wetu-status h3 {
margin: 0px; }
body.tour-operator_page_lsx-wetu-importer .subsubsub {
margin-bottom: 10px; }
body.tour-operator_page_lsx-wetu-importer .documentation {
list-style: none;
margin: 11px 0 0;
padding: 0;
font-size: 13px;
float: left;
text-decoration: none; }
body.tour-operator_page_lsx-wetu-importer .documentation:before {
content: "|";
color: #666;
margin-right: 6px;
margin-left: 6px; }
body.tour-operator_page_lsx-wetu-importer #lsx-wetu-importer-search-form .normal-search {
display: inline-block; }
body.tour-operator_page_lsx-wetu-importer #lsx-wetu-importer-search-form .normal-search input.keyword {
height: 30px; }
body.tour-operator_page_lsx-wetu-importer .advanced-search-toggle {
display: inline-block;
margin-top: 0; }
body.tour-operator_page_lsx-wetu-importer .widefat .check-column {
padding: 6px 0 6px;
vertical-align: middle; }
body.tour-operator_page_lsx-wetu-importer .widefat #the-list .check-column {
vertical-align: top;
padding: 10px 0 6px; }
body.tour-operator_page_lsx-wetu-importer #import-list .widefat .check-column {
vertical-align: top;
padding: 10px 0 6px; }
}
}
@media (max-width: 690px) {
body .to-wrapper .welcome-block.postbox {
width: calc(48% - 27px);
}
}
@media (max-width: 1600px) {
body .to-wrapper .welcome-block.postbox {
width: calc(24% - 26px);
}
}
@media (max-width: 1200px) {
body .to-wrapper .welcome-block.postbox {
width: calc(24% - 27px);
}
}
@media (max-width: 768px) {
body .to-wrapper .welcome-block.postbox {
width: calc(48% - 27px);
}
}

Comment on lines 154 to 166
body .to-wrapper .welcome-block-header .value span {
display: block;
color: white;
background-color: #0073aa;
padding: 10px;
border-radius: 25px 25px 25px 25px;
width: 20px;
height: 20px;
display: inline-block;
}
body .to-wrapper .welcome-block-header .value a:hover span {
background-color: #00a0d2;
}
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

White text contrast needs improvement for accessibility

The current color combinations do not meet WCAG 2.1 Level AA requirements:

  • White text on #0073aa background has a contrast ratio of 4.47:1 (fails AA level for small text)
  • White text on #00a0d2 hover state has a contrast ratio of 3.79:1 (fails AA level)

Recommendations:

  • Change background color to #005B87 for normal state (contrast ratio 7:1)
  • Change hover state to #004C70 (contrast ratio 8:1)
🔗 Analysis chain

Verify colour contrast ratios for accessibility

The white text on #0073aa background and hover state #00a0d2 should be tested for WCAG compliance.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check colour contrast ratios using WCAG guidelines
# Note: This would typically use a colour contrast analyzer tool
echo "Background colors found in CSS:"
rg -A 1 'background-color:' | grep -E '#[0-9a-fA-F]{6}'

Length of output: 29080

Comment on lines 126 to 128
<li><a href="<?php echo esc_attr( admin_url( 'admin.php' ) . '?import=lsx-wetu-importer&tab=tour' ); ?>#publish"><?php esc_html_e( 'Published', 'lsx-wetu-importer' ); ?> (<?php echo esc_attr( lsx_wetu_get_post_count( 'tour', 'publish ' ) ); ?>)</a></li>
<li><a href="<?php echo esc_attr( admin_url( 'admin.php' ) . '?import=lsx-wetu-importer&tab=tour' ); ?>#pending"><?php esc_html_e( 'Pending', 'lsx-wetu-importer' ); ?> (<?php echo esc_attr( lsx_wetu_get_post_count( 'tour', 'pending ' ) ); ?>)</a></li>
<li><a href="<?php echo esc_attr( admin_url( 'admin.php' ) . '?import=lsx-wetu-importer&tab=tour' ); ?>#draft"><?php esc_html_e( 'Draft', 'lsx-wetu-importer' ); ?> (<?php echo esc_attr( lsx_wetu_get_post_count( 'tour', 'draft ' ) ); ?>)</a></li>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove trailing spaces in status parameters

The lsx_wetu_get_post_count() calls contain trailing spaces in the status parameters which might cause unexpected behaviour.

Apply this diff:

-lsx_wetu_get_post_count( 'tour', 'publish ' )
+lsx_wetu_get_post_count( 'tour', 'publish' )
-lsx_wetu_get_post_count( 'tour', 'pending ' )
+lsx_wetu_get_post_count( 'tour', 'pending' )
-lsx_wetu_get_post_count( 'tour', 'draft ' )
+lsx_wetu_get_post_count( 'tour', 'draft' )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<li><a href="<?php echo esc_attr( admin_url( 'admin.php' ) . '?import=lsx-wetu-importer&tab=tour' ); ?>#publish"><?php esc_html_e( 'Published', 'lsx-wetu-importer' ); ?> (<?php echo esc_attr( lsx_wetu_get_post_count( 'tour', 'publish ' ) ); ?>)</a></li>
<li><a href="<?php echo esc_attr( admin_url( 'admin.php' ) . '?import=lsx-wetu-importer&tab=tour' ); ?>#pending"><?php esc_html_e( 'Pending', 'lsx-wetu-importer' ); ?> (<?php echo esc_attr( lsx_wetu_get_post_count( 'tour', 'pending ' ) ); ?>)</a></li>
<li><a href="<?php echo esc_attr( admin_url( 'admin.php' ) . '?import=lsx-wetu-importer&tab=tour' ); ?>#draft"><?php esc_html_e( 'Draft', 'lsx-wetu-importer' ); ?> (<?php echo esc_attr( lsx_wetu_get_post_count( 'tour', 'draft ' ) ); ?>)</a></li>
<li><a href="<?php echo esc_attr( admin_url( 'admin.php' ) . '?import=lsx-wetu-importer&tab=tour' ); ?>#publish"><?php esc_html_e( 'Published', 'lsx-wetu-importer' ); ?> (<?php echo esc_attr( lsx_wetu_get_post_count( 'tour', 'publish' ) ); ?>)</a></li>
<li><a href="<?php echo esc_attr( admin_url( 'admin.php' ) . '?import=lsx-wetu-importer&tab=tour' ); ?>#pending"><?php esc_html_e( 'Pending', 'lsx-wetu-importer' ); ?> (<?php echo esc_attr( lsx_wetu_get_post_count( 'tour', 'pending' ) ); ?>)</a></li>
<li><a href="<?php echo esc_attr( admin_url( 'admin.php' ) . '?import=lsx-wetu-importer&tab=tour' ); ?>#draft"><?php esc_html_e( 'Draft', 'lsx-wetu-importer' ); ?> (<?php echo esc_attr( lsx_wetu_get_post_count( 'tour', 'draft' ) ); ?>)</a></li>

@krugazul krugazul changed the base branch from master to block-enhancements December 17, 2024 14:35
@krugazul krugazul merged commit 05fda6c into block-enhancements Dec 19, 2024
3 of 5 checks passed
@krugazul krugazul deleted the plugin-header-updates branch December 19, 2024 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants