Open
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
WalkthroughAdds a backend PUT /images/rename-image endpoint, DB migration and accessor for image paths, frontend API + UI to rename images, tests for rename flows, OpenAPI update, minor dependency and conditional import changes, and keyboard handling to avoid shortcut interference during renaming. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as "Client (UI)"
participant Frontend as "Frontend API"
participant Backend as "Backend /rename-image"
participant DB as "Database (images table)"
participant FS as "Filesystem"
rect rgba(200,200,255,0.5)
Client->>Frontend: PUT /images/rename-image {image_id, rename}
Frontend->>Backend: forward request
Backend->>DB: db_get_image_path_by_id(image_id)
DB-->>Backend: image_path or None
alt image not found
Backend-->>Frontend: 404 Image Not Found
Frontend-->>Client: 404
else image found
Backend->>FS: check target path exists (open/flags)
alt target exists
Backend-->>Frontend: 400 File Exists
Frontend-->>Client: 400
else target free
Backend->>FS: os.rename(old_path, new_path)
alt rename success
Backend-->>Frontend: 200 RenameImageResponse
Frontend-->>Client: 200
else rename failure
Backend->>FS: attempt cleanup (unlink if needed)
Backend-->>Frontend: 500 Internal Error
Frontend-->>Client: 500
end
end
end
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested Labels
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
frontend/src/api/api-functions/images.ts (1)
16-25: Consider typing theapiClient.putcall for type safety.The
putcall lacks a generic type parameter, soresponse.datais implicitlyany. While the function's return type annotation provides some safety, adding the generic ensures the response shape is validated at the call site.♻️ Proposed fix
- const response = await apiClient.put(imagesEndpoints.renameImage, { + const response = await apiClient.put<{ success: boolean; message: string }>(imagesEndpoints.renameImage, { image_id: imageId, rename: newName, });As per coding guidelines, "Avoid 'any', use explicit types."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/api/api-functions/images.ts` around lines 16 - 25, The apiClient.put call in renameImage is untyped causing response.data to be any; update the call to supply a generic response type matching the function return (e.g., { success: boolean; message: string }) so TypeScript can validate the shape at the call site (apply the generic to apiClient.put in the renameImage function and keep the function's declared return type).frontend/src/components/Media/MediaInfoPanel.tsx (1)
52-71:getBaseNameandgetExtensionasuseCallbackfunctions returning derived strings are better expressed asuseMemovalues.They are pure derivations of
currentImage, never passed as callbacks to child components. UsinguseMemois more idiomatic for computed values and removes the double-call indirection (getBaseName→getFileName()).♻️ Proposed refactor
- const getFileName = useCallback(() => { - if (!currentImage) return 'Image'; - return currentImage.path?.split(/[/\\]/).pop() || 'Image'; - }, [currentImage]); - - const getBaseName = useCallback(() => { - const fileName = getFileName(); - const lastDotIndex = fileName.lastIndexOf('.'); - if (lastDotIndex <= 0) return fileName; - return fileName.substring(0, lastDotIndex); - }, [getFileName]); - - const getExtension = useCallback(() => { - const fileName = getFileName(); - const lastDotIndex = fileName.lastIndexOf('.'); - if (lastDotIndex === -1 || lastDotIndex === fileName.length - 1) return ''; - return fileName.substring(lastDotIndex); - }, [getFileName]); - - const displayName = useMemo(() => getFileName(), [getFileName]); + const displayName = useMemo(() => { + if (!currentImage) return 'Image'; + return currentImage.path?.split(/[/\\]/).pop() || 'Image'; + }, [currentImage]); + + const baseName = useMemo(() => { + const lastDotIndex = displayName.lastIndexOf('.'); + if (lastDotIndex <= 0) return displayName; + return displayName.substring(0, lastDotIndex); + }, [displayName]); + + const extension = useMemo(() => { + const lastDotIndex = displayName.lastIndexOf('.'); + if (lastDotIndex === -1 || lastDotIndex === displayName.length - 1) return ''; + return displayName.substring(lastDotIndex); + }, [displayName]);Then replace
getBaseName()→baseNameinstartRenamingandgetExtension()→extensionin the JSX.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Media/MediaInfoPanel.tsx` around lines 52 - 71, getBaseName and getExtension are pure derived values but are implemented as useCallback functions; change them to useMemo computed values (e.g., baseName and extension) that depend on getFileName (or currentImage) instead of callbacks, update displayName to useMemo if needed, and replace all usages of getBaseName() and getExtension() (such as in startRenaming and JSX) with the new baseName and extension variables.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/routes/images.py`:
- Around line 169-180: After the invalid_chars check in the rename handler (the
code working with new_name and raising HTTPException), add additional
Windows-safe validations: reject names that are exactly "." or ".." or that
consist only of dots; reject names that end with a dot or a space (trimless
trailing dot/space); and reject names whose base stem (the part before the first
dot) matches any Windows reserved device names (CON, PRN, NUL, AUX, COM1–COM9,
LPT1–LPT9) in a case-insensitive manner (treat names like "CON", "con.txt",
"CON.", "con " as invalid). On any failure raise the same
HTTPException/status.HTTP_400_BAD_REQUEST pattern used currently with a clear
ErrorResponse message indicating the specific validation that failed.
- Around line 193-208: There is a TOCTOU between os.path.exists(new_file_path)
and os.rename(image_path, new_file_path); replace the check+rename with an
atomic reservation pattern: attempt to create a placeholder at new_file_path
using os.open(new_file_path, os.O_CREAT | os.O_EXCL) (catch FileExistsError to
return the same 400 error), close the placeholder, then perform
os.rename(image_path, new_file_path); on any exception during rename remove the
placeholder (os.unlink) and re-raise/translate to an HTTPException as needed;
reference image_path, new_file_path, os.open(..., os.O_CREAT|os.O_EXCL),
os.rename and os.unlink in the updated code to locate where to change.
In `@backend/tests/test_images.py`:
- Line 78: The pytest parametrize list for invalid_char is wrong: replace the
double-escaped backslash string '\\\\' with a single backslash '\\' and add the
missing disallowed characters '<', '>', ':', and '"' to the list used in
`@pytest.mark.parametrize` (the invalid_char parameter in the test in
test_images.py) so the test covers the full set of forbidden characters.
In `@frontend/src/components/Media/MediaInfoPanel.tsx`:
- Around line 33-36: Add a useEffect in the MediaInfoPanel component that
watches currentImage?.id and, when it changes, cancels any in-progress rename by
calling setIsRenaming(false) and clearing related state (setNewName(''),
setError(null), setIsSaving(false)) so the rename input and stale value are
reset when navigating to a new image; reference the existing state vars
isRenaming, setIsRenaming, newName, setNewName, setError, setIsSaving and key
the effect on currentImage?.id.
- Around line 104-105: The displayed filename stays stale because MediaInfoPanel
derives displayName from currentImage.path; add a local state (e.g.,
renamedName) in the MediaInfoPanel component, set renamedName to newName.trim()
after the successful renameImageApi call (where setIsRenaming(false) is called),
use renamedName as the preferred source for the shown filename when present, and
add/reset logic in the existing useEffect that watches currentImage?.id to clear
renamedName whenever the current image changes (and also clear it when exiting
rename mode if needed).
- Line 106: Replace the catch clause in the MediaInfoPanel component from "catch
(e: any)" to "catch (e: unknown)" and then narrow e before use: for logging or
accessing message, use a type guard (e.g., if (e instanceof Error) { /* use
e.message */ } else { /* handle/stringify e */ }) or a helper isError; update
any direct uses of e inside the catch to follow this pattern so no 'any' types
remain.
---
Nitpick comments:
In `@frontend/src/api/api-functions/images.ts`:
- Around line 16-25: The apiClient.put call in renameImage is untyped causing
response.data to be any; update the call to supply a generic response type
matching the function return (e.g., { success: boolean; message: string }) so
TypeScript can validate the shape at the call site (apply the generic to
apiClient.put in the renameImage function and keep the function's declared
return type).
In `@frontend/src/components/Media/MediaInfoPanel.tsx`:
- Around line 52-71: getBaseName and getExtension are pure derived values but
are implemented as useCallback functions; change them to useMemo computed values
(e.g., baseName and extension) that depend on getFileName (or currentImage)
instead of callbacks, update displayName to useMemo if needed, and replace all
usages of getBaseName() and getExtension() (such as in startRenaming and JSX)
with the new baseName and extension variables.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
backend/app/database/images.pybackend/app/routes/images.pybackend/app/schemas/images.pybackend/tests/test_images.pydocs/backend/backend_python/openapi.jsonfrontend/package.jsonfrontend/src-tauri/src/main.rsfrontend/src/api/api-functions/images.tsfrontend/src/api/apiEndpoints.tsfrontend/src/components/Media/MediaInfoPanel.tsxfrontend/src/hooks/useKeyboardNavigation.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/Media/MediaInfoPanel.tsx (1)
304-314:⚠️ Potential issue | 🟡 MinorStub button appears interactive but silently does nothing — add
disabledor remove it.The "Open Original File" button has no
disabledprop and no visual disabled styling, so users will click it and get no feedback. Either add adisabledattribute (and appropriate styling) until the feature is implemented, or remove it from the UI entirely.💡 Proposed fix
<button + disabled className="w-full rounded-lg bg-white/10 py-2 text-white hover:bg-white/20 + disabled:cursor-not-allowed disabled:opacity-40" onClick={(e) => { e.preventDefault(); - // Button disabled - does nothing }} > Open Original File </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Media/MediaInfoPanel.tsx` around lines 304 - 314, The "Open Original File" button in the MediaInfoPanel component currently appears clickable but is a silent stub; update the <button> with the disabled attribute and matching visual disabled styling (e.g., replace hover/bg classes with a muted/opacity style and add aria-disabled as needed) and remove the misleading onClick no-op, or alternatively remove the button entirely—locate the button by its text "Open Original File" inside MediaInfoPanel and apply the disabled state and styling changes or delete the element accordingly.
🧹 Nitpick comments (3)
backend/tests/test_images.py (3)
15-24: Fixture return types are missing — coding guidelines require proper type hints.Neither
sample_image_idnorsample_image_pathdeclares a return type, which is a minor deviation from the project's Python type-hint requirement.♻️ Proposed fix
`@pytest.fixture` -def sample_image_id(): +def sample_image_id() -> str: return "image_123" `@pytest.fixture` -def sample_image_path(tmp_path): +def sample_image_path(tmp_path: pytest.TempPathFactory) -> str: image_file = tmp_path / "old_name.jpg" image_file.write_bytes(b"dummy image data") return str(image_file)As per coding guidelines, Python code should "Ensure proper use of type hints."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/test_images.py` around lines 15 - 24, The fixtures sample_image_id and sample_image_path are missing return type hints; update their signatures to include explicit return types (e.g., def sample_image_id() -> str and def sample_image_path(tmp_path: pathlib.Path) -> str) so they conform to the project's type-hint rules, and ensure any imported types (like pathlib.Path) are imported at top of the test file if not already present.
59-59: Strengthenmock_openassertion to verify path and flags.
mock_open.assert_called_once()only checks call count, so a wrong path or incorrect flags (e.g., missingos.O_EXCL) would go undetected.♻️ Proposed improvement
- mock_open.assert_called_once() + mock_open.assert_called_once_with( + expected_new_path, + os.O_WRONLY | os.O_CREAT | os.O_EXCL, + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/test_images.py` at line 59, Replace the loose call-count check on mock_open with a strict argument assertion: update mock_open.assert_called_once() to mock_open.assert_called_once_with(expected_path, expected_flags) (or include any additional args like mode/encoding if your code uses them). Define expected_path as the file path variable used by the code under test and define expected_flags using os constants (e.g. expected_flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL) so the test verifies both the correct path and that the O_EXCL flag (and other required flags) are passed to os.open/open.
154-158:mock_unlink.assert_called_once()should also assert the path.The cleanup logic must unlink the placeholder at the new path. Without verifying the argument, a bug passing the wrong path to
os.unlink(e.g., the original source path) would not be caught.♻️ Proposed improvement
+ folder = os.path.dirname(sample_image_path) + ext = os.path.splitext(sample_image_path)[1] + expected_new_path = os.path.join(folder, "new_name" + ext) + - mock_unlink.assert_called_once() + mock_unlink.assert_called_once_with(expected_new_path)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/test_images.py` around lines 154 - 158, The test currently only checks that mock_unlink was called but not what path was removed; update the assertion to verify it was called with the placeholder file at the destination/new path used in the test (e.g., replace mock_unlink.assert_called_once() with mock_unlink.assert_called_once_with(expected_path)), where expected_path is the variable or constructed path the code should unlink (the placeholder at the new/destination path referenced earlier in the test).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/tests/test_images.py`:
- Around line 30-60: The tests calling the rename route are not mocking
os.close, so the real os.close(fd) in app.routes.images (used to reserve the
target filename) may close a live descriptor; update both
test_rename_image_success and test_rename_image_unexpected_error to add
`@patch`("app.routes.images.os.close") to their decorators and add the
corresponding mock_close parameter to the test function signatures (ensure the
mock parameters order matches the decorator order), so os.close is mocked and no
real file descriptor is closed during the tests.
In `@frontend/src/components/Media/MediaInfoPanel.tsx`:
- Around line 67-79: getBaseName and getExtension capture renamedName but their
useCallback deps only include getFileName, causing stale closures after renames;
update the dependency arrays for both getBaseName and getExtension to include
renamedName (e.g., change dependencies from [getFileName] to [getFileName,
renamedName]) so the callbacks are recreated when renamedName changes.
---
Outside diff comments:
In `@frontend/src/components/Media/MediaInfoPanel.tsx`:
- Around line 304-314: The "Open Original File" button in the MediaInfoPanel
component currently appears clickable but is a silent stub; update the <button>
with the disabled attribute and matching visual disabled styling (e.g., replace
hover/bg classes with a muted/opacity style and add aria-disabled as needed) and
remove the misleading onClick no-op, or alternatively remove the button
entirely—locate the button by its text "Open Original File" inside
MediaInfoPanel and apply the disabled state and styling changes or delete the
element accordingly.
---
Nitpick comments:
In `@backend/tests/test_images.py`:
- Around line 15-24: The fixtures sample_image_id and sample_image_path are
missing return type hints; update their signatures to include explicit return
types (e.g., def sample_image_id() -> str and def sample_image_path(tmp_path:
pathlib.Path) -> str) so they conform to the project's type-hint rules, and
ensure any imported types (like pathlib.Path) are imported at top of the test
file if not already present.
- Line 59: Replace the loose call-count check on mock_open with a strict
argument assertion: update mock_open.assert_called_once() to
mock_open.assert_called_once_with(expected_path, expected_flags) (or include any
additional args like mode/encoding if your code uses them). Define expected_path
as the file path variable used by the code under test and define expected_flags
using os constants (e.g. expected_flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL)
so the test verifies both the correct path and that the O_EXCL flag (and other
required flags) are passed to os.open/open.
- Around line 154-158: The test currently only checks that mock_unlink was
called but not what path was removed; update the assertion to verify it was
called with the placeholder file at the destination/new path used in the test
(e.g., replace mock_unlink.assert_called_once() with
mock_unlink.assert_called_once_with(expected_path)), where expected_path is the
variable or constructed path the code should unlink (the placeholder at the
new/destination path referenced earlier in the test).
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/app/routes/images.pybackend/tests/test_images.pyfrontend/src/components/Media/MediaInfoPanel.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/app/routes/images.py
Comment on lines
+30
to
+60
| @patch("app.routes.images.db_get_image_path_by_id") | ||
| @patch("app.routes.images.os.rename") | ||
| @patch("app.routes.images.os.open") | ||
| def test_rename_image_success( | ||
| self, | ||
| mock_open, | ||
| mock_rename, | ||
| mock_get_path, | ||
| sample_image_id, | ||
| sample_image_path, | ||
| ): | ||
| mock_get_path.return_value = sample_image_path | ||
| mock_open.return_value = 3 # dummy file descriptor | ||
|
|
||
| response = client.put( | ||
| "/images/rename-image", | ||
| json={"image_id": sample_image_id, "rename": "new_name"}, | ||
| ) | ||
|
|
||
| assert response.status_code == 200 | ||
| data = response.json() | ||
| assert data["success"] is True | ||
| assert "Successfully renamed image" in data["message"] | ||
|
|
||
| folder = os.path.dirname(sample_image_path) | ||
| ext = os.path.splitext(sample_image_path)[1] | ||
| expected_new_path = os.path.join(folder, "new_name" + ext) | ||
|
|
||
| mock_get_path.assert_called_once_with(sample_image_id) | ||
| mock_open.assert_called_once() | ||
| mock_rename.assert_called_once_with(sample_image_path, expected_new_path) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Locate the images route handler
find . -path "*/app/routes/images.py" -o -path "*/app/routes/images/*" | head -20Repository: AOSSIE-Org/PictoPy
Length of output: 92
🏁 Script executed:
# Search for os.close in the codebase
rg -n "os\.close" --type pyRepository: AOSSIE-Org/PictoPy
Length of output: 176
🏁 Script executed:
# Search for the rename-image route handler
rg -n "rename-image|def.*rename" --type py -A 5 | head -100Repository: AOSSIE-Org/PictoPy
Length of output: 6831
🏁 Script executed:
# Show the rename-image route handler context around line 253
sed -n '240,265p' backend/app/routes/images.pyRepository: AOSSIE-Org/PictoPy
Length of output: 1096
🏁 Script executed:
# Show a broader context to understand the flow
sed -n '220,270p' backend/app/routes/images.pyRepository: AOSSIE-Org/PictoPy
Length of output: 2034
Mock os.close alongside os.open in the rename image tests to prevent closing live file descriptors.
The route handler at backend/app/routes/images.py:253 explicitly calls os.close(fd) after os.open() to atomically reserve the target filename and avoid TOCTOU race conditions. In test_rename_image_success and test_rename_image_unexpected_error, os.close is not mocked, meaning mock_open.return_value = 3 causes the real os.close(3) syscall to execute. File descriptor 3 is often in use by pytest, coverage tools, or the logging system; closing it can corrupt the test session or cause spurious failures.
Add @patch("app.routes.images.os.close") and corresponding mock parameter to both affected test methods:
Patch for test_rename_image_success (lines 30–60)
`@patch`("app.routes.images.db_get_image_path_by_id")
`@patch`("app.routes.images.os.rename")
+@patch("app.routes.images.os.close")
`@patch`("app.routes.images.os.open")
def test_rename_image_success(
self,
mock_open,
+ mock_close,
mock_rename,
mock_get_path,
sample_image_id,
sample_image_path,
):Apply the same fix to test_rename_image_unexpected_error.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/tests/test_images.py` around lines 30 - 60, The tests calling the
rename route are not mocking os.close, so the real os.close(fd) in
app.routes.images (used to reserve the target filename) may close a live
descriptor; update both test_rename_image_success and
test_rename_image_unexpected_error to add `@patch`("app.routes.images.os.close")
to their decorators and add the corresponding mock_close parameter to the test
function signatures (ensure the mock parameters order matches the decorator
order), so os.close is mocked and no real file descriptor is closed during the
tests.
Comment on lines
+67
to
+79
| const getBaseName = useCallback(() => { | ||
| const fileName = renamedName || getFileName(); | ||
| const lastDotIndex = fileName.lastIndexOf('.'); | ||
| if (lastDotIndex <= 0) return fileName; | ||
| return fileName.substring(0, lastDotIndex); | ||
| }, [getFileName]); | ||
|
|
||
| const getExtension = useCallback(() => { | ||
| const fileName = renamedName || getFileName(); | ||
| const lastDotIndex = fileName.lastIndexOf('.'); | ||
| if (lastDotIndex === -1 || lastDotIndex === fileName.length - 1) return ''; | ||
| return fileName.substring(lastDotIndex); | ||
| }, [getFileName]); |
Contributor
There was a problem hiding this comment.
renamedName missing from getBaseName and getExtension dependency arrays — stale-closure bug for consecutive renames.
getBaseName reads renamedName from its closure but only declares [getFileName] as a dependency. Because a successful rename does not change currentImage, getFileName stays stable and useCallback never recreates the callback. The second time the user clicks Rename, startRenaming() calls the stale getBaseName(), which still sees renamedName === null and falls back to getFileName(), pre-filling the input with the original on-disk base name instead of the already-renamed name.
getExtension has the same omission; it's incidentally harmless today because the extension is invariant across renames, but the same stale reference means the guard renamedName || getFileName() is never used as intended after the first rename.
🐛 Proposed fix
const getBaseName = useCallback(() => {
const fileName = renamedName || getFileName();
const lastDotIndex = fileName.lastIndexOf('.');
if (lastDotIndex <= 0) return fileName;
return fileName.substring(0, lastDotIndex);
- }, [getFileName]);
+ }, [renamedName, getFileName]);
const getExtension = useCallback(() => {
const fileName = renamedName || getFileName();
const lastDotIndex = fileName.lastIndexOf('.');
if (lastDotIndex === -1 || lastDotIndex === fileName.length - 1) return '';
return fileName.substring(lastDotIndex);
- }, [getFileName]);
+ }, [renamedName, getFileName]);As per coding guidelines: "Check for proper React hook dependencies and prevent unnecessary re-renders."
📝 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
| const getBaseName = useCallback(() => { | |
| const fileName = renamedName || getFileName(); | |
| const lastDotIndex = fileName.lastIndexOf('.'); | |
| if (lastDotIndex <= 0) return fileName; | |
| return fileName.substring(0, lastDotIndex); | |
| }, [getFileName]); | |
| const getExtension = useCallback(() => { | |
| const fileName = renamedName || getFileName(); | |
| const lastDotIndex = fileName.lastIndexOf('.'); | |
| if (lastDotIndex === -1 || lastDotIndex === fileName.length - 1) return ''; | |
| return fileName.substring(lastDotIndex); | |
| }, [getFileName]); | |
| const getBaseName = useCallback(() => { | |
| const fileName = renamedName || getFileName(); | |
| const lastDotIndex = fileName.lastIndexOf('.'); | |
| if (lastDotIndex <= 0) return fileName; | |
| return fileName.substring(0, lastDotIndex); | |
| }, [renamedName, getFileName]); | |
| const getExtension = useCallback(() => { | |
| const fileName = renamedName || getFileName(); | |
| const lastDotIndex = fileName.lastIndexOf('.'); | |
| if (lastDotIndex === -1 || lastDotIndex === fileName.length - 1) return ''; | |
| return fileName.substring(lastDotIndex); | |
| }, [renamedName, getFileName]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/Media/MediaInfoPanel.tsx` around lines 67 - 79,
getBaseName and getExtension capture renamedName but their useCallback deps only
include getFileName, causing stale closures after renames; update the dependency
arrays for both getBaseName and getExtension to include renamedName (e.g.,
change dependencies from [getFileName] to [getFileName, renamedName]) so the
callbacks are recreated when renamedName changes.
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
1 participant
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.
Fixes #1192
Screenshots/Recordings:
Screen.Recording.2026-02-23.200129.mp4
*or!), and successful rename reflected after saving.Additional Notes:
PUT /images/rename-imagewhich looks up an image by ID, validates the new name (disallowing characters like<,>,:,",/,\,|,?,*,!,^), checks for existing files, and renames the file on disk without touching the database (which is updated by the sync microservice).db_get_image_path_by_idhelper and request/response Pydantic models for the rename operation, plus unit tests inbackend/tests/test_images.pycovering success, validation errors, not-found, existing-target, and unexpected filesystem errors.renameImage(imageId, newName)API helper and wired it intoMediaInfoPanelso users can rename the current image directly from the info panel via an inline text field.useKeyboardNavigationto ignore key events when typing in inputs/textareas/contenteditable elements so shortcuts like rotation (r) or toggling the info panel (i) do not fire while the rename field is focused.Checklist
/images/rename-image)AI Usage Disclosure
Summary by CodeRabbit
New Features
Tests
Chores