Port project_projects_select_options() to D7 and make it handle multiple project node types

Thanks for this patch. Upon visual inspection:

A) PHPDoc blocks should have 1-liner summaries.

B) The $project_urls param should probably be renamed to $project_machine_names and the comments (and code) updated accordingly. Confusingly, in D6 (actually all the way back to 4.6 and earlier), the project "shortname" aka machine-name was called the "uri". Which looks a lot like "url". And it's not technically a URI, either. :( So, I'm trying to eradicate this confusion by just calling it a shortname or better yet, "machine-name" whenever possible. ;)

C) Code style whitespace bugs (extra spaces) here:

+  if ( empty($project_entity_types) ) {

There are also some extra spaces in a few other spots (e.g. inside comment blocks)

D) Code style whitespace bugs (missing spaces) here:

+  $query = db_select('node','n')->fields('n',array('nid','title','type'));

That should look like this:

+  $query = db_select('node', 'n')->fields('n', array('nid', 'title', 'type'));

E) A couple problems with this code block:

+  $types = array();
+  foreach( $project_entity_types as $id => $project_entity_type ) {
+    $types[] = $project_entity_type;
+  }
+  $query->condition('type', $types, 'IN');

- Why not just use $project_entity_types directly? Can DBTNG not handle it if the array is keyed without sequential keys?
- If it has to be numerical sequential keys, you can just use array_values() instead of foreach like this.
- If you were going to use foreach the code style should be:

  foreach ($project_entity_types as $id => $project_entity_type) {
    $types[] = $project_entity_type;
  }

F) The way you're JOINing here for field_data_field_project_type assumes fields are stored in the database. Field storage is pluggable. I think there's a Field API way to do what you're doing here without manually building up DBTNG queries which will work even if fields are stored in mongo or something. I'm in a bit of a hurry right now, so I can't find you the function and link it here, but you can ask chx if you need help with this. ;)

G) This isn't what this function wants to be doing:

+      $uri = drupal_lookup_path('alias','node/'.$project->nid);

As per point B we just want the machine names here, not the full path alias. However, while I'm talking about this line, again, it's missing spaces for code style.

Thanks!
-Derek

Re: tying this to nodes, see #1586210: Clean up confusion where we appear to support more entity types -- trying to be general is great, but there are limits. ;) To avoid needless complication, we have to just accept the fact that Project* is assuming that projects, releases, and issues are all *nodes*. We can't really handle using arbitrary entities as projects without much more complication than we can afford (given how much work we already have ahead of us to port all this and convert it to Field API), especially since there's dubious value in making this fully entity-agnostic. Attaching an issue queue to a user sounds funny, but I can't really justify the maintenance overhead to begin to try to support that. ;)

And, maybe I'm just stuck in my old thinking, but I really believe that all of these qualify for all the things that make a node a node -- a real path, the possibility for revisions, comments, etc. I don't see a problem requiring nodes for any of these, and it definitely makes the code simpler and easier to understand.

@Senpai: thanks for setting the status. I was going to review anyway, but it's nice to actually stick to the real workflow with the issue status, especially since we've got minor help for obvious problems from the bot.

@rgristroph: Thanks for the patch. Mostly looking great. A few remaining problems:

H) As I linked in #4, see #1586210: Clean up confusion where we appear to support more entity types. We removed PROJECT_ENTITY_TYPE. Now I'm having second thoughts. If we reopen that, we can fix this. But for now, that constant isn't defined so you should just use 'node'.

I) If $issues is FALSE and $include_sandbox is set to 'include_sandboxes', this code will simply return all nodes in the system. Nothing is restricting it to project nodes.

Thanks again,
-Derek

Also, I just noticed another thing here:

J) Similar to point H, project_project_entity_bundles() no longer exists, either. It's now called project_project_node_types()...

(You forgot to update the status to "needs review"). ;)

I was about to bump this back to "needs work" when I saw this:

    $project_entities = entity_load($entity_type, array_keys($project_entities));

However, upon further research, D7 gives us no way to just load a few fields on all 10K projects. We have to entity_load() all of them. :( In fact, entityreference has the exact same problem unless you use the autocomplete widget -- they have to entity_load() everything, too. Shocking.

So, #1549808: Port the lame D6 node/add/project-issue UI to D7 is likely to crash d.o once we deploy it. But, it seems pointless to keep pouring energy into this to try to play crazy tricks to make a totally broken UI work on a d.o-sized site. This code here (and the UI it supports) works fine on small Project* sites. It's a totally broken mess (to be clear, this patch is perfectly fine code, D7 itself is to blame) for something d.o-sized. But whatever... that's really for #1044950: Re-architect the project-dropdown UI selector on node/add/project-issue to prevent server crashes not here.

To move things forward, I just fixed the tiny whitespace bug in your patch, and pushed 37a81345 to 7.x-2.x.

Thanks for all your work on this!
-Derek

p.s. tagging for sprint 3 since this bled over into there...