-
-
Notifications
You must be signed in to change notification settings - Fork 224
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: Consider entry node when filtering in reference editor #5195
Changes from all commits
9084d69
989e7c5
ef93f94
4715f33
3bbabb3
5a8ab49
5bce349
0190e16
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,8 +38,6 @@ private function __construct( | |
} | ||
|
||
/** | ||
* If the value is NULL an unset-property instruction will be returned instead. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unrelated and otherwise not changed file There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know yeah that the comment is false was my fault and i fixed it ^^ |
||
* | ||
* @param Value $value | ||
*/ | ||
public static function create( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,11 +12,17 @@ | |
|
||
declare(strict_types=1); | ||
|
||
namespace Neos\ContentRepository\Core\Projection\ContentGraph; | ||
namespace Neos\ContentRepository\Core\Projection\ContentGraph\Filter\SearchTerm; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think, we had it in the main ContentGraph folder by intention because it is likely to be reused for other parts. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, I would also let it stay where it was. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With the rest of the changes I came to understand why you moved it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would almost argue the SearchTermMatcher doesn't belong in Projection/ContentGraph/Filters and that strengthens the argument the SearchTerm should be generic as well. Yes the SearchTermMatcher applies to contentgraph projection nodes, BUT I tihnk it's clear that everything else in filters is used in conjunction with a ContentGraph or Subgraph query (query in the loosest term not necessarily DB query), while the SearchTermMatcher applies to a set of nodes that have already been assembled. I think that is its own kind of operation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i moved it to align it to the |
||
|
||
/** | ||
* A search term for use in Filters for the {@see ContentSubgraphInterface} API. | ||
* | ||
* The search is defined the following: | ||
* - test all properties if one contains the term | ||
* - the term is checked case-insensitive | ||
* - an empty term will lead to no filtering | ||
* - FIXME: define the search behaviour across non-string-typed properties | ||
* | ||
* @api DTO for {@see ContentSubgraphInterface} | ||
*/ | ||
final readonly class SearchTerm | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😆