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

!!! BUGFIX: Consider entry node when filtering in reference editor #5195

Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\Criteria\PropertyValueLessThan;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\Criteria\PropertyValueLessThanOrEqual;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\Criteria\PropertyValueStartsWith;
use Neos\ContentRepository\Core\Projection\ContentGraph\SearchTerm;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\SearchTerm\SearchTerm;
use Neos\ContentRepository\Core\SharedModel\Id\UuidFactory;
use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId;
use Neos\ContentRepository\Core\SharedModel\Node\PropertyName;
Expand Down Expand Up @@ -178,6 +178,9 @@ public function addNodeTypeCriteria(QueryBuilder $queryBuilder, ExpandedNodeType

public function addSearchTermConstraints(QueryBuilder $queryBuilder, SearchTerm $searchTerm, string $nodeTableAlias = 'n'): void
{
if ($searchTerm->term === '') {
return;
}
$queryBuilder->andWhere('JSON_SEARCH(' . $nodeTableAlias . '.properties, "one", :searchTermPattern, NULL, "$.*.value") IS NOT NULL')->setParameter('searchTermPattern', '%' . $searchTerm->term . '%');
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,14 @@ Feature: Find and count nodes using the findChildNodes and countChildNodes queri
| a1 | Neos.ContentRepository.Testing:Page | a | {"text": "a1"} | {} |
| a2 | Neos.ContentRepository.Testing:Page | a | {"text": "a2"} | {} |
| a2a | Neos.ContentRepository.Testing:SpecialPage | a2 | {"text": "a2a"} | {} |
| a2a1 | Neos.ContentRepository.Testing:Page | a2a | {"text": "a2a1", "stringProperty": "the brown fox", "booleanProperty": true, "integerProperty": 33, "floatProperty": 12.345, "dateProperty": {"__type": "DateTimeImmutable", "value": "1980-12-13"}} | {} |
| a2a1 | Neos.ContentRepository.Testing:Page | a2a | {"text": "a2a1", "stringProperty": "the brown fox likes Äpfel", "booleanProperty": true, "integerProperty": 33, "floatProperty": 12.345, "dateProperty": {"__type": "DateTimeImmutable", "value": "1980-12-13"}} | {} |
Copy link
Member

Choose a reason for hiding this comment

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

😆

| a2a2 | Neos.ContentRepository.Testing:Page | a2a | {"text": "a2a2", "stringProperty": "the red fox", "booleanProperty": false, "integerProperty": 22, "floatProperty": 12.34, "dateProperty": {"__type": "DateTimeImmutable", "value": "1980-12-14"}} | {} |
| a2a3 | Neos.ContentRepository.Testing:Page | a2a | {"text": "a2a3", "stringProperty": "the red Bear", "integerProperty": 19, "dateProperty": {"__type": "DateTimeImmutable", "value": "1980-12-12"}} | {} |
# Note that the node a2a3 is disabled! See below.
| a2a3-disabled | Neos.ContentRepository.Testing:Page | a2a | {"text": "a2a3", "stringProperty": "the red Bear", "integerProperty": 19, "dateProperty": {"__type": "DateTimeImmutable", "value": "1980-12-12"}} | {} |
| a2a4 | Neos.ContentRepository.Testing:Page | a2a | {"text": "a2a4", "stringProperty": "the brown Bear", "integerProperty": 19, "dateProperty": {"__type": "DateTimeImmutable", "value": "1980-12-12"}} | {} |
| a2a5 | Neos.ContentRepository.Testing:Page | a2a | {"text": "a2a5", "stringProperty": "the brown bear", "integerProperty": 19, "dateProperty": {"__type": "DateTimeImmutable", "value": "1980-12-13"}} | {} |
# Note that the node a2a6 must not contain any properties!
| a2a6-empty | Neos.ContentRepository.Testing:Page | a2a | {} | {} |
| b | Neos.ContentRepository.Testing:Page | home | {"text": "b"} | {} |
| b1 | Neos.ContentRepository.Testing:Page | b | {"text": "b1"} | {} |
And the current date and time is "2023-03-16T13:00:00+01:00"
Expand All @@ -89,16 +92,19 @@ Feature: Find and count nodes using the findChildNodes and countChildNodes queri
| nodeAggregateId | "a2a5" |
| propertyValues | {"integerProperty": 20} |
And the command DisableNodeAggregate is executed with payload:
| Key | Value |
| nodeAggregateId | "a2a3" |
| nodeVariantSelectionStrategy | "allVariants" |
| Key | Value |
| nodeAggregateId | "a2a3-disabled" |
| nodeVariantSelectionStrategy | "allVariants" |

Scenario:
# Child nodes without filter
When I execute the findChildNodes query for parent node aggregate id "home" I expect the nodes "terms,contact,a,b" to be returned
When I execute the findChildNodes query for parent node aggregate id "a" I expect the nodes "a1,a2" to be returned
When I execute the findChildNodes query for parent node aggregate id "a1" I expect no nodes to be returned
When I execute the findChildNodes query for parent node aggregate id "a2a" I expect the nodes "a2a1,a2a2,a2a4,a2a5" to be returned
When I execute the findChildNodes query for parent node aggregate id "a2a" I expect the nodes "a2a1,a2a2,a2a4,a2a5,a2a6-empty" to be returned
# Child nodes with empty filter
When I execute the findChildNodes query for parent node aggregate id "a2a" and filter '{"searchTerm": ""}' I expect the nodes "a2a1,a2a2,a2a4,a2a5,a2a6-empty" to be returned
When I execute the findChildNodes query for parent node aggregate id "a2a" and filter '{"nodeTypes": ""}' I expect the nodes "a2a1,a2a2,a2a4,a2a5,a2a6-empty" to be returned

# Child nodes filtered by node type
When I execute the findChildNodes query for parent node aggregate id "home" and filter '{"nodeTypes": "Neos.ContentRepository.Testing:AbstractPage"}' I expect the nodes "terms,contact,a,b" to be returned
Expand All @@ -109,7 +115,15 @@ Feature: Find and count nodes using the findChildNodes and countChildNodes queri

# Child nodes filtered by search term
When I execute the findChildNodes query for parent node aggregate id "a2a" and filter '{"searchTerm": "brown"}' I expect the nodes "a2a1,a2a4,a2a5" to be returned
# The total count highlights that the search is case insensitive
When I execute the findChildNodes query for parent node aggregate id "a2a" and filter '{"searchTerm": "bear", "pagination": {"limit": 3, "offset": 1}}' I expect the nodes "a2a5" to be returned and the total count to be 2
# Case insensitive multibyte search
When I execute the findChildNodes query for parent node aggregate id "a2a" and filter '{"searchTerm": "äpfel"}' I expect the nodes "a2a1" to be returned
# Search for numbers (could be considered useless)
When I execute the findChildNodes query for parent node aggregate id "a2a" and filter '{"searchTerm": "22"}' I expect the nodes "a2a2" to be returned
When I execute the findChildNodes query for parent node aggregate id "a2a" and filter '{"searchTerm": "12.34"}' I expect the nodes "a2a1,a2a2" to be returned
# Search for boolean (could be considered useless)
When I execute the findChildNodes query for parent node aggregate id "a2a" and filter '{"searchTerm": "true"}' I expect the nodes "a2a1" to be returned

# Child nodes paginated
When I execute the findChildNodes query for parent node aggregate id "home" and filter '{"pagination": {"limit": 3}}' I expect the nodes "terms,contact,a" to be returned and the total count to be 4
Expand Down Expand Up @@ -140,10 +154,10 @@ Feature: Find and count nodes using the findChildNodes and countChildNodes queri
When I execute the findChildNodes query for parent node aggregate id "a2a" and filter '{"propertyValue": "stringProperty $=~ \"Bear\""}' I expect the nodes "a2a4,a2a5" to be returned

# Child nodes with custom ordering
When I execute the findChildNodes query for parent node aggregate id "a2a" and filter '{"ordering": [{"type": "propertyName", "field": "text", "direction": "ASCENDING"}]}' I expect the nodes "a2a1,a2a2,a2a4,a2a5" to be returned
When I execute the findChildNodes query for parent node aggregate id "a2a" and filter '{"ordering": [{"type": "propertyName", "field": "text", "direction": "DESCENDING"}]}' I expect the nodes "a2a5,a2a4,a2a2,a2a1" to be returned
When I execute the findChildNodes query for parent node aggregate id "a2a" and filter '{"ordering": [{"type": "propertyName", "field": "non_existing", "direction": "ASCENDING"}]}' I expect the nodes "a2a1,a2a2,a2a4,a2a5" to be returned
When I execute the findChildNodes query for parent node aggregate id "a2a" and filter '{"ordering": [{"type": "propertyName", "field": "booleanProperty", "direction": "ASCENDING"}, {"type": "propertyName", "field": "dateProperty", "direction": "ASCENDING"}]}' I expect the nodes "a2a4,a2a5,a2a2,a2a1" to be returned
When I execute the findChildNodes query for parent node aggregate id "a2a" and filter '{"ordering": [{"type": "timestampField", "field": "CREATED", "direction": "ASCENDING"}]}' I expect the nodes "a2a1,a2a2,a2a4,a2a5" to be returned
When I execute the findChildNodes query for parent node aggregate id "a2a" and filter '{"ordering": [{"type": "timestampField", "field": "LAST_MODIFIED", "direction": "DESCENDING"}]}' I expect the nodes "a2a5,a2a1,a2a2,a2a4" to be returned
When I execute the findChildNodes query for parent node aggregate id "a2a" and filter '{"ordering": [{"type": "propertyName", "field": "text", "direction": "ASCENDING"}]}' I expect the nodes "a2a6-empty,a2a1,a2a2,a2a4,a2a5" to be returned
When I execute the findChildNodes query for parent node aggregate id "a2a" and filter '{"ordering": [{"type": "propertyName", "field": "text", "direction": "DESCENDING"}]}' I expect the nodes "a2a5,a2a4,a2a2,a2a1,a2a6-empty" to be returned
When I execute the findChildNodes query for parent node aggregate id "a2a" and filter '{"ordering": [{"type": "propertyName", "field": "non_existing", "direction": "ASCENDING"}]}' I expect the nodes "a2a1,a2a2,a2a4,a2a5,a2a6-empty" to be returned
When I execute the findChildNodes query for parent node aggregate id "a2a" and filter '{"ordering": [{"type": "propertyName", "field": "booleanProperty", "direction": "ASCENDING"}, {"type": "propertyName", "field": "dateProperty", "direction": "ASCENDING"}]}' I expect the nodes "a2a6-empty,a2a4,a2a5,a2a2,a2a1" to be returned
When I execute the findChildNodes query for parent node aggregate id "a2a" and filter '{"ordering": [{"type": "timestampField", "field": "CREATED", "direction": "ASCENDING"}]}' I expect the nodes "a2a1,a2a2,a2a4,a2a5,a2a6-empty" to be returned
When I execute the findChildNodes query for parent node aggregate id "a2a" and filter '{"ordering": [{"type": "timestampField", "field": "LAST_MODIFIED", "direction": "DESCENDING"}]}' I expect the nodes "a2a5,a2a1,a2a2,a2a4,a2a6-empty" to be returned

Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ private function __construct(
}

/**
* If the value is NULL an unset-property instruction will be returned instead.
Copy link
Member

Choose a reason for hiding this comment

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

unrelated and otherwise not changed file

Copy link
Member Author

Choose a reason for hiding this comment

The 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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\NodeType\NodeTypeCriteria;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\Criteria\PropertyValueCriteriaInterface;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\PropertyValueCriteriaParser;
use Neos\ContentRepository\Core\Projection\ContentGraph\SearchTerm;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\SearchTerm\SearchTerm;
use Neos\ContentRepository\Core\SharedModel\Node\ReferenceName;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\NodeType\NodeTypeCriteria;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\Criteria\PropertyValueCriteriaInterface;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\PropertyValueCriteriaParser;
use Neos\ContentRepository\Core\Projection\ContentGraph\SearchTerm;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\SearchTerm\SearchTerm;

/**
* Immutable filter DTO for {@see ContentSubgraphInterface::countChildNodes()}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\NodeType\NodeTypeCriteria;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\Criteria\PropertyValueCriteriaInterface;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\PropertyValueCriteriaParser;
use Neos\ContentRepository\Core\Projection\ContentGraph\SearchTerm;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\SearchTerm\SearchTerm;

/**
* Immutable filter DTO for {@see ContentSubgraphInterface::countDescendantNodes()}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\NodeType\NodeTypeCriteria;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\Criteria\PropertyValueCriteriaInterface;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\PropertyValueCriteriaParser;
use Neos\ContentRepository\Core\Projection\ContentGraph\SearchTerm;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\SearchTerm\SearchTerm;
use Neos\ContentRepository\Core\SharedModel\Node\ReferenceName;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\Pagination\Pagination;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\Criteria\PropertyValueCriteriaInterface;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\PropertyValueCriteriaParser;
use Neos\ContentRepository\Core\Projection\ContentGraph\SearchTerm;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\SearchTerm\SearchTerm;
use Neos\ContentRepository\Core\SharedModel\Node\ReferenceName;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\Pagination\Pagination;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\Criteria\PropertyValueCriteriaInterface;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\PropertyValueCriteriaParser;
use Neos\ContentRepository\Core\Projection\ContentGraph\SearchTerm;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\SearchTerm\SearchTerm;

/**
* Immutable filter DTO for {@see ContentSubgraphInterface::findChildNodes()}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\Pagination\Pagination;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\Criteria\PropertyValueCriteriaInterface;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\PropertyValueCriteriaParser;
use Neos\ContentRepository\Core\Projection\ContentGraph\SearchTerm;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\SearchTerm\SearchTerm;

/**
* Immutable filter DTO for {@see ContentSubgraphInterface::findDescendantNodes()}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\Pagination\Pagination;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\Criteria\PropertyValueCriteriaInterface;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\PropertyValueCriteriaParser;
use Neos\ContentRepository\Core\Projection\ContentGraph\SearchTerm;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\SearchTerm\SearchTerm;

/**
* Immutable filter DTO for {@see ContentSubgraphInterface::findPrecedingSiblingNodes()}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\Pagination\Pagination;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\Criteria\PropertyValueCriteriaInterface;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\PropertyValueCriteriaParser;
use Neos\ContentRepository\Core\Projection\ContentGraph\SearchTerm;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\SearchTerm\SearchTerm;
use Neos\ContentRepository\Core\SharedModel\Node\ReferenceName;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\Pagination\Pagination;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\Criteria\PropertyValueCriteriaInterface;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\PropertyValue\PropertyValueCriteriaParser;
use Neos\ContentRepository\Core\Projection\ContentGraph\SearchTerm;
use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\SearchTerm\SearchTerm;

/**
* Immutable filter DTO for {@see ContentSubgraphInterface::findSucceedingSiblingNodes()}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,17 @@

declare(strict_types=1);

namespace Neos\ContentRepository\Core\Projection\ContentGraph;
namespace Neos\ContentRepository\Core\Projection\ContentGraph\Filter\SearchTerm;
Copy link
Member

Choose a reason for hiding this comment

The 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.
Can be discussed, but IMO we should not sneak in such changes in unrelated PRs as it makes the review needlessly harder and could confuse debugging

Copy link
Member

Choose a reason for hiding this comment

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

Yep, I would also let it stay where it was.

Copy link
Member

Choose a reason for hiding this comment

The 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

Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

Choose a reason for hiding this comment

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

i moved it to align it to the PropertyValueCriteriaMatcher it resides as well in the PropertyValue namespace and it was only logical to me that the same should apply for SearchTerm things. Id be fine with that non-ideal namespace for now as the SearchTermMatcher is marked as @internal for now and we can later decide on a better location for this and the PropertyValueCriteriaMatcher. Or is there a better suggestion out there? Probably with the introduction of the inmemory content graph well have a better namespace at hand.


/**
* 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
Expand Down
Loading
Loading