-
Notifications
You must be signed in to change notification settings - Fork 14
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
BUGFIX: Refactor node tree using Query DTOs #63
Merged
Merged
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
…lter and perform filtering by node type on the server-side
…nOfTreeNode and move the whole thing to a separate query
grebaldi
force-pushed
the
feature/tree-cqrs
branch
from
May 15, 2024 15:02
7519617
to
9771a80
Compare
grebaldi
changed the title
WIP: Refactor node tree using Query DTOs
BUGFIX: Refactor node tree using Query DTOs
May 15, 2024
Thanks a lot for your effort. Do we want this as a minor eg 1.5 or as a full major 2.0? |
As a bugfix, this would technically be a patch release. There's no breaking change API-wise in here. It's just a lot of code. If this feels uncomfortable, I'd go for a minor release. |
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.
fixes: #54
TLDR; This PR replaces the mechanism through which Sitegeist.Archaeopteryx builds its own node tree with a custom-made backend API. The new approach has allowed to move a lot of responsibility from the client application to the server, which is much better suited to answer domain-specific questions. @nezaniel and I have designed this with Neos.Neos.Ui in mind. If the idea works for Archaeopteryx, we may be able to move the approach into the Neos UI as well.
General considerations
The entire approach is pretty much modeled after the ideas outlined in neos/neos-ui#3331. However, the implementation in this PR is more Neos/Flow-idiomatic and requires less reflection. The overall idea is to move responsibility away from the client application to the server by creating a thin API layer, that maps directly to UI concepts.
This PR introduces multiple query data transfer objects (DTOs) with co-located query handlers. Each query has its own namespace beneath
Application
. Together they form the newly introduced Application layer.*Query
DTOs arefinal readonly
classes with afromArray
static factory method. Each query DTO is accompanied by a*QueryHandler
class, that is#[Flow\Scope('singleton')]
and performs the actual operation. Each query DTO is also accompanied by a*QueryResult
DTO class, that is alsofinal readonly
and additionally implements\JsonSerializable
. The*QueryResult
class is always the return type of the respective*QueryHandler->handle
method.There are some supporting DTO classes (like
TreeNode
orBreadcrumb
). Those classes are usually co-located with the*QueryResult
- or*Query
DTO that are using them. In case multiple queries or query results need the same DTO, those DTOs have been moved into theApplication\Shared
namespace.Each query has its own
Controller
. For this, a little bit of framework code has been introduced to reduce boilerplate. But in principle, the controllers do nothing more but to implement Flow'sControllerInterface
and run their logic directly inprocessRequest
(so: no actions or any other MVC concepts). All query controllers have very limited responsibility: Deserialize the query, invoke the query handler, serialize and return the query result. Everything else is responsibility of the query handler.Also, all query controllers are folded into the Neos backend authentication.
Neos.Neos:AbstractEditor
is given access by default.The Queries
Query
GetTree
GetTree
is used to render the node treeworkspaceName
string
dimensionValues
array
startingPoint
NodePath
startingPoint
in the editor configuration)loadingDepth
int
loadingDepth
in the editor configuration)baseNodeTypeFilter
string
baseNodeType
in the editor configuration)linkableNodeTypes
NodeTypeNames
allowedNodeTypes
in the editor configuration)narrowNodeTypeFilter
string
searchTerm
string
selectedNodeId
(optional)NodeAggregateIdentifier
loadingDepth
would allow, it's entire branch will be build from the bottom-upThe
GetTree
query returns a hierarchy ofTreeNode
DTOs, which contain all the information the UI needs to render a tree. The tree is rendered with the node atstartingPoint
at its root.By default, the depth of the rendered tree will be limited by
loadingDepth
. Descendants that are excluded thusly, can later be loaded using theGetChildrenForTreeNode
query.If
searchTerm
is set, the query will perform a search instead and return a tree containing every node that matches thesearchTerm
plus its ancestors (untilstartingPoint
).The query is informed by the editor configuration, by what is currently set in the Neos UI document tree (initially) and by user input.
If a node is selected whose depth in the tree exceeds
loadingDepth
and the Archaeopteryx dialog is opened after that, the tree will be loaded down to the depth of the selected node, including all ancestors and (ancestor-)siblings up tostartingPoint
.EXAMPLE
Query:
Result:
baseNodeTypeFilter
vs.narrowNodeTypeFilter
vs.allowedNodeTypes (???)
I found it a bit confusing to differentiate between all those filters in relation to the tree.
There is a thing called
baseNodeTypeFilter
, which is configured via the editor configuration (analogous to how that's done with the document and content trees in Neos UI). The configuration key just saysbaseNodeType
, but it is actually a filter string. This filter determines which node types can show up in the tree. So, any node type that doesn't match this filter will never show up in the tree, regardless of any other configuration or user selection.Then there's a thing, that I called
narrowNodeTypeFilter
in this PR. This is the user-selected node type filter. If a user selects a filter option, theGetTreeQuery
will actually perform a search and return all nodes that match the node type filter plus all of their ancestors.There's also the configuration option
allowedNodeTypes
. This has nothing to do with filters at all. This option determines which node types are allowed to be linked (introduced in #11).DTO
TreeNode
The various states a tree node can be in
nodeAggregateIdentifier
icon
label
nodeTypeLabel
title
attribute of the tree node icon)isMatchedByFilter
searchTerm
and/ornarrowNodeTypeFilter
) given in the query. If this isfalse
the tree node will appear with reduced opacityisLinkable
allowedNodeTypes
configuration)isDisabled
true
, a little red icon will mark this tree node as disabledisHiddenInMenu
true
the tree node will appear with reduced opacityhasScheduledDisabledState
hiddenBeforeDateTime
andhiddenAfterDateTime
). If this istrue
, a little clock icon will mark this tree node as scheduled to be disabledhasUnloadedChildren
loadingDepth
children
loadingDepth
, this array will be empty andhasUnloadedChildren
will be trueQuery
GetChildrenForTreeNode
Peek.2024-05-15.15-04.GetChildrenForTreeNode.mp4
Loading children that have been excluded due to
loadingDepth
workspaceName
dimensionValues
treeNodeId
nodeTypeFilter
linkableNodeTypes
allowedNodeTypes
in the editor configuration)This query loads a single level of tree nodes for the parent identified by
treeNodeId
. This is used to load children that have been initially excluded due toloadingDepth
in theGetTree
query. For plausible results, theworkspaceName
,dimensionValues
andlinkableNodeTypes
properties should be the same as with the initialGetTree
query.EXAMPLE
Query:
Result:
Query
GetNodeSummary
The node summary used for Archaeopteryx' main dialog
workspaceName
dimensionValues
nodeId
When a link is selected, Archaeopteryx displays a panel in its main dialog that summarizes the link. The
GetNodeSummary
query informs this panel.EXAMPLE
Query:
Result:
Query
GetNodeTypeFilterOptions
The node type filter options as displayed in the select box in Archaeopteryx' main dialog
baseNodeTypeFilter
nodeTypePreset
s are configured, the resulting options will consist of all non-abstract node types that match this filter stringThe result of the
GetNodeTypeFilterOptions
query informs the node type filter select box in Archaeopteryx' main dialog.Analogous to Neos.Neos.Ui, it will use
nodeTreePreset
s to calculate the filter options, if any are configured. If not, the filter options will simply be a list of all non-abstract node types that match the givenbaseNodeTypeFilter
.EXAMPLE
Query:
Result: