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

!!! FEATURE: Overhaul NodeCreationHandlerInterface #3519

Merged
merged 32 commits into from
Mar 15, 2024

Conversation

mhsdesign
Copy link
Member

@mhsdesign mhsdesign commented Jun 2, 2023

Neos.Neos adjustment neos/neos-development-collection#4630

Replaces: #3720 for Neos9
Resolves: #3615
Resolves: #3509

For now the api will be internal, Flowpack.NodeTemplates provides a good abstraction for this.

The NodeCreationHandlerInterface

Contract to hook into the process before the node creation command is handled by the content repository
You can add additional steps to the node creation.
For example adding initial properties NodeCreationCommands::withInitialPropertyValues(),
or queuing additional commands like to create a child via NodeCreationCommands::withAdditionalCommands()
The node creation handlers factory can be registered on a NodeType:

Vendor.Site:Content:
  options:
    nodeCreationHandlers:
      myHandler:
        factoryClassName: 'Vendor\Site\MyHandlerFactory'
        position: end

The factory must implement the NodeCreationHandlerFactoryInterface and
return an implementation with this NodeCreationHandlerInterface interface.
The current content-repository or NodeType-manager will be accessible via the content repository.

The new signature of the interface is as follows:

interface NodeCreationHandlerInterface extends ContentRepositoryServiceInterface
{
    /**
     * @param NodeCreationCommands $commands original or previous commands,
     *                                       with the first command being the initial intended node creation
     * @param NodeCreationElements $elements incoming data from the creationDialog
     * @return NodeCreationCommands the enriched node creation commands,
     *                              to be passed to the next handler or run at the end
     */
    public function handle(NodeCreationCommands $commands, NodeCreationElements $elements): NodeCreationCommands;
}

Additionally the interface was relocated to Neos\Neos\Ui\Domain\NodeCreation\NodeCreationHandlerInterface.

The NodeCreationCommands

A collection of commands that describe a node creation from the Neos Ui.
The node creation can be enriched via a node creation handler.
The first command points to the triggered node creation command.
To not contradict the users intend it is ensured that the initial node
creation will be mostly preserved by only allowing to add additional properties.
Additional commands can be also appended, to be run after the initial node creation command.
All commands will be executed blocking.
You can retrieve the subgraph or the parent node (where the first node will be created in) the following way:

$subgraph = $contentRepository->getContentGraph()->getSubgraph(
    $commands->first->contentStreamId,
    $commands->first->originDimensionSpacePoint->toDimensionSpacePoint(),
    VisibilityConstraints::frontend()
);
$parentNode = $subgraph->findNodeById($commands->first->parentNodeAggregateId);

The NodeCreationElements

Holds the deserialized elements of the submitted node creation dialog form

Elements are configured like properties or references in the schema,
but its up to the node-creation-handler if they are handled in any way or just left out.

Elements that are of simple types or objects, will be available according to its type.
For example myImage will be an actual image object instance.

Vendor.Site:Content:
  ui:
    creationDialog:
      elements:
        myString:
          type: string
        myImage:
          type: Neos\Media\Domain\Model\ImageInterface

Elements that refer to nodes are of type references or reference.
They will be available as NodeAggregateIds collection.

Vendor.Site:Content:
  ui:
    creationDialog:
      elements:
        myReferences:
          type: references

The naming references in the element configuration does not refer to the content repository reference edges.
Referring to a node will just denote that an editor will be used capable of returning node ids.
The node ids might be used for setting references but that is up to a node-creation-handler.

To promoted properties / references the same rules apply:

Vendor.Site:Content:
  properties:
    myString:
      type: string
      ui:
        showInCreationDialog: true

@github-actions github-actions bot added the 9.0 label Jun 2, 2023
@mhsdesign
Copy link
Member Author

mhsdesign commented Jun 9, 2023

@bwaidelich suggested, that instead of passing the ContentRepository into the NodeCreationHandlerInterface::handle() we could also consider using the new domain services from the ESCR.

Those domain services can be build via ContentRepositoryRegistry::getService($crId, $serviceFactory) https://github.com/neos/neos-development-collection/blob/f3d553315045c23b9deb7d9b897c84e39d399305/Neos.ContentRepositoryRegistry/Classes/ContentRepositoryRegistry.php#L107

This would in turn mean, that the NodeCreationHandlerInterface would extend the ContentRepositoryServiceInterface (the domain service)

interface NodeCreationHandlerInterface extends ContentRepositoryServiceInterface
{
    public function handle(NodeCreationCommands $commands, array $data): NodeCreationCommands;
}

and we wouldn't register the NodeCreationHandlerInterface directly in the configuration anymore, but its factory (the above mentioned $serviceFactory):

'Neos.Neos:Content':
  options:
    nodeCreationHandlers:
      fooBar:
        factoryClassName: 'Some\NodeCreationHandlerFactoryInterface'

the factory would look like:

interface NodeCreationHandlerFactoryInterface extends ContentRepositoryServiceFactoryInterface
{
    public function build(ContentRepositoryServiceFactoryDependencies $serviceFactoryDependencies): NodeCreationHandlerInterface;
}

and the $serviceFactoryDependencies would hold among others the ContentRepository:
https://github.com/neos/neos-development-collection/blob/01f7f8f6e5d7daa50a8bc68bdc280ddf3ccdb7f9/Neos.ContentRepository.Core/Classes/Factory/ContentRepositoryServiceFactoryDependencies.php#L45

@mhsdesign mhsdesign force-pushed the feature/improveNodeCreationHandlerInterface branch 2 times, most recently from bb21d12 to d7b460b Compare June 11, 2023 13:14
@mhsdesign mhsdesign changed the title WIP: FEATURE: Improve NodeCreationHandlerInterface FEATURE: Improve NodeCreationHandlerInterface Jun 11, 2023
@mhsdesign mhsdesign marked this pull request as ready for review June 11, 2023 13:14
@github-actions github-actions bot added the Feature Label to mark the change as feature label Jun 11, 2023
@mhsdesign mhsdesign force-pushed the feature/improveNodeCreationHandlerInterface branch from 4f9db10 to 00230e3 Compare July 23, 2023 16:32
@mhsdesign mhsdesign linked an issue Oct 17, 2023 that may be closed by this pull request
@mhsdesign mhsdesign force-pushed the feature/improveNodeCreationHandlerInterface branch from 5ca0e4f to 6f8fd2d Compare February 17, 2024 09:31
@neos-bot
Copy link

neos-bot commented Feb 17, 2024

🎥 End-to-End Test Recordings

These videos demonstrate the end-to-end tests for the changes in this pull request.

@mhsdesign mhsdesign force-pushed the feature/improveNodeCreationHandlerInterface branch from db48f62 to f233e63 Compare February 17, 2024 15:09
@mhsdesign mhsdesign changed the title FEATURE: Improve NodeCreationHandlerInterface !!! FEATURE: Overhaul NodeCreationHandler Feb 17, 2024
@mhsdesign mhsdesign force-pushed the feature/improveNodeCreationHandlerInterface branch from f233e63 to 0eb406c Compare February 17, 2024 15:18
@mhsdesign mhsdesign changed the title !!! FEATURE: Overhaul NodeCreationHandler !!! FEATURE: Overhaul NodeCreationHandlerInterface Feb 17, 2024
Comment on lines 55 to 56
private array $propertyLikeValues,
private array $referenceLikeValues,
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
private array $propertyLikeValues,
private array $referenceLikeValues,
private array $elementValues,

array<string, NodeAggregateIds|mixed>

Copy link
Member

Choose a reason for hiding this comment

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

as far as I understand it, this DTO is an object that can be used to prime the creation of nodes with some parameters and we (currently) use it to map the values of the NodeCreation dialog.

As mentioned in our weekly today, I wonder why we need to separate the values already at this point. If we do I would suggest $propertyValues and $referencedNodeAggregateIds but I would personally prefer to keep this as a simple key-value DTO for arbitrary parameters.

That would mean that the code below would instead of

if (!$elements->hasReferenceLike($referenceName)) {
    continue;
}
$setReferencesCommands[] = SetNodeReferences::create(
    $commands->first->contentStreamId,
    $commands->first->nodeAggregateId,
    $commands->first->originDimensionSpacePoint,
      ReferenceName::fromString($referenceName),
      NodeReferencesToWrite::fromNodeAggregateIds($elements->getReferenceLike($referenceName))
);

be s.th. like:

if (!$parameters->has($referenceName)) {
    continue;
}

$setReferencesCommands[] = SetNodeReferences::create(
    $commands->first->contentStreamId,
    $commands->first->nodeAggregateId,
    $commands->first->originDimensionSpacePoint,
      ReferenceName::fromString($referenceName),
      NodeReferencesToWrite::fromNodeAggregateIds(NodeAggregateIds::fromArrray($parameters->get($referenceName)),
);

(potentially with some error handling if the expected format does not match)

And I would even leave out the deserialization at this point..

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes thanks i had a little blockade in my brain as i somehow wanted to port the idea of separating properties and references neos/neos-development-collection#4677 into the elements as well.

I also talked to Wilhelm about that and we found that references might be a little misleading as type here but introducing nodeAggregateIds or anything else is not worth it.
Its just important to note that the type references thingies have at that stage nothing in common with real reference edges. There is just a node editor that allows to "references" a node by its id.

I documented it like

The naming references in the element configuration does not refer to the content repository reference edges.
Referring to a node will just denote that an editor will be used capable of returning node ids.
The node ids might be used for setting references but that is up to a node-creation-handler.

So yes i simplified the vo and now it contains all elements in one blob.

And I would even leave out the deserialisation at this point.

Here i have a strong objection. The data in a format only the neos ui understands. If every handler serialises it differently it leads to bugs like #3719
By encapsulating it we make it an implementation detail and are later-on easier able to change the neos ui serialisation format.

to `PromotedElementsCreationHandler` as in the future it will also handle references.
The naming will also better reflect what it does, and that it only handles promote elements (eg ui.showInCreationDialog)
…operties and references

At this point, they are all elements and some might have the special case of referring to a node which is done via `type: references`
NodeType configuration, which resides in `ui.creationDialog` is part of the Neos.Neos.Ui
…Postprocessor`

... and colocate code with `PromotedElementsCreationHandler`
…sor` to Neos.Ui

Into the `CreationDialogPostprocessor`

NodeType configuration, which resides in `ui.creationDialog` is part of the Neos.Neos.Ui
The declaration `showInCreationDialog` was copied over from Neos.Neos
and the explicit configuration of `creationDialog.elements.title` is obsolete.

The explicit declaration of ui.label: i18n would lead to `Neos.Neos:NodeTypes.Document:creationDialog.title` instead of `Neos.Neos:NodeTypes.Document:properties.title` being used.
That made it hard to set a simple title as the translation was more eager. Resolves #3509

The previously used creationDialog translation is now obsolete (resided in Neos.Neos)

See also #1539
@mhsdesign mhsdesign force-pushed the feature/improveNodeCreationHandlerInterface branch from 669d400 to ea20dc1 Compare March 2, 2024 11:09
As this is a promoted property, it will otherwise appear "after" the explicit elements, which causes the test

> SelectBox opens above in creation dialog if there's not enough space below

to fail, and just makes sense correcting
…ntNodeCreationHandler`

Normally due to the `showInCreationDialog` property the `PromotedElementsCreationHandler` will handle this already.

Also, the references editor with `createNew` will leverage this silently but with #3730 this has to be refactored either way
…420885`

As with #3515 the `nodeName` will be null, so we dont need to use it for generating the uripath
@mhsdesign
Copy link
Member Author

mhsdesign commented Mar 5, 2024

Thank you so much for your kind words and the extensive feedback! ❤️

I could answer a few questions already:

It is even possible to replace the original intent of creating a node of the selected node type (correct me if I'm wrong).

It is technically, but 100% marked as footgun and not to do, as it contradicts the intention, and will crash if the ui cant find the node id afterwards ;)

This is by no means a problem. I rather think that it points us towards a more generic task-based interface, that is not NodeType-centric.

Hmm i think as well that this would be moving to fast and maybe we can implement this for a NodeCreationDialog version 3
For now i would say strong coupling to the content repository (via the cr services #3519 (comment)) is already a good step forward.

…e Neos.Ui

after a discussion with Wilhelm we concluded that this object should rather stay internal and not be part of the api of the Ui
the public api is rather the yaml configuration of FlowPack.NodeTemplates
@mhsdesign
Copy link
Member Author

As discussed with Wilhelm and Christian we will make the node creation handlers internal for now, as there is not much demand for the api and the main consumers are the NeosUi itself or Flowpack.NodeTemplates. Currently its not worth to draft out the perfect api and declare it as such, even though the implementation will unlikely change in the next time (hopefully ^^)

Additionally using the ContentRepositoryServiceInterface as discussed in #3519 (comment) was reverted in favour of a much simpler factory interface which will not leak the hardcore ContentRepositoryServiceFactoryDependencies:

interface NodeCreationHandlerFactoryInterface
{
    public function build(ContentRepository $contentRepository): NodeCreationHandlerInterface;
}

but as its internal now either way, it shouldnt matter too much ^^

@mhsdesign mhsdesign merged commit f9c9a23 into 9.0 Mar 15, 2024
11 checks passed
@mhsdesign mhsdesign deleted the feature/improveNodeCreationHandlerInterface branch March 15, 2024 15:26
samsauter added a commit to samsauter/form-builder that referenced this pull request Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
9.0 Feature Label to mark the change as feature
Projects
Status: Done
4 participants