Repository navigation
Copy placed images to the next or previous cloud page - #272
Open
SunkenInTime wants to merge 33 commits into
Open
SunkenInTime wants to merge 33 commits into
SunkenInTime wants to merge 33 commits into
Conversation
Resolving an unrelated cloud conflict (Use cloud, Keep both) reloads the page on screen, which closed Edit placement and threw away the user's drags. A reload of the same page, the one that keeps undo history, now keeps the edit open on whichever of its lineups survive. A drag stays while the reload has its end where the drag started; an end the cloud moved or removed shows the cloud's version, with a toast. Opening another page still ends the edit without writing it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A reload in the middle of a drag (pointer still down) left the editor's drag based on the old position: the next pointer move put back the drag the reload had just undone, and when the dragged end was gone, the next drag of another end started from its numbers. The editor now lets a drag carry on only if the reload kept its end where the drag began. fromHive sets the reopened edit in one assignment, so nothing watching sees the edit end on the way. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The editor compared a reload against where its own gesture began, which for a second drag of an end is the earlier draft, not the saved position fromHive compares against. The two could disagree: a reload that moved the spot to the first draft dropped the drag, and the editor then put it back. The editor now asks the reopened edit whether it kept the draft. A drag taken back on an end the reload removed no longer outlives its gesture, and dragging another end clears it. The screenshot view's lineup fake takes fromHive's new samePage flag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A reload that took a drag back and a second reload that removed its end, both before the pointer lifted, left the marker set: when a later reload brought the end back, its first drag was ignored. Every reopen now clears a marker whose end is no longer in the edit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Use cloud and Keep both reloaded the whole page to show the cloud's version, which closed Edit placement and threw away its drags even when the conflict was on another lineup. Teammates' changes already apply item by item (#208); conflict resolution now goes the same way, holding what the user has open except what they chose the cloud's version of. A placement edit of a lineup in the conflict ends. This replaces the earlier commits' reopen-after-reload logic in the lineup provider and editor, which go back to #265's versions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When part of Use cloud's discard fails, the redraw that follows puts the user's version back on every refused entity, held or not; holding one kept the cloud's version on screen while the work still waited. A teammate's change held back by a hold the user let go of during the discard waited for a reapply that conflict resolution then cleared. It now resumes once resolution ends. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On a server that merges by field, Save names only the spots the user dragged, measured from the version the edit started from: the user's drag wins over a teammate's move of that spot, and the teammate's move of another spot the edit held back stays. The test that Save waits as a conflict now says it runs on a server that checks whole groups. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolve cloud conflicts item by item so Edit placement stays open
A placed image's id is also its image's id, so copying one to another page of a cloud strategy needs a row of its own. images:copyAsset gives the copy's id a row pointing at the same stored bytes, as a duplicated strategy's images do, taking over a placeholder if the copy's content landed first. Additive: no client calls it yet. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An image's id also names its picture on the server, so a copy, which gets an id of its own, first has the server give that id the original's picture (images:copyAsset, sharing the stored bytes), and only then is sent. An image still uploading, or one the cloud cannot show, is not copied, and a toast says why. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Astra's review: a copied image whose content never reaches the server kept its row, and so its bytes, for good. A day after each copy, the copy now goes through the reclaim check, which removes it only if nothing shows it. A failed upload under the copy's id no longer reads as one still on its way: the copy replaces it. Tests cover both, and a viewer collaborator being refused. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Astra's review of the app side: a copy can wait in a device's outbox for days and still lands needing its picture, so the day-later reclaim of unused copies broke copies delivered late. Copied rows now go as any image does, once content that showed it is purged or with the strategy; a copy whose content never arrives keeps its row until then. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Astra's review: an image placed on this device whose upload had not reached the server yet read as one the cloud doesn't have. The copy now checks the upload queue and says it is still uploading. The test also proves the copy waits for its picture before it is queued. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CodeRabbit's review: an active row under the target's id counted as the copy whatever it showed. It now counts only if it has the source's stored bytes; otherwise the copy is refused. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Astra's review: an image whose own save failed keeps an upload job that never runs, since an upload waits for its image to be saved to send. A copy of it said "still uploading" for good. Only uploads that will go count now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 10, 2026
An image element may name the picture it shows (`assetId`); without one, its own id names it, as before. A copy of an image can then be a new element showing its original's picture, with nothing copied or uploaded, and nothing to wait for while the original is still uploading. References, upload placeholders and cleanup all read the picture through collectAssetIdFromElementPayload, so they follow. For builds from before the field, page and strategy snapshots also list each picture under the id of every image showing it, and a whole write that leaves the field out keeps it: an image never changes picture. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A placed image may name the picture it shows (assetId, Hive field 11); without one, its own id names the picture, as before, so every image, .ica file and backup made so far reads exactly as it did. pictureId is the id the picture is stored, uploaded and found under: rendering, the page transition, screenshots, video export, the upload queue, cloud export, local-to-cloud migration and the cleanup of unused picture files all use it. Item identity (undo, transitions, sync rows, hero tags) stays the image's own id. Nothing makes such images yet; copies will (see the server half). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copies will show their original's picture instead (see the picture id change), so the server never needs to copy a picture within a strategy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A copied image now names its original's picture (assetId), so copying needs no call to the server first, and an image still uploading copies like any other: its copy shows the picture once the upload lands. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Three gaps for builds and rows from before picture ids: - Image payloads go without assetId to clients that don't ask for it (acceptsPictureIds on the snapshots and applyBatch): such a client keeps no assetId, so a copy would read to it as an unsaved change. - images:getAssetUrl answers for an image's own id with the picture it shows, as old builds ask when a picture's address expires. - A picture from before upload statuses is looked up among its own strategy's rows first: copies keep pictures' ids across strategies, so twenty copies could fill a lookup of every strategy's rows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This client keeps PlacedImage.assetId, so it reads snapshots and op results with it (acceptsPictureIds); older clients get them without. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… t3/cloud-image-copy-rework
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ge-picture-id-client
… t3/cloud-image-copy-rework
This branch has not been deployed
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
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.
After #267, "Copy to next page" and "Copy to previous page" work on cloud strategies for everything except placed images. This adds images.
What changes
assetId, from Let an image show another image's picture #276/Find an image's picture by its picture id #277). Nothing is copied or uploaded, and the server needs no call first. A copy of a copy names the first original's picture.Changed since the first version
The first version asked the server to copy the picture under the copy's id first (#271), and refused to copy an image that was still uploading. Dara pointed out that the waiting was the wrong model. #271 is closed. This branch takes its server code back out and builds on #276/#277 instead.
Ground truth
Web build in Edge against the dev deployment, which runs this stack, signed in as a test account on
localhost:8765:assetIdnaming its picture, and no new picture rows. The next page showed it.main's client code, standing in for an old build, the copies showed their pictures.Tests
strategy_page_session_provider_test, "an image":The full suite passes on the combined branch (#274).
Merge order: #276, #277, then this (it contains #267's changes and both of those).
ci: test analyze web
🤖 Generated with Claude Code