Feat/permission management feature unlock - #248
myself-alif wants to merge 4 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThe PR primarily normalizes formatting across PHP, JavaScript, CSS, Less, configuration, documentation, and generated assets. It also updates Tailwind configuration, an asset version, and the AddSectionModal success alert option. ChangesRepository formatting and generated assets
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.2.5)PHP Fatal error: Uncaught Error: Undefined constant "ABSPATH" in /includes/functions.php:307 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 |
3d22523 to
d6e9097
Compare
302c61b to
b4cc066
Compare
42861d5 to
0777628
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@includes/Ajax.php`:
- Around line 230-233: The hide_pro_notice() method updates user meta without
nonce or capability checks; add a CSRF-safe validation by calling
check_ajax_referer with a named nonce action (e.g. the same token your front-end
sends) at the start of hide_pro_notice(), verify the current user
(get_current_user_id) and optionally capability (current_user_can) before
calling update_user_meta, and return a proper JSON error/success
(wp_send_json_error/wp_send_json_success) and exit when verification fails or
after success; ensure the AJAX action that enqueues the nonce on the client uses
the same nonce action name used in check_ajax_referer.
In `@includes/API/API.php`:
- Around line 784-785: The guard in get_doc() is validating against
$this->post_type instead of the fetched post’s actual type, allowing non-'docs'
posts to slip through; update the condition to check the fetched post’s type
(e.g. use get_post_type($post) or $post->post_type) so the line becomes: if (
empty($post) || empty($post->ID) || 'docs' !== get_post_type($post) ) { return
$error; } thereby ensuring get_doc() only proceeds for posts of type 'docs'.
- Around line 937-947: The REST callback handle_hide_promotion_notice uses
wp_send_json_success/wp_send_json_error which exits and bypasses REST flow;
change it to return proper REST responses (use rest_ensure_response() or a
WP_REST_Response instance) instead of calling
wp_send_json_success/wp_send_json_error, returning a success response (with
message and 200) when update_option succeeds and returning a WP_Error or
rest_ensure_response with appropriate error data and HTTP status (e.g., 400/500)
when the param is missing or update fails; update references in
handle_hide_promotion_notice so no wp_send_json_* calls remain.
In `@includes/Assets.php`:
- Around line 52-56: The wp_register_style calls register 'wedocs-app-style' and
'wedocs-blocks-style' with the version passed as the 3rd parameter; swap the
arguments so the dependencies array is the 3rd parameter and the version string
is the 4th. Locate the wp_register_style invocations in Assets.php (handles
'wedocs-app-style' and 'wedocs-blocks-style') and change the parameter order to:
handle, $assets_url... path, dependencies array (e.g. $react_dependencies or
$block_dependencies), then the version (e.g. $react_dependencies['version'] or
$block_dependencies['version']). Ensure no other calls to wp_register_style use
the wrong parameter order.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c9f9c267-9033-438c-a95f-763c9220bf25
⛔ Files ignored due to path filters (2)
assets/build/5f4138884ca3828fa2ff.svgis excluded by!**/*.svgpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (92)
.gitignore.svnignoreappsero.jsonassets/build/block.asset.phpassets/build/blocks/DocsGrid/block.jsonassets/build/blocks/DocsGrid/render.phpassets/build/frontend.asset.phpassets/build/frontend.cssassets/build/index.asset.phpassets/build/index.cssassets/build/index.jsassets/build/print.asset.phpassets/build/print.cssassets/build/store.asset.phpassets/build/style-block.cssassets/js/frontend.jsincludes/API/API.phpincludes/API/SettingsApi.phpincludes/Admin.phpincludes/Admin/Admin.phpincludes/Admin/Docs_List_Table.phpincludes/Admin/Menu.phpincludes/Admin/Promotion.phpincludes/Ajax.phpincludes/Assets.phpincludes/Capability.phpincludes/Frontend.phpincludes/Post_Types.phpincludes/Shortcode.phpincludes/Upgrader/Abstracts/UpgradeHandler.phpincludes/Upgrader/Upgrades/Upgrades.phpincludes/Upgrader/Upgrades/V_2_0_2.phpincludes/functions.phplanguages/wedocs.potpackage.jsonpostcss.config.jsreadme.mdreadme.txtsrc/assets/less/frontend.lesssrc/assets/less/responsive.lesssrc/blocks/DocsGrid/StyleControls.jssrc/blocks/DocsGrid/block.jsonsrc/blocks/DocsGrid/edit.jssrc/blocks/DocsGrid/index.jssrc/blocks/DocsGrid/render.phpsrc/blocks/DocsGrid/save.jssrc/blocks/DocsGrid/style.scsssrc/blocks/Search/edit.jssrc/blocks/Search/index.jssrc/blocks/Search/save.jssrc/blocks/index.jssrc/components/AddArticleModal.jssrc/components/AddDocModal.jssrc/components/AddSectionModal.jssrc/components/App.jssrc/components/ConfirmationModal.jssrc/components/DocActions.jssrc/components/DocListing/ArticleChildrens.jssrc/components/DocListing/DocSections.jssrc/components/DocListing/QuickEditModal.jssrc/components/DocListing/SectionArticles.jssrc/components/DraggableDocs.jssrc/components/Migrations/Modals/MigrationContentMappingModal.jssrc/components/Migrations/Modals/MigrationProgressModal.jssrc/components/Migrations/Modals/MigrationSelectionModal.jssrc/components/MultiSelectBox.jssrc/components/PermissionSettingsDemo/PrivacySettings.jssrc/components/ProPreviews/PermissionSettings.jssrc/components/ProPreviews/common/UpgradeButton.jssrc/components/ProPreviews/common/UpgradePopup.jssrc/components/ProPreviews/common/UpgradeTooltip.jssrc/components/ProPreviews/index.jssrc/components/RestrictionModal.jssrc/components/Settings/GeneralSettings.jssrc/components/Settings/Menu.jssrc/components/Settings/index.jssrc/data/docs/actions.jssrc/data/docs/controls.jssrc/data/docs/resolvers.jssrc/data/docs/selectors.jssrc/data/settings/controls.jssrc/data/settings/reducer.jssrc/index.jssrc/utils/helper.jstailwind.config.jstemplates/content-modal.phptemplates/doc-search-form.phptemplates/docs-sidebar.phptemplates/shortcode.phptemplates/single-docs.phpwebpack.config.jswedocs.php
✅ Files skipped from review due to trivial changes (33)
- appsero.json
- .gitignore
- .svnignore
- assets/build/frontend.asset.php
- assets/build/store.asset.php
- src/blocks/DocsGrid/index.js
- assets/build/print.asset.php
- assets/build/block.asset.php
- assets/build/index.asset.php
- includes/Admin/Menu.php
- postcss.config.js
- includes/API/SettingsApi.php
- includes/Upgrader/Upgrades/Upgrades.php
- includes/Upgrader/Abstracts/UpgradeHandler.php
- src/blocks/index.js
- includes/Admin/Docs_List_Table.php
- src/blocks/DocsGrid/save.js
- src/blocks/Search/index.js
- includes/Post_Types.php
- src/blocks/DocsGrid/block.json
- package.json
- src/assets/less/responsive.less
- includes/Admin/Promotion.php
- assets/build/blocks/DocsGrid/block.json
- includes/Shortcode.php
- src/blocks/Search/save.js
- assets/js/frontend.js
- src/components/AddDocModal.js
- src/components/AddArticleModal.js
- includes/Admin/Admin.php
- src/blocks/DocsGrid/edit.js
- src/blocks/DocsGrid/style.scss
- src/blocks/Search/edit.js
| public function hide_pro_notice() { | ||
| $user_id = get_current_user_id(); | ||
| update_user_meta( $user_id, 'wedocs_hide_pro_notice', true ); | ||
| } |
There was a problem hiding this comment.
Add nonce validation in hide_pro_notice() to prevent CSRF.
This action mutates user state (Line 232) but has no nonce check, and it is exposed via AJAX hooks.
Suggested fix
public function hide_pro_notice() {
+ check_ajax_referer( 'wedocs-admin-nonce' );
+
$user_id = get_current_user_id();
+ if ( ! $user_id ) {
+ wp_send_json_error( __( 'Unauthorized.', 'wedocs' ), 401 );
+ }
update_user_meta( $user_id, 'wedocs_hide_pro_notice', true );
+ wp_send_json_success();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function hide_pro_notice() { | |
| $user_id = get_current_user_id(); | |
| update_user_meta( $user_id, 'wedocs_hide_pro_notice', true ); | |
| } | |
| public function hide_pro_notice() { | |
| check_ajax_referer( 'wedocs-admin-nonce' ); | |
| $user_id = get_current_user_id(); | |
| if ( ! $user_id ) { | |
| wp_send_json_error( __( 'Unauthorized.', 'wedocs' ), 401 ); | |
| } | |
| update_user_meta( $user_id, 'wedocs_hide_pro_notice', true ); | |
| wp_send_json_success(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@includes/Ajax.php` around lines 230 - 233, The hide_pro_notice() method
updates user meta without nonce or capability checks; add a CSRF-safe validation
by calling check_ajax_referer with a named nonce action (e.g. the same token
your front-end sends) at the start of hide_pro_notice(), verify the current user
(get_current_user_id) and optionally capability (current_user_can) before
calling update_user_meta, and return a proper JSON error/success
(wp_send_json_error/wp_send_json_success) and exit when verification fails or
after success; ensure the AJAX action that enqueues the nonce on the client uses
the same nonce action name used in check_ajax_referer.
| if ( empty( $post ) || empty( $post->ID ) || 'docs' !== $this->post_type ) { | ||
| return $error; |
There was a problem hiding this comment.
Fix post type validation in get_doc().
Line 784 checks $this->post_type instead of the fetched post’s type, so non-docs posts can pass this guard.
Suggested fix
- if ( empty( $post ) || empty( $post->ID ) || 'docs' !== $this->post_type ) {
+ if ( empty( $post ) || empty( $post->ID ) || 'docs' !== $post->post_type ) {
return $error;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ( empty( $post ) || empty( $post->ID ) || 'docs' !== $this->post_type ) { | |
| return $error; | |
| if ( empty( $post ) || empty( $post->ID ) || 'docs' !== $post->post_type ) { | |
| return $error; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@includes/API/API.php` around lines 784 - 785, The guard in get_doc() is
validating against $this->post_type instead of the fetched post’s actual type,
allowing non-'docs' posts to slip through; update the condition to check the
fetched post’s type (e.g. use get_post_type($post) or $post->post_type) so the
line becomes: if ( empty($post) || empty($post->ID) || 'docs' !==
get_post_type($post) ) { return $error; } thereby ensuring get_doc() only
proceeds for posts of type 'docs'.
| public function handle_hide_promotion_notice( $request ) { | ||
| if ( ! empty( $request->get_param('option_name') ) ) { | ||
| $offer_key = sanitize_text_field( wp_unslash( $request->get_param('option_name') ) ); | ||
| update_option( $offer_key, 'hide' ); | ||
| wp_send_json_success( 'Successfully dismissed.' ); | ||
| } | ||
| wp_send_json_error( 'Faild to dismiss.' ); | ||
| } |
There was a problem hiding this comment.
Return a REST response instead of wp_send_json_* in REST route callbacks.
Lines 943 and 946 use wp_send_json_*, which exits execution and bypasses standard REST response flow.
Suggested fix
public function handle_hide_promotion_notice( $request ) {
if ( ! empty( $request->get_param('option_name') ) ) {
$offer_key = sanitize_text_field( wp_unslash( $request->get_param('option_name') ) );
update_option( $offer_key, 'hide' );
- wp_send_json_success( 'Successfully dismissed.' );
+ return rest_ensure_response( [ 'success' => true, 'message' => 'Successfully dismissed.' ] );
}
- wp_send_json_error( 'Faild to dismiss.' );
+ return new WP_Error( 'wedocs_promo_dismiss_failed', __( 'Failed to dismiss.', 'wedocs' ), [ 'status' => 400 ] );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function handle_hide_promotion_notice( $request ) { | |
| if ( ! empty( $request->get_param('option_name') ) ) { | |
| $offer_key = sanitize_text_field( wp_unslash( $request->get_param('option_name') ) ); | |
| update_option( $offer_key, 'hide' ); | |
| wp_send_json_success( 'Successfully dismissed.' ); | |
| } | |
| wp_send_json_error( 'Faild to dismiss.' ); | |
| } | |
| public function handle_hide_promotion_notice( $request ) { | |
| if ( ! empty( $request->get_param('option_name') ) ) { | |
| $offer_key = sanitize_text_field( wp_unslash( $request->get_param('option_name') ) ); | |
| update_option( $offer_key, 'hide' ); | |
| return rest_ensure_response( [ 'success' => true, 'message' => 'Successfully dismissed.' ] ); | |
| } | |
| return new WP_Error( 'wedocs_promo_dismiss_failed', __( 'Failed to dismiss.', 'wedocs' ), [ 'status' => 400 ] ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@includes/API/API.php` around lines 937 - 947, The REST callback
handle_hide_promotion_notice uses wp_send_json_success/wp_send_json_error which
exits and bypasses REST flow; change it to return proper REST responses (use
rest_ensure_response() or a WP_REST_Response instance) instead of calling
wp_send_json_success/wp_send_json_error, returning a success response (with
message and 200) when update_option succeeds and returning a WP_Error or
rest_ensure_response with appropriate error data and HTTP status (e.g., 400/500)
when the param is missing or update fails; update references in
handle_hide_promotion_notice so no wp_send_json_* calls remain.
| wp_register_style( | ||
| 'wedocs-app-style', | ||
| $assets_url . '/build/index.css', | ||
| $react_dependencies['version'], | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the exact WordPress function signature for wp_register_style(), and which parameter index corresponds to $deps and $ver?
💡 Result:
The exact WordPress function signature for wp_register_style is: function wp_register_style( $handle, $src, $deps = array, $ver = false, $media = 'all') [1][2][3] Regarding the parameter indices: - The $deps parameter corresponds to the 3rd index (index 2, if using 0-based indexing) [1][2][3]. - The $ver parameter corresponds to the 4th index (index 3, if using 0-based indexing) [1][2][3].
Citations:
- 1: https://developer.wordpress.org/reference/functions/wp_register_style/
- 2: https://docs.classicpress.net/reference/functions/wp_register_style/
- 3: https://wpsocket.com/wpref/function/wp_register_style/
🏁 Script executed:
# First, let's locate and check the file structure
find . -name "Assets.php" -path "*/includes/*" | head -20Repository: weDevsOfficial/wedocs-plugin
Length of output: 94
🏁 Script executed:
# Read the file to verify the issues at lines 52-56 and 79-83
cat -n ./includes/Assets.php | sed -n '50,85p'Repository: weDevsOfficial/wedocs-plugin
Length of output: 1620
Fix wp_register_style() argument order: dependencies array must be 3rd parameter, version as 4th.
Lines 55 and 82 pass $react_dependencies['version'] and $block_dependencies['version'] as the 3rd parameter, but wp_register_style() expects the dependencies array there. The version string should be the 4th parameter.
Suggested fix
wp_register_style(
'wedocs-app-style',
$assets_url . '/build/index.css',
- $react_dependencies['version'],
+ array(),
+ $react_dependencies['version']
);
...
wp_register_style(
'wedocs-block-style',
$assets_url . '/build/style-block.css',
- $block_dependencies['version']
+ array(),
+ $block_dependencies['version']
);📝 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.
| wp_register_style( | |
| 'wedocs-app-style', | |
| $assets_url . '/build/index.css', | |
| $react_dependencies['version'], | |
| ); | |
| wp_register_style( | |
| 'wedocs-app-style', | |
| $assets_url . '/build/index.css', | |
| array(), | |
| $react_dependencies['version'] | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@includes/Assets.php` around lines 52 - 56, The wp_register_style calls
register 'wedocs-app-style' and 'wedocs-blocks-style' with the version passed as
the 3rd parameter; swap the arguments so the dependencies array is the 3rd
parameter and the version string is the 4th. Locate the wp_register_style
invocations in Assets.php (handles 'wedocs-app-style' and 'wedocs-blocks-style')
and change the parameter order to: handle, $assets_url... path, dependencies
array (e.g. $react_dependencies or $block_dependencies), then the version (e.g.
$react_dependencies['version'] or $block_dependencies['version']). Ensure no
other calls to wp_register_style use the wrong parameter order.
0777628 to
0e7ef57
Compare
url change and popup ui issue fix (Popup was not showing correctly)
Summary by CodeRabbit
Style
overflow-visible, reducing clipping for menus/popovers.Chores
Documentation