Allow uploads folder used for generated css file#2301
Conversation
|
""" WalkthroughThe changes refactor the file creation logic within the Changes
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 0
🧹 Nitpick comments (1)
classes/models/FrmStyle.php (1)
403-432: Well-structured new method with configurable file pathThis new method nicely encapsulates the logic for determining where to save the CSS file, allowing flexibility through the
frm_add_css_to_uploads_dirfilter. This addresses the core requirement to optionally use the uploads directory for generated CSS files.However, the PHPDoc comment contains placeholder version information (
@since x.x), which should be updated with the actual version number.- * @since x.x + * @since 6.x
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
classes/models/FrmStyle.php(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
classes/models/FrmStyle.php (2)
classes/controllers/FrmStylesController.php (1)
FrmStylesController(6-1365)classes/helpers/FrmAppHelper.php (1)
FrmAppHelper(6-4460)
🔇 Additional comments (3)
classes/models/FrmStyle.php (3)
394-396: Good refactoring to improve code organizationThe code now delegates file creation arguments to a dedicated method instead of hardcoding them, which is a good separation of concerns.
434-436: Clear implementation of uploads directory pathThe method correctly uses WordPress's
wp_upload_dir()function to determine the uploads directory path for CSS files.
438-440: Good use of existing helper method for plugin pathThe method properly leverages the
FrmAppHelper::plugin_path()method to get the plugin directory, maintaining consistency with how paths are handled elsewhere in the codebase.
AbdiTolesa
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
classes/models/FrmStyle.php (2)
408-427: Clean up redundant code in argument preparationThe method contains unnecessary code with the
$path_argsvariable being defined but never used.Simplify the code by removing the unused variable:
private function get_create_style_file_args() { $add_css_to_uploads_dir = self::add_css_to_uploads_dir(); $create_file_args = array( 'file_name' => FrmStylesController::get_file_name(), 'new_file_path' => self::get_generated_css_file_path( $add_css_to_uploads_dir ), ); if ( ! $add_css_to_uploads_dir ) { return $create_file_args; } - $path_args = array( - 'folder_name' => 'formidable/css', - ); return array_merge( $create_file_args, array( 'folder_name' => 'formidable/css', ) ); }
442-450: Extensible CSS storage location controlThe filter allows for dynamic control of where CSS files are stored, which is excellent for plugin flexibility.
The version placeholder "x.x" in the PHPDoc should be updated with the actual version number before release:
/** - * @since x.x + * @since 3.07.03 (or whatever the actual version is) * * @param bool $add_css_to_uploads_dir */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
classes/controllers/FrmStylesController.php(1 hunks)classes/models/FrmStyle.php(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
classes/models/FrmStyle.php (2)
classes/controllers/FrmStylesController.php (2)
FrmStylesController(6-1371)get_file_name(251-260)classes/helpers/FrmAppHelper.php (1)
FrmAppHelper(6-4460)
classes/controllers/FrmStylesController.php (2)
classes/models/FrmStyle.php (3)
add_css_to_uploads_dir(442-450)FrmStyle(6-876)get_generated_css_file_path(435-440)classes/helpers/FrmAppHelper.php (1)
FrmAppHelper(6-4460)
🔇 Additional comments (5)
classes/controllers/FrmStylesController.php (1)
230-240: Improved CSS file path flexibilityThe method now intelligently determines whether to store CSS files in the uploads directory or plugin directory, based on a filter. This change improves compatibility with environments where the plugin directory isn't writable.
Some key improvements:
- Uses the new
FrmStyle::add_css_to_uploads_dir()function to determine storage location- Constructs file paths with
FrmStyle::get_generated_css_file_path()- Dynamically sets appropriate base URL based on storage location
classes/models/FrmStyle.php (4)
396-396: Well-structured refactoring of file creation logicThe implementation now uses a dedicated method to prepare arguments, improving code organization and maintainability.
435-440: Good abstraction for file path determinationThis method cleanly handles the logic for deciding where the generated CSS file should be stored.
456-458: Path construction for plugin directoryThis method correctly uses the plugin path helper.
452-454:Details
✅ Verification successful
Path construction for uploads directory
This method correctly utilizes WordPress's upload directory functions.
🌐 Web query:
Is using direct array access like `wp_upload_dir()['basedir']` compatible with all supported WordPress PHP versions?💡 Result:
Using direct array access syntax like
wp_upload_dir()['basedir']is compatible with all PHP versions currently supported by WordPress (7.4 and newer)[2][8]. This syntax, known as "array dereferencing," was introduced in PHP 5.4 and remains valid in modern PHP versions.Key Details:
WordPress PHP Version Support:
- WordPress 6.7 supports PHP 7.4+ and newer versions (8.0–8.4)[2][8].
- PHP 7.2 and 7.3 remain compatible but with exceptions[2].
Technical Compatibility:
- Array dereferencing (
function()['key']) works in all WordPress-supported PHP versions[2][8].- The
wp_upload_dir()function returns an associative array, and direct key access is the intended usage[3][9].Environment-Specific Considerations:
- Some hosting platforms (e.g., Pantheon, Kinsta) may alter filesystem paths, but this doesn't affect PHP syntax compatibility[1][3].
- The array structure returned by
wp_upload_dir()is standardized, withbasediralways present when the function succeeds[9][10].For modern WordPress installations, this syntax is safe and recommended. Developers should still test environment-specific path resolutions, as platform configurations (not PHP versions) might cause unexpected
basedirvalues[1][3].Citations:
- 1: https://docs.pantheon.io/wordpress-known-issues
- 2: https://make.wordpress.org/core/handbook/references/php-compatibility-and-wordpress-versions/
- 3: https://community.kinsta.com/t/wp-upload-dir-not-reporting-correct-path/1948
- 4: https://timber.github.io/docs/v2/upgrade-guides/2.0/
- 5: https://talk.plesk.com/threads/php-7-4-compatibility-issue-with-wordpress-toolkit.356780/
- 6: https://complianz.io/custom-upload-directory-for-complianz-files/
- 7: https://wordpress.stackexchange.com/questions/28729/wordpress-plugin-dev-using-array-for-wp-options
- 8: https://wpvip.com/how-to-prepare-your-wordpress-site-for-php-8/
- 9: https://developer.wordpress.org/reference/functions/wp_upload_dir/
- 10: https://kinsta.com/blog/wordpress-user-roles/
Verified Compatibility: Path Construction for Uploads Directory is Correct
The code in
classes/models/FrmStyle.php(lines 452-454) correctly constructs the uploads directory path usingwp_upload_dir()['basedir'], which is fully compatible with all supported WordPress PHP versions (PHP 7.4 and above). The web verification confirms that direct array access is safe and recommended. No changes are needed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
classes/models/FrmStyle.php (3)
408-427: Fix duplicate 'folder_name' declaration in method.There's a redundant declaration of 'folder_name'. The method defines it in
$path_argsbut then doesn't use this variable, instead repeating the same value in the array_merge call.private static function get_create_style_file_args() { $add_css_to_uploads_dir = self::add_css_to_uploads_dir(); $create_file_args = array( 'file_name' => FrmStylesController::get_file_name(), 'new_file_path' => self::get_generated_css_file_path( $add_css_to_uploads_dir ), ); if ( ! $add_css_to_uploads_dir ) { return $create_file_args; } $path_args = array( 'folder_name' => 'formidable/css', ); return array_merge( $create_file_args, - array( - 'folder_name' => 'formidable/css', - ) + $path_args ); }
447-455: Update PHPDoc version tags.The
@since x.xtags should be updated with the actual version number before release./** - * @since x.x + * @since 6.x.x * * @return bool */ public static function add_css_to_uploads_dir() { /** - * @since x.x + * @since 6.x.x * * @param bool $add_css_to_uploads_dir */ return apply_filters( 'frm_add_css_to_uploads_dir', false ); }
462-464: Improve array access readability intarget_css_uploads_dir.Direct array access to
wp_upload_dir()['basedir']is less readable. Consider using a variable assignment first.private static function target_css_uploads_dir() { - return wp_upload_dir()['basedir'] . '/formidable/css'; + $upload_dir = wp_upload_dir(); + return $upload_dir['basedir'] . '/formidable/css'; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
classes/models/FrmStyle.php(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
classes/models/FrmStyle.php (2)
classes/controllers/FrmStylesController.php (2)
FrmStylesController(6-1371)get_file_name(251-260)classes/helpers/FrmAppHelper.php (1)
FrmAppHelper(6-4460)
🔇 Additional comments (4)
classes/models/FrmStyle.php (4)
394-396: Refactoring of file creation insave_settingsmethod looks good.The code has been updated to use a dedicated method for preparing file creation arguments, which improves code organization and readability.
435-440: Well-structured path selection method.This method cleanly determines which directory to use for the generated CSS file based on the configuration parameter.
471-473: Consistent path resolution method for plugin directory.This method correctly returns the plugin's CSS directory path using the existing helper.
396-396:Details
❓ Verification inconclusive
Verify file creation in uploads directory.
It's important to ensure the uploads directory is writable when the filter is applied. Consider adding a directory existence check and possibly auto-create the directory if it doesn't exist.
🏁 Script executed:
#!/bin/bash # Check if the filter is being used in any theme or plugin files rg -l "add_filter\(\s*['\"]frm_add_css_to_uploads_dir['\"]" --type php # Check uploads directory structure and permissions echo "Checking uploads directory structure:" if [ -d "$(wp_upload_dir()['basedir'])/formidable/css" ]; then echo "Directory exists" else echo "Directory does not exist" fiLength of output: 417
Action Required: Verify Upload Directory Checks & Permissions
The current verification attempt using the shell script returned a syntax error due to improper usage of PHP’s WP function inside Bash. It appears that the directory check (i.e. ensuring that the "uploads/formidable/css" directory exists and is writable) isn’t directly verifiable with the provided shell snippet.
- In
classes/models/FrmStyle.phpat line 396, the code instantiates a file creation class. However, since this process relies on thefrm_add_css_to_uploads_dirfilter, you need to ensure that the uploads directory is present and writable before any file creation occurs.- Action Needed:
- Update the directory check logic to use an approach that correctly retrieves the uploads directory (e.g., using PHP to call
wp_upload_dir()and then performing the check/auto-creation within PHP, or otherwise verifying the directory through a manual check).- Manually verify the writable state of the uploads directory when the filter is applied.
Please review and adjust the implementation accordingly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
classes/models/FrmStyle.php (3)
407-426: Simplify the folder_name assignment code by removing redundant array.The method properly builds the arguments for file creation, but there's a redundant
$path_argsarray (lines 417-420) that's defined but never used, while the same folder_name is repeated in the array_merge.private static function get_create_style_file_args() { $add_css_to_uploads_dir = self::add_css_to_uploads_dir(); $create_file_args = array( 'file_name' => FrmStylesController::get_file_name(), 'new_file_path' => self::get_generated_css_file_path( $add_css_to_uploads_dir ), ); if ( ! $add_css_to_uploads_dir ) { return $create_file_args; } - $path_args = array( - 'folder_name' => 'formidable/css', - ); return array_merge( $create_file_args, array( 'folder_name' => 'formidable/css', ) ); }
446-454: Update the @SInCE tag with the actual version number.The
@since x.xplaceholder should be replaced with the actual version number before release. This applies to all new methods added.Also, the filter is well designed, allowing developers to override the default behavior and store CSS files in the uploads directory.
461-463: Add error handling for the uploads directory path.The current implementation directly accesses the 'basedir' key without checking if it exists, which could lead to a PHP notice if there's an issue with
wp_upload_dir(). Consider adding a safeguard.private static function target_css_uploads_dir() { - return wp_upload_dir()['basedir'] . '/formidable/css'; + $upload_dir = wp_upload_dir(); + return isset($upload_dir['basedir']) ? $upload_dir['basedir'] . '/formidable/css' : ''; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
classes/models/FrmStyle.php(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
classes/models/FrmStyle.php (2)
classes/controllers/FrmStylesController.php (2)
FrmStylesController(6-1371)get_file_name(251-260)classes/helpers/FrmAppHelper.php (1)
FrmAppHelper(6-4460)
🔇 Additional comments (3)
classes/models/FrmStyle.php (3)
395-396: Code improvement: Refactoring file creation arguments.The code has been improved by extracting the file creation arguments into a separate method, enhancing readability and maintainability.
434-439: LGTM! Path selection logic is clear and well-structured.The method correctly decides which path to use based on the input parameter.
470-472: LGTM! Plugin directory path is correctly determined.The method properly returns the plugin's CSS directory path.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
classes/models/FrmStyle.php (3)
407-426: Refactor redundant array definition.There's a redundant array definition in your code. The
$path_argsarray is defined but never used directly - instead, its contents are duplicated in thearray_merge()call.- $path_args = array( - 'folder_name' => 'formidable/css', - ); - return array_merge( - $create_file_args, - array( - 'folder_name' => 'formidable/css', - ) - ); + return array_merge( + $create_file_args, + array( + 'folder_name' => 'formidable/css', + ) + );Alternatively, use the variable:
$path_args = array( 'folder_name' => 'formidable/css', ); return array_merge( $create_file_args, - array( - 'folder_name' => 'formidable/css', - ) + $path_args );
462-464: Add error handling for wp_upload_dir() access.The current implementation assumes
wp_upload_dir()['basedir']will always be defined, which might not be the case in certain environments.- return wp_upload_dir()['basedir'] . '/formidable/css'; + $upload_dir = wp_upload_dir(); + if ( ! empty( $upload_dir['basedir'] ) ) { + return $upload_dir['basedir'] . '/formidable/css'; + } + // Fallback to plugin directory if uploads directory is not available + return self::target_css_plugin_dir();
402-426: Update version placeholder in PHPDoc comments.The PHPDoc comment contains a placeholder version number (
@since x.x). This should be replaced with the actual version number for proper documentation.- /** - * @since x.x + /** + * @since 6.15This applies to all the new methods that have PHPDoc comments with
@since x.x.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
classes/models/FrmStyle.php(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
classes/models/FrmStyle.php (2)
classes/controllers/FrmStylesController.php (2)
FrmStylesController(6-1367)get_file_name(247-256)classes/helpers/FrmAppHelper.php (1)
FrmAppHelper(6-4460)
🪛 GitHub Check: Psalm
classes/models/FrmStyle.php
[failure] 454-454: TooFewArguments
classes/models/FrmStyle.php:454:57: TooFewArguments: Too few arguments for wp_is_file_mod_allowed - expecting context to be passed (see https://psalm.dev/025)
🪛 GitHub Check: PHPStan
classes/models/FrmStyle.php
[failure] 454-454:
Function wp_is_file_mod_allowed invoked with 0 parameters, 1 required.
🪛 GitHub Actions: PHPUnit
classes/models/FrmStyle.php
[error] 454-454: ArgumentCountError: Too few arguments to function wp_is_file_mod_allowed(), 0 passed and exactly 1 expected.
🪛 GitHub Actions: Psalm Code Analysis
classes/models/FrmStyle.php
[error] 454-454: TooFewArguments: Too few arguments for wp_is_file_mod_allowed - expecting context to be passed (see https://psalm.dev/025)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cypress
🔇 Additional comments (3)
classes/models/FrmStyle.php (3)
434-439: The path generation logic looks good.This method properly delegates to the appropriate target directory method based on the condition.
471-473: Path construction for plugin directory looks good.This method correctly returns the path to the CSS directory within the plugin folder.
395-396: Improved code organization with new method extraction.The refactoring of the file creation logic into separate methods improves code organization and readability. The
save_settingsmethod now delegates the argument preparation to the newget_create_style_file_argsmethod.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
classes/models/FrmStyle.php (1)
441-455: Fixed missing argument in wp_is_file_mod_allowed() function call.The function call now properly includes the context parameter 'frm_can_install_plugin', which resolves the pipeline failures from the previous commit.
🧹 Nitpick comments (2)
classes/models/FrmStyle.php (2)
402-426: Refactor the redundant path arguments.The
$path_argsvariable on lines 417-419 is defined but never used. Instead, the same array structure is directly used in thearray_merge()call.- $path_args = array( - 'folder_name' => 'formidable/css', - ); - return array_merge( - $create_file_args, - array( - 'folder_name' => 'formidable/css', - ) - ); + return array_merge( + $create_file_args, + array( + 'folder_name' => 'formidable/css', + ) + );
457-464: Remove extra whitespace and improve array access.There's an extra space after the
returnkeyword, and the direct array access might not be compatible with older PHP versions.- return wp_upload_dir()['basedir'] . '/formidable/css'; + return wp_upload_dir()['basedir'] . '/formidable/css';For better compatibility with older PHP versions, consider:
- return wp_upload_dir()['basedir'] . '/formidable/css'; + $upload_dir = wp_upload_dir(); + return $upload_dir['basedir'] . '/formidable/css';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
classes/models/FrmStyle.php(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
classes/models/FrmStyle.php (2)
classes/controllers/FrmStylesController.php (2)
FrmStylesController(6-1367)get_file_name(247-256)classes/helpers/FrmAppHelper.php (1)
FrmAppHelper(6-4460)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: Cypress
🔇 Additional comments (3)
classes/models/FrmStyle.php (3)
395-395: Improved code organization by extracting file creation arguments.Good refactoring to move the file creation arguments to a dedicated method. This enhances readability and maintainability.
428-439: Well-structured path determination logic.The method clearly determines the appropriate file path based on the provided condition.
466-473: Clear and concise plugin directory path retrieval.The method efficiently reuses the existing helper method to construct the path.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
classes/models/FrmStyle.php (3)
459-461: Consider adding directory existence check.The method returns a path without verifying if the directory exists or is writable. Consider adding a check to ensure the uploads directory exists and is writable, creating it if necessary.
private static function target_css_uploads_dir() { - return wp_upload_dir()['basedir'] . '/formidable/css'; + $upload_dir = wp_upload_dir(); + $css_dir = $upload_dir['basedir'] . '/formidable/css'; + + // Ensure directory exists and is writable + if (!file_exists($css_dir)) { + wp_mkdir_p($css_dir); + } + + return $css_dir; }
450-451: Document the filter purpose in PHPDoc.The
frm_add_css_to_uploads_dirfilter is documented with a placeholder version but lacks a description of its purpose. Add a description for developers who might want to use this filter./** * @since x.x * + * Filter whether to add CSS to the uploads directory instead of the plugin directory. + * This is useful for environments where the plugin directory is not writable. * * @param bool $add_css_to_uploads_dir */
459-461:Details
❓ Verification inconclusive
Consider using array access notation for better compatibility.
Modern array syntax
['basedir']might not be compatible with older PHP versions (before 5.4). For better compatibility, consider using standard array access notation.private static function target_css_uploads_dir() { - return wp_upload_dir()['basedir'] . '/formidable/css'; + $upload_dir = wp_upload_dir(); + return $upload_dir['basedir'] . '/formidable/css'; }
🏁 Script executed:
#!/bin/bash # Check for PHP version requirements in plugin files grep -r "php" --include="*.php" | grep -i "require\|version" | head -n 10Length of output: 959
Compatibility with Older PHP Versions – Action Required
The current implementation uses inline array dereferencing on the function’s return value:
private static function target_css_uploads_dir() { return wp_upload_dir()['basedir'] . '/formidable/css'; }This syntax is only supported in PHP 5.4 and later. If you need to support environments running PHP versions prior to 5.4, please update the code as follows:
private static function target_css_uploads_dir() { $upload_dir = wp_upload_dir(); return $upload_dir['basedir'] . '/formidable/css'; }If your project already requires PHP 5.4 or higher, then this change is optional. However, if backward compatibility with older PHP versions is desired, please make the suggested modification.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
classes/models/FrmStyle.php(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Cypress
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: PHP 7.4 tests in WP trunk
🔇 Additional comments (4)
classes/models/FrmStyle.php (4)
395-395: Good refactoring of file creation arguments.The code now uses a dedicated method for preparing the arguments for
FrmCreateFileinstead of using an inline array. This improves code readability and maintainability.
407-423: Well-structured method for determining file creation arguments.The new
get_create_style_file_argsmethod effectively encapsulates the logic for determining file path arguments. It uses the result fromadd_css_to_uploads_dir()to decide whether to include afolder_nameparameter, providing flexibility in where CSS files are stored.
431-436: Clean implementation of file path resolution.This method clearly delegates to the appropriate target directory method based on the
$add_css_to_uploads_dirparameter. The code is concise and easy to understand.
445-452: Fixed WordPress function argument issue.The implementation correctly passes a context parameter to
wp_is_file_mod_allowed(), addressing the previously identified pipeline failures.
| /** | ||
| * @since x.x |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Update version placeholders in PHPDoc comments.
All the new methods have a placeholder version x.x in their PHPDoc comments. These should be updated with the actual version number where these methods are being introduced.
- * @since x.x
+ * @since 6.14.2Also applies to: 425-426, 438-439, 454-455, 463-464
🌐 Web query:
What is the latest version of Formidable Forms?
💡 Result:
The latest version of Formidable Forms is 6.20, as indicated by the plugin's listing on WordPress.org, which was updated 6 hours ago[4]. This version includes ongoing optimizations and fixes, such as recent updates to form styling and database query improvements seen in earlier releases[1][5].
For Pro users, updates can be downloaded directly from the Formidable Forms account page or via the WordPress plugins interface, ensuring compatibility with the latest features and security patches[8]. Developers should note that version 6.20 requires Formidable Forms Lite (the free version) to be installed alongside Pro for full functionality[8].
Citations:
- 1: https://github.com/Strategy11/formidable-forms/releases
- 2: https://formidableforms.com/formidable-anniversary-2024/
- 3: https://connect.formidableforms.com/question/category/general-questions/where-can-i-find-different-versions-of-formidable-views/
- 4: https://wordpress.org/plugins/formidable/
- 5: https://github.com/Strategy11/formidable-forms/blob/master/changelog.txt
- 6: https://www.npmjs.com/package/formidable
- 7: https://wordpress.com/plugins/formidable
- 8: https://formidableforms.com/knowledgebase/update-formidable-forms-pro/
- 9: https://plugintests.com/plugins/wporg/formidable/latest
- 10: https://github.com/Strategy11/formidable-forms
Action Required: Update PHPDoc Version Placeholders
The PHPDoc comments in classes/models/FrmStyle.php still show the placeholder x.x for the @since annotations. Based on the latest version of Formidable Forms (6.20), please update these comments accordingly. For example:
- * @since x.x
+ * @since 6.20This change is required at the following locations:
- Lines 402-403
- Lines 425-426
- Lines 438-439
- Lines 454-455
- Lines 463-464
Please ensure that all new methods’ PHPDoc comments reflect version 6.20 to maintain consistency with the current release.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
classes/models/FrmStyle.php (1)
402-404: Update PHPDoc version placeholders.All the new methods have placeholder versions (
x.x) in their PHPDoc comments. These should be updated with the actual version number (6.20) where these methods are being introduced.Also applies to: 425-426, 438-439, 454-455, 463-464
🧹 Nitpick comments (1)
classes/models/FrmStyle.php (1)
459-461: Consider adding error handling for the uploads directory.The method uses
wp_upload_dir()which returns an array, but doesn't check if there are any errors returned in the array (via the 'error' key).private static function target_css_uploads_dir() { - return wp_upload_dir()['basedir'] . '/formidable/css'; + $upload_dir = wp_upload_dir(); + if (!empty($upload_dir['error'])) { + // Fall back to plugin directory if there's an error + return self::target_css_plugin_dir(); + } + return $upload_dir['basedir'] . '/formidable/css'; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
classes/models/FrmStyle.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
classes/models/FrmStyle.php (1)
classes/controllers/FrmStylesController.php (2)
FrmStylesController(6-1367)get_file_name(247-256)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cypress
🔇 Additional comments (5)
classes/models/FrmStyle.php (5)
395-396: Good refactoring of the file creation arguments!The code now uses a dedicated method to generate the file creation arguments, which improves maintainability and readability.
407-423: Well-structured method for determining file creation arguments.This method cleanly determines whether to use the uploads directory or plugin directory for CSS files, and constructs the appropriate arguments accordingly. Good use of the conditional return pattern to avoid unnecessary conditional assignment.
431-436: Good separation of concerns for file path determination.This method properly delegates to the appropriate target directory methods based on the input parameter, maintaining clean code organization.
445-452: Filter implementation correctly integrates with WordPress file permissions.The method properly uses
wp_is_file_mod_allowed()with the required context parameter, which addresses a previous issue flagged in the pipeline failures.
468-470: Clean and straightforward plugin directory path implementation.This method returns the plugin's CSS directory path using the helper method.
truongwp
left a comment
truongwp
left a comment
There was a problem hiding this comment.
This looks good. Thanks @AbdiTolesa!
AbdiTolesa
requested review from
Crabcyborg
and removed request for
Crabcyborg
2 participants
Fix https://github.com/Strategy11/formidable-pro/issues/3179
Test steps
add_filter( 'frm_add_css_to_uploads_dir', '__return_true' );Updatebutton.formidableforms.cssfile is served from uploads dir insideformidable/csssub directory.