🔨 chore: Mozart integration and update Appsero for WP 6.7 compatibility #48
Conversation
WalkthroughThe changes in this pull request primarily involve the addition of new classes and modifications to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (13)
composer.json (1)
38-44: Consider adding error handling to the scripts.The commands are correctly structured but could benefit from error handling.
"post-install-cmd": [ - "[ $COMPOSER_DEV_MODE -eq 0 ] || \"vendor/bin/mozart\" compose", + "[ $COMPOSER_DEV_MODE -eq 0 ] || (\"vendor/bin/mozart\" compose || exit 1)", "composer dump-autoload" ], "post-update-cmd": [ - "[ $COMPOSER_DEV_MODE -eq 0 ] || \"vendor/bin/mozart\" compose", + "[ $COMPOSER_DEV_MODE -eq 0 ] || (\"vendor/bin/mozart\" compose || exit 1)", "composer dump-autoload" ]texty.php (1)
147-150: Consider security and privacy improvements.A few suggestions for enhancing security and privacy:
- Move the client ID to a configuration file or environment variable to follow security best practices
- Add explicit user consent handling before initializing tracking
- Consider adding a privacy policy notice about data collection
Example implementation:
public function appsero_init() { - $client = new Texty\Dependencies\Appsero\Client( 'd4c17b0f-8f01-4b95-a8de-42b0641eec9a', 'Texty', __FILE__ ); + $client_id = defined('TEXTY_APPSERO_ID') ? TEXTY_APPSERO_ID : 'd4c17b0f-8f01-4b95-a8de-42b0641eec9a'; + $client = new Texty\Dependencies\Appsero\Client($client_id, 'Texty', __FILE__); - // Active insights - $client->insights()->init(); + // Initialize insights only if user has opted in + if (get_option('texty_tracking_allowed', false)) { + $client->insights()->init(); + } }dependencies/Appsero/Client.php (4)
115-128: Consider relying on autoloading instead of manual class inclusion.In the
insights()method, manually includingInsights.phpwithrequire_oncemight be unnecessary if autoloading is properly configured. Utilizing autoloading enhances maintainability and reduces the need for explicit file inclusions.Apply this diff to remove the manual inclusion:
- if ( ! class_exists( __NAMESPACE__ . '\Insights' ) ) { - require_once __DIR__ . '/Insights.php'; - }
155-168: Leverage autoloading by removing explicit class inclusion.Similarly, in the
license()method, the manual inclusion ofLicense.phpmay be redundant if autoloading is in place. Relying on autoloading simplifies class management.Apply this diff to eliminate the manual inclusion:
- if ( ! class_exists( __NAMESPACE__ . '\License' ) ) { - require_once __DIR__ . '/License.php'; - }
215-219: Document all parameters in thesend_requestmethod's DocBlock.The
$blockingparameter is currently undocumented. Adding it to the DocBlock enhances clarity for developers using or maintaining the code.Apply this diff to update the DocBlock:
* @param array $params * @param string $route + * @param bool $blocking Whether the request should be blocking. * * @return array|WP_Error array of results including HTTP headers or WP_Error if the request failed
141-148: Ensure WordPress functions are available before usage.The
_deprecated_function()call assumes a WordPress environment. If this code might run outside WordPress, wrapping the call with a function check prevents potential errors.Apply this diff to add a function existence check:
- _deprecated_function( __CLASS__ . '::updater', '2.0', '\Texty\Dependencies\Appsero\Updater::init($client);, for more details please visit: https://appsero.com/docs/appsero-developers-guide/appsero-client/appsero-sdk-updater-changes/' ); + if ( function_exists( '_deprecated_function' ) ) { + _deprecated_function( __CLASS__ . '::updater', '2.0', '\Texty\Dependencies\Appsero\Updater::init($client);, for more details please visit: https://appsero.com/docs/appsero-developers-guide/appsero-client/appsero-sdk-updater-changes/' ); + }dependencies/Appsero/License.php (4)
319-319: Typographical Error: Correct 'vefification' to 'verification'There's a typo in the error message 'Nonce vefification failed.'; 'vefification' should be corrected to 'verification'.
Apply the following diff to fix the typo:
-$this->error = $this->client->__trans( 'Nonce vefification failed.' ); +$this->error = $this->client->__trans( 'Nonce verification failed.' );
290-292: Correct CSS Class Name: Use 'deactivate-button'Update the CSS class from 'deactive-button' to 'deactivate-button' in the button element.
Apply this diff to update the class name:
-<button type="submit" name="submit" class="<?php echo 'deactive' === $action ? 'deactive-button' : ''; ?>"> +<button type="submit" name="submit" class="<?php echo 'deactivate' === $action ? 'deactivate-button' : ''; ?>"> <?php echo $action === 'active' ? $this->client->__trans( 'Activate License' ) : $this->client->__trans( 'Deactivate License' ); ?> </button>
498-501: Update CSS Selector to '.deactivate-button'Rename the CSS selector from '.deactive-button' to '.deactivate-button' to match the updated class name.
Apply this diff to update the CSS selector:
-.license-input-fields button.deactive-button { +.license-input-fields button.deactivate-button { background-color: #E40055; border-color: #E40055; }
410-541: Code Organization: Move Inline CSS to an External StylesheetConsider moving the inline CSS in the
licenses_stylemethod to an external stylesheet and enqueue it properly. This improves maintainability and adheres to best practices by separating presentation from logic.dependencies/Appsero/Insights.php (3)
1048-1062: Avoid dependency on external service inget_user_ip_addressmethodRelying on an external service (
icanhazip.com) to retrieve the server's IP address can lead to failures or delays if the service is unreachable. Consider using$_SERVER['SERVER_ADDR']or$_SERVER['REMOTE_ADDR']to obtain the IP address locally, enhancing reliability and performance.Suggested refactor:
- $response = wp_remote_get( 'https://icanhazip.com/' ); - if ( is_wp_error( $response ) ) { - return ''; - } - $ip = trim( wp_remote_retrieve_body( $response ) ); - if ( ! filter_var( $ip, FILTER_VALIDATE_IP ) ) { - return ''; - } + $ip = isset( $_SERVER['SERVER_ADDR'] ) ? $_SERVER['SERVER_ADDR'] : ''; + if ( ! filter_var( $ip, FILTER_VALIDATE_IP ) ) { + $ip = ''; + } return $ip;
658-694: Avoid directly including core WordPress filesIn the
get_all_plugins()method, directly includingplugin.phpusinginclude ABSPATH . '/wp-admin/includes/plugin.php';can lead to issues if paths change or in different environments. Instead, consider usingrequire_onceand checking if the function exists beforehand.Suggested change:
if ( ! function_exists( 'get_plugins' ) ) { - include ABSPATH . '/wp-admin/includes/plugin.php'; + require_once ABSPATH . 'wp-admin/includes/plugin.php'; }
705-711: Consider checking for empty roles inget_user_countsmethodIn the
get_user_counts()method, roles with zero count are skipped. If the intention is to include all roles, even those with zero users, consider adjusting the condition.foreach ( $user_count_data['avail_roles'] as $role => $count ) { - if ( ! $count ) { - continue; - } $user_count[ $role ] = $count; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
composer.json(2 hunks)dependencies/Appsero/Client.php(1 hunks)dependencies/Appsero/Insights.php(1 hunks)dependencies/Appsero/License.php(1 hunks)texty.php(1 hunks)
🔇 Additional comments (6)
composer.json (3)
28-28: LGTM! Version constraint is appropriate.
Using ^0.7.1 allows for compatible updates while maintaining stability.
✅ Verification successful
Version constraint ^0.7.1 is appropriate and matches the latest stable release
The package version specified in composer.json (^0.7.1) correctly targets the latest stable release of coenjacobs/mozart, which is indeed 0.7.1. The caret operator will allow updates to compatible minor versions while preventing breaking changes from major versions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check latest Mozart version and any reported issues
curl -s https://packagist.org/packages/coenjacobs/mozart.json | jq -r '.package.versions | keys[0]'
Length of output: 107
Script:
#!/bin/bash
# Let's verify the version information more thoroughly
curl -s https://packagist.org/packages/coenjacobs/mozart.json | jq -r '.package.versions | keys | .[]' | sort -V
Length of output: 291
19-20: LGTM! Verify the directory structure.
The PSR-4 autoloading configuration is correctly set up for both main namespace and dependencies.
✅ Verification successful
✓ Directory structure verified and properly configured
The PSR-4 autoloading configuration in composer.json correctly maps to existing directories:
"Texty\\": "includes/"maps to theincludes/directory which contains the main plugin classes"Texty\\Dependencies\\": "dependencies/"maps to thedependencies/directory which contains the Appsero dependency
Both directories exist and contain appropriate PHP files following the expected namespace structure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the existence of the mapped directories
fd -t d -e dependencies
fd -t d -e includes
Length of output: 47
Script:
#!/bin/bash
# Let's try a different approach to verify the directory structure
# List all directories in the repository
fd --type d
# Also check if there are any PHP files in these directories
echo "PHP files in includes directory:"
fd -e php . includes/
echo "PHP files in dependencies directory:"
fd -e php . dependencies/
Length of output: 1775
72-72: Review the implications of vendor directory deletion.
The delete_vendor_directories: true setting will remove original vendor directories after processing. Ensure this doesn't affect any runtime dependencies that might look for files in their original locations.
✅ Verification successful
Vendor directory deletion is safe in this context
The delete_vendor_directories: true setting is used with Mozart, a dependency isolation tool. The codebase is properly configured to:
- Move vendor dependencies to a custom namespace
Texty\Dependencies - Relocate files to
/dependencies/directory - Use PSR-4 autoloading for both main code and dependencies
- Generate proper autoload mappings via
composer dump-autoload
The only direct vendor reference found is the standard require 'vendor/autoload.php' which is generated by Composer and will continue to work after Mozart processes the dependencies.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for hardcoded vendor paths in the codebase
rg -l "vendor/appsero"
rg -l "vendor/google"
Length of output: 44
Script:
#!/bin/bash
# Let's check for any file paths or includes that might reference vendor directories
rg -i "require.*vendor|include.*vendor" -g "!{vendor,node_modules}/*"
# Check composer.json for more context about the vendor directories
cat composer.json
# Look for any autoload configurations
rg -i "autoload|classmap|psr-4" composer.json
Length of output: 2745
texty.php (2)
147-147: LGTM! Namespace change aligns with Mozart integration.
The namespace change from Appsero\Client to Texty\Dependencies\Appsero\Client is correct and follows the Mozart dependency prefixing pattern to prevent conflicts with other plugins.
147-150: Verify the impact of Mozart integration.
Let's ensure that the Mozart integration is working as expected and no namespace conflicts exist.
✅ Verification successful
Mozart integration is working correctly
The verification shows that:
- Mozart is properly configured in composer.json with correct namespace prefixing (Texty\Dependencies)
- The Appsero Client class is correctly prefixed and exists at dependencies/Appsero/Client.php
- The namespace is properly set to
Texty\Dependencies\Appsero - No unprefixed Appsero usage was found in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential namespace conflicts and verify Mozart integration
# Check if Mozart is properly configured in composer.json
echo "Checking Mozart configuration..."
cat composer.json | jq '.extra.mozart'
# Look for any remaining non-prefixed Appsero usage
echo "Checking for non-prefixed Appsero usage..."
rg "new Appsero\\Client" --type php
# Verify the prefixed namespace exists
echo "Verifying prefixed namespace..."
rg "namespace Texty\\Dependencies\\Appsero" --type php
Length of output: 891
Script:
#!/bin/bash
# Let's fix the regex escaping and try again
# Check for non-prefixed Appsero usage
echo "Checking for non-prefixed Appsero usage..."
rg "new Appsero\\\\Client" --type php
# Verify the prefixed namespace exists
echo "Verifying prefixed namespace..."
rg "namespace Texty\\\\Dependencies\\\\Appsero" --type php
# Check the actual class file existence and content
echo "Checking for prefixed Client class..."
fd "Client.php" dependencies/
# If found, verify its content
fd "Client.php" dependencies/ -x cat {}
Length of output: 7211
dependencies/Appsero/Insights.php (1)
845-855: Ensure proper sanitization of user input in uninstall_reason_submission method
While sanitize_text_field and wp_unslash are used to sanitize $_POST data, consider adding more specific validation for reason_id and reason_info to prevent potential security issues. For example, validate that reason_id corresponds to expected values.
To automate verification, run the following script to check if reason_id and reason_info are properly validated:
✅ Verification successful
Input validation for reason_id is sufficient through predefined reasons system
The current implementation is secure for the following reasons:
- The
reason_idvalues are strictly controlled through a predefined set of reasons inget_uninstall_reasons()method - Custom reasons can only be added through WordPress filters (
appsero_custom_deactivation_reasons) - The frontend form only allows selection from these predefined radio buttons
- The input is properly sanitized using
sanitize_text_fieldandwp_unslash - The nonce verification adds an extra layer of security
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that 'reason_id' matches predefined uninstall reasons.
# List of allowed reason IDs
allowed_reasons=("could-not-understand" "found-better-plugin" "not-have-that-feature" "is-not-working" "looking-for-other" "did-not-work-as-expected" "other")
# Search for where 'reason_id' is used and ensure validation occurs
rg -A5 "\\$data\\['reason_id'\\]" -t php
# Expected: Validation code that checks if 'reason_id' is in the list of allowed reasons
Length of output: 281
Script:
#!/bin/bash
# Let's try a different approach to find usage of reason_id
rg -A 5 "reason_id" dependencies/Appsero/Insights.php
# Also search for any predefined reasons or validation
rg "reasons\s*=" dependencies/Appsero/Insights.php
# Look for the complete uninstall_reason_submission method implementation
ast-grep --pattern 'function uninstall_reason_submission() {
$$$
}'
Length of output: 1326
Script:
#!/bin/bash
# Let's find the get_uninstall_reasons method to see predefined reasons
ast-grep --pattern 'function get_uninstall_reasons() {
$$$
}'
# Also look for the complete context around where reasons are defined
rg -B2 -A10 "reasons = \[" dependencies/Appsero/Insights.php
Length of output: 2437
Script:
#!/bin/bash
# Let's see the complete list of predefined reasons and how they are used
rg -B2 -A30 "reasons = \[" dependencies/Appsero/Insights.php
# Also check how custom reasons are merged and validated
rg -A10 "custom_reasons" dependencies/Appsero/Insights.php
Length of output: 7521
| "mozart": { | ||
| "dep_namespace": "Texty\\Dependencies\\", | ||
| "dep_directory": "/dependencies/", | ||
| "classmap_directory": "/dependencies/classes/", | ||
| "classmap_prefix": "Texty_", | ||
| "packages": [ | ||
| "appsero/updater", | ||
| "appsero/client" | ||
| ], | ||
| "excluded_packages": [ | ||
| "psr/container" | ||
| ], | ||
| "override_autoload": { | ||
| "google/apiclient": { | ||
| "classmap": [ | ||
| "src/" | ||
| ] | ||
| } | ||
| }, |
There was a problem hiding this comment.
💡 Codebase verification
Mozart configuration needs directory adjustments and .gitignore updates
The verification revealed several issues that need attention:
- The
/dependencies/classes/directory specified in Mozart config is missing - Neither
/dependencies/nor/dependencies/classes/are listed in .gitignore - The existing
/dependenciesdirectory structure doesn't match Mozart's expected layout
Required actions:
- Add
/dependencies/and/dependencies/classes/to .gitignore - Create the
/dependencies/classes/directory to match Mozart configuration - Ensure the existing
/dependencies/Appserocontent aligns with the expected Mozart output structure
🔗 Analysis chain
Verify Mozart configuration alignment with project structure.
The Mozart configuration appears well-structured, but please ensure:
- The
/dependencies/and/dependencies/classes/directories are gitignored - The
appsero/clientandappsero/updaterpackages are compatible with namespace prefixing
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check .gitignore entries and directory structure
echo "Checking .gitignore entries:"
rg "dependencies/" .gitignore
rg "dependencies/classes/" .gitignore
# Verify Appsero package compatibility
echo "Checking Appsero package structure:"
rg -l "namespace\s+Appsero" vendor/appsero
Length of output: 315
Script:
#!/bin/bash
# Let's check if .gitignore exists and its content
echo "Checking if .gitignore exists:"
ls -la .gitignore
echo "\nChecking current .gitignore content:"
cat .gitignore
# Check if dependencies directories exist
echo "\nChecking dependencies directories:"
ls -la dependencies 2>/dev/null || echo "dependencies/ directory not found"
ls -la dependencies/classes 2>/dev/null || echo "dependencies/classes/ directory not found"
# Check composer.lock for actual package versions
echo "\nChecking if appsero packages are in composer.lock:"
if [ -f composer.lock ]; then
rg -A 5 '"name":\s*"appsero/' composer.lock
else
echo "composer.lock not found"
fi
Length of output: 1207
| list( $this->slug, $mainfile ) = explode( '/', $this->basename ); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent undefined offset errors when using explode.
In the set_basename_and_slug() method, using list() with explode() assumes that $this->basename contains a /. If it doesn't, this could lead to undefined offset notices. Safeguard against this by checking the array size before assignment.
Apply this diff to handle the array elements safely:
- list( $this->slug, $mainfile ) = explode( '/', $this->basename );
+ $parts = explode( '/', $this->basename, 2 );
+ $this->slug = $parts[0];
+ $mainfile = isset( $parts[1] ) ? $parts[1] : '';Also applies to: 201-202
| $action = ( $license && isset( $license['status'] ) && 'activate' === $license['status'] ) ? 'deactive' : 'active'; | ||
| $this->licenses_style(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Correct Action Variable Value to 'deactivate'
The action variable should use 'deactivate' instead of 'deactive' for consistency.
Apply this diff to correct the action value:
-$action = ( $license && isset( $license['status'] ) && 'activate' === $license['status'] ) ? 'deactive' : 'active';
+$action = ( $license && isset( $license['status'] ) && 'activate' === $license['status'] ) ? 'deactivate' : 'activate';📝 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.
| $action = ( $license && isset( $license['status'] ) && 'activate' === $license['status'] ) ? 'deactive' : 'active'; | |
| $this->licenses_style(); | |
| $action = ( $license && isset( $license['status'] ) && 'activate' === $license['status'] ) ? 'deactivate' : 'activate'; | |
| $this->licenses_style(); |
| case 'deactive': | ||
| $this->deactive_client_license(); | ||
| break; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update Action Case Statement to Use 'deactivate'
In the switch statement, replace 'deactive' with 'deactivate' to match the corrected method name.
Apply this diff to update the action case:
case 'active':
$this->active_client_license( $license_key );
break;
- case 'deactive':
- $this->deactive_client_license();
+ case 'deactivate':
+ $this->deactivate_client_license();
break;
case 'refresh':
$this->refresh_client_license();
break;📝 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.
| case 'deactive': | |
| $this->deactive_client_license(); | |
| break; | |
| case 'deactivate': | |
| $this->deactivate_client_license(); | |
| break; |
| public function refresh_license_api() { | ||
| $this->check_license_status(); | ||
|
|
||
| wp_send_json_success( | ||
| [ | ||
| 'message' => 'License refreshed successfully.', | ||
| ], | ||
| 200 | ||
| ); | ||
| } |
There was a problem hiding this comment.
Security Enhancement: Add Capability Check in 'refresh_license_api'
To ensure only authorized users can access the 'refresh_license_api' method, add a capability check at the beginning of the method.
Apply this code change to add the capability check:
public function refresh_license_api() {
+ if ( ! current_user_can( 'manage_options' ) ) {
+ wp_send_json_error(
+ [
+ 'message' => $this->client->__trans( 'You do not have permission to perform this action.' ),
+ ],
+ 403
+ );
+ return;
+ }
$this->check_license_status();
wp_send_json_success(
[
'message' => 'License refreshed successfully.',
],
200
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function refresh_license_api() { | |
| $this->check_license_status(); | |
| wp_send_json_success( | |
| [ | |
| 'message' => 'License refreshed successfully.', | |
| ], | |
| 200 | |
| ); | |
| } | |
| public function refresh_license_api() { | |
| if ( ! current_user_can( 'manage_options' ) ) { | |
| wp_send_json_error( | |
| [ | |
| 'message' => $this->client->__trans( 'You do not have permission to perform this action.' ), | |
| ], | |
| 403 | |
| ); | |
| return; | |
| } | |
| $this->check_license_status(); | |
| wp_send_json_success( | |
| [ | |
| 'message' => 'License refreshed successfully.', | |
| ], | |
| 200 | |
| ); | |
| } |
| /** | ||
| * Deactive client license | ||
| */ | ||
| private function deactive_client_license() { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inconsistent Naming: Rename 'deactive_client_license' to 'deactivate_client_license'
The method name 'deactive_client_license' should be renamed to 'deactivate_client_license' for consistency and correct terminology.
Apply this diff to rename the method:
-private function deactive_client_license() {
+private function deactivate_client_license() {Update all references to this method accordingly.
Committable suggestion skipped: line range outside the PR's diff.
| esc_url( 'https://appsero.com/' ), | ||
| esc_url( 'https://appsero.com/privacy-policy' ) | ||
| ); | ||
| ?> | ||
| </p> | ||
| </div> | ||
|
|
||
| <div class="wd-dr-modal-footer"> | ||
| <a href="#" class="dont-bother-me wd-dr-button-secondary"><?php $this->client->_etrans( 'Skip & Deactivate' ); ?></a> | ||
| <button class="wd-dr-button-secondary wd-dr-cancel-modal"><?php $this->client->_etrans( 'Cancel' ); ?></button> | ||
| <button class="wd-dr-submit-modal"><?php $this->client->_etrans( 'Submit & Deactivate' ); ?></button> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| <script type="text/javascript"> | ||
| (function($) { | ||
| $(function() { | ||
| var modal = $('#<?php echo $this->client->slug; ?>-wd-dr-modal'); | ||
| var deactivateLink = ''; | ||
|
|
||
| // Open modal | ||
| $('#the-list').on('click', 'a.<?php echo $this->client->slug; ?>-deactivate-link', function(e) { | ||
| e.preventDefault(); | ||
|
|
||
| modal.addClass('modal-active'); | ||
| deactivateLink = $(this).attr('href'); | ||
| modal.find('a.dont-bother-me').attr('href', deactivateLink).css('float', 'left'); | ||
| }); | ||
|
|
||
| // Close modal; Cancel | ||
| modal.on('click', 'button.wd-dr-cancel-modal', function(e) { | ||
| e.preventDefault(); | ||
| modal.removeClass('modal-active'); | ||
| }); | ||
|
|
||
| // Reason change | ||
| modal.on('click', 'input[type="radio"]', function() { | ||
| var parent = $(this).parents('li'); | ||
| var isCustomReason = parent.data('customreason'); | ||
| var inputValue = $(this).val(); | ||
|
|
||
| if (isCustomReason) { | ||
| $('ul.wd-de-reasons.wd-de-others-reasons li').removeClass('wd-de-reason-selected'); | ||
| } else { | ||
| $('ul.wd-de-reasons li').removeClass('wd-de-reason-selected'); | ||
|
|
||
| if ( "other" !== inputValue ) { | ||
| $('ul.wd-de-reasons.wd-de-others-reasons').css('display', 'none'); | ||
| } | ||
| } | ||
|
|
||
| // Show if has custom reasons | ||
| if ( "other" === inputValue ) { | ||
| $('ul.wd-de-reasons.wd-de-others-reasons').css('display', 'flex'); | ||
| } | ||
|
|
||
| parent.addClass('wd-de-reason-selected'); | ||
| $('.wd-dr-modal-reason-input').show(); | ||
|
|
||
| $('.wd-dr-modal-reason-input textarea').attr('placeholder', parent.data('placeholder')).focus(); | ||
| }); | ||
|
|
||
| // Submit response | ||
| modal.on('click', 'button.wd-dr-submit-modal', function(e) { | ||
| e.preventDefault(); | ||
|
|
||
| var button = $(this); | ||
|
|
||
| if (button.hasClass('disabled')) { | ||
| return; | ||
| } | ||
|
|
||
| var $radio = $('input[type="radio"]:checked', modal); | ||
| var $input = $('.wd-dr-modal-reason-input textarea'); | ||
|
|
||
| $.ajax({ | ||
| url: ajaxurl, | ||
| type: 'POST', | ||
| data: { | ||
| nonce: '<?php echo wp_create_nonce( 'appsero-security-nonce' ); ?>', | ||
| action: '<?php echo $this->client->slug; ?>_submit-uninstall-reason', | ||
| reason_id: (0 === $radio.length) ? 'none' : $radio.val(), | ||
| reason_info: (0 !== $input.length) ? $input.val().trim() : '' | ||
| }, | ||
| beforeSend: function() { | ||
| button.addClass('disabled'); | ||
| button.text('Processing...'); | ||
| }, | ||
| complete: function() { | ||
| window.location.href = deactivateLink; | ||
| } | ||
| }); | ||
| }); | ||
| }); | ||
| }(jQuery)); | ||
| </script> | ||
|
|
||
| <?php | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor inline scripts and styles in deactivate_scripts method
Embedding extensive HTML, CSS, and JavaScript directly within the PHP code reduces code maintainability and readability. Instead, use WordPress's enqueue functions (wp_enqueue_script, wp_enqueue_style) to properly load external script and style files. This aligns with WordPress best practices, improves modularity, and simplifies future maintenance.
| if ( | ||
| in_array( $ip, array( '127.0.0.1', '::1' ), true ) || | ||
| ! strpos( $host, '.' ) || | ||
| in_array( strrchr( $host, '.' ), array( '.test', '.testing', '.local', '.localhost', '.localdomain' ), true ) | ||
| ) { | ||
| $is_local = true; | ||
| } |
There was a problem hiding this comment.
Fix logical error in hostname check within is_local_server method
In the condition ! strpos( $host, '.' ), if the dot . is at the beginning of $host, strpos returns 0, which evaluates to false. The negation ! 0 becomes true, causing the condition to incorrectly identify the host as local. To accurately check if the dot is not found in $host, use strict comparison with false === strpos( $host, '.' ).
Suggested fix:
- ! strpos( $host, '.' ) ||
+ false === strpos( $host, '.' ) ||📝 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.
| if ( | |
| in_array( $ip, array( '127.0.0.1', '::1' ), true ) || | |
| ! strpos( $host, '.' ) || | |
| in_array( strrchr( $host, '.' ), array( '.test', '.testing', '.local', '.localhost', '.localdomain' ), true ) | |
| ) { | |
| $is_local = true; | |
| } | |
| if ( | |
| in_array( $ip, array( '127.0.0.1', '::1' ), true ) || | |
| false === strpos( $host, '.' ) || | |
| in_array( strrchr( $host, '.' ), array( '.test', '.testing', '.local', '.localhost', '.localdomain' ), true ) | |
| ) { | |
| $is_local = true; | |
| } |
devAsadNur
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
composer.json (1)
38-44: Consider adding error handling to the scriptsThe scripts are well-structured, but could benefit from error handling.
"post-install-cmd": [ - "[ $COMPOSER_DEV_MODE -eq 0 ] || \"vendor/bin/mozart\" compose", + "[ $COMPOSER_DEV_MODE -eq 0 ] || \"vendor/bin/mozart\" compose || { echo 'Mozart compose failed'; exit 1; }", "composer dump-autoload" ], "post-update-cmd": [ - "[ $COMPOSER_DEV_MODE -eq 0 ] || \"vendor/bin/mozart\" compose", + "[ $COMPOSER_DEV_MODE -eq 0 ] || \"vendor/bin/mozart\" compose || { echo 'Mozart compose failed'; exit 1; }", "composer dump-autoload" ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
composer.json(2 hunks)
🔇 Additional comments (4)
composer.json (4)
19-20: LGTM! PSR-4 autoloading configuration is correct
The namespace mappings are properly configured following PSR-4 standards.
28-28: LGTM! Mozart dependency is correctly configured
The Mozart package will help manage dependencies and prevent conflicts in the WordPress plugin environment.
53-73: Mozart configuration needs directory adjustments
The previous review already identified issues with the Mozart configuration regarding directory structure. These concerns are still valid.
15-15: Verify appsero/client version security status
The version constraint ^v2.0.4 allows compatible updates. Let's verify this version for known security issues.
✅ Verification successful
✓ appsero/client version v2.0.4 is secure and up-to-date
The version being used (v2.0.4) is the latest available version on Packagist, and no security vulnerabilities were found in the GitHub Security Advisory Database for this package.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for security advisories and latest version
echo "Checking latest version on Packagist:"
curl -s https://repo.packagist.org/p2/appsero/client.json | jq -r '.packages."appsero/client"[0].version'
echo "\nChecking for security advisories:"
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: COMPOSER, package: "appsero/client") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
}
}
}'
Length of output: 578
devAsadNur
changed the title
Integrated Mozart.
Summary by CodeRabbit
New Features
Clientclass for managing plugin/theme data and updates.Insightsclass for tracking plugin usage with user consent.Licenseclass for managing license verification and activation.Improvements
Bug Fixes