fix phpcs issues #563
fix phpcs issues #563
Conversation
arifulhoque7
added
Needs Dev Review
WalkthroughBootstrap loader functions now include runtime validation and file inclusion logic. Public API scaffolding functions added for database migration, routing, and plugin initialization. Extensive PHPCS annotations introduced for public API hook names. Date handling converted from local time to UTC using gmdate. Output escaping improvements applied across notification and error messages. Translation and localization infrastructure enhanced throughout. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
src/Task/Transformers/New_Task_Transformer.php (2)
52-76: Fix incorrect return value.The method builds a
$dataarray with structured user information but returns the raw$userobject instead, which breaks the expected data structure for the creator field.Apply this diff to fix the return value:
- return $user; + return $data;
88-88: Add visibility modifier.The method is missing a visibility modifier (public, protected, or private). This violates PHP coding standards.
Apply this diff to add the visibility modifier:
-function pm_user_can_complete_task( $item ) { +public function pm_user_can_complete_task( $item ) {core/Router/Uri_Parser.php (3)
20-28: Remove premature escaping from exception messages.Escaping exception messages at construction violates the "escape late" principle. Exception messages are primarily consumed by:
- Logging systems where HTML entities reduce readability
- Error handlers that may output to JSON, logs, or HTML
- Debugging tools and stack traces
The correct approach is to escape at output time, not construction time. This allows the exception message to be used flexibly in different contexts.
Apply this diff to remove the premature escaping:
- throw new Invalid_Route_Handler( esc_html( str_replace( '/', '\\', $str ) ) ); + throw new Invalid_Route_Handler( str_replace( '/', '\\', $str ) );Ensure that any code displaying these exceptions to users properly escapes them at the point of output.
38-46: Remove premature escaping from exception messages.Same issue as the previous method: exception messages should remain unescaped until the point of output. This preserves the raw class name for logging and debugging while allowing output handlers to escape appropriately for their context.
Apply this diff:
- throw new Class_Not_Found( esc_html( $class ) ); + throw new Class_Not_Found( $class );
58-66: Remove premature escaping from exception messages.Both exception parameters are being escaped prematurely. Keep the controller and method names unescaped in the exception for proper debugging output.
Apply this diff:
- throw new Undefined_Method_Call( esc_html( $controller ), esc_html( $method ) ); + throw new Undefined_Method_Call( $controller, $method );core/Upgrades/Upgrade_2_2_2.php (1)
36-44: Use the$sqlvariable instead of duplicating the query.The
$sqlvariable is defined on line 40 but never used. The query is duplicated inline on line 43. This should be refactored to use the variable.Apply this diff:
$sql = "UPDATE {$wpdb->prefix}pm_boards SET `order`=99999999 WHERE `title`='Inbox' AND `type`='task_list' AND `description`='This is a system default task list. Any task without an assigned tasklist will appear here.'"; // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- No placeholders needed, static query - $wpdb->query( "UPDATE {$wpdb->prefix}pm_boards SET `order`=99999999 WHERE `title`='Inbox' AND `type`='task_list' AND `description`='This is a system default task list. Any task without an assigned tasklist will appear here.'" ); + $wpdb->query( $sql );src/Task/Transformers/Task_Transformer.php (1)
90-114: Add type hint and explicit sanitization toget_type()method for defensive programming.The method is only called safely from
transform()with an Eloquent model's integer ID, but the public method has no type hint or parameter validation. The phpcs comment claims "values are sanitized" but sanitization occurs upstream (via the ORM), not in the method itself. Since the method is public and could be called from other code paths in the future, adding explicit defensive measures is recommended:- public function get_type( $item_id ) { + public function get_type( int $item_id ) { global $wpdb; + $item_id = absint( $item_id );This ensures the method is robust against future misuse while the current call site is safe.
core/Upgrades/Upgrade_2_3.php (1)
530-531: Pre-existing issue: Incorrect prepared statement pattern.These lines (not modified in this PR) use
$wpdb->prepare()with%sas a placeholder for an entire SQL VALUES/fragment, but%sis for string literals, not SQL syntax. The pattern:$wpdb->prepare( "INSERT INTO table (col1, col2) VALUES %s", $value )would produce invalid SQL like:
INSERT INTO table (col1, col2) VALUES '(1, 1),(1, 2),(1, 3)'Since
$valueis built from database results in loops (lines 518-522, 624-630, 704-706), the data should be trusted, but this usage is incorrect. WordPress's$wpdb->prepare()is meant for values, not SQL fragments.Consider refactoring to build proper INSERT statements or use
$wpdb->insert()in a loop, though this is outside the scope of the current PHPCS fixes.Also applies to: 637-638, 713-714
src/Task/Helper/Task.php (1)
990-1014: Multisite assignee query appears broken (undefined$project_format, inconsistent prepare)In
include_assignees()’s multisite branch:
- The query references
$project_format, but that variable is never defined in the method.$query_datais initialized from$this->task_ids, which doesn’t match theproject_idfilter in the query.- The constructed SQL string does not contain any
%d/%splaceholders, yet it is passed to$wpdb->prepare( $query, $query_data ), which is likely to trigger “wpdb::prepare was called incorrectly” notices and/or unexpected behavior.This combination suggests the multisite path is currently unreliable. It would be safer to either:
- Rewrite the branch to mirror the single‑site case (derive a proper placeholder string and data array for the IDs you actually intend to filter on, and add matching placeholders to
$query), or- If multisite support here is not needed, remove or clearly deprecate this branch to avoid silent failures.
I’d recommend addressing this in or alongside this PHPCS cleanup, since it’s adjacent to the newly annotated code.
src/Task_List/Controllers/Task_List_Controller.php (1)
621-648: Preserve keyword filters when normalizing$due_date.Running
gmdate()on$filter_params['due_date']before this branch turns special values like'overdue','today', and'week'into'1970-01-01', so none of the comparisons fire and every list loses those filters. This is a functional regression for the task widgets.Please keep the original keyword intact (and only normalize actual date strings, if needed):
- $due_date = empty( $filter_params['due_date'] ) ? false : gmdate( 'Y-m-d', strtotime( $filter_params['due_date'] ) ); + $due_date = empty( $filter_params['due_date'] ) ? false : $filter_params['due_date'];This restores the prior behavior where tokens pass through untouched.
🧹 Nitpick comments (16)
src/Common/Traits/Transformer_Manager.php (1)
15-18: Consider consolidating redundant phpcs:ignore comments.The phpcs:ignore comment is added both before the
ifstatement (lines 15, 35) and inside the block (lines 17, 37), which appears redundant. Typically, a PHPCS directive on theifline should suppress warnings on that line; the inner comment on the direct$_GETaccess may or may not be necessary depending on how PHPCS processes scope.The explanatory comments clearly document that nonce verification is intentionally not implemented, which aligns with the PR's objective of adding rationale for suppressions. However, verify whether both comments are truly needed or if one would suffice—this could reduce noise.
If the inner comment (lines 17, 37) is not required by PHPCS, consider removing it:
- // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Nonce verification is not implemented for this AJAX call if ( isset( $_GET['with'] ) ) { - // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Nonce verification is not implemented for this AJAX call $manager->parseIncludes( sanitize_text_field( wp_unslash( $_GET['with'] ) ) ) ; }To confirm whether the inner comment is necessary, run PHPCS with
--show-sourceson the original code to see exactly which line(s) trigger the warning.Also applies to: 35-38
src/Imports/Helpers/Trello_helper.php (1)
37-49: PHPCS suppression is correctly scoped and documented.The disable/enable comments properly wrap all four cURL functions and provide a clear explanation for the suppression. The format follows WordPress coding standards for exceptions.
However, consider whether WordPress remote functions (
wp_remote_get()) could replace this cURL usage. The current code sets only basic options (return transfer, URL) that are standard in WordPress remote functions, which would eliminate the need for the suppression entirely.// Alternative approach using wp_remote_get: $response = wp_remote_get( $this->remote_addr( $source, $querystring ) ); if ( is_wp_error( $response ) ) { return null; // or handle error appropriately } $resp = wp_remote_retrieve_body( $response ); return $calback( $resp );This would align with WordPress best practices and eliminate the need for the PHPCS suppression.
src/Task/Transformers/New_Task_Transformer.php (2)
104-104: Use strict comparison.Using loose comparison (
==) can lead to unexpected type coercion. Consider using strict comparison (===) for more predictable behavior.Apply this diff:
- if ( $item->created_by == $user_id ) { + if ( $item->created_by === $user_id ) {
158-158: Add braces for control structure.WordPress coding standards recommend using braces for all control structures, including single-line statements.
Apply this diff:
- if ( empty( $user_id ) ) continue; + if ( empty( $user_id ) ) { + continue; + }languages/wedevs-project-manager.php (2)
365-365: Duplicate translation string detected.The string "Add List" with domain 'wedevs-project-manager' now appears twice in this file (lines 365 and 519). While this doesn't break functionality, having duplicate entries is redundant and inefficient.
Consider removing one of the duplicate entries:
- __( 'Add List', 'wedevs-project-manager' ),
532-532: Duplicate translation string detected.The string "Pusher" with domain 'wedevs-project-manager' now appears twice in this file (lines 532 and 567). While this doesn't break functionality, having duplicate entries is redundant and inefficient.
Consider removing one of the duplicate entries:
- __( 'Pusher', 'wedevs-project-manager' ),src/Project/Helper/Project_Role_Relation.php (3)
52-57: Remove unused loop variable.The
$keyvariable in theforeachloop is never used.Apply this diff:
-foreach ( $role_project_ids as $key => $role_project_id ) { +foreach ( $role_project_ids as $role_project_id ) {
64-72: Use consistent phpcs:ignore annotations.Lines 64 and 69 omit
PluginCheck.Security.DirectDB.UnescapedDBParameterfrom the suppression list, while line 59 includes it. Since all three queries have identical structure, the annotations should match for consistency.Apply this diff:
-// phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Table name is interpolated, values are sanitized +// phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared,PluginCheck.Security.DirectDB.UnescapedDBParameter -- Table name is interpolated, values are sanitized. Table names cannot be placeholders. $role_project_id = $wpdb->get_var( $wpdb->prepare( "SELECT id FROM $tb_role_project WHERE project_id=%d AND role_id=%d", $project_id, 3 ) ); $this->role_project_id = $role_project_id; $this->role_project_user( $project, 3, $role_project_id ); -// phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Table name is interpolated, values are sanitized +// phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared,PluginCheck.Security.DirectDB.UnescapedDBParameter -- Table name is interpolated, values are sanitized. Table names cannot be placeholders. $role_project_id = $wpdb->get_var( $wpdb->prepare( "SELECT id FROM $tb_role_project WHERE project_id=%d AND role_id=%d", $project_id, 1 ) );
164-169: Remove unused loop variable.The
$keyvariable in theforeachloop is never used.Apply this diff:
-foreach ( $rpids as $key => $rpid ) { +foreach ( $rpids as $rpid ) {core/Admin_Notice/Admin_Notice.php (1)
174-179: Escaping looks solid; you can simplify by dropping the redundantsprintf/variable.
$offeris built from static HTML plusesc_html__calls and then passed throughwp_kses_postat output, which is a good escaping pattern for this notice. The intermediatesprintf( '%s', $offer )and$offer_msgindirection are unnecessary though.You can simplify to:
- $offer_msg = sprintf( '%s', $offer ); ... - <div class="pm-offer-msg-wrap"><?php echo wp_kses_post( $offer_msg ); ?></div> + // $offer already contains the complete, escaped markup. ... + <div class="pm-offer-msg-wrap"><?php echo wp_kses_post( $offer ); ?></div>Keeps behavior the same while reducing a bit of noise.
Please re-run PHPCS on this file after simplifying to confirm no new sniffs are triggered around the escaping.
src/Pusher/Src/Controllers/Pusher_Controller.php (1)
43-45: Consider using consistent JSON error responses and WordPress conventions.The error handling has a few inconsistencies:
The
header('', true, 403)syntax is unusual. Consider using a proper status message:header('HTTP/1.1 403 Forbidden', true, 403)or WordPress functions likewp_send_json_error().The success path returns JSON, but the error path returns plain text "Forbidden", creating inconsistent response formats.
For consistency with the authenticated path, the output should be escaped.
Consider refactoring to use WordPress JSON response helpers:
} else { - header('', true, 403); - echo "Forbidden"; - exit; + wp_send_json_error( + array( 'message' => 'Forbidden' ), + 403 + ); }This provides consistent JSON responses, proper HTTP status codes, and automatic output escaping.
routes/category.php (1)
5-10: Route variable PHPCS suppressions look good; consider removing unused variableThe
phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFoundannotations correctly describe these as route-file variables rather than true globals and won’t affect behavior. As a minor follow-up,$project_manage_capabilityappears unused in this file, so it could be removed in a later cleanup to avoid dead code.core/Permissions/Create_File.php (1)
12-20: Hook-name PHPCS suppression is correct; error message text could be alignedThe
phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFoundonpm_check_permissionis appropriate and keeps the public hook name intact without altering logic. As a small future improvement, consider updating the error message (“create message”) to mention “file” to match thecreate_fileaction for clearer feedback.src/Pusher/Libs/action.php (1)
86-86: Consider removing unused parameters.The
$newand$oldparameters are not used within the function body. If these parameters are required by the action hook signature, consider adding a comment to document why they're unused. Otherwise, they could be removed for clarity.src/Settings/Helper/Settings.php (1)
78-86: Inconsistent phpcs:ignore annotations across similar query patterns.The annotation at line 78 for
InterpolatedNotPreparedis missing for the identical query pattern inset_client_role_project_id(line 92) andset_manager_role_project_id(line 105). All three methods build queries with interpolated table names in the same way.Apply consistent annotations:
private function set_client_role_project_id( $project_id ) { global $wpdb; $tb_rol = $wpdb->prefix . 'pm_role_project'; + // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- $tb_rol is a table name $query = "SELECT id FROM $tb_rol WHERE project_id=%d and role_id=%d"; // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- Query is prepared above $this->client_role_project_id = $wpdb->get_var( $wpdb->prepare( $query, $project_id, 3 ) );private function set_manager_role_project_id( $project_id ) { global $wpdb; $tb_rol = $wpdb->prefix . 'pm_role_project'; + // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- $tb_rol is a table name $query = "SELECT id FROM $tb_rol WHERE project_id=%d and role_id=%d"; // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- Query is prepared above $this->manager_role_project_id = $wpdb->get_var( $wpdb->prepare( $query, $project_id, 1 ) );bootstrap/loaders.php (1)
10-116: Initialize the accumulators before returning.If
glob()ever yields an empty array (or if the directories are user-pruned),$config/$langremain undefined, triggering notices and returningnull. A quick guard keeps the helpers robust:- foreach ( $files as $file ) { + $config = []; + foreach ( $files as $file ) { $config[basename( $file, '.php' )] = include $file; }Apply the same pattern to
pm_load_texts()for symmetry.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (107)
bootstrap/loaders.php(9 hunks)bootstrap/start.php(1 hunks)config/scripts.php(3 hunks)config/style.php(1 hunks)core/Admin_Notice/Admin_Notice.php(1 hunks)core/Cli/Commands.php(4 hunks)core/DB_Connection/Model.php(2 hunks)core/File_System/File_System.php(2 hunks)core/Notifications/Background_Emailer.php(1 hunks)core/Notifications/Email.php(4 hunks)core/Notifications/Emails/Complete_Task_Notification.php(1 hunks)core/Notifications/Emails/New_Comment_Notification.php(1 hunks)core/Notifications/Emails/New_Message_Notification.php(1 hunks)core/Notifications/Emails/New_Project_Notification.php(1 hunks)core/Notifications/Emails/New_Task_Notification.php(1 hunks)core/Notifications/Emails/Update_Comment_Notification.php(1 hunks)core/Notifications/Emails/Update_Message_Notification.php(1 hunks)core/Notifications/Emails/Update_Project_Notification.php(1 hunks)core/Notifications/Emails/Update_Task_Notification.php(1 hunks)core/Notifications/Notification.php(4 hunks)core/Permissions/Create_Discuss.php(1 hunks)core/Permissions/Create_File.php(1 hunks)core/Permissions/Create_Milestone.php(1 hunks)core/Permissions/Create_Task.php(1 hunks)core/Permissions/Create_Task_List.php(1 hunks)core/Permissions/Delete_Task.php(1 hunks)core/Promotions/Offers.php(3 hunks)core/Router/Uri_Parser.php(3 hunks)core/Upgrades/Upgrade_2_0.php(4 hunks)core/Upgrades/Upgrade_2_2_1.php(1 hunks)core/Upgrades/Upgrade_2_2_2.php(1 hunks)core/Upgrades/Upgrade_2_3.php(6 hunks)core/User_Profile/Profile_Update.php(2 hunks)core/WP/Enqueue_Scripts.php(5 hunks)core/WP/Frontend.php(2 hunks)core/WP/Menu.php(3 hunks)db/Create_Table.php(1 hunks)db/seeds/RoleTableSeeder.php(1 hunks)languages/wedevs-project-manager.php(2 hunks)libs/configurations.php(2 hunks)libs/functions.php(76 hunks)libs/sanitization-filters.php(1 hunks)libs/validation-rules.php(2 hunks)routes/activity.php(1 hunks)routes/category.php(1 hunks)routes/comment.php(1 hunks)routes/discussion-board.php(1 hunks)routes/file.php(1 hunks)routes/milestone.php(1 hunks)routes/mytask.php(1 hunks)routes/project.php(1 hunks)routes/pusher.php(1 hunks)routes/role.php(1 hunks)routes/search.php(1 hunks)routes/settings.php(1 hunks)routes/task-list.php(1 hunks)routes/task.php(1 hunks)routes/trello.php(1 hunks)routes/user.php(1 hunks)src/Activity/Controllers/Activity_Controller.php(1 hunks)src/Activity/Helper/Activity.php(4 hunks)src/Category/Transformers/Category_Transformer.php(1 hunks)src/Comment/Controllers/Comment_Controller.php(3 hunks)src/Comment/Helper/Comment.php(3 hunks)src/Comment/Transformers/Comment_Transformer.php(1 hunks)src/Common/Traits/Assignee_Filter.php(1 hunks)src/Common/Traits/Transformer_Manager.php(2 hunks)src/Common/Transformers/Assignee_Transformer.php(1 hunks)src/Discussion_Board/Controllers/Discussion_Board_Controller.php(8 hunks)src/Discussion_Board/Helper/Discussion_Board.php(9 hunks)src/Discussion_Board/Transformers/Discussion_Board_Transformer.php(2 hunks)src/File/Controllers/File_Controller.php(2 hunks)src/File/Helper/File.php(2 hunks)src/File/Transformers/File_Transformer.php(1 hunks)src/Imports/Helpers/Trello_helper.php(2 hunks)src/Milestone/Controllers/Milestone_Controller.php(9 hunks)src/Milestone/Helper/Milestone.php(9 hunks)src/Milestone/Transformers/Milestone_Transformer.php(4 hunks)src/My_Task/Controllers/MyTask_Controller.php(4 hunks)src/Project/Controllers/Project_Controller.php(13 hunks)src/Project/Helper/Project.php(27 hunks)src/Project/Helper/Project_Role_Relation.php(2 hunks)src/Project/Models/Project.php(1 hunks)src/Project/Transformers/Project_Transformer.php(5 hunks)src/Project/Validators/Create_Project.php(1 hunks)src/Pusher/Core/Auth/Pusher_Crypto.php(2 hunks)src/Pusher/Libs/action.php(13 hunks)src/Pusher/Libs/filter.php(1 hunks)src/Pusher/Libs/function.php(2 hunks)src/Pusher/Src/Controllers/Pusher_Controller.php(1 hunks)src/Settings/Controllers/AI_Settings_Controller.php(2 hunks)src/Settings/Controllers/Settings_Controller.php(1 hunks)src/Settings/Helper/Settings.php(4 hunks)src/Settings/Transformers/Settings_Transformer.php(1 hunks)src/Task/Controllers/Task_Controller.php(34 hunks)src/Task/Helper/Task.php(28 hunks)src/Task/Models/Task.php(2 hunks)src/Task/Observers/Task_Observer.php(4 hunks)src/Task/Task_Model_Trait.php(0 hunks)src/Task/Transformers/New_Task_Transformer.php(1 hunks)src/Task/Transformers/Task_Transformer.php(3 hunks)src/Task_List/Controllers/Task_List_Controller.php(12 hunks)src/Task_List/Helper/Task_List.php(13 hunks)src/Task_List/Models/Task_List.php(1 hunks)src/Task_List/Transformers/List_Task_Transformer.php(1 hunks)src/Task_List/Transformers/New_Task_List_Transformer.php(1 hunks)src/Task_List/Transformers/Task_List_Transformer.php(8 hunks)
⛔ Files not processed due to max files limit (24)
- src/Tools/Helpers/ImportActivecollab.php
- src/Tools/Helpers/ImportAsana.php
- src/Tools/Helpers/ImportTrello.php
- src/Tools/Library/PM_Asana.php
- src/User/Controllers/User_Controller.php
- src/User/Helper/User.php
- src/User/Transformers/User_Transformer.php
- tests/_support/AcceptanceTester.php
- tests/_support/FunctionalTester.php
- tests/_support/UnitTester.php
- tests/acceptance/FirstCest.php
- tests/acceptance/Scenario/01CreateActivitiesCest.php
- texts/activities.php
- views/emails/html/complete-task.php
- views/emails/html/footer.php
- views/emails/html/header.php
- views/emails/html/new-comment.php
- views/emails/html/new-message.php
- views/emails/html/new-project.php
- views/emails/html/new-task.php
- views/emails/html/update-comment.php
- views/emails/html/update-message.php
- views/emails/html/update-project.php
- views/emails/html/update-task.php
💤 Files with no reviewable changes (1)
- src/Task/Task_Model_Trait.php
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-02-04T05:54:21.894Z
Learnt from: iftakharul-islam
Repo: weDevsOfficial/wp-project-manager PR: 529
File: src/Project/Helper/Project.php:1340-1350
Timestamp: 2025-02-04T05:54:21.894Z
Learning: In WordPress, table names constructed using `$wpdb->prefix` concatenated with hardcoded strings (e.g., `$wpdb->prefix . 'pm_projects'`) are secure and don't need `$wpdb->prepare()`.
Applied to files:
core/Upgrades/Upgrade_2_2_2.phpcore/Upgrades/Upgrade_2_3.phpdb/Create_Table.phpsrc/Project/Helper/Project_Role_Relation.phplibs/configurations.phpsrc/Task/Helper/Task.phpsrc/Pusher/Libs/action.phpsrc/Project/Helper/Project.phplibs/functions.php
🧬 Code graph analysis (34)
core/Notifications/Emails/New_Task_Notification.php (2)
core/Notifications/Email.php (2)
get_template_path(45-69)get_blogname(179-181)views/assets/src/helpers/i18n/i18n.js (1)
__(45-47)
src/Pusher/Src/Controllers/Pusher_Controller.php (1)
src/Pusher/Core/Auth/Auth.php (1)
socket_auth(36-61)
routes/search.php (2)
src/Pusher/Pusher.php (1)
router(51-55)core/Singletonable.php (1)
singleton(27-37)
src/Comment/Controllers/Comment_Controller.php (2)
src/File/Models/File.php (1)
comment(28-30)src/Common/Traits/Transformer_Manager.php (1)
get_response(11-29)
src/Milestone/Transformers/Milestone_Transformer.php (2)
src/Milestone/Helper/Milestone.php (2)
task_lists(233-287)discussion_boards(176-231)src/Milestone/Models/Milestone.php (2)
task_lists(87-91)discussion_boards(103-107)
routes/task.php (2)
src/Pusher/Pusher.php (1)
router(51-55)core/Singletonable.php (1)
singleton(27-37)
languages/wedevs-project-manager.php (1)
views/assets/src/helpers/i18n/i18n.js (1)
__(45-47)
src/Milestone/Controllers/Milestone_Controller.php (3)
src/Milestone/Helper/Milestone.php (1)
achieve_date(289-318)src/Milestone/Transformers/Milestone_Transformer.php (1)
meta(54-61)src/File/Models/File.php (1)
meta(32-34)
routes/settings.php (2)
src/Pusher/Pusher.php (1)
router(51-55)core/Singletonable.php (1)
singleton(27-37)
core/Notifications/Emails/Complete_Task_Notification.php (1)
core/Notifications/Email.php (2)
get_template_path(45-69)get_blogname(179-181)
core/Notifications/Emails/Update_Comment_Notification.php (2)
core/Notifications/Email.php (2)
get_template_path(45-69)get_blogname(179-181)views/assets/src/helpers/i18n/i18n.js (1)
__(45-47)
core/Notifications/Emails/Update_Project_Notification.php (2)
core/Notifications/Email.php (2)
get_template_path(45-69)get_blogname(179-181)views/assets/src/helpers/i18n/i18n.js (1)
__(45-47)
libs/configurations.php (1)
core/Config/Config.php (1)
Config(7-66)
routes/comment.php (2)
src/Pusher/Pusher.php (1)
router(51-55)core/Singletonable.php (1)
singleton(27-37)
core/Notifications/Emails/Update_Message_Notification.php (2)
core/Notifications/Email.php (2)
get_template_path(45-69)get_blogname(179-181)views/assets/src/helpers/i18n/i18n.js (1)
__(45-47)
src/Task/Controllers/Task_Controller.php (2)
src/Task/Helper/Task.php (3)
Task(22-1881)status(357-367)where(1183-1201)src/Task/Models/Task.php (1)
Task(20-139)
src/Task/Helper/Task.php (2)
src/Project/Helper/Project.php (3)
where(1138-1150)join(1112-1131)get_results(66-88)src/Task/Observers/Task_Observer.php (1)
due_date(76-87)
core/Notifications/Emails/New_Project_Notification.php (1)
core/Notifications/Email.php (2)
get_template_path(45-69)get_blogname(179-181)
src/Project/Transformers/Project_Transformer.php (3)
src/User/Transformers/User_Transformer.php (1)
includeMeta(115-232)src/Project/Helper/Project.php (2)
Project(7-1451)where(1138-1150)src/Project/Models/Project.php (6)
Project(21-136)task_lists(71-73)tasks(75-77)discussion_boards(79-81)milestones(83-85)files(92-94)
core/Notifications/Emails/Update_Task_Notification.php (2)
core/Notifications/Email.php (2)
get_template_path(45-69)get_blogname(179-181)views/assets/src/helpers/i18n/i18n.js (1)
__(45-47)
core/Notifications/Emails/New_Message_Notification.php (2)
core/Notifications/Email.php (2)
get_template_path(45-69)get_blogname(179-181)views/assets/src/helpers/i18n/i18n.js (1)
__(45-47)
core/WP/Menu.php (2)
views/assets/src/helpers/i18n/i18n.js (1)
__(45-47)libs/functions.php (2)
pm_user_can_access(462-475)pm_manager_cap_slug(1422-1425)
src/Pusher/Libs/action.php (1)
libs/functions.php (3)
pm_clean(1034-1040)pm_get_list_url(766-773)pm_get_discuss_url(735-741)
routes/discussion-board.php (2)
src/Pusher/Pusher.php (1)
router(51-55)core/Singletonable.php (1)
singleton(27-37)
core/Admin_Notice/Admin_Notice.php (1)
libs/configurations.php (1)
config(7-9)
src/Discussion_Board/Controllers/Discussion_Board_Controller.php (4)
src/Comment/Models/Comment.php (1)
discussion_board(46-48)src/Discussion_Board/Transformers/Discussion_Board_Transformer.php (1)
Discussion_Board_Transformer(16-115)libs/functions.php (1)
pm_get_text(16-18)src/Common/Traits/Transformer_Manager.php (1)
get_response(11-29)
src/Project/Controllers/Project_Controller.php (3)
src/Project/Transformers/Project_Transformer.php (1)
Project_Transformer(15-164)src/Common/Traits/Transformer_Manager.php (1)
get_response(11-29)libs/functions.php (1)
pm_get_text(16-18)
bootstrap/loaders.php (3)
db/seeds/RoleTableSeeder.php (1)
RoleTableSeeder(10-64)core/Router/WP_Router.php (2)
WP_Router(11-322)register(26-30)core/WP/Frontend.php (1)
Frontend(26-395)
routes/category.php (2)
src/Pusher/Pusher.php (1)
router(51-55)core/Singletonable.php (1)
singleton(27-37)
core/Notifications/Emails/New_Comment_Notification.php (1)
core/Notifications/Email.php (2)
get_template_path(45-69)get_blogname(179-181)
src/Project/Helper/Project.php (1)
src/Task/Helper/Task.php (1)
where(1183-1201)
core/Notifications/Email.php (1)
libs/functions.php (1)
pm_get_setting(146-172)
src/Task_List/Controllers/Task_List_Controller.php (5)
src/Task_List/Transformers/Task_List_Transformer.php (1)
Task_List_Transformer(20-210)src/Common/Traits/Transformer_Manager.php (1)
get_response(11-29)core/File_System/File_System.php (1)
delete(161-165)src/Task/Observers/Task_Observer.php (1)
due_date(76-87)src/Task/Controllers/Task_Controller.php (1)
filter(890-969)
libs/functions.php (5)
core/Textdomain/Textdomain.php (2)
Textdomain(7-95)get_text(28-53)src/Project/Helper/Project.php (1)
Project(7-1451)src/Task/Helper/Task.php (1)
Task(22-1881)src/Task_List/Helper/Task_List.php (2)
Task_List(20-1026)milestone(402-457)src/Milestone/Helper/Milestone.php (1)
Milestone(19-626)
🪛 PHPMD (2.15.0)
src/Project/Helper/Project_Role_Relation.php
52-52: Avoid unused local variables such as '$key'. (undefined)
(UnusedLocalVariable)
164-164: Avoid unused local variables such as '$key'. (undefined)
(UnusedLocalVariable)
core/WP/Menu.php
66-66: Avoid unused local variables such as '$today'. (undefined)
(UnusedLocalVariable)
src/Pusher/Libs/action.php
86-86: Avoid unused parameters such as '$new'. (undefined)
(UnusedFormalParameter)
86-86: Avoid unused parameters such as '$old'. (undefined)
(UnusedFormalParameter)
libs/functions.php
531-531: Avoid unused parameters such as '$user_id'. (undefined)
(UnusedFormalParameter)
| $offer = '<h2>' . esc_html__( 'WP Project Manager Pro required version 2.0 or above.', 'wedevs-project-manager' ) . '</h2>'; | ||
| $offer .= '<p>' . esc_html__( 'To migrate version 2.0, Please read migration docs', 'wedevs-project-manager' ) . '</p>'; |
There was a problem hiding this comment.
Consider minor copy edits for clearer English in the notice.
The strings are fine functionally, but the wording is slightly awkward. If you’re touching this text anyway, you might want to make it read more naturally:
- “required version 2.0 or above” → “requires version 2.0 or above”
- “To migrate version 2.0, Please read migration docs” → “To migrate to version 2.0, please read the migration docs.”
Example diff:
- $offer = '<h2>' . esc_html__( 'WP Project Manager Pro required version 2.0 or above.', 'wedevs-project-manager' ) . '</h2>';
- $offer .= '<p>' . esc_html__( 'To migrate version 2.0, Please read migration docs', 'wedevs-project-manager' ) . '</p>';
+ $offer = '<h2>' . esc_html__( 'WP Project Manager Pro requires version 2.0 or above.', 'wedevs-project-manager' ) . '</h2>';
+ $offer .= '<p>' . esc_html__( 'To migrate to version 2.0, please read the migration docs.', 'wedevs-project-manager' ) . '</p>';Purely optional; no impact on PHPCS.
🤖 Prompt for AI Agents
In core/Admin_Notice/Admin_Notice.php around lines 171-172, update the wording
for clarity: change "WP Project Manager Pro required version 2.0 or above." to
"WP Project Manager Pro requires version 2.0 or above." and change "To migrate
version 2.0, Please read migration docs" to "To migrate to version 2.0, please
read the migration docs." Keep the existing esc_html__ calls and
concatenation/HTML structure intact, only adjust the string literals and
punctuation/capitalization as noted.
| // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound -- Legacy hook name, part of public API | ||
| do_action( 'cpm_after_upload_file', $$attachment_id ); |
There was a problem hiding this comment.
CRITICAL: Fix variable syntax error on line 17.
Line 17 contains $$attachment_id (double dollar sign), which uses PHP's variable variable syntax. This is almost certainly unintended and will cause a runtime error. Should be $attachment_id (single dollar sign).
Apply this fix:
- // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound -- Legacy hook name, part of public API
- do_action( 'cpm_after_upload_file', $$attachment_id );
+ // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound -- Legacy hook name, part of public API
+ do_action( 'cpm_after_upload_file', $attachment_id );📝 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.
| // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound -- Legacy hook name, part of public API | |
| do_action( 'cpm_after_upload_file', $$attachment_id ); | |
| // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound -- Legacy hook name, part of public API | |
| do_action( 'cpm_after_upload_file', $attachment_id ); |
🤖 Prompt for AI Agents
In core/File_System/File_System.php around lines 16 to 17, the do_action call
uses PHP variable-variable syntax `$$attachment_id` by mistake; replace the
double dollar sign with a single dollar sign so the call passes the actual
$attachment_id variable (keep the existing phpcs comment and hook name
unchanged).
| // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error -- Used for debugging within WP_DEBUG block | ||
| trigger_error( esc_attr__( 'Transactional email triggered fatal error for callback ', 'wedevs-project-manager' ) . esc_attr( $callback['filter'] ), E_USER_WARNING ); |
There was a problem hiding this comment.
Reconsider the escaping strategy for the error message.
The use of esc_attr__() on the error message string is incorrect. The __() function returns a translated string but does not escape it—the esc_attr prefix in esc_attr__() is misleading. Additionally, applying esc_attr() to $callback['filter'] (a hook name) is unnecessary since filter names are controlled by code, not user input.
For trigger_error(), no HTML escaping is needed since the output goes to error logs, not HTML contexts.
Apply this diff to remove unnecessary escaping:
- // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error -- Used for debugging within WP_DEBUG block
- trigger_error( esc_attr__( 'Transactional email triggered fatal error for callback ', 'wedevs-project-manager' ) . esc_attr( $callback['filter'] ), E_USER_WARNING );
+ // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error -- Used for debugging within WP_DEBUG block
+ trigger_error( __( 'Transactional email triggered fatal error for callback ', 'wedevs-project-manager' ) . $callback['filter'], E_USER_WARNING );📝 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.
| // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error -- Used for debugging within WP_DEBUG block | |
| trigger_error( esc_attr__( 'Transactional email triggered fatal error for callback ', 'wedevs-project-manager' ) . esc_attr( $callback['filter'] ), E_USER_WARNING ); | |
| // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error -- Used for debugging within WP_DEBUG block | |
| trigger_error( __( 'Transactional email triggered fatal error for callback ', 'wedevs-project-manager' ) . $callback['filter'], E_USER_WARNING ); |
🤖 Prompt for AI Agents
In core/Notifications/Background_Emailer.php around lines 48-49, the error
message currently uses esc_attr__() and esc_attr() unnecessarily; replace the
trigger_error call to use a non-escaped translation function and interpolate the
hook name directly, e.g. build the message with sprintf( __( 'Transactional
email triggered fatal error for callback %s', 'wedevs-project-manager' ),
$callback['filter'] ) and pass that to trigger_error(..., E_USER_WARNING) so no
HTML escaping functions are used for a log message.
| // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error -- Used for debugging within WP_DEBUG block | ||
| trigger_error( esc_attr__( 'Transactional email triggered fatal error for callback ', 'wedevs-project-manager' ) . esc_attr( current_filter() ), E_USER_WARNING ); |
There was a problem hiding this comment.
Inappropriate use of esc_attr() for error messages.
The use of esc_attr() is designed for escaping HTML attribute values, not for sanitizing error messages. For trigger_error(), you don't need HTML escaping since it's a PHP error handler, not HTML output. Additionally, current_filter() returns a hook name string that doesn't require sanitization.
Apply this fix:
-// phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error -- Used for debugging within WP_DEBUG block
-trigger_error( esc_attr__( 'Transactional email triggered fatal error for callback ', 'wedevs-project-manager' ) . esc_attr( current_filter() ), E_USER_WARNING );
+// phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error -- Used for debugging within WP_DEBUG block
+trigger_error( __( 'Transactional email triggered fatal error for callback ', 'wedevs-project-manager' ) . current_filter(), E_USER_WARNING );If you want to sanitize for logging, use sanitize_text_field() instead, though it's not necessary for current_filter().
📝 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.
| // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error -- Used for debugging within WP_DEBUG block | |
| trigger_error( esc_attr__( 'Transactional email triggered fatal error for callback ', 'wedevs-project-manager' ) . esc_attr( current_filter() ), E_USER_WARNING ); | |
| // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error -- Used for debugging within WP_DEBUG block | |
| trigger_error( __( 'Transactional email triggered fatal error for callback ', 'wedevs-project-manager' ) . current_filter(), E_USER_WARNING ); |
🤖 Prompt for AI Agents
In core/Notifications/Notification.php around lines 117-118, the code uses
esc_attr__() to build a message for trigger_error(); change this to a
translation function intended for non-HTML output (e.g. __() or _x()) and stop
using esc_attr() for the hook name; if you must sanitize for logging wrap
current_filter() with sanitize_text_field(), otherwise use current_filter()
directly, then concatenate that into the translated message and pass it to
trigger_error() as before.
| if ( $current_time >= $promo_notice['start_date'] && $current_time <= $promo_notice['end_date'] ) { | ||
| $offer->link = $promo_notice['action_url']; | ||
| $offer->key = $promo_notice['key']; | ||
| $offer->btn_txt = ! empty( $promo_notice['action_title'] ) ? $promo_notice['action_title'] : 'Get Now'; |
There was a problem hiding this comment.
Make the default button text translatable.
The hardcoded fallback 'Get Now' should be wrapped in a translation function to support multilingual sites.
Apply this diff:
- $offer->btn_txt = ! empty( $promo_notice['action_title'] ) ? $promo_notice['action_title'] : 'Get Now';
+ $offer->btn_txt = ! empty( $promo_notice['action_title'] ) ? $promo_notice['action_title'] : __( 'Get Now', 'wedevs-project-manager' );📝 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.
| $offer->btn_txt = ! empty( $promo_notice['action_title'] ) ? $promo_notice['action_title'] : 'Get Now'; | |
| $offer->btn_txt = ! empty( $promo_notice['action_title'] ) ? $promo_notice['action_title'] : __( 'Get Now', 'wedevs-project-manager' ); |
🤖 Prompt for AI Agents
In core/Promotions/Offers.php around line 50, the fallback hardcoded button text
'Get Now' must be made translatable; replace the literal string with a WordPress
translation call (e.g. esc_html__( 'Get Now', 'your-text-domain' ) or __('Get
Now', 'your-text-domain') depending on context/escaping conventions in this
file) so the default label can be localized, using the plugin/theme text domain
already in use (or replace 'your-text-domain' with the correct one).
| // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_var_dump -- Debug function for development | ||
| var_dump('aslkdjhflaksdjfhasdf'); die(); |
There was a problem hiding this comment.
Remove debug statements instead of suppressing PHPCS warnings.
The PR description states that var_dump() calls should be removed from development code, but this one is being suppressed instead. The var_dump() combined with die() appears to be debug code that will break functionality in production.
Apply this diff to remove the debug statement:
public function assigned_to() {
- // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_var_dump -- Debug function for development
- var_dump('aslkdjhflaksdjfhasdf'); die();
}Alternatively, if this method should have actual business logic, please implement the intended functionality instead of the debug statement.
🤖 Prompt for AI Agents
In src/Common/Traits/Assignee_Filter.php around lines 10-11 there is a leftover
debug statement "var_dump(...); die();" that is suppressed by PHPCS; remove
those debug calls entirely and any PHPCS suppression comment. Replace them with
the proper implementation for this method (implement the intended business
logic) or, if no action is required here, return an appropriate value (e.g.,
null/false/empty array) or throw a meaningful exception — do not leave debug
output or die() in production code.
| { | ||
| if (!self::is_encrypted_channel($channel)) { | ||
| throw new PusherException('You must specify a channel of the form private-encrypted-* for E2E encryption. Got '.$channel); | ||
| throw new PusherException('You must specify a channel of the form private-encrypted-* for E2E encryption. Got '.esc_html($channel)); |
There was a problem hiding this comment.
Remove HTML escaping from exception messages.
Exception messages are not HTML output and should not be escaped with esc_html(). This function is designed for escaping content that will be rendered in HTML contexts, not for exception messages that are typically used in logs, debugging, or error handling. Applying HTML escaping here will:
- Convert characters like
<,>, and&to HTML entities in logs and error messages, making them harder to read - Violate separation of concerns—exception construction shouldn't assume the display context
- Create a misleading precedent suggesting all string concatenation requires HTML escaping
If these exception messages are ever displayed in HTML, escape them at the point of display, not during construction.
Apply this diff to remove the inappropriate escaping:
- throw new PusherException('You must specify a channel of the form private-encrypted-* for E2E encryption. Got '.esc_html($channel));
+ throw new PusherException('You must specify a channel of the form private-encrypted-* for E2E encryption. Got '.$channel);- throw new PusherException('Cannot encrypt plaintext for a channel that is not of the form private-encrypted-*. Got '.esc_html($channel));
+ throw new PusherException('Cannot encrypt plaintext for a channel that is not of the form private-encrypted-*. Got '.$channel);If PHPCS is flagging these lines, add suppression comments with proper justification:
// phpcs:ignore WordPress.Security.EscapeOutput.ExceptionNotPrepared -- Exception messages are not HTML output; escape at display time if needed
throw new PusherException('You must specify a channel of the form private-encrypted-* for E2E encryption. Got '.$channel);Also applies to: 90-90
🤖 Prompt for AI Agents
In src/Pusher/Core/Auth/Pusher_Crypto.php around lines 73 and 90, remove the
esc_html() call wrapped around $channel when constructing the PusherException so
the exception message contains the raw channel value (exception messages should
not be HTML-escaped); update the throw statements to concatenate $channel
directly into the message, and if PHPCS flags escaping issues add a phpcs
suppression comment like: phpcs:ignore
WordPress.Security.EscapeOutput.ExceptionNotPrepared with a brief justification
that exception messages are not HTML output and should be escaped at display
time.
|
|
||
| // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedFunctionFound -- Function name is part of public API | ||
| function PM_pusher_before_assignees( $task, $assignees ) { | ||
| // phpcs:ignore WordPress.Security.NonceVerification.Missing,WordPress.Security.ValidatedSanitizedInput.MissingUnslash,WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- Nonce verification and sanitization handled by pm_clean() |
There was a problem hiding this comment.
🧩 Analysis chain
Verify nonce handling in pusher callbacks.
The suppression comments state that "nonce verification and sanitization handled by pm_clean()", but pm_clean() only performs sanitization (via sanitize_text_field() and wp_unslash()), not nonce verification. These pusher callback functions process $_POST data without explicit nonce checks, which could be a security concern if these callbacks are triggered by user requests.
Run the following script to check if nonce verification is performed upstream before these callbacks are invoked:
Also applies to: 87-87, 158-158, 373-373, 381-381, 502-502
🏁 Script executed:
#!/bin/bash
# Search for where these pusher functions are hooked/called
ast-grep --pattern 'add_action($$$, $$$PM_pusher$$$)'
# Search for nonce verification in pusher initialization/routing
rg -n "check_ajax_referer|wp_verify_nonce" --type=php -g "*usher*"Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find and examine the action.php file for the suppression comments
fd -type f -name "action.php" | xargs cat
# Check pm_clean() definition to confirm it only sanitizes
rg -n "function pm_clean" --type=php -A 10Length of output: 1640
🏁 Script executed:
#!/bin/bash
# Search for how pusher functions are registered/called more broadly
rg -n "PM_pusher|Pusher.*callback|pusher.*action" --type=php -i -C 3
# Search for nonce handling in the broader codebase (not just pusher files)
rg -n "check_ajax_referer|wp_verify_nonce" --type=php -C 3 | head -50Length of output: 23126
🏁 Script executed:
#!/bin/bash
# Find what triggers these pusher endpoints (webhook/API endpoints)
rg -n "rest_route|endpoint|webhook" --type=php -g "*/Pusher/*" -C 2
# Look for WP hooks registration in pusher context
rg -n "add_action|add_filter|do_action" --type=php -g "*/Pusher/*" -C 2Length of output: 1389
🏁 Script executed:
#!/bin/bash
# Find where the internal hooks are triggered (do_action calls)
rg -n "do_action.*pm_before_assignees|do_action.*pm_update_task_status|do_action.*pm_after_new_comment|do_action.*pm_after_update_comment|do_action.*pm_after_new_message|do_action.*pm_after_update_message" --type=php -B 5 -A 2Length of output: 5665
🏁 Script executed:
#!/bin/bash
# Search more broadly for where these actions are triggered
rg -n "do_action" --type=php | rg "pm_before_assignees|pm_update_task_status|pm_after_new_comment|pm_after_update_comment|pm_after_new_message|pm_after_update_message|pm_updated"Length of output: 937
🏁 Script executed:
#!/bin/bash
# Check the context around where pm_clean($_POST['is_admin']) is accessed
# Find the calling code for PM_pusher_before_assignees to understand control flow
fd -type f -name "*.php" -exec grep -l "pm_before_assignees" {} \;Length of output: 170
🏁 Script executed:
#!/bin/bash
# Check if REST controllers verify nonce before do_action
rg -n "class Task_Controller|class Comment_Controller|class Discussion_Board_Controller" -A 50 --type=php | head -100Length of output: 10069
🏁 Script executed:
#!/bin/bash
# Look for nonce verification in Task_Controller specifically
fd -name "Task_Controller.php" -exec cat {} \; | head -150Length of output: 230
🏁 Script executed:
#!/bin/bash
# Check where $_POST['is_admin'] comes from - is it passed through $request->get_params()?
rg -n "get_params\(\)" --type=php src/Task/Controllers/Task_Controller.php src/Comment/Controllers/Comment_Controller.php src/Discussion_Board/Controllers/Discussion_Board_Controller.php -B 3 -A 3Length of output: 14315
🏁 Script executed:
#!/bin/bash
# Find REST route registration and middleware
rg -n "register_rest_route|rest_authentication_errors" --type=php -B 3 -A 3 | head -80Length of output: 536
🏁 Script executed:
#!/bin/bash
# Check if WP_Router has nonce verification middleware
cat core/Router/WP_Router.php | head -300Length of output: 9050
🏁 Script executed:
#!/bin/bash
# Verify the actual data flow: does $_POST['is_admin'] come from REST request params?
rg -n "is_admin" --type=php src/Pusher/Libs/action.php -B 2 -A 2 | head -60Length of output: 3651
Correct the misleading suppression comments about nonce verification location.
The suppression comments claim "nonce verification handled by pm_clean()", but pm_clean() only sanitizes input—nonce verification actually occurs at the REST API layer via WP_Router.php (line 247: wp_verify_nonce($nonce, 'wp_rest')), before these callbacks are invoked. The comments should reflect this accurate flow.
Additionally, these functions access $_POST['is_admin'] directly rather than using the sanitized request parameters passed to them via do_action(), which is unnecessary given the REST layer already handles verification.
Update comment at line 42 to:
// phpcs:ignore WordPress.Security.NonceVerification.Missing,WordPress.Security.ValidatedSanitizedInput.MissingUnslash,WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- Nonce verification performed at REST layer; sanitization via pm_clean()
Also applies to: 87-87, 158-158, 373-373, 381-381
For line 502 in PM_pusher_message_url(): The comment states nonce verification is "not implemented", but it should acknowledge that verification occurs at the REST layer. Update comment and sanitization:
// phpcs:ignore WordPress.Security.NonceVerification.Missing,WordPress.Security.ValidatedSanitizedInput.MissingUnslash,WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- Nonce verified at REST layer; sanitize is_admin value
$is_admin = empty( intval( pm_clean( $_POST['is_admin'] ) ) ) ? false : true;🤖 Prompt for AI Agents
In src/Pusher/Libs/action.php around lines 42, 87, 158, 373, 381 and 502, the
phpcs suppression comments wrongly claim nonce verification is handled by
pm_clean() and some callbacks read $_POST['is_admin'] unsanitized; update each
suppression comment to state "Nonce verification performed at REST layer;
sanitization via pm_clean()", and for line 502 replace the direct
$_POST['is_admin'] access with a sanitized value using pm_clean() and intval()
(i.e. compute $is_admin from pm_clean($_POST['is_admin']) and cast to bool) so
the comment and code accurately reflect that the REST layer verifies the nonce
and input is sanitized locally before use.
| $response = $pusher->socket_auth( $channel_name, $socket_id ); | ||
| header( 'Content-Type: application/json' ); | ||
| // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- JSON response from validated data | ||
| echo $response; | ||
| exit; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that validation in Auth class is adequate for security.
The addition of the explicit Content-Type: application/json header and structured response handling is good. The output is from json_encode(), which is inherently safe for JSON responses.
However, ensure that the validation methods validate_channel() and validate_socket_id() in the Auth class provide adequate security for the request parameters before they're used in the authentication signature.
🏁 Script executed:
#!/bin/bash
# Description: Verify the validation logic in the Auth class
# Find and examine the validation methods
ast-grep --pattern $'class Auth {
$$$
validate_channel($$$) {
$$$
}
$$$
}'
ast-grep --pattern $'class Auth {
$$$
validate_socket_id($$$) {
$$$
}
$$$
}'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find Auth class file and examine validation methods
# Find the Auth.php file
fd -t f "Auth.php" --path "*Pusher*"
# Search for validate_channel method definition
rg -n "validate_channel" --type php
# Search for validate_socket_id method definition
rg -n "validate_socket_id" --type php
# Search for socket_auth method definition
rg -n "socket_auth" --type php -A 15Length of output: 3000
🏁 Script executed:
#!/bin/bash
# Examine the validation methods and socket_auth return
# Get full validate_channel implementation
rg -n "private function validate_channel" --type php -A 10
# Get full validate_socket_id implementation
rg -n "private function validate_socket_id" --type php -A 10
# Get full socket_auth method including return
rg -n "public function socket_auth" --type php -A 20Length of output: 2857
🏁 Script executed:
#!/bin/bash
# Check if validation methods throw exceptions or have side effects
# Check for any throw statements in Auth.php
rg -n "throw" src/Pusher/Core/Auth/Auth.php
# Check if there's error handling after validate calls
rg -n "validate_channel\|validate_socket_id" src/Pusher/Core/Auth/Auth.php -B 2 -A 5Length of output: 351
🏁 Script executed:
#!/bin/bash
# Check controller for exception handling
rg -n "try\|catch" src/Pusher/Src/Controllers/Pusher_Controller.php -B 3 -A 3
# Get complete context around socket_auth call in controller
rg -n "socket_auth" src/Pusher/Src/Controllers/Pusher_Controller.php -B 5 -A 10
# Check complete validate_channel implementation with all context
cat src/Pusher/Core/Auth/Auth.php | sed -n '68,77p'
# Check complete validate_socket_id implementation with all context
cat src/Pusher/Core/Auth/Auth.php | sed -n '79,91p'Length of output: 1267
🏁 Script executed:
#!/bin/bash
# Get complete socket_auth implementation including return
cat src/Pusher/Core/Auth/Auth.php | sed -n '36,65p'Length of output: 1272
Fix validation return value handling in Auth class socket_auth() method.
The validation methods validate_channel() and validate_socket_id() return false on invalid input, but their return values are ignored in socket_auth() (lines 37-38). Invalid parameters proceed directly to the HMAC signature calculation, defeating validation entirely.
Required fixes:
src/Pusher/Core/Auth/Auth.phplines 37-38: Check validation return values and throwPusherExceptionon failure (as documented with @throws), or handle false returns to exit earlysrc/Pusher/Src/Controllers/Pusher_Controller.phpline 36: Add try-catch to handle validation exceptions if thrown fromsocket_auth()
The phpcs:ignore comment claiming "validated data" is incorrect until validation failures are properly handled.
| public function task_model( $key = '' ) { | ||
| // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound -- Hook name is part of public API | ||
| return apply_filters( 'task_model', $this, $key ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Public API hook suppression is appropriate but verify consistency.
The PHPCS ignore comment for the task_model hook is correctly formatted and the rationale aligns with the PR's intent to suppress naming violations for public API hooks. However, there's an inconsistency worth noting: this hook lacks any prefix, while the labels() method's hook (line 136) has a pm_ prefix yet receives the same suppression.
Verify that both hooks intentionally follow different naming patterns as part of your public API contract, or consider standardizing them to align with plugin namespace conventions (e.g., wdevs_task_model or similar).
Run the script below to check if the PHPCS rule triggers differently for each hook and confirm the suppression is necessary for both:
🏁 Script executed:
#!/bin/bash
# Search for usages of these hooks outside the models file to understand public API usage
echo "=== Searching for task_model hook usage ==="
rg -nP '\btask_model\b' --type=php -B2 -A2
echo ""
echo "=== Searching for pm_task_model_labels hook usage ==="
rg -nP '\bpm_task_model_labels\b' --type=php -B2 -A2Length of output: 924
🏁 Script executed:
#!/bin/bash
# Search for other hook definitions to understand the plugin's hook naming convention
echo "=== Searching for all apply_filters calls to see hook naming patterns ==="
rg -nP "apply_filters\s*\(" --type=php -A1 | head -50
echo ""
echo "=== Searching for PHPCS ignore comments related to hook naming ==="
rg -nP "NonPrefixedHooknameFound" --type=php -B1 -A1Length of output: 93103
Consider standardizing hook names in the Task model for consistency.
The PHPCS ignore comments are correctly applied, but the two public API hooks exposed by the Task model follow different naming conventions: task_model (unprefixed) vs. pm_task_model_labels (with pm_ prefix). While unprefixed hooks appear elsewhere in the codebase as intentional public API design, standardizing these two Task model hooks to use the same naming pattern would improve consistency. If task_model is meant to be foundational/generic, the naming works; otherwise, consider prefixing it to pm_task_model for alignment with pm_task_model_labels.
|
Still some issue remains @arifulhoque7 vai |
Rubaiyat-E-Mohammad
added
bug
and removed
Needs Testing
|
@Rubaiyat-E-Mohammad vai. Hidden file errors will removed after build. And rest of issues can't be solved because it will encounter fatal error. |
Rubaiyat-E-Mohammad
added
QA Approved
2 participants
Fix PHPCS warnings and errors for WordPress coding standards compliance.
Closes issue
This commit addresses all PHPCS warnings and errors to ensure the plugin
meets WordPress coding standards requirements.
Changes include:
Added phpcs:ignore comments for naming convention violations:
Added phpcs:ignore comments for database security warnings:
Fixed internationalization issues:
Replaced date() with gmdate() for timezone-safe date handling
Removed unnecessary debug statements:
Added phpcs:ignore comments for nonce verification warnings:
All suppressions include explanatory comments to maintain code clarity
and document the reasoning behind each exception.
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.