Closed (fixed)
Project:
Search API (8.x)
Component:
User interface
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
2 May 2014 at 10:13 UTC
Updated:
20 Aug 2014 at 08:40 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
ytsurkComment #2
ytsurkComment #3
ytsurkComment #4
ytsurkComment #5
drunken monkey"no test" probably means that there is none, I'm pretty sure it should get one.
Also, a new type for creating just a verbatim copy of one or more fields would be very helpful, I think – and since we don't distinguish between single- and multi-valued any more, also pretty easy. (This change might also make most other functionality of this processor much less useful.)
Comment #6
ytsurkso - this processor can be dropped ? or is there still a need ?
somehow it was already process code written, but that is now broken :(
Comment #7
drunken monkeyNo, I think we should keep it. "Much less" was probably exaggerated. But we should probably make "Copy verbatim" (or whatever the label) the default option, once we've added it.
Comment #8
drunken monkeyIn the course of #2252079: Switch to proper objects for extracted items I noticed that this is currently not really ported at all. More or less only the method names were changed, but the implementation is still the same as in D7 (which of course won't work anymore).
@ ytsurk: Are you still working on this? If so, just ask any questions you might have (here or in IRC), I'd love to assist you in implementing this.
Comment #9
nick_vhPorted and confirmed as working. Pending is to write tests for it. Committed to the D8 master branch of the sandbox.
Some important differences. The add_aggregated processor was renamed to aggregated_field processor to more accurately reflect what it does. The whole form ajax was updated to D8 standard and uses static function to return the form element it needed.
Also it now checks if the field that the site builder wants to aggregate is from the same datasource type as the item that was given to the processor. If not, it will ignore it.
It was manually tested to see if the data reflected well in the database backend.
Comment #10
nick_vhPending:
Write tests for all cases
Add the notification that it won't be saved until the user saves the processor page.
Comment #11
drunken monkeyAlready added #2305021: Test field extraction in processors as a follow-up for the slightly more complicated (and hopefully less critical) task of testing the field extraction for unindexed fields that are aggregated.
Already worked quite a bit on the rest, should be ready to commit soon.
Comment #12
nick_vhStarted to work on a test for the aggregated fields processor. Bunch of schema validation processor errors were corrected first as the validation was committed to Drupal 8.
Comment #13
drunken monkeySorry, but we probably won't have use for that – as written above (and marked by the "Assigned" change), I've been working on this, and now I already have a working processor, the schema fixes and a decent test class.
Of course, if you have additional fixes or tests, please merge them in.
Anyways, the tests all pass for me, except the Views test which now outright fails with an exception. No idea what that's about, but will look into it if I can.
Comment #15
drunken monkeyOh, and one additional thing I'm not sure about, is whether we shouldn't rename the class (along with its test class, of course) to
AggregatedFields(with appended "s") to reflect its label. After all, it doesn't just provide a single field. Any comments on that?Comment #20
drunken monkey