Skip to content

fix(gallery): validate album rename and image_ids input (#6822) - #6823

Merged
alteixeira20 merged 1 commit into
odysseus-dev:devfrom
dxrpl:fix/6822-gallery-album-input
Oct 31, 2026
Merged

alteixeira20 merged 1 commit into
odysseus-dev:devfrom
dxrpl:fix/6822-gallery-album-input

Conversation

@dxrpl

@dxrpl dxrpl commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

Summary

PUT /api/gallery/albums/{id} stored name and description without checks, so a blank or whitespace name was accepted (create already rejects it), a number was stored as text, and a list or object made SQLAlchemy fail with a 500. POST .../add and .../remove passed image_ids straight to GalleryImage.id.in_(...), so anything but a list of ids (a string, number, null, objects, or a non-object body) was a 500. The update now requires a non-empty string name (trimmed, same message as create) and a string description; add/remove require image_ids to be a list of strings. Everything else answers 400. Valid requests behave as before.

Target branch

  • This PR targets the correct integration branch: lab in the private maintainer-preview repository, or dev in the public repository. main remains release-curated.

Linked Issue

Fixes #6822

Type of Change

  • Bug fix (non-breaking — fixes a confirmed issue)
  • New feature (non-breaking — adds new behaviour)
  • Breaking change (changes or removes existing behaviour)
  • Refactor / cleanup (behaviour unchanged)
  • Documentation only
  • CI / tooling / configuration

Checklist

  • I searched open issues and open PRs — this is not a duplicate.
  • This PR targets the correct integration branch (lab in maintainer-preview; dev in the public repository)
  • My changes are limited to the scope described above — no unrelated refactors or whitespace changes mixed in.
  • I actually ran the app (docker compose up or uvicorn app:app) and verified the change works end-to-end. Type-checks and unit tests are not enough.
  • I did not run the app/runtime validation and stated that gap in How to Test. Leave this unchecked when the app-run box above is checked.

How to Test

I ran the branch natively (uvicorn app:app, scratch data dir), logged in as admin and used curl. PUT with {"name":" "}, {"name":123}, {"name":["x"]}, {"description":{"a":1}} and a [1] body gave 400 (on dev: 200, 200, 500, 500, 500). PUT with {"name":" Renamed ","description":"d"} gave 200 and the album list showed Renamed. add with "abc" and null as image_ids gave 400 (500 on dev), [] and ["x"] gave 200. I did not use the gallery UI or move real images into an album. The new tests/test_gallery_album_input.py fails on dev (18 failures) and passes with the change.

To check by hand:

  1. Start the app, log in, create an album (POST /api/gallery/albums with {"name":"Keep"}).
  2. curl -b cj -X PUT -H 'content-type: application/json' -d '{"name":" "}' localhost:7000/api/gallery/albums/<id>
  3. It should return 400 Album name required; on dev it returns {"ok":true} and the album ends up with a blank name.

Visual / UI changes — REQUIRED if you touched anything that renders

No visual change: the edit only fixes variable scoping so the existing timeout message is reachable. No markup, CSS or styling was touched.

  • Screenshot or short clip: not applicable, see above.
  • I am not an LLM agent submitting a bulk PR.

Screenshots / clips

The reply bubble after the 120 s timeout, with the fix applied:

timeout-bubble

@github-actions github-actions Bot added the ready for review Description complete — ready for maintainer review label Oct 24, 2026
@dxrpl
dxrpl force-pushed the fix/6822-gallery-album-input branch from 47949b9 to 29ee240 Compare October 28, 2026 17:24

@alteixeira20 alteixeira20 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM. Rejects non-string or blank names and descriptions on album rename and validates image_ids lists on add and remove. Regression tests pass on the PR head and integrated tree.

@alteixeira20
alteixeira20 merged commit 7606149 into odysseus-dev:dev Oct 31, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review Description complete — ready for maintainer review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gallery albums: renaming to a blank name is accepted, and bad image_ids / name types return 500

2 participants