Skip to content

VPS-96/Allowed renaming of resources in edit view#413

Open
kmck133 wants to merge 5 commits into
masterfrom
VPS-96/change-resource-name
Open

VPS-96/Allowed renaming of resources in edit view#413
kmck133 wants to merge 5 commits into
masterfrom
VPS-96/change-resource-name

Conversation

@kmck133

@kmck133 kmck133 commented May 26, 2026

Copy link
Copy Markdown
Contributor

Issue

After uploading a resource, there was no way to rename it. Users were stuck with the original filename even if it was unclear or incorrect.

Solution

Added the ability to rename uploaded resources from the collections sidebar in the manage resources view. A pencil icon appears next to each file; clicking it replaces the filename with an inline input field. The user can confirm with Enter or the checkmark button, or cancel with Escape or the X button.
This required a new PATCH /api/files/:fileId endpoint on the backend that validates and updates the name field on StoredFile. The frontend calls this endpoint, then updates both the sidebar list and the active preview panel (if the renamed file is currently selected) without a page reload.

Risk

  • Low. Renaming a resource updates the display name in StoredFile but not the filename stored in GridFS. This is intentional, as the application always resolves filenames through StoredFile.name (including when downloading the resource), so there is no functional impact. The divergence is only visible when inspecting the raw resources.files collection in MongoDB directly.
  • The new PATCH /:fileId route sits alongside the existing DELETE /:fileId route with no ambiguity since they use different HTTP methods.

Summary by CodeRabbit

  • New Features
    • Inline file renaming in the Resources management UI with Save/Cancel controls, keyboard shortcuts (Enter to save, Escape to cancel), auto-focus, and name trimming.
  • Usability
    • Validation enforcing a maximum filename length and preventing no-op renames.
    • Minor layout and preview/header styling improvements for clearer file selection and download actions.
  • Reliability
    • Improved handling of rename operations and clearer error responses when updates fail.

@linear

linear Bot commented May 26, 2026

Copy link
Copy Markdown

VPS-96

@leowla leowla left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be a character limit

Image

@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a PATCH backend endpoint to update stored file names and implements frontend inline renaming: state, API integration, conditional rename editor UI, and minor layout/styling adjustments.

Changes

File Renaming Feature

Layer / File(s) Summary
Backend file rename endpoint
backend/src/routes/api/files.js
PATCH /api/files/:fileId validates and trims the file name, enforces a 255-character limit, checks file existence, persists the update via meta.save(), and returns the updated file object with gridFsId omitted.
Frontend rename state and API integration
frontend/src/features/resources/ManageResourcesPage.jsx
Adds renamingFileId and renameInput state, imports pencil/check icons, and implements renameFile() to PATCH the backend endpoint with auth token and sync groups and selectedFile state. handleRenameSubmit() trims/validates input, calls renameFile() when changed, and clears rename mode.
Frontend rename UI rendering and styling adjustments
frontend/src/features/resources/ManageResourcesPage.jsx
Adds min-w-0 to the collections card, constrains group <details> overflow, and reworks file list to conditionally render an inline rename editor (auto-focused input with Enter/Escape handling and Save/Cancel buttons) or a normal file row; also tweaks preview header and download button styling.

Sequence Diagram

sequenceDiagram
  participant User
  participant ManageResourcesPage
  participant Backend
  User->>ManageResourcesPage: Click rename button / open inline editor
  ManageResourcesPage->>ManageResourcesPage: Set renamingFileId and show input
  User->>ManageResourcesPage: Enter new name and press Enter or click Save
  ManageResourcesPage->>ManageResourcesPage: handleRenameSubmit validates and trims name
  ManageResourcesPage->>Backend: PATCH /api/files/:fileId with name payload
  Backend->>Backend: Validate, trim, enforce 255-char limit
  Backend->>Backend: meta.save() persists updated name
  Backend->>ManageResourcesPage: Return updated file object
  ManageResourcesPage->>ManageResourcesPage: Update groups and selectedFile and clear renamingFileId
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A rabbit taps keys soft and light,
PATCHing names by moonlit byte,
Frontend glows with pencil's gleam,
Backend saves the new-file dream,
Hooray—each file now says its name! 🐇✏️

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding the ability to rename resources in the edit view, which aligns with the primary purpose of the changeset.
Description check ✅ Passed The description includes all required template sections (Issue, Solution, Risk) with comprehensive details about the changes, implementation approach, and risk mitigation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch VPS-96/change-resource-name

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
frontend/src/features/resources/ManageResourcesPage.jsx (1)

390-404: 💤 Low value

Prefer the truncate utility and a focusable control for the file name.

Two small refinements on the default file row:

  • The inline overflow/textOverflow/whiteSpace styles duplicate Tailwind's truncate (already used for the group name on Line 329).
  • The clickable <a> has no href, so it isn't keyboard-focusable/operable. A <button> (or adding role="button" + tabIndex={0} and key handling) makes selection accessible.
♻️ Optional
-                                    <a
-                                      className="text--1"
-                                      title={f.name}
-                                      style={{ overflow: "hidden", textOverflow: "ellipsis", whiteSpace: "nowrap" }}
-                                      onClick={() =>
-                                        setSelectedFile({
-                                          ...f,
-                                          groupId: group.id,
-                                          groupName: group.name,
-                                        })
-                                      }
-                                    >
-                                      {f.name}
-                                    </a>
+                                    <button
+                                      type="button"
+                                      className="text--1 truncate text-left"
+                                      title={f.name}
+                                      onClick={() =>
+                                        setSelectedFile({
+                                          ...f,
+                                          groupId: group.id,
+                                          groupName: group.name,
+                                        })
+                                      }
+                                    >
+                                      {f.name}
+                                    </button>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/features/resources/ManageResourcesPage.jsx` around lines 390 -
404, Replace the inline truncation styles on the file name link with the
existing truncate utility and make the clickable element keyboard-focusable:
remove overflow/textOverflow/whiteSpace inline styles and use the truncate class
(e.g., className="truncate text--1") for the element that displays f.name, and
change the non-href <a> to a focusable control that calls setSelectedFile
(either convert to a <button type="button"> that invokes setSelectedFile({...f,
groupId: group.id, groupName: group.name}) or keep the element but add
role="button" tabIndex={0} and key handlers for Enter/Space to call
setSelectedFile). Ensure the handler calls the same setSelectedFile signature
used in the diff.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@backend/src/routes/api/files.js`:
- Around line 159-161: The if-check using name.trim().length > 255 returns a
JSON error response that exceeds the project's Prettier print width; run
prettier --write on the file or reflow the call so the res.status(400).json(...)
is wrapped across lines (preserving the same message string) to satisfy
formatting. Locate the snippet containing name.trim().length and the
res.status(400).json({ error: "Name must be 255 characters or fewer" }) call and
apply Prettier formatting (or manually break the .json call into multiple lines)
so the linter/formatter CI passes.
- Around line 155-158: Validate that req.body.name is a string before calling
name.trim(): change the guard around "const { name } = req.body" to check typeof
name !== 'string' || !name.trim() and return res.status(400).json({ error: "Name
is required" }) for non-string or empty values so name.trim() cannot throw;
reference the existing "name" variable, the name.trim() call, and the
res.status(...) return to locate and update the check.
- Around line 152-172: The PATCH handler is missing an authorization check and
allows IDOR; after fetching meta via StoredFile.findById(fileId) (and similarly
in DELETE /:fileId and state-conditionals handlers), verify the caller has
access to the file's scenarioId/groupId using the existing authorization helpers
(e.g., call scenarioAuth or dashboardAuth or the same access-check logic used
elsewhere) using the authenticated principal from req (uid or user on req set by
router.use(auth)); if the check fails return 403 and do not modify/save the
StoredFile, otherwise proceed to set meta.name and save. Ensure the same pattern
is applied to delete and other mutation routes that touch StoredFile.

---

Nitpick comments:
In `@frontend/src/features/resources/ManageResourcesPage.jsx`:
- Around line 390-404: Replace the inline truncation styles on the file name
link with the existing truncate utility and make the clickable element
keyboard-focusable: remove overflow/textOverflow/whiteSpace inline styles and
use the truncate class (e.g., className="truncate text--1") for the element that
displays f.name, and change the non-href <a> to a focusable control that calls
setSelectedFile (either convert to a <button type="button"> that invokes
setSelectedFile({...f, groupId: group.id, groupName: group.name}) or keep the
element but add role="button" tabIndex={0} and key handlers for Enter/Space to
call setSelectedFile). Ensure the handler calls the same setSelectedFile
signature used in the diff.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4d4a9819-71a0-46f8-9d55-0cf623aaa679

📥 Commits

Reviewing files that changed from the base of the PR and between 736892a and 121e598.

📒 Files selected for processing (2)
  • backend/src/routes/api/files.js
  • frontend/src/features/resources/ManageResourcesPage.jsx

Comment on lines +152 to +172
router.patch("/:fileId", async (req, res) => {
try {
const { fileId } = req.params;
const { name } = req.body;
if (!name || !name.trim()) {
return res.status(400).json({ error: "Name is required" });
}
if (name.trim().length > 255) {
return res.status(400).json({ error: "Name must be 255 characters or fewer" });
}
const meta = await StoredFile.findById(fileId);
if (!meta) return res.status(404).json({ error: "File not found" });
meta.name = name.trim();
await meta.save();
const file = meta.toObject();
delete file.gridFsId;
return res.json(file);
} catch (err) {
return res.status(500).json({ error: err.message });
}
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Look for any scenario/file ownership or role checks in the files routes and middleware
rg -nP -C3 '(ownerUid|uploaderUid|scenarioId|authoriz|permission|forbidden|403)' backend/src/routes/api/files.js
fd -t f . backend/src/middleware --exec rg -nP -C3 '(role|owner|permission|scenario|403|forbidden)' {}
# Check how collections routes guard scenario access for comparison
fd -t f 'collections.js' backend/src --exec rg -nP -C3 '(owner|uid|role|permission|403)' {}

Repository: UoaWDCC/VPS

Length of output: 5875


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Show the PATCH /:fileId handler and surrounding code in backend/src/routes/api/files.js
sed -n '1,260p' backend/src/routes/api/files.js | nl -ba | sed -n '120,220p'

# 2) Confirm auth middleware wiring in that file
rg -n --hidden "router\.use\(|auth|scenarioAuth|dashboardAuth|permission|role|403|forbidden|owner|uid" backend/src/routes/api/files.js

# 3) Find any authorization helper used for fileId-based routes (rename/delete) in this route file
rg -n --hidden "patch\\(\"/:fileId\"|delete\\(\"/:fileId\"|StoredFile\\.findById\\(|findById\\(fileId\\)|fileId\\)" backend/src/routes/api/files.js

# 4) Check if any middleware elsewhere enforces ownership for StoredFile routes
#    (search for StoredFile access-control patterns)
fd -t f . backend/src/middleware backend/src/routes/api backend/src/db/daos backend/src/db/models --max-depth 6 2>/dev/null | head -n 2000 >/tmp/filelist.txt
rg -n --hidden "(StoredFile|gridFsId|uploaderUid|scenarioId|groupId|ownerUid|can[A-Z]|hasAccess|authorize|permission|role)" $(cat /tmp/filelist.txt) || true

# 5) Specifically locate scenario/file ownership checks in middleware and list them
rg -n --hidden "(assert.*Scenario|assert.*Group|scenarioAuth|dashboardAuth|access|ownerId)" backend/src/middleware backend/src/routes/api | head -n 200 || true

Repository: UoaWDCC/VPS

Length of output: 98


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Show backend/src/routes/api/files.js around the PATCH handler using cat -n (nl not available)
sed -n '1,260p' backend/src/routes/api/files.js | cat -n | sed -n '120,220p'

# 2) Confirm auth middleware wiring in that file
rg -n --hidden "router\.use\(|auth|scenarioAuth|dashboardAuth|permission|role|403|forbidden|owner|uid" backend/src/routes/api/files.js || true

# 3) Locate the patch/delete routes and where fileId is used
rg -n --hidden "router\.(patch|delete)\(\"/:fileId\"|StoredFile\.findById|findById\(\s*fileId|uploaderUid|scenarioId|groupId" backend/src/routes/api/files.js || true

# 4) Search for any middleware/utility used by files.js for access control
#    (start with scenarioAuth/dashboardAuth and access checks; then also search for StoredFile ownership checks)
fd -t f backend/src/middleware backend/src/routes/api backend/src/db/daos backend/src/db/models --max-depth 6 2>/dev/null > /tmp/vps_files.txt
rg -n --hidden "(scenarioAuth|dashboardAuth|accessList|ownerId|uploaderUid|StoredFile|forbidden|403|permission|role|authorize|hasAccess|assert.*Scenario|assert.*Group)" $(cat /tmp/vps_files.txt) || true

Repository: UoaWDCC/VPS

Length of output: 5413


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show top of backend/src/routes/api/files.js to capture middleware wiring/order
sed -n '1,120p' backend/src/routes/api/files.js | nl -ba | head -n 120

# Inspect firebase auth middleware to see what it guarantees (e.g., req.user fields)
sed -n '1,220p' backend/src/middleware/firebaseAuth.js | cat -n

# Locate StoredFile model to see available fields (scenarioId/groupId/uploaderUid/etc.)
fd -t f 'storedfile*|StoredFile*|file*model*' backend/src/db/models backend/src/db --max-depth 6 2>/dev/null > /tmp/storedfile_models.txt || true
cat /tmp/storedfile_models.txt || true
for f in $(cat /tmp/storedfile_models.txt 2>/dev/null); do echo "---- $f"; sed -n '1,260p' "$f" | cat -n; done

# Find any ownership/permission checks around StoredFile elsewhere
rg -n --hidden "uploaderUid|ownerUid|scenarioId|groupId|access|permission|role|authorize|hasAccess|403|forbidden" backend/src/routes/api backend/src/middleware backend/src/db/daos backend/src/db/models | rg -n "StoredFile|gridFsId|files.js|/files|state-conditionals" || true

# Check if other handlers in this same files.js enforce scenario/group membership
rg -n --hidden "assertGroupInScenario|scenarioId|groupId" backend/src/routes/api/files.js

Repository: UoaWDCC/VPS

Length of output: 98


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Show full middleware wiring at top of files.js (no nl; use cat -n)
sed -n '1,120p' backend/src/routes/api/files.js | cat -n

# 2) Inspect firebase auth middleware
sed -n '1,260p' backend/src/middleware/firebaseAuth.js | cat -n

# 3) Locate and inspect StoredFile model (search by filename first)
fd -t f 'storedfile*|StoredFile*' backend/src/db/models backend/src/db | head -n 50

# If we found candidates, print them; otherwise rg for "name: " fields in StoredFile schema
for f in $(fd -t f 'storedfile*|StoredFile*' backend/src/db/models backend/src/db 2>/dev/null | head -n 20); do
  echo "---- $f"
  sed -n '1,260p' "$f" | cat -n
done

# 4) Find any scenario/role/ownership checks referenced from files.js (and any middleware that looks like it)
rg -n --hidden "(scenarioAuth|dashboardAuth|permission|role|ownerUid|uploaderUid|accessList|hasAccess|authorize|forbidden|403|assert.*Scenario|assert.*Group)" backend/src/routes/api/files.js backend/src/routes/api backend/src/middleware backend/src/db/daos backend/src/db/models || true

# 5) Confirm how this router is mounted (to see if additional middleware wraps it)
rg -n --hidden "routes/api/files|api/files|require\\(.*files|from '.*files|import .*files\\.js" backend/src | head -n 100 || true

Repository: UoaWDCC/VPS

Length of output: 20512


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect backend/src/index.js around where files router is mounted
sed -n '1,120p' backend/src/index.js | cat -n
rg -n "app\.use\(\"/api/files\"|filesRouter" backend/src/index.js

# Inspect remaining part of backend/src/routes/api/files.js for any ownership checks in other file mutations
rg -n "router\.(patch|delete|post|put)\\(\"/:fileId\"|state-conditionals|scenarioAuth|dashboardAuth|assertGroupInScenario|accessList|getAccessList|403|forbidden" backend/src/routes/api/files.js
sed -n '140,280p' backend/src/routes/api/files.js | cat -n

# Inspect delete/state-conditionals handlers quickly for auth/ownership checks usage
sed -n '170,280p' backend/src/routes/api/files.js | cat -n

Repository: UoaWDCC/VPS

Length of output: 10731


Add authorization for file renames (and other file mutations) to prevent IDOR

backend/src/routes/api/files.js only applies router.use(auth) (Firebase token validation) and then PATCH /:fileId does StoredFile.findById(fileId) + updates meta.name without checking the caller’s access to the file’s scenarioId/groupId (same issue in DELETE /:fileId and the state-conditionals routes). Any authenticated user can rename/delete/modify arbitrary files by guessing fileId. Enforce scenario/group access (e.g., via the existing dashboardAuth/scenarioAuth/access checks) before mutating StoredFile.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/routes/api/files.js` around lines 152 - 172, The PATCH handler is
missing an authorization check and allows IDOR; after fetching meta via
StoredFile.findById(fileId) (and similarly in DELETE /:fileId and
state-conditionals handlers), verify the caller has access to the file's
scenarioId/groupId using the existing authorization helpers (e.g., call
scenarioAuth or dashboardAuth or the same access-check logic used elsewhere)
using the authenticated principal from req (uid or user on req set by
router.use(auth)); if the check fails return 403 and do not modify/save the
StoredFile, otherwise proceed to set meta.name and save. Ensure the same pattern
is applied to delete and other mutation routes that touch StoredFile.

Comment on lines +155 to +158
const { name } = req.body;
if (!name || !name.trim()) {
return res.status(400).json({ error: "Name is required" });
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Guard against non-string name to avoid a 500.

A truthy non-string name (e.g. { "name": 123 }) passes the !name check, then name.trim() throws a TypeError, which is caught and returned as a generic 500 instead of a 400.

🛡️ Proposed fix
-    const { name } = req.body;
-    if (!name || !name.trim()) {
+    const { name } = req.body;
+    if (typeof name !== "string" || !name.trim()) {
       return res.status(400).json({ error: "Name is required" });
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/routes/api/files.js` around lines 155 - 158, Validate that
req.body.name is a string before calling name.trim(): change the guard around
"const { name } = req.body" to check typeof name !== 'string' || !name.trim()
and return res.status(400).json({ error: "Name is required" }) for non-string or
empty values so name.trim() cannot throw; reference the existing "name"
variable, the name.trim() call, and the res.status(...) return to locate and
update the check.

Comment thread backend/src/routes/api/files.js

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/src/features/resources/ManageResourcesPage.jsx (1)

163-165: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix stale-state overwrite when rename resolves asynchronously.

Line 163 uses a stale selectedFile snapshot after await, so selecting another file mid-request can incorrectly overwrite that file’s name in Line 164.

Suggested fix
-      if (selectedFile?.id === fileId) {
-        setSelectedFile((prev) => ({ ...prev, name: data.name }));
-      }
+      setSelectedFile((prev) =>
+        prev?.id === fileId ? { ...prev, name: data.name } : prev
+      );
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/features/resources/ManageResourcesPage.jsx` around lines 163 -
165, The code reads the stale selectedFile snapshot before an async rename
completes and may overwrite a newly selected file; replace the external if
(selectedFile?.id === fileId) check with a functional state update that compares
prev.id inside setSelectedFile so the comparison is done against the latest
state, e.g. call setSelectedFile(prev => prev?.id === fileId ? { ...prev, name:
data.name } : prev) so only the currently selected file with matching id is
updated after the async response.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@frontend/src/features/resources/ManageResourcesPage.jsx`:
- Around line 580-582: In the ManageResourcesPage component, update the anchor
element that renders the download link (the <a> with href={downloadUrl}) to
replace the deprecated Tailwind utility class "flex-shrink-0" with the v4
canonical "shrink-0" in its className string so the download button uses the
correct shrinking behavior.

---

Outside diff comments:
In `@frontend/src/features/resources/ManageResourcesPage.jsx`:
- Around line 163-165: The code reads the stale selectedFile snapshot before an
async rename completes and may overwrite a newly selected file; replace the
external if (selectedFile?.id === fileId) check with a functional state update
that compares prev.id inside setSelectedFile so the comparison is done against
the latest state, e.g. call setSelectedFile(prev => prev?.id === fileId ? {
...prev, name: data.name } : prev) so only the currently selected file with
matching id is updated after the async response.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4cee5d3e-a027-4c3a-b0cf-1b1a6d52b3b2

📥 Commits

Reviewing files that changed from the base of the PR and between 121e598 and bab4def.

📒 Files selected for processing (2)
  • backend/src/routes/api/files.js
  • frontend/src/features/resources/ManageResourcesPage.jsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • backend/src/routes/api/files.js

Comment on lines +580 to +582
<a
className="btn btn-phantom btn-xs flex-shrink-0"
href={downloadUrl}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

In Tailwind CSS v4.0.7, is flex-shrink-0still supported, or shouldshrink-0 be used instead?

💡 Result:

In Tailwind CSS v4.0.7, you should use shrink-0 instead of flex-shrink-0. While flex-shrink-0 was previously deprecated and initially removed in the early stages of Tailwind CSS v4 [1], it was subsequently brought back as a legacy utility to maintain backward compatibility [2]. However, shrink-0 is the modern, preferred utility class [3][4]. You are encouraged to use shrink-0 in new code, as flex-shrink-0 is considered a legacy alias [1][2].

Citations:


🌐 Web query:

Tailwind CSS v4 flex-shrink-0alias forshrink-0 documentation

💡 Result:

In Tailwind CSS v4, the documentation for the flex-shrink utilities uses shrink-* as the canonical class names. Specifically, v4 documents the utility as shrink-0 (not flex-shrink-0) to set flex-shrink: 0 [1]. flex-shrink-0 is a v3 legacy class name that maps to the v4 alternative shrink-0 during the v4 migration/upgrade process [2][3].

Citations:


Use Tailwind v4 shrink-0 instead of flex-shrink-0

In frontend/src/features/resources/ManageResourcesPage.jsx (line ~581), change flex-shrink-0 to shrink-0 since Tailwind v4 documents shrink-* as the canonical utility for flex-shrink.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/features/resources/ManageResourcesPage.jsx` around lines 580 -
582, In the ManageResourcesPage component, update the anchor element that
renders the download link (the <a> with href={downloadUrl}) to replace the
deprecated Tailwind utility class "flex-shrink-0" with the v4 canonical
"shrink-0" in its className string so the download button uses the correct
shrinking behavior.

@harbassan harbassan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just this one + the coderabbit stuff which is valid.

Comment on lines +143 to +146
if (!user) {
toast.error("You must be logged in.");
return;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this check is unecessary, given that this is on a protected route, so we can make the assumption that they are logged in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants