[UPY-11] Implement the rename file fucntion #12
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes introduce a new file renaming feature across the project. A new method Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client as UTApi/AsyncUTApi
participant API as UploadThingAPI
User->>Client: rename_files(updates)
Client->>API: POST /v6/renameFiles (with updates)
API-->>Client: RenameFilesResponse (success, renamed_count)
Client-->>User: RenameFilesResponse
Possibly Related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (10)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
examples/main.py (1)
63-70: Enhance verification logic to be more precise.The current verification using
issubsetonly checks if the new names exist but doesn't verify that the old names were actually replaced.Consider this more thorough verification:
- expected_names = {"renamed_test.png", "renamed_test.jpg"} - actual_names = {file.name for file in updated_files.files} - if expected_names.issubset(actual_names): + old_names = {"test.png", "test.jpg"} + new_names = {"renamed_test.png", "renamed_test.jpg"} + actual_names = {file.name for file in updated_files.files} + if new_names.issubset(actual_names) and not old_names.intersection(actual_names):examples/async_main.py (1)
74-81: Enhance verification logic to be more precise.Similar to the synchronous example, the verification could be more thorough by also checking that old names were replaced.
Consider this more thorough verification:
- expected_names = {"renamed_test.png", "renamed_test.jpg"} - actual_names = {file.name for file in updated_files.files} - if expected_names.issubset(actual_names): + old_names = {"test.png", "test.jpg"} + new_names = {"renamed_test.png", "renamed_test.jpg"} + actual_names = {file.name for file in updated_files.files} + if new_names.issubset(actual_names) and not old_names.intersection(actual_names):tests/test_async_client.py (1)
311-332: Consider adding test cases for error scenarios.While the current test thoroughly verifies the mixed update functionality, consider adding test cases for:
- Error cases (e.g., invalid file key, invalid custom ID)
- Empty updates list
- Invalid response from the API
README.md (1)
63-84: Consider adding examples for error handling and response usage.While the examples are clear and comprehensive, consider adding:
- Error handling example (similar to the general error handling section)
- Example showing how to use the response (e.g., checking success and renamed count)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
README.md(3 hunks)examples/async_main.py(1 hunks)examples/main.py(1 hunks)tests/test_async_client.py(2 hunks)tests/test_client.py(2 hunks)upyloadthing/__init__.py(2 hunks)upyloadthing/async_client.py(2 hunks)upyloadthing/base_client.py(2 hunks)upyloadthing/client.py(2 hunks)upyloadthing/schemas.py(1 hunks)
👮 Files not reviewed due to content moderation or server errors (4)
- upyloadthing/client.py
- upyloadthing/async_client.py
- upyloadthing/base_client.py
- tests/test_client.py
🔇 Additional comments (4)
upyloadthing/__init__.py (1)
7-7: LGTM!The
RenameFilesResponseschema is correctly imported and exposed in the public interface, maintaining consistency with other response types.Also applies to: 21-21
upyloadthing/schemas.py (1)
37-40: LGTM!The
RenameFilesResponseschema follows the established pattern, with appropriate field types and clear purpose.tests/test_async_client.py (2)
267-286: LGTM!The test case thoroughly verifies the file renaming functionality using file keys, including response type checking and attribute validation.
288-309: LGTM!The test case thoroughly verifies the file renaming functionality using custom IDs, including response type checking and attribute validation.
README.md
Outdated
| - `rename_files(updates: List[dict[str, str]]) -> RenameFilesResponse` | ||
| - Rename files or update their custom IDs | ||
| - Updates list should contain dicts with: | ||
| - `fileKey` (required) and either: | ||
| - `newName` or `customId` | ||
| - Returns rename operation result | ||
|
|
There was a problem hiding this comment.
Fix inconsistency in method documentation.
The documentation states that fileKey is required, but the examples show that either fileKey or customId is required.
Apply this diff to fix the documentation:
- - `fileKey` (required) and either:
- - `newName` or `customId`
+ - Either `fileKey` or `customId` (one is required)
+ - `newName` (required)📝 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.
| - `rename_files(updates: List[dict[str, str]]) -> RenameFilesResponse` | |
| - Rename files or update their custom IDs | |
| - Updates list should contain dicts with: | |
| - `fileKey` (required) and either: | |
| - `newName` or `customId` | |
| - Returns rename operation result | |
| - `rename_files(updates: List[dict[str, str]]) -> RenameFilesResponse` | |
| - Rename files or update their custom IDs | |
| - Updates list should contain dicts with: | |
| - Either `fileKey` or `customId` (one is required) | |
| - `newName` (required) | |
| - Returns rename operation result |
farmisen
force-pushed
the
farmisen/upy-11-implement-the-rename-file-fucntion
branch
from
9f396f8 to
1cd2f27
Compare
Implements file rename functionality for both sync and async uploadthing clients
Adds new rename_files() method to both client implementations allowing files to be renamed using either file keys or custom IDs. Includes comprehensive test coverage and example implementations.
Changes
Impact
Summary by CodeRabbit
New Features
Documentation
Tests