taxonomy_term view not behaving like default when using taxonomy/term/<tid_1>,<tid_2> syntax

Hmm. OK, well I think this might be a bug... I'm sure someone will tell me if it's not.

I've jumped in to have a look and I found the following bit of code in views_handler_argument_term_node_tid_depth.inc which says to me that it's not looking at $tids->operator to check if it's an and/or condition but assuming that it's an "or".

      if (count($tids->value) > 1) {
        $placeholder = " IN (" . implode(', ', array_fill(0, sizeof($tids->value), '%d')) . ")";
      }

Trying to think how to do the "and" query economically and thinking a "UNION ALL" might be the best option (I'm from a SQL Server background, so assuming there is a UNION ALL... sure it's ANSI 92 but I digress). For the moment I'm going to modify views_handler_argument_term_node_tid_depth.inc because I can't work out how to do this without changing the core code... well not without doing a global rewrite of the query which would be quite painful when I can do it easily when the query is getting built.

Anyways, if anyone has input into doing this better I'd appreciate it, but I'll update when I make my change.

Thanks,
Anthony.

"UNION ALL"? What was I thinking. Anyway I used a query that worked. I did a group by of all the rows that matched the tid values we're looking for. If there was exactly the same number of rows as terms searched for then it matches all of them (this works because vid/tid form a composite primary key) and added this to the sub select done by the query to get all the required nid values. This seems to work for me, but I've only done a quick bit of testing. Is anyone aware of further reaching implications by changing this code that I may not be aware of?

Thanks,
Anthony.

I've replaced:

      if (count($tids->value) > 1) {
        $placeholder = " IN (" . implode(', ', array_fill(0, sizeof($tids->value), '%d')) . ")";
      }

with:

      $group_by = "";
      if (count($tids->value) > 1) {
       $placeholder = " IN (" . implode(', ', array_fill(0, sizeof($tids->value), '%d')) . ")";
        // @AM 20090910 condition added to cater for the ',' argument used to specify AND conditions
        if ($tids->operator == "and") {
          // @AM 20090910 If it is an 'and' condition then use a group by and having to 
          // work out which rows have all the required tid values
          $group_by = "\nGROUP BY tn.vid\n";
          $group_by .= "HAVING count(*) = " . count($tids->value);
        }
      }

and after this line:

    $where = "  WHERE tn.tid $placeholder\n";

I've added:

    // @AM 20090910 Add group_by if it's been defined
    $where .= $group_by;

Patch created as it seems to work for me in case anyone wants it.

Hello Merlinofchaos,

Thanks for your reply. Yes you're right I didn't specify (explicitly) the file that the patch is to be applied to. Is there a best practice way to do this that I've missed? At any rate, the file that the patch applies to is views_handler_argument_term_node_tid_depth.inc

As for what it is that I'm trying to patch, perhaps I didn't clearly state the problem clearly enough. If I use the the taxonony view provided with views as a default view, and I try and use it to show nodes that are tagged with more than one taxonomy term (using the comma syntax, i.e. taxonomy/term/1268,2000 where 1268 and 2000 are both valid taxonomy term ids) I get the behaviour that doesn't agree with the default behaviour of taxonomy, i.e. I get nodes that are tagged with EITHER term, not thos that are tagged with BOTH those terms. Does this agree with how you see the taxonomy view work or does it work correctly for you? Have I specified the problem clearly enough or is there more that I can do to make it clear? To be honest, looking at the source I can't see how it would work, but there's a good chance I'm missing some subtlety.

For the group by, it works because vid/tid form a composite key, that is, it returns all nodes from that table that are tagged (at least) with all the terms in the argument. As I've stated the query works for me, but I can probably find a reference online that explains the query in more detail if it helps? I've tried to be careful to not go just "slamming them in like that" by putting conditional code in that only adds the group by when required (I actually copied the condition form other views code).

I would certainly like to avoid breaking more than I'm fixing so I look forward to your input into making this patch more acceptable.

Regards,
Anthony.

Hi Vadym,

Are you saying it worked for you? Maybe I should put this as needs review and see what happens. I'm not overly up with the process for getting patches done (as is probably obvious) but happy to help out if you have a specific question.

Regards,
Anthony.

Well the GROUP BY is on a sub-query, but never mind... given that there are no better options given to fixing this and the current fix is obviously not suitable I might as well just change this to "won't fix" and move on.