Add multi-field search support to value filter. #161
Conversation
Codecov Report
@@ Coverage Diff @@
## master #161 +/- ##
=========================================
Coverage 99.68% 99.68%
+ Complexity 140 138 -2
=========================================
Files 11 11
Lines 321 321
=========================================
Hits 320 320
Misses 1 1
Continue to review full report at Codecov.
|
You are right, using |
|
Yes, it only makes sense for the field part. |
|
Given that it would be kind of a bug fix, would you be OK with a "hard break" in terms of removing that feature/option, ie only use |
|
Lets ship it together with #156 as major bump. All good then. |
|
I don't think we are breaking anything as |
|
That would be my stance on it too, just wanted to ask as people have different opinions when it comes to removing not-correctly/non working, but possibly in use behavior. I'll remove it. |
| { | ||
| $articles = TableRegistry::get('Articles'); | ||
| $manager = new Manager($articles); | ||
| $filter = new Value('title', $manager, [ | ||
| 'multiValue' => true, | ||
| 'mode' => 'Or' | ||
| 'valueMode' => 'Or' |
This never worked, as a single field can only ever hold one of the supplied values, hence using `AND` would never match anything. `mode` is now being used for defining the mode for multiple fields instead.
ndm2
force-pushed
the
multi-field-search-support
branch
from
62c6a30
to
3ba9321
Compare
|
@ADmad Do you want to tag a new minor before we merge and release the new major change? |
Basically a copy of #135, but complete with tests.
However, I've just noticed that having a mode for the values doesn't seem to make any sense, given that this filter applies an EQ comparison, applying AND to multiple values would never match, as a field can only hold one of the values in such a comparison.
Am I missing something, or did I just made a mistake when I added this initially, and nobody noticed it?