cases in edit/releases tab on project nodes: default release broken

yeah, sorry, there are a few bugs in these edge cases for the "releases" tab UI. :(

the good news is that pressing "submit" on http://drupal.org/node/25406/edit/releases solves this (which i just did).

now that the support request is fixed, i'm turning this into a bug report against project_release.module, so i don't forget to try to track down and fix this particular case (some day). ;)

that's because the dev snapshot hasn't been packaged yet, and therefore, the release node isn't published yet. if you wait until the next packaging run (like the drupal_set_message() when you created the node, and handbook, both tell you to) it'll all Just Work(tm). at least, it should. ;)

found the problem:

in function project_release_db_save(), there's this section of code:

$has_default = db_result(db_query("SELECT COUNT(*) FROM {project_release_default_versions} WHERE nid = %d AND tid = %d", $node->pid, $tid));
      if (!$has_default) {
        db_query("INSERT INTO {project_release_default_versions} (nid, tid, major) VALUES (%d, %d, %d)", $node->pid, $tid, $node->version_major);
      }

there nothing wrong with that per se, but the problem arises because there is no code in place to police the default version when you unpublish a node. so if you unpublish all releases for a major version, then create a new release, it's not entered as the release default.

not really sure about the best way to solve this long term. suggestions welcome :)

i ended up rewriting this logic quite a bit, in an attempt to handle all cases. here's an overview:

1. check the default release table for an existing major setting the API version in question
2. if one exists, then make sure there is at least one published release node on that major
3. if there is no entry in the table, or the data is determined to be stale, then first clear any stale information in the default release table for that API version
4. look for the highest major that has a published release node for the API version
5. add it to the default table if found

brutal :)

initial testing seems to indicate this will handle everything well. since i don't see any existing support for the deletion of release nodes, we might want to abstract this logic to support that as well.

Thanks for trying to clean up my mess, hunmonk. ;) However, a visual inspection reveals a few problems with this initial patch:

A) Major could be 0 in a valid case. It doesn't look like your code handles that. I'll admit that the Drupal DB layer doesn't make this easy, since it's a pain to manipulate NULL values, but it's still possible.

B) I really don't like reusing the variable called $has_default like this. :( With your patch applied, depending on where you are in the function, it could now mean a couple of different things (do we have a default major version vs. the specific current default major version). This is confusing. I'd rather see separate variables, each named exactly what they mean.

C) + // For a sensible default, pull the highest major number that has a published release.
IMHO, that's not a sensible default. Just because a --2 or --3 branch exists doesn't mean that should be the default. The basic use-case is that --1 is stable, and --2 is for unstable new features.

attached addresses all of the previous concerns. i've also broken out the checking code and re-used it to perform the check in the case of release node deletions.

i can confirm through testing that the table is properly updated when:

• it contains no previous entry for the API version
• it contains stale data from a branch that has no active release nodes
• the table contains current data (no adjustments are made in this case)

i can confirm that all queries work in both mysql and pgsql.

I re-reviewed this and it looks good. I tested, and confirmed that things are basically sane. Some of the weird cases I tried in testing didn't always work exactly as planned, but then again, those were cases where I was directly manipulating the DB, instead of going through the Drupal UI, so these code paths weren't getting hit. ;) So, as long as people play by the rules and use the Drupal UI, everything seems to work. Attached patch just fixes some trivial comment formatting problems (word wrap at 80 columns), but otherwise, doesn't touch the code.

hunmonk, thanks for working on this admittedly very confusing part of the code! Nice job.

p.s. I even tested that a major version of 0 is handled correctly. ;)

Argh, whoops, not so fast. Something's broken on the table on the edit/releases tab when the major version is reset. The "current default release" isn't correctly displayed. I'll dig into this for a little while and see if I can clean it up.

Ok, it was just a trivial bug with empty() returning true when major == 0. :( It's unrelated to hunmonk's patch, but we agreed to just roll it into here since it's trivial and at least related code. ;)

tested, great. committed to HEAD.

i have no interest in porting this to 4.7

Backported to DRUPAL-4-7--2: http://drupal.org/cvs?commit=84434

This seems to still be happening on d.o. Over the past couple days killes (can't remember which module) and bjaspan (schema module) hit this issue. I directed them to save the edit/releases tab and that fixed it.

I believe in the case of schema module, it already had a 6.x branch and release. A new 5.x branch was created and released (1.0) and it wasn't showing up on the project page even after the release node was auto published.

Alas that this is still happening. However, all of this code is about to change, in a fairly big way, for http://drupal.org/node/203313 and http://drupal.org/node/176776. Let's see if we can just magically fix this problem while we work on those other two.

This is no edge case! http://drupal.org/node/204869

I saw this on *every* 6.x-1.x release node i've created (for e.g. Global Redirect, Gotwo, etc.).

@hass: thanks for the helpful tone. :/ Adding a release for an entirely new version of core is less common than adding any new release. Hence, I was calling this an edge case, since in a given year, a contributor might create dozens of releases, only 1 of which triggers this bug. Glad to see you haven't lost your knack for being aggressive, unhelpful, and overreacting.

I think an error is around project_release_check_default_version($pid, $tid) function.

When we selecting $active_branch_releases the taxonomy on new release node is not yet saved into the table {term_node}. And so we've got 0 here.

My suggestion: force run taxonomy_node_save() before check default version.

Attached patch provide this behavior.

@vladimir.dolgopolov: Thanks! Finally, some useful feedback. Someone who did a little testing and debugging, and proposed a solution to one of the possible problems. Yay!

However, your patch is needlessly complicated. Instead of this:

+        $terms = taxonomy_node_get_terms_by_vocabulary($node->nid, $vid);
+        taxonomy_node_save($node->nid, array_merge($terms, array($tid)));

All you need is:

+        taxonomy_node_save($node->nid, $node->taxonomy);

Furthermore, this doesn't actually explain or solve the problem on d.o itself, since there, all new release nodes start life unpublished, which is the only time we'd hit this $is_new case, and unpublished releases shouldn't count in project_release_check_default_version(), anyway.

However, after heavy testing, I decided something like this was useful for my latest patch here:

http://drupal.org/node/203313#comment-696550

so that we correctly handle the case where release nodes are initially created as published nodes.

Anyway, as I said above, I'm not going to fix this bug in the existing code. See http://drupal.org/node/203313 for the real solution here. Marking this postponed until #203313 is committed and deployed on d.o, then I'll just mark this fixed.

However, thanks again for contributing productively to this issue!

You are welcome :)

The only drawback of my suggestion is: taxonomy_node_save() called twice by node - here and in taxonomy_nodeapi().
But it works though.

Right, I was thinking of that last night as I drifted off to sleep. There's a (slightly funky, but totally workable) solution. project_release already has to be weighted heavier than taxonomy for other things to work. The problem here is that we're doing this checking code directly from hook_insert() and hook_update(), which happen first, then hook_nodeapi() is invoked. So, if we move the logic to trigger the check into our own nodeapi() implementation specific to project_release nodes, we know that will fire *after* taxonomy has a chance to update the DB. I'll roll a new patch at #203313 to reflect this.

See http://drupal.org/node/203313#comment-696949.

This was majorly improved via #203313: Add a way for maintainers to indicate multiple supported branches. Unfortunately, there's still one more problem here. package-release-nodes.php doesn't actually use nodeapi to edit the node when it publishes a release, it just directly updates the {node} table (which is better for performance than a full node_load(), node_save()). However, that means that project_release_check_supported_versions() isn't getting called. :( So, when the new release node is added, project_release_check_supported_versions() is called once, but since the new release isn't published, prcsv() ignores it. We need to trigger that check again from package-release-nodes.php once it's published. We should also fix prcsv() to itself clear the {cache_project_release} table for the project if it changed something. Stay tuned, I'm rolling a patch now.

This *should* do it, but after I installed it on p.d.o, it doesn't seem to actually be working. :( The wd_msg() call I added there is getting hit with the right arguments, so I have no idea why it's not doing what it should. Weird DB replication delays, perhaps? I guess I'd have to pepper the project_release_check_supported_versions() with some watchdog() debugging to see what's happening inside there, but I don't have time for that now. I figured I'd post the work-in-progress in case someone else wants to take a look. I'm busy.

FYI: the work-around now is to edit the release node after it was packaged and press submit. That should trigger everything that needs to happen.

I've used the work-around but still can't seem to get the table to show up for the Bronto module. Here's what I did to try to trigger table creation:

- Went to http://drupal.org/node/231226/edit
- Pressed "Submit"
- Waited 24 hours for the table to show up

Please let me know if I 'm missing something.

@JuliaKM: It looks like you only have a development release for your module, so on the http://drupal.org/node/231203/edit/releases page you need to click the "Show snapshot release" check box.

@aclight thanks! I don't know how I missed that.

I think I'm having a similar problem, even though in my case there was no "unpublishing" involved. So my version of kbahey's original post is:

I created a new 5.x-2.0 release for imediasee off of a DRUPAL-5--2-0 tag.
The release was successfully created: http://drupal.org/node/236989
and shows up on the list here: http://drupal.org/node/226766/release
However, on the main project page http://drupal.org/project/imediasee the 5.x-2.0 release does not show at all.
And on http://drupal.org/node/226766/edit/releases there doesn't seem to be any way to point to the new release as being the new recommended release.

How can this be rectified? (This is my first release after my initial release, so I apologize if I've missed a step due to noob-ness.)

@steve: go to http://drupal.org/node/236989/edit, hit the submit button to save the release again, then go to the project's page and click the edit tab, then click releases, then you should have the option to set that release as supported.

subscribing

As of 3/25 I too encountered this problem while doing my first project. I posted comments at Step-by-step: Create a CVS project and CVS maintainer quick-start guide the two places that I spent the most time at while learning how to do it.

I was getting so sick of the support load and duplicate issues about this that I just came back for a closer look. Ugh, I can't believe it: #26 was broken due to an incredibly stupid error: i reversed the args for project_release_check_supported_versions(). ;) DOH!!!!

Now fixed, tested on p.d.o, committed to HEAD and DRUPAL-5, and installed on d.o. This long nightmare is finally over. PHEW!

Attaching the final patch I committed for reference, in case anyone's interested.

@WorldFallz: thanks for commenting about the work-arounds. Now that the bug is fixed, I removed those comments. Cheers.

Happy to help...even just a little. Thanks for fixing this and all your work on project--- it gets even better with each release!