Release/v0.3.0 asset handling improvements#51
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR improves asset handling by adding renaming support and cleaning up legacy components. Key changes include:
- Adding an API function and corresponding backend support for renaming assets.
- Replacing the old AssetManager component (in features/page) with a new one in components/assets that supports file renaming.
- Minor UI text updates and improvements in user interactions (e.g. escape key handling in the asset modal).
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| ui/leafwiki-ui/src/lib/api.ts | Adds the renameAsset API function using fetchWithAuth. |
| ui/leafwiki-ui/src/features/page/AssetManager.tsx | Removes the legacy AssetManager component. |
| ui/leafwiki-ui/src/components/editor/MarkdownToolbar.tsx | Updates asset import and improves escape key handling during renaming. |
| ui/leafwiki-ui/src/components/assets/AssetManager.tsx | Introduces a new AssetManager component with multi-file upload support. |
| ui/leafwiki-ui/src/components/assets/AssetItem.tsx | Adds a new component for individual asset items with rename and delete functionality. |
| ui/leafwiki-ui/src/components/EditorTitleBar.tsx | Updates UI text from a non-English label to "Changes". |
| internal/wiki/wiki.go | Adds RenameAsset method routing to the asset service. |
| internal/http/router.go | Introduces a new PUT endpoint for asset renaming. |
| internal/http/api/rename_asset.go | Implements the API handler for renaming assets. |
| internal/core/assets/assets_service_test.go | Adds tests for the asset renaming feature. |
| internal/core/assets/assets_service.go | Implements RenameAsset to perform file renaming with validations. |
Comments suppressed due to low confidence (1)
ui/leafwiki-ui/src/components/editor/MarkdownToolbar.tsx:194
- [nitpick] Consider adding an inline comment explaining why the escape key event is prevented when a renaming operation is in progress to clarify the intended user experience.
onEscapeKeyDown={(e) => { if (isRenamingRef.current) {
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
a647c3c to
60b0edc
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for renaming assets and batch uploading in both backend and UI for the v0.3.0 release.
- Introduce
RenameAssetendpoint and service logic in Go. - Extend the API client and update UI components (
AssetManager/AssetItem) for rename functionality and multi-file uploads. - Update toolbar integration, docs, and tests to cover the happy-path rename.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/leafwiki-ui/src/lib/api.ts | Add renameAsset function to client API |
| ui/leafwiki-ui/src/components/editor/MarkdownToolbar.tsx | Wire up new AssetManager, handle escape while renaming |
| ui/leafwiki-ui/src/components/assets/AssetManager.tsx | New manager with multi-file upload and rename state control |
| ui/leafwiki-ui/src/components/assets/AssetItem.tsx | Add inline rename UI and call to renameAsset |
| ui/leafwiki-ui/src/components/EditorTitleBar.tsx | Change dirty indicator text to English |
| readme.md | Document v0.3.0 improvements and upcoming features |
| internal/wiki/wiki.go | Add RenameAsset wrapper on the Wiki service |
| internal/http/router.go | Register PUT route for asset rename |
| internal/http/api/rename_asset.go | Implement JSON handler for rename |
| internal/core/assets/assets_service.go | Add RenameAsset logic with validations |
| internal/core/assets/assets_service_test.go | Add happy-path test for asset renaming |
Comments suppressed due to low confidence (2)
internal/core/assets/assets_service_test.go:149
- TestAssetRename covers only the happy path. Adding tests for edge cases—extension mismatch, duplicate new name, invalid slug, or missing original file—would improve coverage and robustness.
func TestAssetRename(t *testing.T) {
readme.md:146
- There's a stray double-dash list item (
-- [ ]). Update to a proper Markdown list (- [ ]) to render correctly.
-- [ ] Upload multiple files
|
|
||
| await renameAsset(pageId, baseName, newFilename) | ||
| toast.success('Asset renamed') | ||
| onReload() |
There was a problem hiding this comment.
After a successful rename, the component remains in edit mode. Consider calling setEditingFilename(null) (or equivalent) after onReload() to exit the rename state.
| onReload() | |
| onReload() | |
| setEditingFilename(null) |
| if _, err := os.Stat(newFullPath); !os.IsNotExist(err) { | ||
| return "", fmt.Errorf("new asset already exists: %s", newFilename) |
There was a problem hiding this comment.
This treats any os.Stat error besides "not exist" as the file already existing. Use err == nil to detect an existing file, and handle other errors separately to avoid misclassification.
| if _, err := os.Stat(newFullPath); !os.IsNotExist(err) { | |
| return "", fmt.Errorf("new asset already exists: %s", newFilename) | |
| if _, err := os.Stat(newFullPath); err == nil { | |
| return "", fmt.Errorf("new asset already exists: %s", newFilename) | |
| } else if !os.IsNotExist(err) { | |
| return "", fmt.Errorf("could not check asset existence: %w", err) |
| ) | ||
| } | ||
|
|
||
| export async function renameAsset( |
There was a problem hiding this comment.
[nitpick] Consider adding an explicit return type (e.g., Promise<{ url: string }>) to renameAsset for clearer type safety and better IDE autocomplete.
No description provided.