Closed (fixed)
Project:
Search API
Version:
8.x-1.x-dev
Component:
Plugins
Priority:
Minor
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
1 Jul 2019 at 09:36 UTC
Updated:
9 Dec 2019 at 11:09 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
drunken monkeyTrue, thanks. Should probably wait for the other patch (#3064479: Return translated term name on taxonomy term views argument) to land, though, to avoid conflicts.
Also, a perfect issue for newbies, I think.
Comment #3
gdejonghe commentedHello, I'm at Drupalcon Amsterdam and I will work on this.
Comment #4
gdejonghe commentedHere is the patch.
Comment #5
mbovan commentedI am at DrupalCon Amsterdam and mentoring @gdejonghe on this issue together with @wengerk.
Comment #6
mbovan commentedTriggering the test bot.
Comment #8
drunken monkeyThanks a lot for working on this!
Please remember to set the issue status to “Needs review”, though, when posting a patch, so the test bot will test the patch. (Thanks for doing that afterwards, mbovan!)
The patch looks great already, thanks again!
However, I prefer a slightly different style for dependency injection, so it’s certain that the reference won’t ever be
NULL. (Might also be the issue in the test?)Patch attached, please review if you get the chance!
Comment #9
drunken monkeyOptimally, in this issue, we would also change that test to be a unit test. Do you want to try that out as well?
Comment #10
drunken monkeyAlright, committed. Thanks again for the patch!
Filed a follow-up for converting the test to a unit test: #3096581: Convert TaxonomyTermArgumentTest to a unit test.