FEATURE: Support search by property & exact value in NodeDataRepository #1
Conversation
Currently it is only possible to search through the properties by
giving a string that matches for any key or value found in the
jsonified properties field.
With this change, the term can also be an array to match exactly on a
given key / value combination.
The search term could be given as `array('key' => 'value').
Change-Id: Ie58b66503db561e6cd1b7b2de5f2c1cb80fe03d7
Resolves: NEOS-1460
Releases: master
Change term check to allow arrays. Change-Id: Iddc4d6686cdb8ab1941609ab9301bd00800df184 Resolves: NEOS-1460 Releases: master
|
Hi Daniel, pretty nice feature! I'd really like to have a functional test for this because we had a lot of trouble with database (text) search and MySQL / PostgreSQL / Sqlite compatibility. It would also be much easier to cover different cases in the future. One other thing: I think we should convert the given term the same way we encode the JSON value in a node. That way we could search for a Node reference or non-string values too. |
Fixes an issue in NodeView which causes an exception on rare occasions when node type labels are empty. The original error message was:: Uncaught exception #1: Warning: strpos(): Empty needle in TYPO3_Neos_Service_View_NodeView.php line 273 Change-Id: Iedd0b4a2db3a0c3764b7377a8f28c61ef0dcf712 Releases: master, 1.2 Original-Commit-Hash: 8e5fe3448f4d647d0f80fc9e62ab41014cb7b560
Tests the simple property search as well as the search for a specific property.
|
@hlubek - I added a test for the simple search in the property field and the search for a specific named property. |
|
@hlubek ping ;-) |
|
TL;DR: 👎 Ok, first of all – what about closing this PR and creating a new one? It seems the Travis reporting is broken for "old" PRs like this one… The change looks good, and works most of the time on MySQL and (probably) never on PostgreSQL. Why? Because the search query does not specify an ordering so the The question now is: Is the order of returned node important and should be made consistent? Or should the test be adjusted to check for the expected results even when the order changes? |
|
Here is an "example failure" on MySQL: https://travis-ci.org/kdambekalns/neos-development-collection/jobs/89391932 |
|
Closed/reopened to fix test run @kdambekalns: The ordering issue is unrelated to the actual change though, but made visible because of the test.. Adding order is potentially a breaking change, although a improvement. Also what ordering even makes sense in this case? A "semi random" one using the identifiers or the creation date or last modified date? Because of that I'd suggest to make the test order independent and tackle the ordering in a separate change. |
|
Thinking how we use this, order related to paths could make sense maybe |
|
@kitsunet: been down that road, it's a dead end.. ordering in SQL by nested path and sort index correctly is a nightmare Which made me remember https://jira.neos.io/browse/NEOS-989 where I tried it. That's actually the issue exposed by the tests. |
|
right, both won't work, I am aware. I just thought path plain and simple... Although change date could be an interesting choice thinking about it. DESC then. |
[TASK] Merge changes from 2.0
|
good work @daniellienert, lets get this in.. ping @hlubek @kdambekalns @kitsunet |
|
can't have a failed first pull request 😄 |
TASK: Adjust Travis build config to add PostgreSQL 9.5
Refactoring and fixing of various issues and merge conflicts
TASK: Refactor icon name migration to mapper
DOCS: Remove PackageFactory Namspace
TASK: Add PHP 7.2 to the tested plattforms
Introduce Variant Type "same" in API
TASK: Raise dependency doctrine/dbal to 2.7.0
…back-suggestions BUGFIX: Fix behavior of "discard selected changes" button in workspace module
Previously in php 7.4 this `Neos\Fusion\Exception\MissingFusionObjectException` was thrown > No Fusion object found in path "" but with php 8 this `ValueError` is thrown which is unexpected > strrpos(): Argument #3 ($offset) must be contained in argument #1 ($haystack) This change takes care of throwing an explicit `Neos\Fusion\Exception` instead: > Fusion path cannot be empty.
…in tests > Type error: Neos\ContentRepository\Core\Tests\Behavior\Fixtures\PostalAddress::__construct(): Argument neos#1 ($streetAddress) must be of type string, null given
Currently it is only possible to search through the properties by
giving a string that matches for any key or value found in the
jsonified properties field.
With this change, the term can also be an array to match exactly on a
given key / value combination.
The search term could be given as
['key' => 'value'].NEOS-1460 #close