neos-development-collection icon indicating copy to clipboard operation
neos-development-collection copied to clipboard

!!! FEATURE: References on creation and Copy

Open kitsunet opened this issue 1 year ago • 6 comments

This reworks references so that multiple reference properties can be set via a single command and also references can be attached to CreateNodeAggregateWithNode which is also used for copying nodes.

The serialization format is adapted to allow multiple reference properties, which also affects all behat tests with references.

To migrate existing events run:

./flow migrateevents:migratesetreferencestomultinameformat

Marked breaking as it requires an event migration.

This requires https://github.com/neos/neos-ui/pull/3844 for the UI

A behat test to show reference copies work was added.

kitsunet avatar Jun 17 '24 16:06 kitsunet

Do not merge yet, probably want to refactor some of the code in the DTOs. I think we should make the "collection of references for one reference property" an explicit DTO, as the code like this feels a bit too messy for my taste...

kitsunet avatar Jun 17 '24 16:06 kitsunet

On the upside the somewhat weird and unfinished reference snapshots can be removed with this.

kitsunet avatar Jun 17 '24 16:06 kitsunet

And forgot to adapt legacy migration tests (which rely on specific event payloads, see below, before I adjust those we should decide which way to go with b/c)...

kitsunet avatar Jun 17 '24 16:06 kitsunet

oh and I should mention that at the moment this is not forgiving to the existing events. We could do this in the respective fromArray constructors of the DTOs and just "upcast" from the old structure on the fly without much problem, or add a migration for the events.

kitsunet avatar Jun 17 '24 16:06 kitsunet

Screenshot 2024-06-18 at 08 47 52

I think this would be much nicer code wise (obviously duplicated for Serialized...), bit annoying to use 3 objects for this, but I think it will make the code much better to reason with and empty is then really just the new "middle DTO" with createEmpty()

kitsunet avatar Jun 18 '24 06:06 kitsunet

I remember we discussed this some time ago so i was wondering where it goes from now? I think we discussed for an alternative way than to use a "hacky" NodeReferenceNameToEmpty?

mhsdesign avatar Oct 12 '24 06:10 mhsdesign

and the test has to be adjusted as it fails now earlier:

001 Scenario: Node reference cannot hold multiple targets to the same node                                          # Features/05-NodeReferencing/01-SetNodeReferences_ConstraintChecks.feature:190
      Then the last command should have thrown an exception of type "InvalidArgumentException" with code 1700150910 # Features/05-NodeReferencing/01-SetNodeReferences_ConstraintChecks.feature:195
        Expected exception code 1700150910, got exception code 1730365958 instead; Message: Duplicate entry in references to write. Target "anthony-destinode" already exists in collection.
        Failed asserting that 1730365958 is identical to 1700150910.

mhsdesign avatar Oct 31 '24 16:10 mhsdesign

Okay i fixed the tests again and added some documentation ... i also got rid of the duplicate NodeReferencesToWrite::fromReferences and its now just create and also the fromNameAnd parts are out for the NodeReferencesForName as well its guaranteed to stick to a name and thus makes it a little shorter and hopefully easy to use.

SetNodeReferences::create(
    $node->workspaceName,
    $node->aggregateId,
    $node->originDimensionSpacePoint,
    NodeReferencesToWrite::create(
        // sets the 3 nodes as references
        NodeReferencesForName::fromTargets(
            ReferenceName::fromString('first-reference'),
            NodeAggregateIds::fromArray(['node-aggregate-id-1', 'node-aggregate-id-2', 'node-aggregate-id-3'])
        ),
        // unset previously set references (as we do NOT merge the values)
        NodeReferencesForName::createEmpty(
            ReferenceName::fromString('second-reference')
        ),
        // create a simple node reference and additionally a reference with an additional property
        NodeReferencesForName::fromReferences(
            ReferenceName::fromString('third-reference'),
            [
                NodeReferenceToWrite::fromTarget(NodeAggregateId::fromString('node-aggregate-id-6')),
                NodeReferenceToWrite::fromTargetAndProperties(NodeAggregateId::fromString('node-aggregate-id-5'), PropertyValuesToWrite::fromArray([
                    'propertyOnReference' => 'Hi im living on the edge ;)'
                ])),
            ])
    )
)

mhsdesign avatar Oct 31 '24 20:10 mhsdesign