Issue #269 - supporting shortcuts #272
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for resolving Google Drive shortcuts (issue #269) and removes the supportsAllDrives parameter from get_media and export_media calls. The implementation introduces two new helper functions that dereference shortcuts to their target items before performing operations.
- Adds
resolve_drive_item()andresolve_folder_id()functions to handle shortcut resolution - Updates multiple Drive operations to resolve shortcuts before processing
- Removes
supportsAllDrivesparameter from media download operations
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| gdrive/drive_tools.py | Integrates shortcut resolution into file operations (get_drive_file_content, list_drive_items, create_drive_file, get_drive_file_permissions, check_drive_file_public_access, update_drive_file) |
| gdrive/drive_helpers.py | Adds new helper functions for resolving shortcuts with depth limiting and type validation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| resolved_file_id, _ = await resolve_drive_item(service, file_id) | ||
| file_id = resolved_file_id | ||
|
|
There was a problem hiding this comment.
Trailing whitespace on line 463. Remove the extra spaces after the statement for code cleanliness.
| if mime_type != SHORTCUT_MIME_TYPE: | ||
| return current_id, metadata | ||
|
|
||
| shortcut_details = metadata.get("shortcutDetails") or {} |
There was a problem hiding this comment.
Using or {} after .get() can mask None values in the dictionary. If shortcutDetails exists but is explicitly None, this will incorrectly return an empty dict. Use metadata.get('shortcutDetails', {}) instead to only provide a default when the key is missing.
| shortcut_details = metadata.get("shortcutDetails") or {} | |
| shortcut_details = metadata.get("shortcutDetails", {}) |
| mime_type = metadata.get("mimeType") | ||
| if mime_type != FOLDER_MIME_TYPE: | ||
| raise Exception( | ||
| f"Resolved ID '{resolved_id}' (from '{folder_id}') is not a folder; mimeType={mime_type}." | ||
| ) |
There was a problem hiding this comment.
The mimeType field is already included in BASE_SHORTCUT_FIELDS, so the extra_fields='mimeType' parameter on line 173 is redundant. Remove this parameter to avoid unnecessary duplication.
4 participants
#269 This patch also includes a removal of the supportAllDrives parameter for get_media/export_media calls as that was raising errors and it's now further up in dereferencing the file.