Repository navigation
[Bug] Publish a version in place instead of creating a new version - #2162
ValeriaMaltseva wants to merge 4 commits into
Conversation
Publishing a version restored the version data into the element and saved it, which always created a new versions row: the published version stayed an unpublished entry and the endpoint returned whatever version happened to be the newest one. The publishers now write the published state into the version itself (core `existingVersion` save parameter) and the endpoint returns the id of that version. Refs pimcore/service-operations#1628 Co-Authored-By: Claude <noreply@anthropic.com>
The `existingVersion` save parameter the publishers rely on ships with pimcore/pimcore 2026.3.4 (pimcore/pimcore#19620). Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The necessary core dependency bump conflicts with the repository’s manifest restriction and needs explicit maintainer resolution before merging.
1 open issue
What changed in this PR
This PR fixes version publishing in Studio by updating the selected version in place rather than creating a duplicate history entry.
Changes:
- Pass the selected
Versionthrough all three publisher interfaces tosave(['existingVersion' => $version]). - Return that version’s ID instead of querying the newest version.
- Add publisher and service regression tests and raise the minimum core dependency.
The fix targets the underlying save behavior at the publisher/service boundary (DataObjectVersionPublisher.php:40, VersionService.php:133–139). All three publisher contracts are updated and remain @internal. Tests cover argument forwarding and the returned ID, but were not run during this review; actual persistence depends on the still-open companion core PR pimcore/pimcore#19620. Interface documentation is updated; no bundle changelog is included.
| File | Description |
|---|---|
| tests/Unit/Version/Service/VersionServiceTest.php | Tests selected-version forwarding, returned ID, and unsupported publishers. |
| tests/Unit/Version/Publisher/DocumentVersionPublisherTest.php | Tests document publishing and missing-document handling. |
| tests/Unit/Version/Publisher/DataObjectVersionPublisherTest.php | Tests object publishing, folder exclusion, and save failures. |
| tests/Unit/Version/Publisher/AssetVersionPublisherTest.php | Tests asset publishing and save failures. |
| src/Version/Service/VersionService.php | Passes the selected version and returns its ID. |
| src/Version/Publisher/DocumentVersionPublisherInterface.php | Adds the selected version to the document publisher contract. |
| src/Version/Publisher/DocumentVersionPublisher.php | Saves into the selected document version. |
| src/Version/Publisher/DataObjectVersionPublisherInterface.php | Adds the selected version to the object publisher contract. |
| src/Version/Publisher/DataObjectVersionPublisher.php | Saves into the selected object version. |
| src/Version/Publisher/AssetVersionPublisherInterface.php | Adds the selected version to the asset publisher contract. |
| src/Version/Publisher/AssetVersionPublisher.php | Saves into the selected asset version. |
| composer.json | Raises the core requirement to ^2026.3.4. |
Review effort: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Publishing can overwrite another user's autosave or report a draft as published without updating it, and version history may display newly published drafts out of order.
3 open issues
1 resolved since last review
Review effort: Balanced
The version is published in place, so publishing another user's auto-save version would take it over (owner and content), as deleting it is not allowed either. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Note
This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.
There was a problem hiding this comment.
🟡 Changes recommended
Two critical issues remain: a failed publish can overwrite the original draft despite a database rollback, and concurrent autosaves can overwrite newer edits or revert a published version to an autosave.
2 open issues
1 resolved since last review
Review effort: Balanced
Auto-save versions are rewritten by the editor of their user at any time, writing into one would race with that (pimcore/pimcore#19620 rejects it). They are published as a new version, as before, which also makes rejecting the auto-save version of another user unnecessary: nothing is taken over. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Changes in this pull request
Resolves pimcore/service-operations#1628 (Jira PEES-1896, "Problem 2")
"Publish" on a version restored the version data into the element and saved it, which always created a new
versionsrow: the published version stayed an unpublished entry, a copy of its content became the live version, and the endpoint returned whatever version happened to be the newest one.save(['existingVersion' => $version]), see [Bug] Publish a version in place instead of creating a new version pimcore#19620), so the history shows that exactly this version went live.VersionService::publishVersion()returns the id of the published version instead of looking up the "last version" (which, after an in-place publish, could be a newer, unrelated row).pimcore/pimcoreis required at^2026.3.4, the release that ships the save parameter.Companion PR (core, scheduled
publish-versiontask + the save parameter): pimcore/pimcore#19620Additional info
Reproduce: open a data object, "Save draft", then in the Versions tab click "Publish" on that draft version. Before: a new published version appears and the draft stays an unpublished entry. After: the draft version itself is marked as published, no version is added.
Tests: unit tests for the three publishers and
VersionService::publishVersion()(tests/Unit/Version/Publisher,tests/Unit/Version/Service), run locally: 9 tests green. PHPStan is clean for the changed files; the remaining errors insrc/Versionlocally are the pre-existing coauthor accessors missing from the container's older core package.🤖 Generated with Claude Code