project* should support tokens for pathauto and others

The problem is probably about "project_name" not being available as a token. Is that right?

If so, that is something which should be added via hook_token_list and hook_token_values in the project module (or project_issue) itself.

Updating status based on my assumptions about this issue and setting status given that I don't understand.

I think this issue can become an issue for the project* modules to implement hook_token_list and hook_token_values.

These should be pretty easy in general - dww/hunmonk/aclight feel free to ping me with questions.

I'd be willing to take this on as a intro into how to create a patch, apply it, get it through the process, etc. Given the fact that I can ask greggles about implementing those two hooks if I run into trouble, I'm not too worried about the coding part.

Mike Hostetler

That'd be great. We'd be happy to commit a patch to project* that implements these hooks. If they're tiny, they could go right in. If they're a lot of code, they should go into a project.tokens.inc file as _project_token_values(), and then in project.module itself, you'd do this:

function project_token_values($whatever) {
  include_once(drupal_get_path('module', 'project') .'/project.tokens.inc');
  return _project_token_values($whatever);
}

Make sense?

However, I don't think project* and pathauto work together at all yet due to the evil $node namespace bug: #98278: project* namespace bugs in $node

We're planning to fix that at the friday code sprint, so this won't be postponed for long, but no one should work on this patch until that's done.

Cheers,
-Derek

Here are the 3 patches for project, project_issue and project_release. Each patch implements hook_token_list and hook_token_value for each of the project* modules.

Quick small changes - t('The issue\'s project nid'); is better handled by the t() function using something like t("The issue's project nid");

The changes greggles suggested have been added to the patches.

A) minor code-style thing:

  if($type == 'project') {

should be:

  if ($type == 'project') {

B) that "release_directory" column is also dead, we shouldn't expose it as a token. See #231558: {project_projects}.release_directory is a dead column

C) the bigger issue here is this is really blocked on #98278: project* namespace bugs in $node, since the patch is referring to stuff like $node->project['uri']

So, you could re-roll for (A) and (B), but you might want to turn your attention to #98278 for now, so that we're sure we're on the right track for (C).

Thanks!
-Derek

Oh, for example, for (C) -- looking at project_release_token2.patch, I think this:

$object->project['version_major']

is really going to end up as:

$object->project_release['version_major']

D) project_issue_token2.patch -- don't know, but don't we want to export some more of the fields of an issue as tokens? e.g. current status, component, category, priority, etc?

#154183: PathAuto Project_release View All Releases/Cats was marked a duplicate of this issue.

Not knowing Project very well, but desiring to see URLs on modules issues on drupal.org changed from "node/<nid>" to something more meaningful ("/project/<project_name>/issues/<issue_title>") I thought I'd bump this.

I've rerolled mikehostetler's patches in #10 with attempts at accounting for the issues raised:
A) space added in all occurrences of if(
B) removed release_directory stuff
C) (noting that #98278: project* namespace bugs in $node is marked fixed,) changed all
$object->project['...']
in release and issue to
$object->project_release['...']
and
$object->project_issue['...']
D) I can attend to if all of this is still relevant.

@jeffschuler: thanks for reviving this patch. However, we'd never use it as you propose on d.o for a number of reasons:

1) The issue node id (nid) is referenced in CVS commit messages to tie commits to issues.

2) The project an issue belongs to can change -- if the project was in the URL, we'd have link rot every time someone moved an issue to a new queue.

3) The title of an issue can change -- same link rot as (b).

4) The fact that the URLs are nice and short makes them easier to paste and reference externally.

5) As you used in comment #14, there's the cool [#xxx] filter to let you quickly refer to the issue and get the current title and status, etc.

That said, I'll take a closer look at your patches here once I've had a chance to catch up on a bunch of work. I've been traveling (and sick) a lot the past month or so, and am deeply behind on a lot of things...

2 and 3: At the risk of going too far off-topic...using Pathauto and Path Redirect eliminates link rot.

4: You can of course still use the node/NID as a shortened version, we would just have to be sure to expose the NID somewhere obvious on the project page.

As a review of the code:

IMO, the "uri" tokens should be called "project_issue_shortname" or something other than URI. They are not URI's but really one specific fragment of a URI. The token standard is to use "url" to mean the full url to something. I know that URI and URL are not the same thing, but when the w3c has a whole document trying to clarify what they are you can be pretty sure nobody actually knows what they mean. "Short name" is how admins know this bit of text.

+    $tokens['project']['project_homepage'] = t('Link to project home page');
+    $tokens['project']['project_changelog'] = t('Link to projects changelog');
+    $tokens['project']['project_cvs'] = t('Link to projects cvs tree');

These should all be possessive "project's".

Thanks dww & Greggles.

Agreed, Greggles; that's exactly why Pathauto is around... I doubt meaningful URLs really need a supporting argument here... And, yes, solutions like Redirect, Pathologic, and XML Sitemap are available to help manage the best URL for the node. And, yes, node/# (our own tinyurl) would still be available for shortcutting/commits (and would deserve alternate exposure.) Multiple URLs for a node aren't rot as long as they're managed.

The initial impetus for my request is that Drupal issues in Google search results don't give me the faintest idea to which project the issue applies.

Original topic:

Greggles, uri is used throughout the Project module; not nearly covered by these patches. Does this deserve a separate issue?

Thanks for your other suggestion, too. I've changed all such instances to "project's", and re-rolled (combined the two Project project patches.)

URI is used in the code, but "project short name" is used in the admin interface. Tokens are user facing strings and should use the user facing name for the data.

Sorry I didn't understand earlier.

uri in token strings changed to shortname.

Finally had a chance to look closely at this. The previous patches were all kinds of broken on a variety of levels. ;)

A) hook_token_values() gets an $object, not hook_tokens_list() -- all the *_values() implementations were referencing an $object that wasn't there.

B) Projects, releases, and issues are all fundamentally nodes. Therefore, we need to use tokens of type 'node', otherwise, no one will be able to access them. For example, pathauto only calls token_replace() with 'global' and 'node'.

C) Due to continued evil with $node namespace woes for project_issue nodes, the patch for issues was all wrong, and wasn't finding any values. I added some work-arounds, but also discovered a nasty flaw in how project_issue handles its own node form when you edit an issue. See the patch at #542150-1: Make issue $node->project_issue[] namespace consistent, which is required to make this work at all. Ideally, we'd fix that entire issue and remove the special-case in here (and other places).

D) Some cosmetic issues on how to declare the arrays, when to initialize defaults, etc.

Here's a pretty thorough re-roll that addresses all of the above. This is probably ready to go in, but I'm posting here in case anyone wants to give it a look...

(just putting here in addition to the irc discussion we had so others can potentially benefit from the ideas).

These look great to me. I didn't really review if they were parsing the right bits of data - I think you know that much better than I do.

I suggest changing from check_plain to check_url for any of the bits of data that are urls:

1. check_url eventually calls check_plain any so it's just as safe
2. check_url also removes bad protocols, so it's slightly better for anything that might get used as a true url

In general - looks awesome to me. I can't wait for this to land.

In IRC, greggles pointed out that all those project links should use check_url(), not check_plain(), which is utterly true. Fixed that, then committed to HEAD (I just committed the required initial patch from #542150). Thanks everyone!

heh. ;)

Also added release tokens for the API compatibility term: http://drupal.org/cvs?commit=247812 (see #540074: Denormalize release API compatibility tid into {project_release_nodes}).