Closed (fixed)
Project:
Search API
Version:
8.x-1.x-dev
Component:
Views integration
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
24 Feb 2016 at 11:41 UTC
Updated:
5 May 2016 at 20:04 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
stborchertAdding argument handler to fields of type "integer", "string" and "entity:taxonomy_term".
Comment #3
drunken monkeyGreat work, thanks!
I didn't have time to test this yet, but it looks solid, at a glance.
However, what we'd definitely need to commit this is, like for filters and fields, some tests to ensure this actually works correctly (and doesn't get broken later).
Are you planning to port/fix the other argument handlers, too, or just those three for the moment?
But thanks regardless, it's a good start in any case.
Comment #4
stborchertSome more argument handlers.
Please note that "decimal" is not working correctly, since Views cannot handle decimal arguments in general. See #2678620: HandlerBase::breakString does not work with decimal values for a patch required to make this work (hopefully this gets committed soon ;) ).
Tests ... uhm, yeah. I think someone should extend the existing tests, who understands how they work ;)
Comment #5
chx commentedEdit; nevermind. Apparently the first indexing wasn't successful. Oh well.
Comment #6
stborchertHm, I tried this right now using the latest development versions of Search API and Search API Solr and it works as expected.
* added the content type as a field to the index (type "string", boost 1.0)
* indexed some content
* added "Content datasource: Type" as contextual filter
* preview items using "article" as argument
* Result (sorry, its german but you see there are results):
edit:
ah ok. glad it works for you, too :)
Comment #7
drunken monkeyOverhauled your patch a bit, but unfortunately didn't have the time to also add tests. I'll probably get to it next week, or maybe tomorrow.
On another note, though, is there a specific reason why you used the D7 pattern of a single Search API base handler class and sub classes for specific types? For filters and fields, we instead used traits to implement Search API-specific adaptions, and then directly extended the corresponding Views handlers for each type. This will likely result in much better functionality going forward, since we'll automatically support almost all features that the Views handlers provide.
However, if it works well enough, we can also commit it like this and change it at any point in the future, really.
Comment #8
drunken monkeyWasn't as easy as I would have liked, but I got the tests implemented and passing now. Please see the attached patch.
If someone could review and verify this looks good, I'll commit it.
The question about the inheritance structure is still valid, though.
Also, why didn't you include the date argument handles in the Views data? Did you test it, is it not working?
Finally, one thing I noticed is that we currently seem to ignore the operator reported back from
breakString()– i.e., whether commas or plusses were used. However, it seems the normal argument handlers provided by Views mostly do the same, so that doesn't seem to be a problem, I guess. Or what would you say? (Would make the code more complicated again, of course, since we couldn't just use(NOT) INanymore.)Comment #9
drunken monkeyAny feedback on this? If no-one wants to complain, I'll commit this soon. But some tests/reviews would still be great.
Comment #10
borisson_I don't claim to understand all the code in the latest patch, but what I understand looks reasonable and the tests agree. RTBC.
Comment #12
drunken monkeyYeah, I guess it's not much easier to review than to write … But thanks for braving it and doing a review!
Committed.
Thanks again, everyone – especially stBorchert, of course!