Problem/Motivation

At the moment in \Drupal\search_api\Plugin\views\argument\SearchApiTerm::title() we load the terms as such:

        $taxonomy_term = Term::load($tid);

Proposed resolution

We should inject the entity type manager instead. This would allow to write unit tests (instead of kernel tests).

Comments

mpp created an issue. See original summary.

drunken monkey’s picture

Issue summary: View changes
Issue tags: +Novice

True, 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.

gdejonghe’s picture

Hello, I'm at Drupalcon Amsterdam and I will work on this.

gdejonghe’s picture

StatusFileSize
new1.81 KB

Here is the patch.

mbovan’s picture

Issue tags: +Amsterdam2019

I am at DrupalCon Amsterdam and mentoring @gdejonghe on this issue together with @wengerk.

mbovan’s picture

Status: Active » Needs review

Triggering the test bot.

drunken monkey’s picture

Thanks 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!

drunken monkey’s picture

Optimally, in this issue, we would also change that test to be a unit test. Do you want to try that out as well?

drunken monkey’s picture

Status: Needs review » Fixed

Alright, 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.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.