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

fix: AllowDynamicProperty warning #1466

Merged

Conversation

sapayth
Copy link
Member

@sapayth sapayth commented Jul 16, 2024

Some of our plugin pages were showing PHP AllowDynamicProperty warning for PHP 8.* users

Summary by CodeRabbit

  • New Features

    • Introduced a centralized container to manage class instances in the WP_User_Frontend class, improving code organization and extensibility.
  • Bug Fixes

    • Added checks to ensure WooCommerce product existence before updating meta, preventing potential errors.
  • Enhancements

    • Updated admin bar styles and removed unnecessary emoji styles for a cleaner interface.
    • Improved text content and user prompts across the admin settings, menu items, and setup wizard.
  • Maintenance

    • Removed deprecated AllowDynamicProperties attribute from multiple classes for better code compliance.
    • Updated version number to 4.0.9.

Copy link

coderabbitai bot commented Jul 16, 2024

Walkthrough

The changes involve refactoring several classes within the project to remove the AllowDynamicProperties attribute and improve instance management through the use of centralized container properties. This enhances code organization and maintainability by encapsulating instances within a single property, making the structure more modular and easier to extend.

Changes

Files Change Summary
Lib/invisible_recaptcha.php Removed #[AllowDynamicProperties] attribute and adjusted class formatting.
admin/class-admin-settings.php Removed AllowDynamicProperties import and attribute from WPUF_Admin_Settings class.
includes/Admin.php Refactored Admin class to use a container for instance management, replacing individual properties.
includes/Admin/Forms/... Removed AllowDynamicProperties and updated form builder management in Admin_Form and Form classes.
includes/Admin/Forms/...Templates/ Added checks for existence of variables and removed AllowDynamicProperties in Post_Form_Template_WooCommerce and Pro_Form_Preview_EDD.
includes/Ajax/Frontend_Form_Ajax.php Removed fully-qualified AllowDynamicProperties usage.
includes/Fields/Form_Field_Post_Taxonomy.php Removed #[AllowDynamicProperties] attribute and import statement.
includes/Frontend.php Refactored to use a container for instance management instead of individual properties.
includes/Pro_Upgrades.php Updated register_pro_fields method to use the container array for managing pro_fields.
includes/Setup_Wizard.php Added custom_admin_bar_styles method and removed emoji styles from setup_wizard.
languages/wp-user-frontend.pot Updated various strings, labels, and messages in the translation file.
package.json Incremented version number from "4.0.8" to "4.0.9".
wpuf.php Introduced $container property in WP_User_Frontend to hold class instances, refactoring the instantiation process.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant WP_User_Frontend
    participant Admin
    participant Ajax
    participant Component

    User->>WP_User_Frontend: Initialize
    WP_User_Frontend->>Admin: Instantiate if is_admin()
    Admin-->>WP_User_Frontend: Instance stored in container
    WP_User_Frontend->>Ajax: Instantiate if DOING_AJAX
    Ajax-->>WP_User_Frontend: Instance stored in container
    WP_User_Frontend->>Component: Instantiate various components
    Component-->>WP_User_Frontend: Instances stored in container
    User->>WP_User_Frontend: Access instance from container
    WP_User_Frontend->>User: Return instance
Loading

Poem

In the code where instances used to roam,
We found a better, organized home.
No more dynamic properties to see,
Just neat containers, filled with glee.
Version bumped, to four point nine,
Our plugin shines, in organized design. 🌟


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>.
    • 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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @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 as PR comments)

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

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration 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: 0

Outside diff range, codebase verification and nitpick comments (7)
includes/Pro_Upgrades.php (1)

30-30: Direct access to pro_fields without using the container.

There is a direct access to wpuf()->pro_fields->get_fields() that should be updated to use the container for consistency and encapsulation.

  • File: includes/Pro_Upgrades.php
  • Line: wpuf()->pro_fields->get_fields()

Please update this instance to access pro_fields via the container.

Analysis chain

Refactoring to use container for pro_fields.

This change encapsulates the pro_fields instance within a container, which should improve manageability and modularity. Ensure that all parts of the application that interact with pro_fields are updated to access it via the container.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all accesses to `pro_fields` are updated.

# Test: Search for `pro_fields` usage. Expect: Only accesses via container.
rg --type php -A 5 $'wpuf()->pro_fields'

Length of output: 41


Script:

#!/bin/bash
# Description: Verify all accesses to `pro_fields` are updated.

# Test: Search for any usage of `pro_fields`. Expect: Only accesses via container.
rg --type php 'pro_fields'

Length of output: 655

includes/Admin.php (1)

17-32: Direct property accesses found for settings and settings_api.

The search results indicate that the properties settings and settings_api are still being accessed directly in the following files:

  • includes/Admin/Forms/Admin_Form_Builder.php
  • includes/Admin/Admin_Settings.php
  • admin/class-admin-settings.php
  • Lib/WeDevs_Settings_API.php

Please review these instances and update them to access the properties via the container if necessary.

Analysis chain

Refactoring to use container for storing instances in Admin class.

The use of a container to store instances like admin_welcome, menu, dashboard_metabox, etc., enhances modularity and makes the code more maintainable. Ensure that all parts of the application that interact with these instances are updated to access them via the container.


Let's extend the search to include all the properties that were moved to the container to ensure they are only accessed via the container.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all accesses to container instances are updated.

# Test: Search for direct property accesses. Expect: No direct accesses, only via container.
rg --type php -A 5 $'this->admin_welcome|this->menu|this->dashboard_metabox'

Length of output: 8316


Script:

#!/bin/bash
# Description: Verify all accesses to container instances are updated.

# Test: Search for direct property accesses. Expect: No direct accesses, only via container.
rg --type php -A 5 $'this->admin_welcome|this->menu|this->dashboard_metabox|this->form_template|this->admin_form|this->admin_form_handler|this->admin_subscription|this->admin_installer|this->settings|this->forms|this->gutenberg_block|this->promotion|this->plugin_upgrade_notice|this->posting|this->shortcodes_button|this->tools'

Length of output: 18033

includes/Ajax/Frontend_Form_Ajax.php (5)

Line range hint 27-27: Guest Mode and Email Verification

The method handles different scenarios based on whether the guest mode is enabled and whether email verification is required. This adds flexibility to the form handling but also increases the complexity of the method.

Consider refactoring this method to reduce complexity and improve readability by breaking it down into smaller, more focused methods.


Line range hint 29-29: Attachment Deletion

The method processes requests to delete attachments. It's important to ensure that permissions are checked before allowing an attachment to be deleted to prevent unauthorized deletions.

Ensure that permission checks are implemented before deleting attachments.


Line range hint 31-31: Content Restriction Handling

The method includes logic to enforce content restrictions based on character and word counts. This is a useful feature for form validation but can be error-prone.

Consider adding unit tests to cover various edge cases in content restriction handling.


Line range hint 39-39: Post Meta and Taxonomy Handling

The method updates post meta and taxonomy based on the form inputs. It's important to ensure that these updates are secure and do not allow unauthorized changes.

Add security checks to ensure that only authorized users can update post meta and taxonomy.


Line range hint 41-41: Email Handling

The method sends emails based on form settings. It's important to ensure that email contents are properly sanitized to prevent email injection attacks.

Ensure that email contents are sanitized before sending.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 838a757 and 676a45c.

Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
Files selected for processing (15)
  • Lib/invisible_recaptcha.php (1 hunks)
  • admin/class-admin-settings.php (1 hunks)
  • includes/Admin.php (3 hunks)
  • includes/Admin/Forms/Admin_Form.php (2 hunks)
  • includes/Admin/Forms/Form.php (2 hunks)
  • includes/Admin/Forms/Post/Templates/Post_Form_Template_WooCommerce.php (1 hunks)
  • includes/Admin/Forms/Post/Templates/Pro_Form_Preview_EDD.php (2 hunks)
  • includes/Ajax/Frontend_Form_Ajax.php (1 hunks)
  • includes/Fields/Form_Field_Post_Taxonomy.php (1 hunks)
  • includes/Frontend.php (2 hunks)
  • includes/Pro_Upgrades.php (1 hunks)
  • includes/Setup_Wizard.php (2 hunks)
  • languages/wp-user-frontend.pot (21 hunks)
  • package.json (1 hunks)
  • wpuf.php (7 hunks)
Files skipped from review due to trivial changes (1)
  • package.json
Additional comments not posted (35)
includes/Admin/Forms/Post/Templates/Pro_Form_Preview_EDD.php (1)

23-27: Addition of private property $pro_icon is well-documented and initialized.

The new property is clearly documented and initialized in the constructor, which is good practice.

Lib/invisible_recaptcha.php (1)

19-19: Removal of AllowDynamicProperties attribute and associated use statement.

This change is consistent with the PR's objective to address PHP 8.* warnings and does not seem to affect the functionality of the class.

includes/Admin.php (1)

53-53: Correct access to form_template via container in method create_post_form_from_template.

This change ensures that the form_template instance is accessed through the container, aligning with the new architecture.

includes/Frontend.php (1)

17-24: Refactor to centralize component management is well-implemented.

The initialization of various components within the container property in the constructor enhances modularity and maintainability of the code. This approach also aligns with the principle of encapsulation.

wpuf.php (5)

69-69: Introduction of container property is a positive change.

The addition of the public $container = []; property is a good practice for managing class instances centrally. This change should simplify future enhancements and maintenance of the plugin.


169-186: Centralized management of class instances is well-implemented.

The instantiation of class instances within the $container property is done correctly. This should ensure that all instances are managed in a unified manner, which is beneficial for code organization and maintainability.


261-261: Conditional instantiation of the free loader class.

The conditional instantiation of the free_loader class within the $container property based on the presence of the PRO version is a smart implementation, ensuring that resources are used efficiently.


341-341: Widget manager instantiation within container.

The instantiation of the widgets class within the $container property is correctly implemented. This centralizes the management of widgets, which is beneficial for maintainability.


212-212: Ensure proper instantiation of upgrades class.

The instantiation of the upgrades class within the $container property is correctly implemented. However, ensure that all references to this class throughout the codebase are updated to reflect this change.

Verification successful

Proper instantiation of upgrades class confirmed.

The instantiation of the upgrades class within the $container property is correctly implemented, and all references to this class throughout the codebase are updated to reflect this change.

  • Verified that $this->container['upgrades'] is used for accessing the upgrades class.
  • No direct instantiations or other access methods for upgrades found.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to `upgrades` class match the new container-based access.

# Test: Search for the class usage. Expect: Only occurances of the new container-based access.
rg --type php -A 5 $'upgrades'

Length of output: 3380


Script:

#!/bin/bash
# Description: Verify all references to `upgrades` class match the new container-based access.

# Test: Search for all references to `upgrades` and check if they use container-based access.
rg --type php '\$this->container\[\'upgrades\'\]' -A 5

Length of output: 93


Script:

#!/bin/bash
# Description: Verify all references to `upgrades` class match the new container-based access.

# Test: Search for all references to `upgrades` and check if they use container-based access.
rg --type php '\$this->container\["upgrades"\]' -A 5

Length of output: 54


Script:

#!/bin/bash
# Description: Verify all references to `upgrades` class in the codebase.

# Test: Search for all references to `upgrades` in the codebase.
rg --type php 'upgrades' -A 5

Length of output: 3380

includes/Admin/Forms/Form.php (1)

24-24: Addition of private $data property enhances encapsulation.

The introduction of the private $data property to hold form data is a good practice. This encapsulation improves data management within the Form class and aligns with principles of modern object-oriented programming.

includes/Admin/Forms/Post/Templates/Post_Form_Template_WooCommerce.php (1)

332-341: Proactive error handling in update_meta function.

The addition of a check for the existence of $product before proceeding with setting the catalog visibility is a good example of defensive programming. This prevents potential errors from accessing properties on a non-existent object, which is crucial for maintaining the robustness of the plugin.

includes/Admin/Forms/Admin_Form.php (2)

13-13: Trait usage updated.

The removal of ContainerTrait aligns with the shift to using a centralized container for instance management, as seen in the changes to post_forms_builder_init.


182-182: Refactored instance management using container.

Switching to wpuf()->container['form_builder'] from wpuf()->form_builder encapsulates instance management within a container, enhancing modularity and maintainability.

includes/Fields/Form_Field_Post_Taxonomy.php (1)

Line range hint 1-1: Removed dynamic properties attribute and import.

Removing the AllowDynamicProperties attribute and its import statement is a positive change for compatibility with PHP 8.*, addressing potential issues with dynamic properties.

includes/Setup_Wizard.php (2)

31-42: Added method to customize admin bar styles.

The new method custom_admin_bar_styles is a good addition for conditionally adding styles to the admin bar, enhancing the admin UI's appearance and performance.


142-142: Optimized setup wizard by removing emoji styles.

Removing the action to print emoji styles in the setup_wizard method is a sensible optimization, reducing unnecessary loads and potentially speeding up the admin interface.

admin/class-admin-settings.php (1)

1-1: Removed dynamic properties attribute and import.

Removing the AllowDynamicProperties attribute and its import statement from WPUF_Admin_Settings class is a positive change for compatibility with PHP 8.*, addressing potential issues with dynamic properties.

includes/Ajax/Frontend_Form_Ajax.php (11)

Line range hint 1-1: File Header and Namespace Declaration

The file begins with the PHP opening tag and namespace declaration. This is standard and correct for a PHP file within a WordPress plugin.


Line range hint 14-14: Use of FieldableTrait

The class uses FieldableTrait, which is a common practice to share methods or properties across multiple classes. Ensure that the methods and properties provided by this trait are used appropriately throughout the class.


Line range hint 16-19: Public and Private Properties

The class defines several properties for managing form settings and post expiration details. It's good practice to keep related properties together and clearly commented, which is done here.


Line range hint 21-21: Method: submit_post

This method handles the submission of posts via AJAX. It begins with a nonce check, which is crucial for security to prevent CSRF attacks.


Line range hint 23-23: Content-Type Header

Setting the content type to JSON is appropriate for an AJAX response. This ensures that the client-side JavaScript can correctly parse the response.


Line range hint 25-25: Form and Field Retrieval

The method retrieves form settings and fields based on the form ID provided in the POST request. It's important to ensure that the form ID is properly sanitized, which is done here using intval.


Line range hint 33-33: Shortcode Restriction

The method checks for restricted shortcodes in the form fields. This is important for security to prevent the execution of unauthorized shortcodes.


Line range hint 35-35: Post Variables Preparation

The method prepares variables for creating or updating a post. It's important to ensure that all user inputs are properly sanitized and validated before use.


Line range hint 37-37: Post Creation and Update

The method handles both creating new posts and updating existing ones. It uses the wp_insert_post function, which is the appropriate function for this purpose in WordPress.


Line range hint 43-43: Final Response Preparation

The method prepares the final response to be sent back to the client. It's important to ensure that the response is properly formatted and contains all necessary information.


10-10: Removal of AllowDynamicProperties Attribute

The AllowDynamicProperties attribute has been removed from the class declaration. This is in line with the PR's objective to address PHP warnings in PHP 8.* environments.

Verification successful

Removal of AllowDynamicProperties Attribute

The AllowDynamicProperties attribute has been removed from the class declaration. This is in line with the PR's objective to address PHP warnings in PHP 8.* environments. The removal does not affect other parts of the codebase.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the removal of AllowDynamicProperties does not affect other parts of the codebase.
ast-grep --lang php --pattern $'class $$_ {
  $$$
  #[AllowDynamicProperties]
  $$$
}'

Length of output: 194


Script:

#!/bin/bash
# Description: Verify that the removal of AllowDynamicProperties does not affect other parts of the codebase.
rg --php 'AllowDynamicProperties'

Length of output: 441


Script:

#!/bin/bash
# Description: Verify that the removal of AllowDynamicProperties does not affect other parts of the codebase.
rg 'AllowDynamicProperties'

Length of output: 27

languages/wp-user-frontend.pot (7)

5-5: Updated Project Version in POT File

The Project-Id-Version has been updated to WP User Frontend 4.0.9. This change is standard for maintaining version consistency across project files.


7-7: Updated POT Creation Date

The POT-Creation-Date has been updated to reflect the current date of the file generation, which is 2024-07-16 10:55:24+00:00. This is a necessary update to track the latest state of translations.


96-96: Updated Translation Strings

Various strings related to admin settings, menu items, and error messages have been updated. These changes are crucial for ensuring that the user interface remains clear and that messages are correctly localized for different languages.

Also applies to: 100-100, 106-106, 110-110, 116-116, 121-121, 125-125, 129-129, 133-133, 136-136, 140-140, 145-145, 149-149, 153-153, 161-161, 165-165, 169-169, 173-173, 179-179, 183-183, 187-187, 191-191


404-404: Consistency in Payment Settings Localization

The string Enable Payments has been consistently used across different files (admin/html/form-settings-payment.php, includes/Setup_Wizard.php), ensuring uniformity in terminology which is good for maintainability.


836-837: Consistent Use of 'Edit' Across Different Contexts

The string Edit is used across various files (admin/installer.php, includes/Admin/Forms/Admin_Form.php), which maintains a consistent language for edit actions throughout the application.


902-902: Confirmation and Error Handling Messages

Strings related to confirmation dialogs (Are you sure?, Yes, delete it, No, cancel it) and error messages related to file uploads have been updated. These are important for ensuring that users receive clear feedback on their actions, especially in potentially destructive operations like deletions.

Also applies to: 909-909, 915-915, 920-920, 925-925, 930-930, 935-935


1460-1460: Updated Post Taxonomy Related Strings

Strings related to post taxonomy operations have been updated. This includes general selection prompts and messages indicating the unavailability of fields, which is crucial for user interactions in taxonomy-related functionalities.

Also applies to: 1466-1466

@sapayth sapayth merged commit c34032d into weDevsOfficial:develop Jul 17, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant