Small typo in the new load multiple terms in taxonomy in that the condition is ignored as it's using $query->conditions() not $query->condition().

Quickly checked the similar code for node, which looks correct.

CommentFileSizeAuthor
#1 taxonomy_tlm_typo.patch1.77 KBcatch
taxonomy_tlm_typo.patch755 bytesasimmonds

Comments

catch’s picture

catch
he/him
Primary language English
commented
StatusFileSize
new taxonomy_tlm_typo.patch1.77 KB

Well found, turns out $query->conditions is a valid method so it doesn't blow up. Patch is good, added a test - the existing ones load stuff by conditions, but didn't check for the possibility of too many terms coming back.

asimmonds’s picture

asimmonds commented

New test passes if bug is fixed, and fail if not. In the new test code, should NULL be used instead of array() in taxonomy_term_load_multiple() so it's consistent with the other occurrences of taxonomy_term_load_multiple()?

catch’s picture

catch
he/him
Primary language English
commented

asimmonds, I think the plan is to move to type hinting all parameters, which would make array() the standard instead of NULL - see #318016: Type hint array parameters - there's not a standard on that yet though.

Status: Needs review » Needs work

The last submitted patch failed testing.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Needs review

Re-testing.

dries’s picture

dries commented
Status: Needs review » Fixed

Committed to CVS HEAD. Thanks!

Status: Fixed » Closed (fixed)

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