Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

!!! 4191 - Adjust and test node type change behavior #5320

Merged
merged 13 commits into from
Oct 24, 2024
Merged

Conversation

nezaniel
Copy link
Member

@nezaniel nezaniel commented Oct 23, 2024

Resolves #4191

This introduces a more comprehensive test suite for ChangeNodeType and adds the following features

  • default values are now added if the property is yet missing
  • !!! obsolete properties are now removed
  • missing tethered children are now added
  • disallowed children are now properly removed (in DELETE strategy)
  • already present tethered children will be adjusted in type if necessary
  • all the above is now applied recursively for nested tethered declarations

This also fixes a bug in NodeRemoval where only one child node was removed per removed parent

Additionally, as reported in #4191, the change projection is now able to deal with aggregate scoped changes (NodeAggregateTypeWasChanged, NodeAggregateNameWasChanged)

Upgrade instructions

Requires cr:setup

Review instructions

See change list

Checklist

  • Code follows the PSR-12 coding style
  • Tests have been created, run and adjusted as needed
  • The PR is created against the 9.0 branch
  • Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
  • Reviewer - The first section explains the change briefly for change-logs
  • Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

Bernhard Schmitt added 8 commits September 13, 2024 21:21
This
- changes node types also recursively for tethered nodes
- adds missing tethered nodes after node type change, also recursively
- thoroughly cleans up disallowed children, also recursively
- adds default values after node type change
- removes obsolete property values after node type change
@github-actions github-actions bot added the 9.0 label Oct 23, 2024
dlubitz and others added 2 commits October 24, 2024 15:08
Co-authored-by: Christian Müller <christian@flownative.com>
@nezaniel nezaniel merged commit 6597802 into 9.0 Oct 24, 2024
10 checks passed
@mhsdesign mhsdesign deleted the 4191-nodeTypeChange branch October 27, 2024 18:10
Comment on lines +41 to +42
// null for aggregate scoped changes (e.g. NodeAggregateNameWasChanged, NodeAggregateTypeWasChanged)
public ?OriginDimensionSpacePoint $originDimensionSpacePoint,
Copy link
Member

Choose a reason for hiding this comment

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

todo the neos ui must learn to handle this case: https://github.com/neos/neos-ui/blob/13520defa0b00eee9cef6073fde11ccbfa3346d4/Classes/ContentRepository/Service/WorkspaceService.php#L83

i assume we need to mark all occupied nodes of the noteaggregate as changes then? And if removalAttachmentPoint is set is probably gurantted that originDimensionSpacePoint is set

Copy link
Member

@mhsdesign mhsdesign Nov 1, 2024

Choose a reason for hiding this comment

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

okay it did see neos/neos-ui#3878 but one place was overseen :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: Changing a NodeType type doesn't get marked as "changed"
4 participants