fix(drive): warn when anchor field targets Workspace editor files#487
fix(drive): warn when anchor field targets Workspace editor files#487seang1121 wants to merge 2 commits intogoogleworkspace:mainfrom
Conversation
…ogleworkspace#169) When `gws drive comments create` includes an `anchor` field, the helper now checks the target file's mimeType via `files.get`. If the file is a Google Workspace editor file (Docs, Sheets, Slides, Drawings), a warning is printed to stderr explaining that the anchor will be silently ignored by the editor UI and the comment may show as "Original content deleted". If the mimeType cannot be determined (e.g., unauthenticated), a general warning is printed instead. The command still executes — this is a non-blocking warning only. Fixes googleworkspace#169
🦋 Changeset detectedLatest commit: e04e138 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new warning mechanism for the drive comments create command. It detects when a user attempts to create an anchored comment on Google Workspace editor files (Docs, Sheets, Slides, Drawings), where the anchor field is silently ignored by the editor UI. The changes include adding a constant for Workspace editor MIME types, integrating an asynchronous warning function into the command handler, and implementing helper functions to fetch file MIME types, check for Workspace editor MIME types, and provide display names for them. Unit tests for the new helper functions are also included.
The upstream bot token secret doesn't exist in this fork, causing all workflows to fail at checkout with "Input required and not supplied: token".
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a helpful warning for users creating anchored comments on Google Workspace files, where anchors are not supported. The implementation is well-structured, making a separate API call to check the file's MIME type and providing clear, contextual warnings. The addition of unit tests for the new helper functions is also a great practice. I've found one potential issue with how a JSON payload is constructed, which could be vulnerable to malformed input.
| let scopes: Vec<&str> = get_method.scopes.iter().map(|s| s.as_str()).collect(); | ||
| let token = auth::get_token(&scopes).await?; | ||
|
|
||
| let params = format!(r#"{{"fileId":"{}","fields":"mimeType"}}"#, file_id); |
There was a problem hiding this comment.
Constructing JSON via string formatting is vulnerable to injection if file_id contains special characters like quotes. This can lead to a panic or API errors. It's safer to use serde_json::json! which handles escaping correctly.
| let params = format!(r#"{{"fileId":"{}","fields":"mimeType"}}"#, file_id); | |
| let params = serde_json::json!({ "fileId": file_id, "fields": "mimeType" }).to_string(); |
Summary
Fixes #169
gws drive comments createincludes ananchorfield, the Drive helper now fetches the target file'smimeTypeviafiles.getbefore the command executesmimeTypecannot be determined (e.g., no auth configured), a general warning is printed insteadeprintln!+error::yellow()patterns (same asaccessNotConfiguredguidance)Workspace editor MIME types checked:
application/vnd.google-apps.document(Google Docs)application/vnd.google-apps.spreadsheet(Google Sheets)application/vnd.google-apps.presentation(Google Slides)application/vnd.google-apps.drawing(Google Drawings)Example warning output:
Test plan
is_workspace_editor_mime()— all 4 editor types + 3 non-editor typesmime_display_name()— all 4 named types + fallbackcargo testpasses locally for new testsgws drive comments createwith anchor on a Google Doc shows warninggws drive comments createwith anchor on a PDF does not warngws drive comments createwithout anchor does not warn