While working on #1974560: Improve node changes layout on comments I ran into the case of issue tags. Currently when they're displayed on d.o issues, they show up as links not to the taxonomy term page, but to customized issue queue listing views. I couldn't remember where that logic is happening, since I know we don't actually natively support 'issue tags' at all in project_issue.

Turns out in D6, it's a buch of special-case if (module_exists('project_issue')) hacks inside Comment Alter Taxonomy. Fun! That module is not at all part of the D7 upgrade, so we have to do *something* with issue tags in the D7 port of project_issue. Not sure any code belongs in project_issue for this, but starting life here to at least capture it for the d.o upgrade.

CommentFileSizeAuthor
#4 1979000-4.issue-tags.patch14.27 KBdww

Comments

dww’s picture

dww
we/he/they
commented

D6's hook_term_path() is now gone. Instead, there are two basic options:

A) Use hook_url_outbound_alter() (Change notice)

B) Use hook_entity_info_alter(), swap out the 'uri callback' for the taxonomy term entity to be a custom callback that checks the vocab and either uses the default or does the special-case magic.

B seems much cleaner, easier, and perhaps faster, so that's definitely my vote unless there's a compelling problem with that.

---

Then, there's the problem of actually displaying issue tags as links (e.g. that's not happening in nodechanges diff output yet -- see #1979066: Consider using field formatters to display changes. However, see also #1981478: nodechanges doesn't handle changes to taxonomy term references when terms are removed -- we have to be able to handle if the term is gone and not try to show a link in that case...

I'd move this to the drupalorg queue, but wearing my Project* maintainer hat, this seems like something a lot of sites would want, and if it's not just clicking a field together, I'm wondering if we should add a project_issue_tags module that holds any code related to getting issue tags working. Might be more trouble than it's worth (especially if it means touching the migration code again), but I'd like to think about it before we plow ahead with d.o-specific hacks.

Cheers,
-Derek

dww’s picture

dww
we/he/they
commented
Assigned: Unassigned » dww

Worked on this a bit over the weekend. This was more broken than I thought. Our default views are hard-coded to define exposed filters for 'taxonomy_vocabulary_9_tid' which is d.o's issue tags vocabulary. Of course, that's not going to work for anyone else.

However, I have a nearly working patch, except that it's going to break the D7 d.o site since core is annoyingly inconsistent with taxonomy and term reference fields. Vocabs migrated from D6 get field machine names like vocabulary_9 which and term reference fields like 'taxonomy_vocabulary_9', whereas new vocabs could be anything for the machine name, and the field is just 'field_[machine_name]'. As I was getting this working locally, I wasn't sure how to handle this, but I've since had an idea how to best handle it (just inspect the instances on the issue bundles). Once that's done, I'm pretty happy with this, and I think it's fine to handle it in project_issue.

Wanted to assign myself so no one else starts working on this. I'll post again once I've got a finished patch (probably late Monday night -- I'll be traveling again tomorrow).

Cheers,
-Derek

dww’s picture

dww
we/he/they
commented
Title: Port issue tag link support to D7 » Port issue tag support to D7

Better title to reflect the real scope. This isn't just about the links. It's also about the views that give us a place to link to. Therefore, it's also related to #949372: Port issue views to Search API so we can have a performant backend, but since that's still in so much flux, and I think most of what I'm doing here is going to be necessary regardless of if we're using Search API or raw views, I wanted to get this working properly for now.

dww’s picture

dww
we/he/they
commented
Status: Active » Needs review
StatusFileSize
new 1979000-4.issue-tags.patch14.27 KB

This is working nicely in local testing. I'm not 100% sure it's going to work with the d.o migrated vocabulary, but I tried to make it resilient in the face of core's inconsistency. We'll also need a trivial drupalorg_project_update_N() to set a variable to put this into effect if we like this approach. Anyone want to review before I push?

Thanks,
-Derek

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Needs review » Reviewed & tested by the community

Looks like a good approach from reading the code.

dww’s picture

dww
we/he/they
commented
Status: Reviewed & tested by the community » Fixed

Great, thanks. For posterity and for anyone else's benefit, here was our chat about this:

[2013-05-15 10:50:35 AM] Derek Wright: Great.  You're not horrified at the 4.7-era alter-the-vocab-form approach? ;)
[2013-05-15 10:50:43 AM] Derek Wright: Seemed insane to try to make this a field you could attach to vocab entities.
[2013-05-15 10:51:59 AM] Derek Wright: (esp since that would require an entity_load() on the vocab inside the URI callback, which seemed like a bad thing for performance -- I wanted to be able to decide where to link to with the cheapest approach possible -- a single variable_get() seemed good).
[2013-05-15 10:52:59 AM] Neil Drumm: Yep, I think either approach would be okay. Sounds like this way is cheaper.
[2013-05-15 10:53:16 AM] Derek Wright: Cool.

So, I pushed this to project_issue:

http://drupalcode.org/project/project_issue.git/commit/f95decf

Then wrote and tested a simple drupalorg_project update to configure this on rebuild/migration:

http://drupalcode.org/project/drupalorg.git/commit/d3a2d58

Now merged into bzr and should be live on git7site soon (although I can't access that site right now).

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