feat(cl2k): split one Drive folder into the artwork type layout - #470
Merged
Conversation
Routing art to logos/backgrounds/squareart meant finding and pasting three Drive folder IDs by hand. Add a per-row "Split by type" action: it creates (or reuses) the three subfolders under the parent you already entered and replaces that row with one routed row per type, carrying their real IDs. Opt-in per Drive. A drive that should stay flat never presses it and keeps a single row claiming several types, exactly as before — there is no migration and no default change. Purely additive on Drive: nothing is moved, renamed or deleted, and unrelated sibling folders under the parent are left alone. The subfolder names come from a fixed internal map, never user input, so the rclone argument surface is unchanged; the parent id is still validated by _reject_unsafe_id. Only the types the parent row claimed are carried to children, so splitting a logos-only Drive doesn't silently start uploading backgrounds.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a backend endpoint and Drive utility to create or reuse ChangesGoogle Drive type routing
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SplitSubfoldersButton
participant gdrive_type_subfolders
participant ensure_type_subfolders
participant rclone
SplitSubfoldersButton->>gdrive_type_subfolders: request type subfolders
gdrive_type_subfolders->>ensure_type_subfolders: provision or reuse folders
ensure_type_subfolders->>rclone: list or create Drive folders
rclone-->>ensure_type_subfolders: return folder metadata
ensure_type_subfolders-->>gdrive_type_subfolders: return folder metadata
gdrive_type_subfolders-->>SplitSubfoldersButton: return success or error
SplitSubfoldersButton->>SplitSubfoldersButton: replace eligible Drive row
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Owner
Author
|
@coderabbitai review |
|
Owner
Author
|
@coderabbitai review |
|
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Owner
Author
|
@coderabbitai review |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/util/cl2k/gdrive_upload.py`:
- Around line 325-327: Validate complete subfolder records before replacing a
row: in backend/util/cl2k/gdrive_upload.py lines 325-327, update the check
around TYPE_SUBFOLDERS to reject expected folders whose IDs are absent, blank,
or non-string; in tests/test_cl2k_type_subfolders.py lines 133-140, add
regression coverage for blank IDs in logos, backgrounds, and squareart records;
in frontend/src/extensions/cl2k/SaveLocationsFields.jsx lines 415-417, validate
before onSplit that there is exactly one record per expected type and that each
name and folder_id is non-empty.
In `@frontend/src/extensions/cl2k/SaveLocationsFields.jsx`:
- Around line 511-522: The subfolder filter in the row-update callback must not
treat an empty parent.types list as claiming every type. Change the filter
around claimed and image_type to require claimed.includes(s.image_type) so
unclaimed parent rows produce no children and remain unchanged through the
existing empty-children return.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e87f5019-8943-424c-83e1-4a8c670ce975
📒 Files selected for processing (4)
backend/api/cl2k_maker.pybackend/util/cl2k/gdrive_upload.pyfrontend/src/extensions/cl2k/SaveLocationsFields.jsxtests/test_cl2k_type_subfolders.py
…laimed types
Two defects this PR introduced.
The completeness check only asked whether the NAME was present in the id map, so
a record like {"Name": "logos", "ID": ""} passed and produced a routed row with
an empty folder_id. _drive_targets skips a blank id, so that art type would have
stopped uploading with no error anywhere. Blank, absent and non-string ids are
now filtered out before the check, which makes the existing "did not return an
id" guard actually catch them. Parametrised over "", " ", None and a non-string
— all four verified red against the old check.
The split filter treated an empty parent.types as claiming every type, so a row
claiming nothing split into all three routed rows — the opposite of what this
PR's own description promises. Only claimed types now carry to children; an
unclaimed row produces no children and is left untouched.
The button also refuses to rewrite the row unless every art type came back with a
non-empty folder_id, so a partial answer can't be saved.
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
5 tasks
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.
Summary
Routing artwork to
logos/backgrounds/squareartmeant finding and pasting three Drive folder IDs by hand — you had to create the folders in Drive yourself, then dig each ID out of its URL.This adds a per-row "Split by type" action. Point one row at the parent folder you already have, press it, and CHUB creates (or reuses) the three subfolders and replaces that row with one routed row per type, carrying their real IDs.
Related issue
N/A
Type of change
Not everyone wants subfolders — nothing here changes that
Flat remains the default and requires doing nothing. A single row claiming several types keeps putting all of them in one folder, exactly as today. There is no migration, no schema change, and no new config field — the button only rewrites the rows you explicitly press it on.
Two deliberate choices for the flat case:
Why this shape rather than subpath support
The alternative was storing one parent ID plus a "use type subfolders" flag and appending
logos/at upload time (posterflow's model). Rejected:upload_file,move_fileandlist_filesall address a Drive by a bare folder ID, solist_fileswould need recursion orposter_self_heal's live-Drive source would stop seeing artwork entirely — silently re-breaking what #466 just fixed. It would also have meant relaxing_reject_unsafe_id, which exists to stop a crafted value reaching rclone as a flag.Resolving real child IDs up front keeps the runtime plumbing completely untouched and leaves the config self-describing: three rows, three real IDs, and the Poster Healer coverage panel (#467) shows the literal truth.
The subfolder names come from a fixed internal map (
TYPE_SUBFOLDERS), never user input, so the rclone argument surface is unchanged and the parent ID is still validated by_reject_unsafe_id.Testing
ruff check .→ exit 0python -m pytest→ 1581 passed (9 new)npm run lint,npx prettier --check "src/**/*.{js,jsx,css,json,md}"(whole tree),npx stylelint,npm run build→ all exit 0npx vitest run→ 38 passedtests/test_cl2k_type_subfolders.pystubs every rclone call, so nothing touches Drive. Covers: creating all three under an empty parent; reusing existing ones and issuing no mkdir (and only one listing — no pointless second round-trip); creating only the missing one; leaving unrelated sibling folders alone while asserting no destructive verb is issued; mkdir failure; listing failure; Drive returning no ID for a subfolder (fail loudly rather than store a blank ID that would upload to the parent root); an unsafe parent ID rejected before any rclone runs; and a missing OAuth token.Screenshots
Not captured — the button sits beside the existing "Test upload" button on each Drive row, same styling, and reports via the usual toast.
Checklist
backend/util/config.py) — N/A, no schema change; it writes existinggdrive_uploadsfieldscd frontend && npm run build)DAPSreferences introducedmainonlyAll four touched files are develop-only, so the branch-isolation invariant holds.
Summary by CodeRabbit
New Features
logos,backgrounds, andsquareartfolders.Bug Fixes