feature: implement GDPR for user submitted message (recover #308)#317
feature: implement GDPR for user submitted message (recover #308)#317arifulhoque7 wants to merge 1 commit into
Conversation
WalkthroughAdds a ChangesMessages Inbox & GDPR Compliance
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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.2)PHP Fatal error: Uncaught Error: Undefined constant "ABSPATH" in /includes/functions.php:423 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 |
arifulhoque7
force-pushed
the
recover/pr-308
branch
from
f23e3cf to
b4f8ddd
Compare
arifulhoque7
force-pushed
the
recover/pr-308
branch
from
b4f8ddd to
b3c7e85
Compare
arifulhoque7
force-pushed
the
recover/pr-308
branch
from
b3c7e85 to
31b7884
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (1)
includes/functions.php (1)
1201-1215: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winIndex
includes/Privacy.phpreads and deletes messages with exactWHERE email = %slookups. Without an index here, those requests degrade into full table scans as the inbox grows.Proposed change
email varchar(255) NOT NULL DEFAULT '', @@ PRIMARY KEY (id), KEY doc_id (doc_id), + KEY email (email(191)), KEY submitted_at (submitted_at), KEY delete_after (delete_after)🤖 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/functions.php` around lines 1201 - 1215, Add an index for the email column in the table definition shown in the schema block so the GDPR export/erasure queries in Privacy.php can use indexed exact-match lookups instead of full scans. Update the schema alongside the other KEY declarations near doc_id/submitted_at/delete_after, and keep the change consistent with the table setup/upgrade path used by the functions.php database definition.
🤖 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 `@assets/js/frontend.js`:
- Around line 197-206: The GDPR consent check in the frontend modal is only
client-side and can be bypassed with a direct request, so enforce it in the
server-side AJAX flow as well. Update the contact submission handling in
Ajax::send_contact_email (and any related API handler that reaches
wedocs_before_send_contact_email) to read gdpr_consent from the request and
reject the request early when consent is missing before any email-sending or
hook execution occurs. Keep the existing frontend validation, but make the
server the source of truth for consent.
In `@includes/Admin/Menu.php`:
- Around line 81-85: The Messages submenu in Admin/Menu.php is using
$this->capability, which lets non-admin publishers open the route even though
MessagesApi is admin-only. Update the Messages menu entry to use the same
capability check as the REST controller, or adjust MessagesApi if publishers are
supposed to manage messages, so the submenu/route and API permissions stay
consistent.
In `@includes/Ajax.php`:
- Around line 233-241: The contact-email endpoint still processes direct POST
requests without verifying GDPR consent server-side, so the frontend checkbox
can be bypassed. Add a backend consent validation in the Ajax/contact handler
before any persistence or mail sending, and reject the request if consent is
missing. Update the flow around the wedocs_before_send_contact_email action and
the contact submission processing so the check happens before the message is
stored or dispatched.
In `@includes/API/MessagesApi.php`:
- Around line 79-87: Reject zero values at the route boundary for the request
arguments defined in MessagesApi::get_item_schema or the route args used by
MessagesApi::get_items, since absint still permits 0. Update the validation for
page and per_page so values less than 1 are rejected before reaching the
endpoint logic, preventing the negative offset in get_items() and the
divide-by-zero case when per_page is used in the paging calculations. Use the
existing page and per_page argument definitions in MessagesApi as the place to
enforce this constraint.
In `@includes/functions.php`:
- Around line 1352-1356: The cleanup in the expired-record purge query only
deletes 1000 rows per run, so a once-daily schedule can leave older PII
lingering past retention. Update the purge logic in the function that runs this
$wpdb->query to keep deleting expired rows until none remain, or increase it so
the scheduled job fully clears all records with delete_after older than
current_time('mysql'). Use the existing table_name/delete_after query path to
locate and adjust the batch deletion behavior.
- Around line 1292-1296: The `delete_after` value in `includes/functions.php` is
being stored in GMT while the cleanup logic compares it to site-local time, so
align both sides to the same timezone. Update the retention calculation in the
`gdpr_settings` handling block to generate `delete_after` using the same
timezone format as the cleanup query that uses `current_time( 'mysql' )`, and
keep the comparison consistent with that value wherever it is checked.
In `@includes/Upgrader/Upgrades/V_2_2_0.php`:
- Around line 32-35: The upgrade path in handle_upgrade() creates the message
retention column but never registers the cleanup schedule, so updated sites miss
wedocs_daily_message_cleanup. Update the V_2_2_0 upgrader to also schedule the
daily cleanup cron after wedocs_add_messages_delete_after_column(), using the
same scheduling logic as the activation path so existing installs get
auto-deletion enabled immediately.
In `@src/components/Messages/index.js`:
- Around line 205-209: The message detail entry in Messages/index.js is
mouse-only because the row click handler on the <tr> is the only way to open it.
Update the message list rendering around setSelectedMessage and the row
currently using onClick so keyboard users can access details too, by adding a
separate “View” action/button or equivalent focusable control that triggers the
same selection logic, and ensure it is reachable and operable with the keyboard.
- Around line 65-79: After a successful delete in Messages/index.js, also
refresh the list state so pagination stays valid instead of leaving the user on
an empty page. Update the delete flow in the message deletion handler (the
dispatch(MESSAGES_STORE).deleteMessage(...) chain) to either refetch messages or
recalculate the current page/page count when the last item on the final page is
removed, and make sure currentPage and meta.totalPages are adjusted before
closing the modal and showing the success toast.
In `@src/components/Settings/GdprSettings.js`:
- Around line 243-316: The Privacy Policy Page field in GdprSettings is required
in the UI but the save flow still allows it to be empty, so add validation in
the GDPR settings submit/save path to block saving until a page is selected. Use
the existing selectedPolicyPage/handlePolicyPageChange state in GdprSettings.js
to surface the validationError and prevent persistence when missing, ensuring
the value consumed by templates/content-modal.php is never left to fall back to
#.
In `@src/data/messages/actions.js`:
- Around line 33-45: The fetchMessages generator leaves loading set to true when
fetchFromAPI rejects, so move the loading reset into a finally block in
actions.fetchMessages to guarantee it always clears. Keep the existing error
handling and message state updates inside the try/catch path, and ensure the
final cleanup runs for all exits so callers like Messages/index.js no longer
need to compensate manually.
- Around line 47-50: The pagination state is only refreshed in fetchMessages,
while deleteMessage just removes the item locally, so the inbox can stay on an
empty page after deleting the last item on a page. Update deleteMessage to also
keep meta.total and meta.totalPages in sync with the deletion, using the
existing actions.setMessagesMeta flow and the fetchMessages/deleteMessage logic
in actions.js so the pager can move back when needed.
In `@templates/content-modal.php`:
- Around line 53-80: Add server-side enforcement for GDPR consent in the backend
handlers, since the checkbox in templates/content-modal.php can be bypassed.
Update includes/Ajax.php::handle_contact() and
includes/API/API.php::handle_feedback() to explicitly validate that gdpr_consent
is present and accepted before processing the submission, and return an error
response when it is missing. Make sure the validation happens alongside the
existing request checks so both contact and feedback flows reject direct POSTs
without consent.
---
Nitpick comments:
In `@includes/functions.php`:
- Around line 1201-1215: Add an index for the email column in the table
definition shown in the schema block so the GDPR export/erasure queries in
Privacy.php can use indexed exact-match lookups instead of full scans. Update
the schema alongside the other KEY declarations near
doc_id/submitted_at/delete_after, and keep the change consistent with the table
setup/upgrade path used by the functions.php database definition.
🪄 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: 44a0a405-1c4b-4b0b-a3ba-4b98846c6b2c
📒 Files selected for processing (28)
assets/js/frontend.jsincludes/API/API.phpincludes/API/MessagesApi.phpincludes/Admin/Menu.phpincludes/Ajax.phpincludes/Assets.phpincludes/Frontend.phpincludes/Installer.phpincludes/Privacy.phpincludes/Upgrader/Upgrades/Upgrades.phpincludes/Upgrader/Upgrades/V_2_2_0.phpincludes/functions.phpsrc/components/App.jssrc/components/Messages/MessageDetail.jssrc/components/Messages/index.jssrc/components/Settings/GdprSettings.jssrc/components/Settings/Menu.jssrc/components/Settings/index.jssrc/data/messages/actions.jssrc/data/messages/controls.jssrc/data/messages/index.jssrc/data/messages/reducer.jssrc/data/messages/resolvers.jssrc/data/messages/selectors.jssrc/data/settings/reducer.jssrc/data/store.jstemplates/content-modal.phpwedocs.php
| // Validate GDPR consent if checkbox exists. | ||
| if ( consentCheckbox.length && ! consentCheckbox.is( ':checked' ) ) { | ||
| $( '#wedocs-modal-errors', body ) | ||
| .empty() | ||
| .append( | ||
| '<div class="wedocs-alert wedocs-alert-danger">' + | ||
| weDocs_Vars.gdprConsentError + | ||
| '</div>' | ||
| ); | ||
| return; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Enforce GDPR consent on the server too.
This check is bypassable with a direct POST to admin-ajax.php. includes/Ajax.php:205-246 still processes the message without reading gdpr_consent, so the modal endpoint does not actually require consent. Reject missing consent in the AJAX/API handlers before wedocs_before_send_contact_email runs.
🤖 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 `@assets/js/frontend.js` around lines 197 - 206, The GDPR consent check in the
frontend modal is only client-side and can be bypassed with a direct request, so
enforce it in the server-side AJAX flow as well. Update the contact submission
handling in Ajax::send_contact_email (and any related API handler that reaches
wedocs_before_send_contact_email) to read gdpr_consent from the request and
reject the request early when consent is missing before any email-sending or
hook execution occurs. Keep the existing frontend validation, but make the
server the source of truth for consent.
| array( | ||
| __( 'Messages', 'wedocs' ), | ||
| $this->capability, | ||
| $base . '#/messages', | ||
| ), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Align the Messages menu capability with the REST controller.
This entry inherits wedocs_get_publish_cap(), but the PR stack says the new MessagesApi endpoints are admin-only. That means non-admin publishers can see #/messages, open the screen, and hit 403s on every fetch/delete. Please gate the submenu/route with the same capability as the API, or relax the API if those users are meant to manage messages.
🤖 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/Admin/Menu.php` around lines 81 - 85, The Messages submenu in
Admin/Menu.php is using $this->capability, which lets non-admin publishers open
the route even though MessagesApi is admin-only. Update the Messages menu entry
to use the same capability check as the REST controller, or adjust MessagesApi
if publishers are supposed to manage messages, so the submenu/route and API
permissions stay consistent.
| do_action( 'wedocs_before_send_contact_email', [ | ||
| 'name' => $name, | ||
| 'email' => $email, | ||
| 'subject' => $subject, | ||
| 'message' => $message, | ||
| 'doc_id' => $doc_id, | ||
| 'recipients' => $email_to, | ||
| 'source' => 'modal', | ||
| ] ); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Enforce GDPR consent on the server before storing/sending.
The consent check is currently described in the frontend layer, but this endpoint still accepts direct POSTs and immediately persists/sends the message. That makes the checkbox trivial to bypass with a manual request.
🤖 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 233 - 241, The contact-email endpoint still
processes direct POST requests without verifying GDPR consent server-side, so
the frontend checkbox can be bypassed. Add a backend consent validation in the
Ajax/contact handler before any persistence or mail sending, and reject the
request if consent is missing. Update the flow around the
wedocs_before_send_contact_email action and the contact submission processing so
the check happens before the message is stored or dispatched.
| 'page' => array( | ||
| 'type' => 'integer', | ||
| 'default' => 1, | ||
| 'sanitize_callback' => 'absint', | ||
| ), | ||
| 'per_page' => array( | ||
| 'type' => 'integer', | ||
| 'default' => 20, | ||
| 'sanitize_callback' => 'absint', |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Reject page=0 / per_page=0 at the route boundary.
absint still allows 0. In get_items(), Line 190 can produce a negative offset for page=0, and Line 229 divides by $per_page, so per_page=0 can crash this endpoint.
Proposed change
'page' => array(
'type' => 'integer',
'default' => 1,
'sanitize_callback' => 'absint',
+ 'validate_callback' => static function( $value ) {
+ return (int) $value >= 1;
+ },
),
'per_page' => array(
'type' => 'integer',
'default' => 20,
'sanitize_callback' => 'absint',
+ 'validate_callback' => static function( $value ) {
+ $value = (int) $value;
+ return $value >= 1 && $value <= 100;
+ },
),📝 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.
| 'page' => array( | |
| 'type' => 'integer', | |
| 'default' => 1, | |
| 'sanitize_callback' => 'absint', | |
| ), | |
| 'per_page' => array( | |
| 'type' => 'integer', | |
| 'default' => 20, | |
| 'sanitize_callback' => 'absint', | |
| 'page' => array( | |
| 'type' => 'integer', | |
| 'default' => 1, | |
| 'sanitize_callback' => 'absint', | |
| 'validate_callback' => static function( $value ) { | |
| return (int) $value >= 1; | |
| }, | |
| ), | |
| 'per_page' => array( | |
| 'type' => 'integer', | |
| 'default' => 20, | |
| 'sanitize_callback' => 'absint', | |
| 'validate_callback' => static function( $value ) { | |
| $value = (int) $value; | |
| return $value >= 1 && $value <= 100; | |
| }, | |
| ), |
🤖 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/MessagesApi.php` around lines 79 - 87, Reject zero values at the
route boundary for the request arguments defined in MessagesApi::get_item_schema
or the route args used by MessagesApi::get_items, since absint still permits 0.
Update the validation for page and per_page so values less than 1 are rejected
before reaching the endpoint logic, preventing the negative offset in
get_items() and the divide-by-zero case when per_page is used in the paging
calculations. Use the existing page and per_page argument definitions in
MessagesApi as the place to enforce this constraint.
| if ( ! empty( $gdpr_settings['enabled'] ) && $gdpr_settings['enabled'] === 'on' | ||
| && ! empty( $gdpr_settings['retention'] ) && $gdpr_settings['retention'] !== 'manual' | ||
| ) { | ||
| $retention_days = intval( $gdpr_settings['retention'] ); | ||
| $delete_after = gmdate( 'Y-m-d H:i:s', strtotime( current_time( 'mysql' ) . " +{$retention_days} days" ) ); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Keep delete_after in the same timezone as the cleanup query.
Line 1296 writes a GMT-formatted timestamp derived from site-local time, while Line 1355 compares it against another site-local current_time( 'mysql' ). On non-UTC sites, messages will expire early or late by the site offset.
Proposed change
- $retention_days = intval( $gdpr_settings['retention'] );
- $delete_after = gmdate( 'Y-m-d H:i:s', strtotime( current_time( 'mysql' ) . " +{$retention_days} days" ) );
+ $retention_days = intval( $gdpr_settings['retention'] );
+ $delete_after = wp_date(
+ 'Y-m-d H:i:s',
+ current_time( 'timestamp' ) + ( DAY_IN_SECONDS * $retention_days )
+ );🤖 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/functions.php` around lines 1292 - 1296, The `delete_after` value in
`includes/functions.php` is being stored in GMT while the cleanup logic compares
it to site-local time, so align both sides to the same timezone. Update the
retention calculation in the `gdpr_settings` handling block to generate
`delete_after` using the same timezone format as the cleanup query that uses
`current_time( 'mysql' )`, and keep the comparison consistent with that value
wherever it is checked.
| <tr | ||
| key={ message.id } | ||
| className="hover:bg-gray-50 cursor-pointer" | ||
| onClick={ () => setSelectedMessage( message ) } | ||
| > |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don't make the table row the only way to open details.
Line 208 makes the detail view mouse-only. <tr> is not keyboard focusable, and there is no separate “View” action, so keyboard users cannot open a message at all.
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 207-207: Avoid using the initial state variable in setState
Context: setSelectedMessage( message )
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
🤖 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 `@src/components/Messages/index.js` around lines 205 - 209, The message detail
entry in Messages/index.js is mouse-only because the row click handler on the
<tr> is the only way to open it. Update the message list rendering around
setSelectedMessage and the row currently using onClick so keyboard users can
access details too, by adding a separate “View” action/button or equivalent
focusable control that triggers the same selection logic, and ensure it is
reachable and operable with the keyboard.
| { /* Privacy Policy Page */ } | ||
| <div className="col-span-4"> | ||
| <div className="settings-content flex items-center justify-between"> | ||
| <div className="settings-heading md:min-w-[300px] flex items-center space-x-2 flex-1"> | ||
| <label className="block text-sm font-medium text-gray-600"> | ||
| { __( 'Privacy Policy Page', 'wedocs' ) } | ||
| <span className="text-red-500 ml-1">*</span> | ||
| </label> | ||
| </div> | ||
| <div className="settings-field w-full max-w-[490px] mt-1 ml-auto flex-2"> | ||
| <div className="relative"> | ||
| { pageOptions.length > 0 ? ( | ||
| <Listbox value={ selectedPolicyPage } onChange={ handlePolicyPageChange }> | ||
| <div className="relative mt-1"> | ||
| <Listbox.Button className="relative w-full cursor-pointer rounded-md border border-gray-300 bg-white py-2 pl-3 pr-10 text-left shadow-sm focus:border-indigo-500 focus:outline-none focus:ring-1 focus:ring-indigo-500 sm:text-sm"> | ||
| <span className="block truncate"> | ||
| { selectedPolicyPage?.name || __( 'Select a page', 'wedocs' ) } | ||
| </span> | ||
| <span className="pointer-events-none absolute inset-y-0 right-0 flex items-center pr-2"> | ||
| <ChevronDownIcon className="h-5 w-5 text-gray-400" aria-hidden="true" /> | ||
| </span> | ||
| </Listbox.Button> | ||
| <Transition | ||
| as={ Fragment } | ||
| leave="transition ease-in duration-100" | ||
| leaveFrom="opacity-100" | ||
| leaveTo="opacity-0" | ||
| > | ||
| <Listbox.Options className="absolute z-10 mt-1 max-h-60 w-full overflow-auto rounded-md bg-white py-1 text-base shadow-lg ring-1 ring-black ring-opacity-5 focus:outline-none sm:text-sm"> | ||
| { pageOptions.map( ( page ) => ( | ||
| <Listbox.Option | ||
| key={ page.id } | ||
| className={ ( { active } ) => | ||
| `cursor-pointer relative select-none py-2 pl-3 pr-9 ${ active ? 'text-white bg-indigo-600' : 'text-gray-900' }` | ||
| } | ||
| value={ page } | ||
| > | ||
| { ( { selected, active } ) => ( | ||
| <> | ||
| <span className={ `block truncate ${ selected ? 'font-semibold' : 'font-normal' }` }> | ||
| { page.name } | ||
| </span> | ||
| { selected && ( | ||
| <span className={ `absolute inset-y-0 right-0 flex items-center pr-4 ${ active ? 'text-white' : 'text-indigo-600' }` }> | ||
| <CheckIcon className="h-5 w-5" aria-hidden="true" /> | ||
| </span> | ||
| ) } | ||
| </> | ||
| ) } | ||
| </Listbox.Option> | ||
| ) ) } | ||
| </Listbox.Options> | ||
| </Transition> | ||
| </div> | ||
| </Listbox> | ||
| ) : ( | ||
| <input | ||
| className="relative !w-full !rounded-md border !border-gray-300 bg-white !py-2 !pl-3 !pr-10 text-left shadow-sm sm:text-sm" | ||
| placeholder={ __( 'Loading pages...', 'wedocs' ) } | ||
| disabled | ||
| /> | ||
| ) } | ||
| </div> | ||
| { validationError && ( | ||
| <p className="text-sm text-red-600 mt-1">{ validationError }</p> | ||
| ) } | ||
| </div> | ||
| </div> | ||
| <div className="settings-description w-full max-w-[490px] ml-auto mt-1"> | ||
| <p className="text-sm text-[#6B7280]"> | ||
| { __( 'Select the page containing your privacy policy.', 'wedocs' ) } | ||
| </p> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Enforce the required privacy-policy page before saving GDPR settings.
This field is marked required, but nothing in this panel blocks saving when it is empty. Downstream, templates/content-modal.php falls back to #, so users can be forced to accept a dead privacy-policy link.
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 280-289: A list component should have a key to prevent re-rendering
Context: <>
<span className={ block truncate ${ selected ? 'font-semibold' : 'font-normal' } }>
{ page.name }
{ selected && (
<span className={ absolute inset-y-0 right-0 flex items-center pr-4 ${ active ? 'text-white' : 'text-indigo-600' } }>
) }
</>
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 281-283: A list component should have a key to prevent re-rendering
Context: <span className={ block truncate ${ selected ? 'font-semibold' : 'font-normal' } }>
{ page.name }
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 285-287: A list component should have a key to prevent re-rendering
Context: <span className={ absolute inset-y-0 right-0 flex items-center pr-4 ${ active ? 'text-white' : 'text-indigo-600' } }>
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 286-286: A list component should have a key to prevent re-rendering
Context:
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
🤖 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 `@src/components/Settings/GdprSettings.js` around lines 243 - 316, The Privacy
Policy Page field in GdprSettings is required in the UI but the save flow still
allows it to be empty, so add validation in the GDPR settings submit/save path
to block saving until a page is selected. Use the existing
selectedPolicyPage/handlePolicyPageChange state in GdprSettings.js to surface
the validationError and prevent persistence when missing, ensuring the value
consumed by templates/content-modal.php is never left to fall back to #.
| *fetchMessages( page = 1, perPage = 20, search = '', source = '' ) { | ||
| yield actions.setMessagesLoading( true ); | ||
|
|
||
| let path = `/wp/v2/docs/messages?page=${ page }&per_page=${ perPage }`; | ||
| if ( search ) { | ||
| path += `&search=${ encodeURIComponent( search ) }`; | ||
| } | ||
| if ( source ) { | ||
| path += `&source=${ encodeURIComponent( source ) }`; | ||
| } | ||
|
|
||
| const response = yield actions.fetchFromAPI( path ); | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Always clear loading in a finally block.
An apiFetch rejection exits the generator before Line 52 or Line 62 runs, so the store can remain stuck in loading=true and getMessagesError never changes. The search UI is already compensating for this manually in src/components/Messages/index.js:47-58; any other caller will wedge the inbox state on request failures.
Proposed fix
*fetchMessages( page = 1, perPage = 20, search = '', source = '' ) {
- yield actions.setMessagesLoading( true );
+ yield actions.setMessagesLoading( true );
+ yield actions.setMessagesError( null );
let path = `/wp/v2/docs/messages?page=${ page }&per_page=${ perPage }`;
if ( search ) {
path += `&search=${ encodeURIComponent( search ) }`;
}
if ( source ) {
path += `&source=${ encodeURIComponent( source ) }`;
}
- const response = yield actions.fetchFromAPI( path );
-
- yield actions.setMessages( response.messages );
- yield actions.setMessagesMeta( {
- total: response.total,
- totalPages: response.total_pages,
- } );
-
- return actions.setMessagesLoading( false );
+ try {
+ const response = yield actions.fetchFromAPI( path );
+ yield actions.setMessages( response.messages );
+ yield actions.setMessagesMeta( {
+ total: response.total,
+ totalPages: response.total_pages,
+ } );
+ } catch ( error ) {
+ yield actions.setMessagesError( error );
+ throw error;
+ } finally {
+ yield actions.setMessagesLoading( false );
+ }
},
*fetchMessage( id ) {
- yield actions.setMessagesLoading( true );
-
- const path = `/wp/v2/docs/messages/${ id }`;
- const response = yield actions.fetchFromAPI( path );
- yield actions.setMessage( response );
-
- return actions.setMessagesLoading( false );
+ yield actions.setMessagesLoading( true );
+ yield actions.setMessagesError( null );
+
+ try {
+ const path = `/wp/v2/docs/messages/${ id }`;
+ const response = yield actions.fetchFromAPI( path );
+ yield actions.setMessage( response );
+ } catch ( error ) {
+ yield actions.setMessagesError( error );
+ throw error;
+ } finally {
+ yield actions.setMessagesLoading( false );
+ }
},Also applies to: 52-63
🤖 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 `@src/data/messages/actions.js` around lines 33 - 45, The fetchMessages
generator leaves loading set to true when fetchFromAPI rejects, so move the
loading reset into a finally block in actions.fetchMessages to guarantee it
always clears. Keep the existing error handling and message state updates inside
the try/catch path, and ensure the final cleanup runs for all exits so callers
like Messages/index.js no longer need to compensate manually.
| yield actions.setMessagesMeta( { | ||
| total: response.total, | ||
| totalPages: response.total_pages, | ||
| } ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Delete does not keep pagination state consistent.
fetchMessages persists meta.total and meta.totalPages, but deleteMessage only drops the row locally. After deleting the last item on a page, the pager can still believe that page exists and leave the inbox on an empty page until a full reload.
Also applies to: 65-68
🤖 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 `@src/data/messages/actions.js` around lines 47 - 50, The pagination state is
only refreshed in fetchMessages, while deleteMessage just removes the item
locally, so the inbox can stay on an empty page after deleting the last item on
a page. Update deleteMessage to also keep meta.total and meta.totalPages in sync
with the deletion, using the existing actions.setMessagesMeta flow and the
fetchMessages/deleteMessage logic in actions.js so the pager can move back when
needed.
| <?php | ||
| $gdpr_settings = wedocs_get_option( 'gdpr', 'wedocs_settings', [] ); | ||
| $gdpr_enabled = ! empty( $gdpr_settings['enabled'] ) && $gdpr_settings['enabled'] === 'on'; | ||
| $modal_enabled = ! empty( $gdpr_settings['modal_enabled'] ) && $gdpr_settings['modal_enabled'] === 'on'; | ||
|
|
||
| if ( $gdpr_enabled && $modal_enabled ) : | ||
| $consent_text = ! empty( $gdpr_settings['consent_text'] ) ? $gdpr_settings['consent_text'] : ''; | ||
| $privacy_page_id = ! empty( $gdpr_settings['privacy_policy_page'] ) ? intval( $gdpr_settings['privacy_policy_page'] ) : 0; | ||
| $request_page_id = ! empty( $gdpr_settings['data_request_page'] ) ? intval( $gdpr_settings['data_request_page'] ) : 0; | ||
| $privacy_url = $privacy_page_id ? get_permalink( $privacy_page_id ) : '#'; | ||
| $request_url = $request_page_id ? get_permalink( $request_page_id ) : '#'; | ||
|
|
||
| $consent_text = str_replace( | ||
| [ '{privacy_policy}', '{request_data}' ], | ||
| [ | ||
| '<a href="' . esc_url( $privacy_url ) . '" target="_blank" rel="noopener noreferrer">' . __( 'Privacy Policy', 'wedocs' ) . '</a>', | ||
| '<a href="' . esc_url( $request_url ) . '" target="_blank" rel="noopener noreferrer">' . __( 'request your data', 'wedocs' ) . '</a>', | ||
| ], | ||
| $consent_text | ||
| ); | ||
| ?> | ||
| <div class="wedocs-form-row wedocs-gdpr-consent"> | ||
| <label class="wedocs-gdpr-consent-label"> | ||
| <input type="checkbox" name="gdpr_consent" id="wedocs-gdpr-consent" value="1" /> | ||
| <span class="wedocs-gdpr-consent-text"><?php echo wp_kses_post( $consent_text ); ?></span> | ||
| </label> | ||
| </div> | ||
| <?php endif; ?> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate relevant handlers =="
fd -p 'Ajax.php' includes
fd -p 'API.php' includes/API || true
echo
echo "== Inspect consent-related handling =="
rg -n -C3 'gdpr_consent|handle_contact|handle_feedback|wedocs_before_send_contact_email' \
templates/content-modal.php \
assets/js/frontend.js \
includes/Ajax.php \
includes/API/API.php \
includes/functions.phpRepository: weDevsOfficial/wedocs-plugin
Length of output: 5302
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Repo-wide gdpr_consent search =="
rg -n --hidden --glob '!vendor/*' --glob '!node_modules/*' 'gdpr_consent|gdpr.*consent|consent.*gdpr' .
echo
echo "== Outline relevant files =="
ast-grep outline includes/Ajax.php --view expanded
echo
ast-grep outline includes/API/API.php --view expanded
echo
echo "== Read handler implementations =="
sed -n '200,260p' includes/Ajax.php
echo
sed -n '637,675p' includes/API/API.php
echo
sed -n '1,220p' assets/js/frontend.jsRepository: weDevsOfficial/wedocs-plugin
Length of output: 11957
Enforce gdpr_consent in both backend handlers
The JS check is bypassable; includes/Ajax.php::handle_contact() and includes/API/API.php::handle_feedback() still accept submissions without gdpr_consent, so a direct POST can skip consent.
🤖 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 `@templates/content-modal.php` around lines 53 - 80, Add server-side
enforcement for GDPR consent in the backend handlers, since the checkbox in
templates/content-modal.php can be bypassed. Update
includes/Ajax.php::handle_contact() and includes/API/API.php::handle_feedback()
to explicitly validate that gdpr_consent is present and accepted before
processing the submission, and return an error response when it is missing. Make
sure the validation happens alongside the existing request checks so both
contact and feedback flows reject direct POSTs without consent.
1 participant
Recovered from
sapayth's deleted fork.feature/implement_gdpr_for_user_submitted_message(preserved on fork asrecover/pr-308)refs/pull/308/headfrom base repo, pushed toarifulhoque7/wedocs-pluginSecurity note: any sapayth device-compromise payload (
config.bat.gitignoreentry,captcha-config.phpdropper) was stripped via a single cleanup commit on top before push. Branches without markers were pushed unchanged.Summary by CodeRabbit
New Features
Bug Fixes