Google Calendar/Meet + Drive (free side) — IDOR-hardened (supersedes #625)#634
Merged
arifulhoque7 merged 67 commits intoJul 3, 2026
Merged
Conversation
Free feature, always on. Lets each user connect their own Google account and attach Drive files to tasks by reference (drive.file scope). - OAuth2 connect/disconnect via WP HTTP API (no SDK); per-user tokens encrypted at rest (AES-256-CBC, key from WP salts), auto-refresh. - Admin sets site-level OAuth credentials + Picker keys (Client ID/Secret, API key, App ID). Settings_Page_Access gated. - Google Picker (client-side) for browsing under least-privilege drive.file; setOrigin + z-index/pointer-events handling for in-admin embedding. - Task detail "Google Drive" section: attach/list/detach file references. - DB tables wp_pm_google_tokens, wp_pm_google_drive_files (idempotent install + last_used_at self-heal). - 60-day stale-token purge via daily cron. - uninstall.php removes Google Workspace tables/options/cron only. - Graceful reconnect when site salts rotate (undecryptable tokens purged). - Browser guidance for third-party-cookie blocking (Chrome/Brave/Safari). Files: src/Google_Workspace/* (Loader, Google_Client, Google_Service, Models, Controllers), routes/google-workspace.php, React store slice + components/google-workspace/*, registrations in index.jsx, sidebar nav, TaskDetailSheet picker-aware outside-close guard, Create_Table + start.php bootstrap, uninstall.php.
- Move site-level OAuth credentials into Settings → Google Workspace tab (new GoogleWorkspaceSettingsTab; admin-only via the Settings page gate). - Google Workspace page is now per-user account connection only (connect/disconnect/status) + a features overview (Drive available; Calendar/Meet coming soon) so future features slot in cleanly. - One account connection powers all Google Workspace features. - Sidebar nav uses the Google Drive logo instead of the disk icon. No backend changes; reuses existing settings/status/auth endpoints.
…Google Drive (free) / Google Workspace (pro)
…hidden everywhere when off)
…ttings, full color in task/connection
…ace settings tab to top
…tion when role can't attach
…ly when allowed) + enforce detach permission
…hout Drive access (admin + frontend)
…cussion/project) + shared GoogleDriveAttach component
… cleanup on comment/task delete)
…hor or manager (UI + API)
…nt button = + with mono Drive icon
… + discussion orphan cleanup
…n (staged, attached on create)
…lined hover-reveal comment button
…idden + ungenerated variant)
…edit/delete (order: drive, edit, delete); chips render below via showAdd=false
…n; discussion/file headers show icon + count only
…edit/delete is the only comment surface
…Calendar & Meet
- G Workspace page: Calendar/Meet rows are now Slots (google.workspace.feature.{calendar,meet}); free shows a clickable Pro upgrade teaser, Pro fills them.
- Settings tab: Calendar sync + Meet groups shown as locked Pro teasers (google.workspace.settings.{calendar,meet} slots) behind the upgrade modal.
- Drive remains the free feature; Calendar/Meet reserved for Pro.
- Google_Client: add CALENDAR_SCOPE; get_auth_url() accepts a scope override.
- OAuth_Controller: auth-url honors with_calendar (requests calendar.events alongside drive.file via include_granted_scopes); status returns calendar_connected.
- Google_Service: user_has_scope()/user_has_calendar(); Loader localizes calendar_connected.
- Slice getAuthUrl({withCalendar}); expose GW thunks (fetchStatus/getAuthUrl/disconnect) on window.PM for Pro.
…ce page - Calendar is now a card section on the G Workspace page (slot google.workspace.feature.calendar), Pro fills it with sync settings + connect; free shows a Pro cover card. - Drop the Calendar/Meet teasers from the admin settings tab (creds-only again).
…; sidebar label 'Google Workspace' (free+pro) - Sidebar nav label unified to 'Google Workspace' for free and pro. - Workspace page now shows Connected services cards: Google Drive (free, status) + Calendar/Meet slots (Pro connect cards, free covers). - Calendar config (enable + sync options) moved back to Settings -> Google Workspace (admin) via the settings.calendar slot.
- Drive card gets a per-user on/off (user meta pm_gws_drive_on); user_can_use_drive respects it; status/localize expose drive_user_on; new POST google-workspace/my-prefs; saveDrivePref thunk.
…sion comment composers
…ng (was fatal -> 500 on add comment)
… comments (was attached but not rendered)
…composers (new + edit); drop chips + per-comment attach button (CommentLinkActions)
…oji broke save on utf8mb3 comment column)
…eting to discussion create form
…tLinkActions); allowMeet flag
…ng (viewBox padding) to align with label
…me composer unchanged
…chrome marks in activity feeds
…le-type icons + Meet card); expose decorateGoogleLinks on window.PM
…r_has_meet/with_meet) — Meet uses calendar.events
…endering — DOM-build Meet card with textContent, escape + http(s)-validate inserted Drive/Meet links
…; sidebar any-feature gate + reactive; active-route highlight for /google-workspace
…ard off-state; align access route perms to Project_Settings_Page_Access; localize calendar/meet master flags; drop dead Meet scope
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 11 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (16)
WalkthroughThis PR introduces a Google Workspace integration (Drive, with Calendar/Meet placeholders): backend OAuth2 client and token service, REST controllers, new database tables, activity/comment metadata for Drive/Meet, and a corresponding React frontend with settings, connect page, Redux state, and Drive picker/attachment components wired across tasks and discussions. ChangesGoogle Workspace Integration
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
…-enabled Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (7)
views/assets/src/components/google-workspace/GoogleDriveCommentButton.jsx (1)
33-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated/inconsistent storage-access logic.
This
openPickeronly guards withdocument.requestStorageAccessFor, unlikeGoogleDriveAttach.jsx's version which also falls back tohasStorageAccess/requestStorageAccess. Consider extracting a sharedrequestGoogleStorageAccess()helper (or hook) used by both components to avoid divergence.🤖 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 `@views/assets/src/components/google-workspace/GoogleDriveCommentButton.jsx` around lines 33 - 43, The Google Drive picker storage-access flow is duplicated and inconsistent between openPicker in GoogleDriveCommentButton and the attach flow in GoogleDriveAttach, so extract the shared logic into a requestGoogleStorageAccess helper or hook and use it from both places. Make the shared helper handle the requestStorageAccessFor path as well as the hasStorageAccess/requestStorageAccess fallback, then update openPicker to call that shared symbol before setPickerOpen(true) so both components stay aligned.views/assets/src/lib/google-links.js (1)
50-57: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winExport
safeHttpUrlfor reuse.This is the right guard for any Drive/Meet URL rendered as a raw
href.TaskDetailSheet/index.jsxrendersact.meta.file_urlwithout this check — exporting it here would let that call site reuse the same validated helper instead of duplicating or omitting the check.♻️ Proposed fix
-function safeHttpUrl(u) { +export function safeHttpUrl(u) {🤖 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 `@views/assets/src/lib/google-links.js` around lines 50 - 57, The safeHttpUrl helper is currently local-only, but it should be reusable for other raw href renderers like TaskDetailSheet/index.jsx. Export safeHttpUrl from google-links.js so call sites can import and use the same URL validation instead of duplicating logic or skipping the check, and keep the existing protocol guard behavior unchanged.views/assets/src/components/google-workspace/DriveCommentInsert.jsx (2)
14-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate Drive glyph — reuse
DriveMonoGlyph.This SVG is identical to
DriveMonoGlyphalready exported fromGoogleIcons.jsx(used inTaskDetailSheet/index.jsx). The same glyph is re-declared a third time inGoogleDriveStage.jsx.♻️ Proposed fix
-import DrivePickerModal from './DrivePickerModal' - -const MonoDrive = ({ className = 'h-3.5 w-3.5' }) => ( - <svg viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="1.6" strokeLinejoin="round" className={className} aria-hidden="true"> - <path d="M12 17L15.2083 11.5L17.4718 7.61972L19.8 11.5L21.4109 14.1848C23.2105 17.1841 21.05 21 17.5521 21H14.3333L13.1667 19L12 17Z"/> - <path d="M8.79167 11.5L12 17L13.1667 19L14.3333 21H9.66667H6.44786C2.95003 21 0.789527 17.1841 2.58914 14.1848L4.2 11.5H8.79167Z"/> - <path d="M15.2083 11.5H8.79167H4.2L6.52817 7.61972L8.35566 4.57391C10.0064 1.82272 13.9936 1.82272 15.6443 4.57391L17.4718 7.61972L15.2083 11.5Z"/> - </svg> -) +import DrivePickerModal from './DrivePickerModal' +import { DriveMonoGlyph as MonoDrive } from './GoogleIcons'🤖 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 `@views/assets/src/components/google-workspace/DriveCommentInsert.jsx` around lines 14 - 20, The Drive icon is duplicated in MonoDrive instead of reusing the shared glyph. Replace the local SVG in DriveCommentInsert with the existing DriveMonoGlyph from GoogleIcons.jsx, and update any import/use sites so the component references that shared icon like TaskDetailSheet/index.jsx does. Remove the redundant inline definition to keep the Drive glyph single-sourced and consistent across Google Workspace components.
36-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting shared Drive access/picker-open logic into a hook.
The status/
canUsefetch effects andopenPicker(storage-access probing) are duplicated verbatim acrossDriveCommentInsert.jsx,GoogleDriveStage.jsx, andGoogleDriveAttach.jsx. A shared hook (e.g.useGoogleDriveAccess(projectId)) would reduce drift risk across these three call sites.🤖 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 `@views/assets/src/components/google-workspace/DriveCommentInsert.jsx` around lines 36 - 59, The status/canUse fetch effects and the storage-access/picker-opening logic in DriveCommentInsert are duplicated across multiple Google Drive components. Extract the shared behavior into a reusable hook such as useGoogleDriveAccess(projectId) that owns the fetchStatus/fetchCanUse effects and the openPicker/requestStorageAccessFor flow, then update DriveCommentInsert, GoogleDriveStage, and GoogleDriveAttach to consume it so the logic stays consistent in one place.src/Activity/Transformers/Activity_Transformer.php (1)
155-158: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueMove the Google Workspace flags out of
parse_meta_for_comment()
Comment_Observerstoreshas_drive/has_meetonresource_type = 'task', andparse_meta_for_task()already passes meta through unchanged. This check never fires for the current Drive/Meet flow; remove it here or move it into the task branch.🤖 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/Activity/Transformers/Activity_Transformer.php` around lines 155 - 158, The Google Workspace flag handling is currently in the comment meta parsing path, but the Drive/Meet flow stores these flags on task resources instead, so this branch never applies. Remove the `has_drive`/`has_meet` checks from `Activity_Transformer::parse_meta_for_comment()` and place them in the task-specific path, likely `parse_meta_for_task()`, where the meta is already passed through and the flags will be available for `Comment_Observer`.core/WP/Menu.php (1)
57-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
phpcs:ignoredoesn't cover the actual$submenuwrites.Per PHPCS semantics, a standalone
phpcs:ignorecomment only suppresses the comment's own line and the immediately following line. Line 57's comment therefore only covers line 58 (if (...)), not thearray_merge()assignment on lines 59-63, and the else-branch write on line 65 has no suppression at all. Both are direct$submenuglobal overrides, same pattern flagged elsewhere in this file.🔧 Proposed fix: move/add ignore comments next to the actual writes
// phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited -- Intentionally adding custom submenu items to WordPress admin menu if ( $pos >= 0 ) { + // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited -- Intentionally adding custom submenu items to WordPress admin menu $submenu[ $slug ] = array_merge( array_slice( $list, 0, $pos + 1 ), [ $gw_item ], array_slice( $list, $pos + 1 ) ); } else { + // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited -- Intentionally adding custom submenu items to WordPress admin menu $submenu[ $slug ][] = $gw_item; }If placed on a line by itself, this comment will ignore the line that the comment is on and the following line.
🤖 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 `@core/WP/Menu.php` around lines 57 - 67, The current PHPCS ignore only applies to the conditional line, not the actual $submenu writes in Menu handling. Move or duplicate the phpcs:ignore comment so it sits directly on the assignment statements in the WP\Menu flow, covering both the array_merge-based $submenu[$slug] update and the else-branch append to $submenu[$slug], matching the suppression pattern used elsewhere in this file.Source: Coding guidelines
src/Google_Workspace/Loader.php (1)
192-213: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winHardcoded
utf8charset instead of the site's actual charset/collation.
dbDeltatables here useDEFAULT CHARSET=utf8rather than$wpdb->get_charset_collate(). If the rest of the WP install usesutf8mb4(the modern default), this can cause collation mismatches on any future joins/comparisons against core tables and truncation issues with 4-byte characters (e.g., emoji in file names).🔧 Proposed fix
- dbDelta( "CREATE TABLE IF NOT EXISTS {$tokens} ( + $charset_collate = $wpdb->get_charset_collate(); + dbDelta( "CREATE TABLE IF NOT EXISTS {$tokens} ( ... - ) DEFAULT CHARSET=utf8" ); + ) {$charset_collate}" );🤖 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/Google_Workspace/Loader.php` around lines 192 - 213, The table definition in Loader::dbDelta() is hardcoding DEFAULT CHARSET=utf8 instead of using the WordPress site charset/collation. Update the CREATE TABLE statement for the files table to use the charset/collation from the database connection, via the existing $wpdb integration, so it matches the rest of the install. Keep the change localized to the schema creation block in Loader and preserve the existing indexes and columns.
🤖 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 `@routes/google-workspace.php`:
- Around line 12-63: The new Google Workspace REST routes registered on
$wedevs_pm_router only set permissions and are missing the required router-level
validation/sanitization contract. Add matching ->validator() and ->sanitizer()
handlers to each route in this file, covering the route params used by
Settings_Controller, OAuth_Controller, and Drive_Controller (such as project_id,
task_id, id, client_id, api_key, and drive_on). Keep the checks at the routing
layer rather than relying on controller-side sanitize/filter calls.
In `@src/Comment/Observers/Comment_Observer.php`:
- Around line 76-93: The Drive/Meet metadata is only added in comment_on_task(),
so other comment activity handlers never populate has_drive and has_meet even
though the activity transformer expects them. Update the other comment_on_*
methods in Comment_Observer to merge in gw_meta() using the same delete guard as
comment_on_task(), so comments on task lists, discussion boards, milestones,
projects, and files carry the same metadata.
In `@src/Google_Workspace/Controllers/Drive_Controller.php`:
- Around line 126-146: The Drive_Controller::index() method is missing the same
Drive-usability permission gate used by attach() and destroy(), so it can expose
attached file metadata without verifying project Drive access. Add the
Google_Service::user_can_use_drive( $project_id ) check in index() before
loading or returning any files, and return the same forbidden response path used
elsewhere when the user cannot use Drive for that project. Keep the existing
ownership validation in place after the usability gate.
In `@src/Google_Workspace/Google_Client.php`:
- Around line 73-78: The Google_Client::revoke() method is discarding the
wp_remote_post() result, so failures are invisible to callers; update it to
inspect the response with is_wp_error() and verify the HTTP status before
treating revocation as successful. If the request fails, log the error with
context in Google_Client::revoke() and surface a failure signal back to callers
so Google_Service::disconnect_user() and purge_stale() can avoid deleting the
local token row when revocation did not succeed.
In `@src/Google_Workspace/Google_Service.php`:
- Around line 355-359: The key() helper in Google_Service currently falls back
to fixed literals when AUTH_KEY or AUTH_SALT are missing, which makes the
encryption key predictable. Update Google_Service::key() to require both
constants and fail loudly instead of returning a derived fallback, and ensure
the token storage/encryption path that uses this key logs a critical error and
refuses to persist or encrypt tokens when the constants are undefined.
In `@src/Google_Workspace/Loader.php`:
- Around line 38-47: The delete_attachments helper is bypassing the ORM by
calling $wpdb->delete() directly. Update the Loader::delete_attachments method
to use the Google_Drive_File Eloquent model for the delete-by-attachable query
instead of raw SQL access, matching the rest of the data layer and the
wp-eloquent guideline.
- Around line 17-20: `maybe_install()` is still calling `ensure_columns()` on
every `admin_init`, so the schema checks never stop after the migration is
complete. Update `Loader::maybe_install()` to cache whether columns have already
been ensured (for example with an option/transient keyed to the current schema
version) and skip `ensure_columns()` once that flag is set; keep the existing
`db_version` migration gate in place so `ensure_columns()` only runs when the
schema actually needs work.
In `@views/assets/src/components/google-workspace/DrivePickerModal.jsx`:
- Around line 138-144: The attachment flow in DrivePickerModal.jsx currently
closes the modal even when every attachFileFor dispatch fails, leaving the user
with no feedback. Update the file loop around attachFileFor and the onClose
handling so failures are detected when attached stays 0, and show an error toast
(or equivalent failure message) before closing instead of silently dismissing
the modal. Keep the success path in the same attachFileFor.fulfilled.match check
and only close after reporting the result.
In `@views/assets/src/components/google-workspace/GoogleDriveAttach.jsx`:
- Line 17: The GoogleDriveAttach component is bypassing the app’s shared
notification abstraction by importing toast directly from sonner. Update the
notification usage in GoogleDriveAttach.jsx to use the existing useToast hook
pattern used elsewhere (for example, DiscussionsPage/index.jsx), and replace any
direct toast calls with the hook-provided notifier while keeping the same
success/error behavior.
In `@views/assets/src/components/google-workspace/GoogleWorkspacePage.jsx`:
- Line 22: Replace the direct sonner toast import in GoogleWorkspacePage with
the project’s useToast hook, since this component currently calls
toast.success/toast.error directly and bypasses the standard notification API.
Update GoogleWorkspacePage to initialize the toast helper from useToast and
route all success/error notifications through it, matching the approach used in
GoogleWorkspaceSettingsTab and other views/assets components.
In `@views/assets/src/components/projects/DiscussionsPage/index.jsx`:
- Around line 128-138: The staged Drive file attach flow in DiscussionsPage is
swallowing failures because dispatch(attachFileFor(...)) returns a rejected
action instead of throwing, and the current for-loop serializes each request.
Update the newDisc/stagedDrive attachment block to surface errors by unwrapping
each attachFileFor dispatch (or otherwise checking the rejected result) and run
the attachments in parallel rather than awaiting inside the loop. Keep the
existing success path and form resets only after all attachments complete
successfully.
---
Nitpick comments:
In `@core/WP/Menu.php`:
- Around line 57-67: The current PHPCS ignore only applies to the conditional
line, not the actual $submenu writes in Menu handling. Move or duplicate the
phpcs:ignore comment so it sits directly on the assignment statements in the
WP\Menu flow, covering both the array_merge-based $submenu[$slug] update and the
else-branch append to $submenu[$slug], matching the suppression pattern used
elsewhere in this file.
In `@src/Activity/Transformers/Activity_Transformer.php`:
- Around line 155-158: The Google Workspace flag handling is currently in the
comment meta parsing path, but the Drive/Meet flow stores these flags on task
resources instead, so this branch never applies. Remove the
`has_drive`/`has_meet` checks from
`Activity_Transformer::parse_meta_for_comment()` and place them in the
task-specific path, likely `parse_meta_for_task()`, where the meta is already
passed through and the flags will be available for `Comment_Observer`.
In `@src/Google_Workspace/Loader.php`:
- Around line 192-213: The table definition in Loader::dbDelta() is hardcoding
DEFAULT CHARSET=utf8 instead of using the WordPress site charset/collation.
Update the CREATE TABLE statement for the files table to use the
charset/collation from the database connection, via the existing $wpdb
integration, so it matches the rest of the install. Keep the change localized to
the schema creation block in Loader and preserve the existing indexes and
columns.
In `@views/assets/src/components/google-workspace/DriveCommentInsert.jsx`:
- Around line 14-20: The Drive icon is duplicated in MonoDrive instead of
reusing the shared glyph. Replace the local SVG in DriveCommentInsert with the
existing DriveMonoGlyph from GoogleIcons.jsx, and update any import/use sites so
the component references that shared icon like TaskDetailSheet/index.jsx does.
Remove the redundant inline definition to keep the Drive glyph single-sourced
and consistent across Google Workspace components.
- Around line 36-59: The status/canUse fetch effects and the
storage-access/picker-opening logic in DriveCommentInsert are duplicated across
multiple Google Drive components. Extract the shared behavior into a reusable
hook such as useGoogleDriveAccess(projectId) that owns the
fetchStatus/fetchCanUse effects and the openPicker/requestStorageAccessFor flow,
then update DriveCommentInsert, GoogleDriveStage, and GoogleDriveAttach to
consume it so the logic stays consistent in one place.
In `@views/assets/src/components/google-workspace/GoogleDriveCommentButton.jsx`:
- Around line 33-43: The Google Drive picker storage-access flow is duplicated
and inconsistent between openPicker in GoogleDriveCommentButton and the attach
flow in GoogleDriveAttach, so extract the shared logic into a
requestGoogleStorageAccess helper or hook and use it from both places. Make the
shared helper handle the requestStorageAccessFor path as well as the
hasStorageAccess/requestStorageAccess fallback, then update openPicker to call
that shared symbol before setPickerOpen(true) so both components stay aligned.
In `@views/assets/src/lib/google-links.js`:
- Around line 50-57: The safeHttpUrl helper is currently local-only, but it
should be reusable for other raw href renderers like TaskDetailSheet/index.jsx.
Export safeHttpUrl from google-links.js so call sites can import and use the
same URL validation instead of duplicating logic or skipping the check, and keep
the existing protocol guard behavior unchanged.
🪄 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: d9622421-e4b7-4c82-86d1-fbeb9016ef5d
📒 Files selected for processing (37)
bootstrap/start.phpcore/WP/Menu.phpdb/Create_Table.phproutes/google-workspace.phpsrc/Activity/Transformers/Activity_Transformer.phpsrc/Comment/Observers/Comment_Observer.phpsrc/Google_Workspace/Controllers/Drive_Controller.phpsrc/Google_Workspace/Controllers/OAuth_Controller.phpsrc/Google_Workspace/Controllers/Settings_Controller.phpsrc/Google_Workspace/Google_Client.phpsrc/Google_Workspace/Google_Service.phpsrc/Google_Workspace/Loader.phpsrc/Google_Workspace/Models/Google_Drive_File.phpsrc/Google_Workspace/Models/Google_Token.phpuninstall.phpviews/assets/src/components/admin-settings/SettingsPage.jsxviews/assets/src/components/admin-settings/tabs/GoogleWorkspaceSettingsTab.jsxviews/assets/src/components/google-workspace/CommentLinkActions.jsxviews/assets/src/components/google-workspace/DriveCommentInsert.jsxviews/assets/src/components/google-workspace/DrivePickerModal.jsxviews/assets/src/components/google-workspace/GoogleDriveAttach.jsxviews/assets/src/components/google-workspace/GoogleDriveCommentButton.jsxviews/assets/src/components/google-workspace/GoogleDriveStage.jsxviews/assets/src/components/google-workspace/GoogleDriveTaskSection.jsxviews/assets/src/components/google-workspace/GoogleIcons.jsxviews/assets/src/components/google-workspace/GoogleWorkspacePage.jsxviews/assets/src/components/layout/AppSidebar.jsxviews/assets/src/components/projects/ActivitiesPage/constants.jsviews/assets/src/components/projects/ActivitiesPage/parts/ActivityItem.jsxviews/assets/src/components/projects/DiscussionsPage/DiscussionDetailPage.jsxviews/assets/src/components/projects/DiscussionsPage/index.jsxviews/assets/src/components/tasks/SingleTaskListPage.jsxviews/assets/src/components/tasks/TaskDetailSheet/index.jsxviews/assets/src/index.jsxviews/assets/src/lib/google-links.jsviews/assets/src/store/googleWorkspaceSlice.jsviews/assets/src/store/index.js
Comment on lines
+12
to
+63
| $wedevs_pm_router->get( 'google-workspace/settings', $gw_base . 'Settings_Controller@get' ) | ||
| ->permission( [ $gw_admin ] ); | ||
|
|
||
| $wedevs_pm_router->post( 'google-workspace/settings', $gw_base . 'Settings_Controller@save' ) | ||
| ->permission( [ $gw_admin ] ); | ||
|
|
||
| // ── Per-user connection ────────────────────────────────────────────── | ||
| $wedevs_pm_router->get( 'google-workspace/status', $gw_base . 'OAuth_Controller@status' ) | ||
| ->permission( [ $gw_auth ] ); | ||
|
|
||
| $wedevs_pm_router->get( 'google-workspace/auth-url', $gw_base . 'OAuth_Controller@auth_url' ) | ||
| ->permission( [ $gw_auth ] ); | ||
|
|
||
| $wedevs_pm_router->post( 'google-workspace/disconnect', $gw_base . 'OAuth_Controller@disconnect' ) | ||
| ->permission( [ $gw_auth ] ); | ||
|
|
||
| $wedevs_pm_router->post( 'google-workspace/my-prefs', $gw_base . 'OAuth_Controller@save_my_prefs' ) | ||
| ->permission( [ $gw_auth ] ); | ||
|
|
||
| // ── Drive Picker config (vends caller's own access token + Picker keys) ── | ||
| $wedevs_pm_router->get( 'google-workspace/drive/picker-config', $gw_base . 'Drive_Controller@picker_config' ) | ||
| ->permission( [ $gw_auth ] ); | ||
|
|
||
| // ── Per-project Drive role access (manager configures; members query) ── | ||
| // Same gate as the project Capabilities/Settings tab where these toggles live. | ||
| $gw_manager = 'WeDevs\PM\Core\Permissions\Project_Settings_Page_Access'; | ||
|
|
||
| $wedevs_pm_router->get( 'projects/{project_id}/google-workspace/access', $gw_base . 'Drive_Controller@get_access' ) | ||
| ->permission( [ $gw_manager ] ); | ||
|
|
||
| $wedevs_pm_router->post( 'projects/{project_id}/google-workspace/access', $gw_base . 'Drive_Controller@save_access' ) | ||
| ->permission( [ $gw_manager ] ); | ||
|
|
||
| $wedevs_pm_router->get( 'projects/{project_id}/google-workspace/can-use', $gw_base . 'Drive_Controller@can_use' ) | ||
| ->permission( [ $gw_access ] ); | ||
|
|
||
| // ── Drive attachments (polymorphic: task, comment, discussion, project) ── | ||
| $wedevs_pm_router->get( 'projects/{project_id}/google-drive', $gw_base . 'Drive_Controller@index' ) | ||
| ->permission( [ $gw_access ] ); | ||
|
|
||
| $wedevs_pm_router->post( 'projects/{project_id}/google-drive', $gw_base . 'Drive_Controller@attach' ) | ||
| ->permission( [ $gw_access ] ); | ||
|
|
||
| $wedevs_pm_router->delete( 'projects/{project_id}/google-drive/{id}', $gw_base . 'Drive_Controller@destroy' ) | ||
| ->permission( [ $gw_access ] ); | ||
|
|
||
| // Legacy task-scoped routes (kept for back-compat). | ||
| $wedevs_pm_router->get( 'projects/{project_id}/tasks/{task_id}/google-drive', $gw_base . 'Drive_Controller@index' ) | ||
| ->permission( [ $gw_access ] ); | ||
|
|
||
| $wedevs_pm_router->post( 'projects/{project_id}/tasks/{task_id}/google-drive', $gw_base . 'Drive_Controller@attach' ) | ||
| ->permission( [ $gw_access ] ); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Missing ->validator()/->sanitizer() on every new route.
None of the 13 Google Workspace routes attach ->validator() or ->sanitizer(), only ->permission(). This is required for every REST endpoint in this codebase. Inline sanitize_text_field()/filter_var() calls inside Settings_Controller/OAuth_Controller don't substitute for the router-level contract, and params like project_id, task_id, id, client_id, api_key, drive_on, etc. reach controllers unvalidated at the routing layer.
As per coding guidelines, "Every REST endpoint must have a validator via ->validator() and sanitizer via ->sanitizer() methods."
🤖 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 `@routes/google-workspace.php` around lines 12 - 63, The new Google Workspace
REST routes registered on $wedevs_pm_router only set permissions and are missing
the required router-level validation/sanitization contract. Add matching
->validator() and ->sanitizer() handlers to each route in this file, covering
the route params used by Settings_Controller, OAuth_Controller, and
Drive_Controller (such as project_id, task_id, id, client_id, api_key, and
drive_on). Keep the checks at the routing layer rather than relying on
controller-side sanitize/filter calls.
Source: Coding guidelines
Comment on lines
+76
to
+93
| /** Flags whether a comment body contains a Drive / Meet link (for activity icons). */ | ||
| private function gw_meta( Comment $comment ) { | ||
| $content = (string) $comment->content; | ||
| return [ | ||
| 'has_drive' => ( strpos( $content, 'drive.google.com' ) !== false || strpos( $content, 'docs.google.com' ) !== false ), | ||
| 'has_meet' => ( strpos( $content, 'meet.google.com' ) !== false ), | ||
| ]; | ||
| } | ||
|
|
||
| private function comment_on_task( Comment $comment, Task $task, $action_type ) { | ||
| $meta = [ | ||
| 'comment_id' => $comment->id, | ||
| 'task_title' => $task->title, | ||
| ]; | ||
| // Don't mark a deleted comment with Drive/Meet icons. | ||
| if ( $action_type !== 'delete' ) { | ||
| $meta = array_merge( $meta, $this->gw_meta( $comment ) ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files 'src/Comment/Observers/Comment_Observer.php' 'src/**/Comment_Observer.php' | cat
echo '---'
rg -n "gw_meta\(|comment_on_(task|discussion_board|task_list|milestone|project|file)\(" src/Comment/Observers/Comment_Observer.php src -S
echo '---'
sed -n '1,220p' src/Comment/Observers/Comment_Observer.phpRepository: weDevsOfficial/wp-project-manager
Length of output: 12585
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- usages of has_drive / has_meet ---'
rg -n "\bhas_drive\b|\bhas_meet\b" src core -S
echo '--- remaining part of Comment_Observer.php ---'
sed -n '220,320p' src/Comment/Observers/Comment_Observer.php
echo '--- activity/comment UI references for comment_on_discussion_board and similar actions ---'
rg -n "comment_on_discussion_board|update_comment_on_discussion_board|reply_comment_on_discussion_board|has_drive|has_meet|drive.google.com|meet.google.com" src core -SRepository: weDevsOfficial/wp-project-manager
Length of output: 6976
Propagate Drive/Meet metadata to all comment activity types. gw_meta() is only merged in comment_on_task(), so comments on task lists, discussion boards, milestones, projects, and files never set has_drive/has_meet even though the activity transformer already reads those flags. Merge it into the other comment_on_* handlers as well.
🤖 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/Comment/Observers/Comment_Observer.php` around lines 76 - 93, The
Drive/Meet metadata is only added in comment_on_task(), so other comment
activity handlers never populate has_drive and has_meet even though the activity
transformer expects them. Update the other comment_on_* methods in
Comment_Observer to merge in gw_meta() using the same delete guard as
comment_on_task(), so comments on task lists, discussion boards, milestones,
projects, and files carry the same metadata.
Comment on lines
+126
to
+146
| public function index( WP_REST_Request $request ) { | ||
| $project_id = (int) $request->get_param( 'project_id' ); | ||
| list( $type, $id ) = $this->resolve_attachable( $request ); | ||
|
|
||
| if ( $type === '' ) { | ||
| return [ 'data' => [] ]; | ||
| } | ||
|
|
||
| $owner_project_id = $this->attachable_project_id( $type, $id ); | ||
| if ( ! $owner_project_id || $owner_project_id !== $project_id ) { | ||
| return new \WP_Error( 'pm_google_forbidden', __( 'This attachment target does not belong to the current project.', 'wedevs-project-manager' ), [ 'status' => 403 ] ); | ||
| } | ||
|
|
||
| $files = Google_Drive_File::where( 'attachable_type', $type ) | ||
| ->where( 'attachable_id', $id ) | ||
| ->where( 'project_id', $project_id ) | ||
| ->orderBy( 'created_at', 'desc' ) | ||
| ->get(); | ||
|
|
||
| return [ 'data' => $this->transform_collection( $files ) ]; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
index() skips the Drive-usability gate that attach()/destroy() enforce.
attach() (Line 176) and destroy() (Line 236) both call Google_Service::user_can_use_drive( $project_id ) before proceeding, but index() only checks attachable ownership. A user whose personal Drive toggle is off, whose role lacks project Drive access, or when the admin master switch is disabled, can still list attached Drive file metadata (names, thumbnail/web view links) for that target, since only the ownership check runs.
🔒 Proposed fix to gate index() consistently
public function index( WP_REST_Request $request ) {
$project_id = (int) $request->get_param( 'project_id' );
list( $type, $id ) = $this->resolve_attachable( $request );
+ if ( ! Google_Service::user_can_use_drive( $project_id ) ) {
+ return new \WP_Error( 'pm_google_forbidden', __( 'You are not allowed to view Google Drive files in this project.', 'wedevs-project-manager' ), [ 'status' => 403 ] );
+ }
+
if ( $type === '' ) {
return [ 'data' => [] ];
}📝 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.
Suggested change
| public function index( WP_REST_Request $request ) { | |
| $project_id = (int) $request->get_param( 'project_id' ); | |
| list( $type, $id ) = $this->resolve_attachable( $request ); | |
| if ( $type === '' ) { | |
| return [ 'data' => [] ]; | |
| } | |
| $owner_project_id = $this->attachable_project_id( $type, $id ); | |
| if ( ! $owner_project_id || $owner_project_id !== $project_id ) { | |
| return new \WP_Error( 'pm_google_forbidden', __( 'This attachment target does not belong to the current project.', 'wedevs-project-manager' ), [ 'status' => 403 ] ); | |
| } | |
| $files = Google_Drive_File::where( 'attachable_type', $type ) | |
| ->where( 'attachable_id', $id ) | |
| ->where( 'project_id', $project_id ) | |
| ->orderBy( 'created_at', 'desc' ) | |
| ->get(); | |
| return [ 'data' => $this->transform_collection( $files ) ]; | |
| } | |
| public function index( WP_REST_Request $request ) { | |
| $project_id = (int) $request->get_param( 'project_id' ); | |
| list( $type, $id ) = $this->resolve_attachable( $request ); | |
| if ( ! Google_Service::user_can_use_drive( $project_id ) ) { | |
| return new \WP_Error( 'pm_google_forbidden', __( 'You are not allowed to view Google Drive files in this project.', 'wedevs-project-manager' ), [ 'status' => 403 ] ); | |
| } | |
| if ( $type === '' ) { | |
| return [ 'data' => [] ]; | |
| } | |
| $owner_project_id = $this->attachable_project_id( $type, $id ); | |
| if ( ! $owner_project_id || $owner_project_id !== $project_id ) { | |
| return new \WP_Error( 'pm_google_forbidden', __( 'This attachment target does not belong to the current project.', 'wedevs-project-manager' ), [ 'status' => 403 ] ); | |
| } | |
| $files = Google_Drive_File::where( 'attachable_type', $type ) | |
| ->where( 'attachable_id', $id ) | |
| ->where( 'project_id', $project_id ) | |
| ->orderBy( 'created_at', 'desc' ) | |
| ->get(); | |
| return [ 'data' => $this->transform_collection( $files ) ]; | |
| } |
🤖 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/Google_Workspace/Controllers/Drive_Controller.php` around lines 126 -
146, The Drive_Controller::index() method is missing the same Drive-usability
permission gate used by attach() and destroy(), so it can expose attached file
metadata without verifying project Drive access. Add the
Google_Service::user_can_use_drive( $project_id ) check in index() before
loading or returning any files, and return the same forbidden response path used
elsewhere when the user cannot use Drive for that project. Keep the existing
ownership validation in place after the usability gate.
Comment on lines
+73
to
+78
| public function revoke( $token ) { | ||
| wp_remote_post( self::REVOKE_URL, [ | ||
| 'timeout' => 15, | ||
| 'body' => [ 'token' => $token ], | ||
| ] ); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
revoke() silently swallows request failures.
The response from wp_remote_post() is discarded — no is_wp_error() check, no status check, no logging. Downstream, Google_Service::disconnect_user() and purge_stale() delete the local token row unconditionally after calling revoke(), so a failed revocation leaves the grant still valid at Google with no local record left to retry revoking it.
🔧 Proposed fix: surface and log failures
public function revoke( $token ) {
- wp_remote_post( self::REVOKE_URL, [
+ $res = wp_remote_post( self::REVOKE_URL, [
'timeout' => 15,
'body' => [ 'token' => $token ],
] );
+ if ( is_wp_error( $res ) || (int) wp_remote_retrieve_response_code( $res ) >= 300 ) {
+ Loader::log( 'Token revoke failed: ' . ( is_wp_error( $res ) ? $res->get_error_message() : wp_remote_retrieve_response_code( $res ) ) );
+ return false;
+ }
+ return true;
}📝 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.
Suggested change
| public function revoke( $token ) { | |
| wp_remote_post( self::REVOKE_URL, [ | |
| 'timeout' => 15, | |
| 'body' => [ 'token' => $token ], | |
| ] ); | |
| } | |
| public function revoke( $token ) { | |
| $res = wp_remote_post( self::REVOKE_URL, [ | |
| 'timeout' => 15, | |
| 'body' => [ 'token' => $token ], | |
| ] ); | |
| if ( is_wp_error( $res ) || (int) wp_remote_retrieve_response_code( $res ) >= 300 ) { | |
| Loader::log( 'Token revoke failed: ' . ( is_wp_error( $res ) ? $res->get_error_message() : wp_remote_retrieve_response_code( $res ) ) ); | |
| return false; | |
| } | |
| return true; | |
| } |
🤖 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/Google_Workspace/Google_Client.php` around lines 73 - 78, The
Google_Client::revoke() method is discarding the wp_remote_post() result, so
failures are invisible to callers; update it to inspect the response with
is_wp_error() and verify the HTTP status before treating revocation as
successful. If the request fails, log the error with context in
Google_Client::revoke() and surface a failure signal back to callers so
Google_Service::disconnect_user() and purge_stale() can avoid deleting the local
token row when revocation did not succeed.
Comment on lines
+355
to
+359
| private static function key() { | ||
| $salt = defined( 'AUTH_KEY' ) ? AUTH_KEY : 'pm-google-fallback'; | ||
| $salt .= defined( 'AUTH_SALT' ) ? AUTH_SALT : 'pm-google-fallback-salt'; | ||
| return hash( 'sha256', $salt, true ); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Hardcoded fallback encryption key if AUTH_KEY/AUTH_SALT are undefined.
When these WP constants aren't defined, the key derivation falls back to fixed, source-visible literals. Since this class's docblock promises tokens are "encrypted at rest," a predictable fallback key defeats that guarantee for any install missing these constants — refresh/access tokens would be recoverable by anyone with the plugin source.
Recommend failing loudly (e.g., refuse to store/encrypt and log a critical error) instead of silently using a guessable 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/Google_Workspace/Google_Service.php` around lines 355 - 359, The key()
helper in Google_Service currently falls back to fixed literals when AUTH_KEY or
AUTH_SALT are missing, which makes the encryption key predictable. Update
Google_Service::key() to require both constants and fail loudly instead of
returning a derived fallback, and ensure the token storage/encryption path that
uses this key logs a critical error and refuses to persist or encrypt tokens
when the constants are undefined.
Comment on lines
+38
to
+47
| private static function delete_attachments( $type, $id ) { | ||
| global $wpdb; | ||
| if ( empty( $id ) ) { | ||
| return; | ||
| } | ||
| $wpdb->delete( $wpdb->prefix . 'pm_google_drive_files', [ | ||
| 'attachable_type' => $type, | ||
| 'attachable_id' => (int) $id, | ||
| ] ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use the Google_Drive_File Eloquent model instead of raw $wpdb->delete().
{src,core}/**/*.php is expected to use Eloquent for data queries; this bypasses the model (referenced elsewhere in this PR's layer) for a straightforward delete-by-attachable query.
As per coding guidelines, "Use Eloquent ORM (via tareq1988/wp-eloquent) for database queries."
🤖 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/Google_Workspace/Loader.php` around lines 38 - 47, The delete_attachments
helper is bypassing the ORM by calling $wpdb->delete() directly. Update the
Loader::delete_attachments method to use the Google_Drive_File Eloquent model
for the delete-by-attachable query instead of raw SQL access, matching the rest
of the data layer and the wp-eloquent guideline.
Source: Coding guidelines
Comment on lines
+138
to
+144
| let attached = 0 | ||
| for (const file of files) { | ||
| const res = await dispatch(attachFileFor({ projectId, attachableType, attachableId, file })) | ||
| if (attachFileFor.fulfilled.match(res)) attached++ | ||
| } | ||
| if (attached > 0) toast.success(__('File attached.', 'wedevs-project-manager')) | ||
| onClose() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Silent failure when all attach requests fail.
If every attachFileFor dispatch is rejected (e.g. permission or network error), attached stays 0, no success toast fires, and the modal closes without telling the user anything went wrong.
🩹 Proposed fix
let attached = 0
for (const file of files) {
const res = await dispatch(attachFileFor({ projectId, attachableType, attachableId, file }))
if (attachFileFor.fulfilled.match(res)) attached++
}
- if (attached > 0) toast.success(__('File attached.', 'wedevs-project-manager'))
+ if (attached > 0) toast.success(__('File attached.', 'wedevs-project-manager'))
+ else if (files.length > 0) toast.error(__('Failed to attach file(s).', 'wedevs-project-manager'))
onClose()📝 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.
Suggested change
| let attached = 0 | |
| for (const file of files) { | |
| const res = await dispatch(attachFileFor({ projectId, attachableType, attachableId, file })) | |
| if (attachFileFor.fulfilled.match(res)) attached++ | |
| } | |
| if (attached > 0) toast.success(__('File attached.', 'wedevs-project-manager')) | |
| onClose() | |
| let attached = 0 | |
| for (const file of files) { | |
| const res = await dispatch(attachFileFor({ projectId, attachableType, attachableId, file })) | |
| if (attachFileFor.fulfilled.match(res)) attached++ | |
| } | |
| if (attached > 0) toast.success(__('File attached.', 'wedevs-project-manager')) | |
| else if (files.length > 0) toast.error(__('Failed to attach file(s).', 'wedevs-project-manager')) | |
| onClose() |
🤖 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 `@views/assets/src/components/google-workspace/DrivePickerModal.jsx` around
lines 138 - 144, The attachment flow in DrivePickerModal.jsx currently closes
the modal even when every attachFileFor dispatch fails, leaving the user with no
feedback. Update the file loop around attachFileFor and the onClose handling so
failures are detected when attached stays 0, and show an error toast (or
equivalent failure message) before closing instead of silently dismissing the
modal. Keep the success path in the same attachFileFor.fulfilled.match check and
only close after reporting the result.
| import { fetchStatus, fetchCanUse, fetchAttachmentsFor, detachFileFor } from '@store/googleWorkspaceSlice' | ||
| import { Button } from '@components/ui/button' | ||
| import { FileText, Plus, ExternalLink, Trash2, Link2, Lock, X } from 'lucide-react' | ||
| import { toast } from 'sonner' |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use useToast instead of raw sonner import.
Other files in this cohort (e.g. DiscussionsPage/index.jsx) call notifications via the project's useToast hook, but this component imports toast directly from sonner, bypassing the shared abstraction.
♻️ Proposed fix
-import { toast } from 'sonner'
+import { useToast } from '`@hooks/useToast`' export default function GoogleDriveAttach(...) {
+ const toast = useToast()Also applies to: 80-83
🤖 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 `@views/assets/src/components/google-workspace/GoogleDriveAttach.jsx` at line
17, The GoogleDriveAttach component is bypassing the app’s shared notification
abstraction by importing toast directly from sonner. Update the notification
usage in GoogleDriveAttach.jsx to use the existing useToast hook pattern used
elsewhere (for example, DiscussionsPage/index.jsx), and replace any direct toast
calls with the hook-provided notifier while keeping the same success/error
behavior.
Source: Coding guidelines
| import { ShieldCheck, Unlink, HardDrive, Settings as SettingsIcon, Lock, Info } from 'lucide-react' | ||
| import { CalendarGlyph, MeetGlyph } from '@components/google-workspace/GoogleIcons' | ||
| import ProBadge from '@components/common/ProBadge' | ||
| import { toast } from 'sonner' |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use the useToast hook instead of importing sonner directly.
This file calls toast.success/toast.error from the raw sonner import (lines 93, 106, 111-113), bypassing the project's useToast hook used everywhere else (e.g., in the sibling GoogleWorkspaceSettingsTab.jsx). As per coding guidelines, views/assets/src/**/*.{jsx,js}: "Use custom useToast hook for showing notifications".
♻️ Proposed fix
-import { toast } from 'sonner'
+import { useToast } from '`@hooks/useToast`' export default function GoogleWorkspacePage() {
const dispatch = useAppDispatch()
+ const toast = useToast()
const { status, statusLoading } = useAppSelector(s => s.googleWorkspace)Also applies to: 93-93, 106-106, 111-113
🤖 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 `@views/assets/src/components/google-workspace/GoogleWorkspacePage.jsx` at line
22, Replace the direct sonner toast import in GoogleWorkspacePage with the
project’s useToast hook, since this component currently calls
toast.success/toast.error directly and bypasses the standard notification API.
Update GoogleWorkspacePage to initialize the toast helper from useToast and
route all success/error notifications through it, matching the approach used in
GoogleWorkspaceSettingsTab and other views/assets components.
Source: Coding guidelines
Comment on lines
+128
to
+138
| // Attach any staged Drive files now that the discussion has an id. | ||
| if (newDisc?.id && stagedDrive.length) { | ||
| for (const file of stagedDrive) { | ||
| await dispatch(attachFileFor({ projectId, attachableType: 'discussion', attachableId: newDisc.id, file })); | ||
| } | ||
| } | ||
| setFormTitle(""); | ||
| setFormDesc(""); | ||
| setFormMilestone("-1"); | ||
| setFormFiles([]); | ||
| setStagedDrive([]); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Silent failure & serial execution when attaching staged Drive files.
attachFileFor resolves via rejectWithValue on error rather than throwing, so a failed attach in this loop is never surfaced to the user — the discussion is created but some files silently fail to attach. The await inside the loop also serializes requests unnecessarily.
🐛 Proposed fix
- if (newDisc?.id && stagedDrive.length) {
- for (const file of stagedDrive) {
- await dispatch(attachFileFor({ projectId, attachableType: 'discussion', attachableId: newDisc.id, file }));
- }
- }
+ if (newDisc?.id && stagedDrive.length) {
+ const results = await Promise.all(
+ stagedDrive.map((file) =>
+ dispatch(attachFileFor({ projectId, attachableType: 'discussion', attachableId: newDisc.id, file }))
+ )
+ );
+ if (results.some((r) => attachFileFor.rejected.match(r))) {
+ toast.error(__('Some Drive files failed to attach', '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.
Suggested change
| // Attach any staged Drive files now that the discussion has an id. | |
| if (newDisc?.id && stagedDrive.length) { | |
| for (const file of stagedDrive) { | |
| await dispatch(attachFileFor({ projectId, attachableType: 'discussion', attachableId: newDisc.id, file })); | |
| } | |
| } | |
| setFormTitle(""); | |
| setFormDesc(""); | |
| setFormMilestone("-1"); | |
| setFormFiles([]); | |
| setStagedDrive([]); | |
| // Attach any staged Drive files now that the discussion has an id. | |
| if (newDisc?.id && stagedDrive.length) { | |
| const results = await Promise.all( | |
| stagedDrive.map((file) => | |
| dispatch(attachFileFor({ projectId, attachableType: 'discussion', attachableId: newDisc.id, file })) | |
| ) | |
| ); | |
| if (results.some((r) => attachFileFor.rejected.match(r))) { | |
| toast.error(__('Some Drive files failed to attach', 'wedevs-project-manager')); | |
| } | |
| } | |
| setFormTitle(""); | |
| setFormDesc(""); | |
| setFormMilestone("-1"); | |
| setFormFiles([]); | |
| setStagedDrive([]); |
🤖 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 `@views/assets/src/components/projects/DiscussionsPage/index.jsx` around lines
128 - 138, The staged Drive file attach flow in DiscussionsPage is swallowing
failures because dispatch(attachFileFor(...)) returns a rejected action instead
of throwing, and the current for-loop serializes each request. Update the
newDisc/stagedDrive attachment block to surface errors by unwrapping each
attachFileFor dispatch (or otherwise checking the rejected result) and run the
attachments in parallel rather than awaiting inside the loop. Keep the existing
success path and form resets only after all attachments complete successfully.
…leIcons.jsx Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ect access Google links pasted into comments were shown to every project member. Now the comment transformer runs content through a wedevs_pm_comment_content_visibility filter; Google_Workspace strips Drive/Docs/Meet anchors (replacing them with plain text) when the requesting user fails the existing per-project role permission Google_Service::user_can_use_drive() — managers/admins pass, co_worker/client gated by the project access map. Stripped server-side so the URL never reaches the response. No-op unless the content actually holds a google.com link. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…delete uninstall.php dropped pm_google_tokens + pm_google_drive_files and deleted the settings options, destroying user data on plugin delete/reinstall (this is the free wp.org core plugin). Now it only clears the scheduled cleanup cron; no tables dropped, no options deleted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…alendar-secured # Conflicts: # views/assets/src/components/layout/AppSidebar.jsx
…alendar-secured # Conflicts: # bootstrap/start.php # views/assets/src/store/index.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Continuation of #625 (Google Calendar/Meet + Drive, free side) with the security blocker fixed. anik-fahmid's original commits are preserved — this only adds the fix on top.
Security fix — Drive cross-project IDOR (High · CWE-639 · OWASP A01)
Same
Drive_ControllerIDOR as the drive branch:(attachable_type, attachable_id)from the request was never bound to the routeproject_id, andindex()was unscoped → cross-project read/write of Drive attachments.Added the
attachable_project_id()ownership resolver + 403 on cross-project inindex()/attach()and scoped the list query byproject_id. OAuth/token/crypto handling here reviewed clean.php -lclean.Supersedes #625.
Summary by CodeRabbit
New Features
Bug Fixes