!!! BUGFIX: Aggregate changes must know their affected origin dimension space points #5395
Conversation
This file contains 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
the logic is carried out inside `RebaseableCommands::commandMatchesAtLeastOneNode` The reasoning behind this is that asking the commands which node aggregate id is affected _does_ work but the capabilities stop if we want to ask which dimensions are affected. For that we must use the event instead - at least in the future if we want to reintroduce dimension based publishing again. The matching list from commands to node id will also be simplified once we use the `EmbedsNodeAggregateId` interface instead and operate on events.
…stead of `NodeIdsToPublishOrDiscard` ... as a 3rd option the followup commit will just flush the full asset usage workspace!
…arded in the events (for now) Introduced with neos@f2c66e0 - its not important for the graph projection and only used by the asset usage catchup hook which can be adjusted. If we want to re-introduce `$publishedNodes` to the event should be also careful that we calculate them from the _actual_ events so we can know for sure there is no gibberish in the event. Previously for example discarding a non-existing node or a node in a not valid DSP would just save that directly into the event, which is not correct as that did not happen like that. It was merely a bad intention.
Instead of fine granular updating only the usage references on the nodes that were discarded, well throw away everything and then rely on the reapplied events what is kept. This is more consistent to how other parts react to publishing like the graph just forks a new content stream and doesnt care about the old data, the change projection behaves similar to the graph. And for the content cache flusher we had the discussion to use `discardedNodes` but opted against it and do a full flush there as well: neos#5175 (comment) This change allows us to get rid of the `discardedNodes` property (with dimensions) in the `WorkspaceWasPartiallyDiscarded` event.
reasoning: - the `publishedNodeIds` were removed on the event, as we dont need it, and it would rise a discussion if we need to keep dimension space points there too or not: see neos#5386 how i archived it. - instead of now trying to make `WorkspaceWasPartiallyPublished` contain all the information we dont use or need, the idea is to make it dumb. Now its basically just a `WorkspaceWasPublished` and then the other remaining events are applied as if they were carried out by a user. If we need to add more metadata to the partial publish at some point, that would need a new event id say as we woouldnt be able to mess with the `WorkspaceWasPartiallyPublished` once released. So by just combining them into one we have the option open to create a `WorkspaceWasPartiallyPublished` again but differently which wil be breaking too but differently. Its just a new event then, no need to migrate data.
…d WorkspaceWasPartiallyDiscarded
…ross-all-dimensions
…e dimension for now
…teTypeWasChanged` (and rename) This information is required for the change projection and in the future to select changes for one dimension to publish
…e in all occupied dimensions
mhsdesign
changed the title
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.
ideas to reintroduce partial publishing show here #5386 that it could be non breaking but one thing is bad: The change node type and change node name dont know their affected dsp. This makes the change projection even more hacky (see
neos-development-collection/Neos.Neos/Classes/PendingChangesProjection/Change.php
Line 33 in 39b8d1c
Especially in light if we want to change that node type change can be done per dimension.Even root node creations contains all dsp because it is correct:neos-development-collection/Neos.ContentRepository.Core/Classes/Feature/RootNodeCreation/Event/RootNodeAggregateWithNodeWasCreated.php
Lines 46 to 47 in 8c9e6c7
Upgrade instructions
Review instructions
Checklist
FEATURE|TASK|BUGFIX!!!and have upgrade-instructions