Saving release notes makes release disappear from project feed and project page release table

Thanks for submitting this. Yeah, there still seems to be some problems with how project_release_query_releases() and friends are working. I've noticed that even after all the fixes from #2137201 on production, the project feeds are still not always properly ordered, especially in terms of releases with "extra" (e.g. alpha, beta, etc).

Even worse, based on quick testing from both me and drumm, it appears that the functions/queries work okay on d.o dev sites, but are only doing crazy things on production. So it's possible this is actually a d.o infra bug instead of a project bug. :/ But, this is definitely a critical bug, and a d.o D7 regression, so tagging it as such.

Eureka! The root of our problem is here:
#1611438: fieldOrderBy filters out results with empty field values

Since we've got release nodes with NULL values for various things (e.g. verison_minor), this code is causing us to miss some releases:

function project_release_query_releases($project_nid, $api_tid = NULL, $major =\
 NULL, $access = FALSE, $dev_first = FALSE) {
  $query = new EntityFieldQuery();
  $query->entityCondition('entity_type', 'node')
    ->entityCondition('bundle', project_release_release_node_types())
    ->propertyCondition('status', 1)
    ->fieldCondition('field_release_project', 'target_id', $project_nid)
    // We always want the dev snapshots to show up either first or last.        
    ->fieldOrderBy('field_release_build_type', 'value', ($dev_first ? 'ASC' : 'DESC'))
    // Sort by the obvious integer values along the branch (minor and patch).   
    ->fieldOrderBy('field_release_version_major', 'value', 'DESC')
    ->fieldOrderBy('field_release_version_minor', 'value', 'DESC')
    ->fieldOrderBy('field_release_version_patch', 'value', 'DESC')
...

I'm not 100% sure the best way to handle this. :/ A few ideas:

A) Ensure we always have values (and somehow know what's empty and what's real) for every version element field

B) Query for all the release nodes matching a given project, and order them individually in PHP instead of via the EFQ

C) Somehow fix EFQ to support optional ORDER BY (which from the looks of #1611438 is never going to happen)

D) Basically give up on Doing It Right(tm), ditch EFQ for this, assume the fields live in SQL, and write our own monster query with LEFT JOIN as appropriate

I'll try to talk to drumm ASAP and get his opinions on this. Stay tuned.

p.s. project_release_query_releases() already does a node_load_multiple(), so option B isn't necessarily as evil as it seems. We can use the EFQ with all the conditions, load all the nodes, then sort in PHP, and return the sorted array. That's probably safer and better than the other alternatives.

drumm briefly skimmed and agrees that B is our best option, so I'm going to work on a patch for that.

Stay tuned...
-Derek

Completely untested, but something like this should be what we need. I'll put this on a dev site and see how it behaves ASAP, but I need to run for now...

Yep, looks like a good direction to go in, with testing of course.

Now that I had a chance to test it, #5 was broken in a few subtle ways once it started trying to compare releases with extra. It was also a bit careless with various NULL values. This seems to be working nicely, for example, check out the order of these:

http://dww7-drupal.redesign.devdrupal.org/files/release-history/bootstra...
http://dww7-drupal.redesign.devdrupal.org/files/release-history/authcach...

Now the question is: should we have a DB update that goes through all the releases to recompute all the update status stuff, rebuild the download tables, etc? If so, exactly how/what should it do?

Upon closer inspection, once I got it all working again, the code for comparing major, minor + patch was almost identical to the extra fields, with a minor difference in behavior when $a has extra and $b does not. So I (think I) simplified the code. I think this is a bit more obvious/readable, but the behavior should be identical between this one and #7.

p.s. I misspoke: #7 was actually broken in that the behavior (code) was identical for all fields, where we actually want it different (as expressed in the comments). #8 is what's now deployed on dww7 and I rebuilt the feeds I linked in comment #7.

Further simplification to the logic and fixed a code-style bug.

Thanks for #11! Yeah, that's probably cleaner. Exactly what's going on there with array_multisort() is a bit magic and opaque, but the same is true of the wonky custom sorting function from #10. ;)

I confirmed the two approaches behave the same, and committed/pushed:

http://drupalcode.org/project/project.git/commit/87358ce
http://drupalcode.org/project/project.git/commit/f042eda

Tested again on staging, then deployed live.

I just edited authcache 7.x-2.0 beta1 and it's now properly appearing on the Authcache project page. The release history feeds are basically constantly rebuilding, so within a few hours that should be repaired, too. I'll leave it up to you when to remove the (now confusing and unnecessary) warning on the project page.

Thanks for your help, and sorry for the troubles!
-Derek

p.s. Automated cleanup is at #2151555: Write a drush command to check release status for projects

I attempted to use this function to sort the automated testing page, and discovered that it does not consider core api version in its sorting ... so we can end up with an order of 6.x-1.4, 8.x-1.4, 7.x-1.4, 7.x-1.3, 8.x-1.3, 6.x-1.3, 6.x-1.2, 8.x-1.2 ... etc.

Attached patch adds sorting by the core api taxonomy term to the project_release_query_releases() function.

Can we handle that in a separate follow-up? Yes, it'd be nice to include tid, but it's not a critical bug.

- All existing uses of this function either filter the list by tid in the first place, or they're grouping the results by tid independently.
- Let's not make #1967500: project_release_api_vocabulary multiple erros worse by assuming this vocabulary exists at all.

Thanks,
-Derek