Project release should handle case where api terms are not defined

What page did you visit to generate this error?

Ok, duly noted. Don't forget to set the status back once you provide more info... I'll take a look at some point.

You also installed project_release.module, but you didn't say that. You did *not* add any taxonomy terms for your releases in the "API compatibility vocabulary", which sadly, project_release.module is far too dependent on. See the following issues for related troubles:
#133052: remove project_release dependency on taxonomy
#193498: Include a description of how project vocabulary works from INSTALL.txt
#193504: Add more information about how taxonomy affects releases
#193526: Remind user to configure project* taxonomy after install

All that said, it shouldn't be generating the SQL error like this. We don't normally see this in our testing, since we're using the http://drupal.org/drupalorg_testing installation profile, which puts terms in these vocabularies for us. We should try to remember to test on a plain installation, too.

Meanwhile, do you actually need release nodes on your site? If not, don't enable project_release.module, and you won't have any problems.

Sure, use project_release.module then -- I just wasn't sure if you actually needed that based on the original message here.

In terms of your 4 fields question, you could use the "API compatibility" taxonomy term as your major version (which would solve your other problems of not having any terms in that vocab), then call use 'major' for "minor", 'minor' for "release", and 'patch' for "build"...

Then, you'd just use the funky setting for how you want your version strings formatted to reflect these different fields as one big number. Something like:

!api%major%minor%patch#extra

so, if you had '2' for api, '1' for major, '0' for minor, and '2' for patch, your version string would become "2.1.0.2", and if you filled in 'extra', you'd get "2.1.0.2-beta" or whatever.

But, that's kind of off topic from the bug reported here. if you want more help with your version string questions, please open another support issue about it (and then help turn what you learn into a handbook page or other docs for the readme or something).

Here's a quick patch that seems to fix the problem, though I haven't tested it thoroughly or thought much about whether this is the right fix to the problem.

As dww pointed out, project_release depends on the api compatibility terms being present in a few places, but it really should handle the case where those terms aren't defined. Attached patch fixes db errors at node/N/edit/releases as well.

Sorry, but I seem to only be able to think in single thoughts this evening. Anyway, when a project has releases but no api terms are defined, the download table looks pretty confusing, I think. The releases will be red with the error X, but there is no text under the status table header.

Since I don't believe we're offering a way to designate a release as supported/recommended when the API compatibility terms are not in place (doing so would be nice for projects where the idea of an API compatibility term doesn't really make sense, but that's another issue), my thought is that we should eliminate the "Status" column of the download table and just use the regular background color when the API compatibility terms are not set.

The attached screenshot shows an example of what I'm talking about.

Setting to CNR to get feedback.

The patch #9 works and fixes the error on project creation and node/$pid/edit/releases. I think a message should be displayed informing that the api terms should be created to take full advantage of the module. This message could be displayed when creating a project, or editing the setting of a project releases.

The person creating a project wouldn't necessarily have access to add those terms in the first place, so I'm not sure that's the right place to put such a message, if we want one. Project release settings would be appropriate, however.

Setting appropriate component.

Also, #264406: Creating projects was marked as a duplicate of this issue.

Ok, the patch attached implements hook_help() similarly to how the project module does it and provides some helpful information about the API compatibility vocabulary.

Just what I was looking for - thanks.

After the latest patch is implemented new project releases are saved without errors, but not displayed on the main project page until I edit and resave the release. Editing any one of the releases fixes the problem.

@sjf: That's probably because the project release download tables are cached, and editing a release node causes the cache to be rebuilt. You should get the same results if you manually truncated the project release cache table or used the devel module to clear all caches.

Thanks for the fast reply. I think users will get confused if they add a new project release and it doesn't show up until the admin clears the cache. Is there a way for me to turn caching off just for project releases?

I think you're misunderstanding how the cache system works. You only ran into this problem because the cache for your project release download tables was set before you applied the patch which changes the code that builds the project release download tables. Now that you've applied this patch, any time this code runs and builds the release download table in the future, it will look like you expect it to. If you want to force a rebuild of all of the tables that are in the cache right now, you just need to do what I mentioned in #17. Once you do that, all download tables will be built using the new, patched code. Users won't get confused because the cache only needs to be cleared once (that is, unless you go and change the code again).

In general if you upgrade/modify modules that use caching, it's a good idea to clear your site's cache (or at least the tables used by the module you changed/upgraded).

I cleared the cache_project_release table and everything now works perfectly. Thanks for taking the time to explain that - very much appreciated.

(This is a reply to #7 which I somehow posted out of sequence... sorry)

There seems to be more to it than that... I added several terms, then changed the name of the category to "Status", and I still get the error....

Because of how I plan to use this module, I have preferred to call the content type 'Resource' rather than 'Project'.

Accordingly, I have renamed the Project Types category to Resource Types. However, when creating a 'project', the category title is not picked up but is hard-coded as 'Project Types'. It should reflect the name given to the category.

(More generally, not a bug as such, but an observation: the word 'project' is hard-coded all over the place, which could more properly be picked up from the name of the content type....)

(Not being well versed in PHP I am reluctant to go monkeying around in the code trying to fix this myself.)

@Rasputin Paracelsus: Please don't hijack this issue for something entirely unrelated. Instead, file a new issue (after searching first to make sure it's not a duplicate).

sorry. didn't do it on purpose. just got confused during posting and forgot to change the heading :(

but see #21, that was on topic...

@Rasputin Paracelsus: I don't have any idea what you're asking in comment #21. Comments in issues aren't threaded, so any reply will be at the bottom of the list. Can you please clarify your problem?

Oh, I see.

Ok--I installed Project and Project release, then tried to add a test project, and got the warning above "Unknown column 'prsv.snapshot'" etc.

As I understand the above comments, particularly #7, the warning is caused by the fact that API compatibility category items (and perhaps Project Types) have not been defined, and vanish if/when they have been.

I have created some items in both categories, but am still getting the warning message... I can of course apply the patch#9 above, but I am wondering why I get the message since I have in fact defined taxonomy terms.

@Rasputin Paracelsus: It's still hard to answer your question. It might be helpful if you could outline your project release api compatibility vocabulary and list the terms there.

Also, find the function project_release_compatibility_list() and add a line that prints $terms just before the function returns. If you have the devel module installed on your site, then the function would look like this:

function project_release_compatibility_list() {
  static $terms = array();
  if (empty($terms) && $tree = project_release_get_api_taxonomy()) {
    $tids = variable_get('project_release_active_compatibility_tids', '');
    $terms[-1] = '<'. t('all') .'>';
    foreach ($tree as $term) {
      if (($tids && !empty($tids[$term->tid])) || !$tids) {
        $terms[$term->tid] = $term->name;
      }
    }
  }
dsm($terms);
  return $terms;
}

If you don't have devel you can use print_r($terms) instead. Paste the output of $terms in a comment here so we can see what's going on.

Thanks.

Ok.

1. Your line dsm($terms) does not appear in my version of the function. Added print_r($terms) before the return.
2. Project releast API compatibility vocabulary: In Planning, In Process, Complete, Active, Retired (this is just about various kinds of files as opposed to Drupal modules). These are all the same level, no hierarchy.
3. Project types vocabulary: Programme, Sound File, Text File.
4. Created "Test Project" with project type "Programme". Got the error message. No extra output.
5. Created a dummy release "In Planning". Got no error message, no extra ouput.
6. Went under the navigation menu to Projects, saw: Array ( [-1] => )
7. Clicked on Programme, saw Array ( [-1] => ) Array ( [-1] => ). But nothing listed under Programme, as such. The Categories area was empty.
8. Clicked on Browse by Name, saw Array ( [-1] => ) Array ( [-1] => ). Nothing else listed. Ditto for Browse by Date.

The project and release do show up on the front page though (these content types promoted to front page).

Maybe I should mention that there seems to be something a bit fishy with categories in general; what should be expandable menus in taxonomy_dhtml and taxonomy_manager show up as terminal nodes and don't expand. I'm not sure whether that's relevant here but I mention it just in case (taxonomy_dhtml worked fine under Drupal 4.7, but doesn't now. Upgrade problem? Have posted issues but no response so far.)

NOTE: will be away for one week starting tomorrow, may or may not be able to keep in touch. FYI. Thanks for your help, much appreciated.

@Rasputin: Could you please test this attached patch to see if it fixes your problems.

After you test the patch, you'll also probably want to make sure that you have one or more of your api terms marked as active at admin/project/project-release-settings. But please test this patch first to make sure that it won't produce an error even if you have no terms marked as active (which I suspect is the case for your currently).

Thanks

Thank-you, very much appreciated.

I can't test it now, I'm just about to leave, but I will do so when I get back next week (I *may* be able to work on it while away).

(NOTE: I understand what patch does; but I don't have shell access and so far haven't figured out how to run the patch win32 utility on my own system. I'll work on this.)

Contrary to expectation I actually had some time available before leaving and managed to sort out "patch"--not straightforward or pleasant...

Applied the patch. Created a test project 2, type "Sound File", then a release "In Process", no errors!

Activated the Project API Compatibility terms, and now the project release show up in the menu under Browse by Date and Browse by Name... nothing under Browse by Category, though. (Presumably I am misunderstanding something about that tab? Something else needs to be set?)

Much appreciated again! Getting there... :)

Spoke too soon!

Decided to delete the test projects. Deleted the release, then when I clicked on the project to delete it, saw the following message:

"user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'prsv.snapshot = 1 ORDER BY td.weight, td.name, r.version_major DESC, r.version_' at line 1 query: SELECT DISTINCT(n.nid), n.created, tn.tid, td.name as api_term_name, prsv.supported, prsv.recommended, prsv.snapshot, r.* FROM node n INNER JOIN project_release_nodes r ON r.nid = n.nid INNER JOIN term_node tn ON n.nid = tn.nid AND tn.tid in (150,151,152,153,154) INNER JOIN term_data td ON td.tid = tn.tid INNER JOIN project_release_supported_versions prsv ON prsv.nid = r.pid AND prsv.tid = tn.tid AND prsv.major = r.version_major AND prsv.supported = 1 WHERE (r.pid = 445) AND (n.status = 1) AND r.rebuild = 1AND prsv.snapshot = 1 ORDER BY td.weight, td.name, r.version_major DESC, r.version_minor DESC, r.version_patch DESC, r.file_date DESC in /home/kfeuerhe/public_html/d5/includes/database.mysql.inc on line 172."

:(

@Rasputin: I recommend that you read the documentation for the project module at http://drupal.org/node/116527 if you have not done so already. That page should answer your last question in comment #31.

I will take a look at the db error you're getting when time permits.

aclight: thanks for your post.

as a former analyst programmer myself (but from the 'olden' days), i make it a point always to read documentation, though I don't claim to always catch all the nuances. thanks for encouraging me to reread: i saw this-- "The category option is relevant only if you've created second-level terms for a particular project type." which of course explains my confusion.

i may note however, that this lack of reaction when clicking on the browse by category tab may not be clear to users. maybe "subcategory" would be better?

thanks also for looking into the next database problem when you have the time. i am sure you have many issues to deal with!

best wishes,

R

@Rasputin: I wasn't able to reproduce the db error you mention in comment #32. Could you please try to reproduce this starting with a fresh Drupal installation and a fresh installation of the development version of the project module. If you're able to reproduce, please provide detailed step by step instructions so I can also reproduce.

FYI, here's a new patch that corrects some other db errors related to adding in fields to the query related to project release api terms. I don't think this patch will fix the problem you report in #32, but it might, so it would be great if you could also test it.

#279448: user warning: unkown table ... has been marked as a duplicate of this issue.

aclight: i will do my best, i hope this weekend. thanks

aclight:

Tried to patch using Win32 patch.exe

C:\unix\usr\local\wbin>patch --binary project.module 231614_project_no_terms_35.patch

got this:

patching file `project.module'
Hunk #4 succeeded at 654 (offset -1 lines).
Hunk #5 succeeded at 686 (offset -2 lines).
patching file `project_release.module'
Reversed (or previously applied) patch detected! Assume -R? [n]
Apply anyway? [n]
Skipping patch.
4 out of 4 hunks ignored -- saving rejects to project_release.module.rej

project_release.module had patch #29 above applied to it; should I have done this on the original (virgin) project_release?

Also: am deferring upgrade to drupal 5.8 pending resolution of this issue; have not created test drupal installation, want to resolve the patch issue first.

Thanks!

@Rasputin: That patch was created against HEAD, so unless that's what you apply the patch to you're likely to run into difficulties, as you have.

I don't believe that upgrading to Drupal 5.8 has anything to do with this patch, since the files changed by the upgrade will not be the same files this patch affects. However, applying patches to a test site instead of your live site is a good idea.

aclight: thank-you! will try it again.

aclight: applied patch to clean downloaded versions; created a test project, which generated the following error:

user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'prsv.snapshot = 1 ORDER BY td.weight, td.name, r.version_major DESC, r.version_' at line 1 query: SELECT DISTINCT(n.nid), n.created, tn.tid, td.name as api_term_name, prsv.supported, prsv.recommended, prsv.snapshot, r.* FROM node n INNER JOIN project_release_nodes r ON r.nid = n.nid INNER JOIN term_node tn ON n.nid = tn.nid AND tn.tid in (150,151,152,153,154) INNER JOIN term_data td ON td.tid = tn.tid INNER JOIN project_release_supported_versions prsv ON prsv.nid = r.pid AND prsv.tid = tn.tid AND prsv.major = r.version_major AND prsv.supported = 1 WHERE (r.pid = 467) AND (n.status = 1) AND r.rebuild = 1AND prsv.snapshot = 1 ORDER BY td.weight, td.name, r.version_major DESC, r.version_minor DESC, r.version_patch DESC, r.file_date DESC in /home/kfeuerhe/public_html/d5/includes/database.mysql.inc on line 172.

@Rasputin: I'm also not able to reproduce what you report in #41, but I see how my patch in #35 would cause that. Please try this patch.

aclight: seems to work!

Created test project, then test release; no errors. Then deleted test release, then deleted test project, no errors.

Would you like me to try anything else? it seems like you've resolved the problem, which is much appreciated... and I'm looking forward to beginning to use this feature :)

Thanks for all your effort!

Since test results from Rasputin validate this patch, I did a code review.

1. The first condition in if (variable_get('project_release_browse_versions', 0) && count($active_tids) > 1) seem superfluous. $active_tids only ever contains anything, if project_release_browse_versions was enabled.

2. Some of the newly added functions have phpdoc lead introductions which span multiple lines. This is against code standards.

These both seem to be minor issues, so this looks quite close to being ready.

re #44-1: You mean here:

+          // Make sure the API compatability term vocabulary exists and contains one
+          // or more terms (not including the term with key of -1, which is always
+          // automatically added to the array of terms. If not then browsing
+          // by version is not possible.
+          if (variable_get('project_release_browse_versions', 0) && count($active_tids) > 1) {

Are you saying that my code comment is wrong, or did you just not realize that it explains why the second check is necessary (at least I'm pretty sure it's necessary).

re #44-2: I've never heard that one before. There are plenty of counterexamples to this, both in project* as well as core. I think it's better to have a sufficiently detailed sentence describing the function (that's also <80 chars wide) than to have a hard and fast rule that it be one line only.

@aclight:

In (1) I was saying the first condition is not needed. From the looks of the code above, the second condition is only ever true if the first one was true.

In (2), I've seen dww has code style fixes targeted for the upcoming D5.x release, so this will need to be fixed sooner or later if we introduce it here. The one-line phpdoc rule is documented in the code style at http://drupal.org/node/1354 : "The first line of the block should contain a brief description of what the function does, beginning with a verb in the form "Do such and such" (rather than "Does such and such"). A longer description with usage notes may follow after a blank line." Druplicon on #drupal also calls for action on each function where a multiline intro exists if you ask for the function description:

[16:42] goba: drupal_get_form?
[16:42] Druplicon: drupal_get_form: [Documented summary is not one line. This is a bug. File a patch!] => drupal_get_form($form_id) => http://api.drupal.org/api/function/drupal_get_form/6

@Gabor: Oops, I misread your comment (1) before. So to make sure I understand you, you're saying that the check for variable_get('project_release_browse_versions', 0) is unnecessary because count($active_tids) > 1) implies that variable_get('project_release_browse_versions', 0) is TRUE? If so, it's not obvious to me why you say that.

Note that my patch is not adding the variable_get('project_release_browse_versions', 0) check --it's already there. Perhaps it has always been unnecessary? Note: I don't have the full code to look at right now, just my patch. If there is another check earlier on in the function, then I just don't remember it and you could be right.

re: one line summary. Ah, I see. I've always wondered why Druplicon says that. To be honest that doesn't really concern me much, as Druplicon isn't responding to function lookup requests for project* functions anyway. And the code style patch that's coming up has nothing to do (or very little to do) with Doxygen comments. It's more fundamental problems with whitespace or the lack thereof in various places. But if I can come up with something that is <~75 characters that adequately describes the function I'll reroll.

@aclight. Ok, excepts from your code:

  $active_tids = array();
   if (module_exists('project_release') && variable_get('project_release_browse_versions', 0)) {
     $version_form = drupal_get_form('project_release_version_filter_form');
     $output .= $version_form;
     // Read in requested version, if any.
     $version = isset($_SESSION['project_version']) ? $_SESSION['project_version'] : variable_get('project_release_overview', -1);
    $active_tids = project_release_compatibility_list();
   }
  // ...
  if (variable_get('project_release_browse_versions', 0) && count($active_tids) > 1) {
    // ...
  }

$active_tids only evet gets filled in, if (module_exists('project_release') && variable_get('project_release_browse_versions', 0)) is true. Checking again on variable_get('project_release_browse_versions', 0) is superfluous, because $active_tids would not contain any value, if this check fails. count($active_tids) > 1 already implies that the first check was true. It is as if you say if (green and (green and blue)). Do I miss something?

As far as the one line rule goes, it is not to make Druplicon happy, it is to have reasonable readable summaries in IDEs which are clever enough to parse the source code and hand you tips on functions you use. Examples include Zend Studio (http://www.joshbeall.com/personal/zendstudio.png) Delphi for PHP (http://dn.codegear.com/kr/article/images/37876/thumb03000012.png), PHPEdit (http://doc.waterproof.fr/phpedit/editing_files/code_hint/using_code_hint), Nuspehere PHPed (http://www.nusphere.com/php/cakephp5.htm) and others. This helps other developers working on the same codebase.

In other words for 1, your newly added $active_tids variable makes the variable_get() superfluous, it was not always superfluous.

+1 for 1 line summaries in PHPDoc. I fix that in project* whenever adding new functions, or otherwise fixing PHPdoc for something, but I haven't done a systematic rampage to clean up all remaining instances, since it's not important enough to spend time on separately. ;)

+1 on removing superfluous checks, although given this much discussion about it here, a short little inline comment would probably be useful to explain what's going on. Something like:

  // project_release_compatibility_list() only returns active tids if browsing by version is enabled.
  if (count($active_tids) > 1) {
    ...
  }

Thanks,
-Derek

1. removed the extraneous check for browsing by version, and fixed the associated comment.
2. i also cut if (!empty($active_tids) && count($active_tids) > 1) { down to if (count($active_tids) > 1) { for the same reason.
3. fixed the doxygen to be in line w/ coding standards.

Since the functionality was validated above already, and this does not change it, it looks ready to me.

Yup, I just gave this a final look and testing, it's indeed RTBC. aclight, care to do the honors? Thanks.

Committed the patch from #51 to HEAD and DRUPAL-5

http://drupal.org/cvs?commit=148882
http://drupal.org/cvs?commit=148880

I suspect this may be the problem with project-5.x-1.2 in my installation. Whenever I click on "Administer releases" on a project page, I get

user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ') ORDER BY td.weight, td.name' at line 1 query: SELECT prsv.*, td.name AS term_name FROM project_release_supported_versions prsv INNER JOIN term_data td ON prsv.tid = td.tid WHERE prsv.nid = 96 AND prsv.tid IN () ORDER BY td.weight, td.name in /var/www/PHP/drupal/includes/database.mysql.inc on line 174.

which gets logged as

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ') ORDER BY td.weight, td.name' at line 1 query: SELECT prsv.*, td.name AS term_name FROM.

I've gone to the point of checking that project_no_terms.patch could apply to the current download for project-5.x-1.2, and this bug is listed as "fixed" for project-5.x-1.x-dev. Does that mean it's in the queue to be offered as an update to project-5.x-1.2?

Or do I need to do something to get the fix? Is the patch file supposed to operate on an installed system with project-5.x-1.2 without breaking it? Would I then run update.php? Or is this out of the loop for project-5.x-1.2, and I need to open a new issue?

I'm very new to drupal, so maybe I need to find the site admin FAQ, if such a thing exists.

Dan Hoey
Hoey@AIC.NRL.Navy.Mil

Yes, it means that if you try using the latest 5.x-1.x-dev release, the bug should go away. Once we finish up a few other known issues, we'll create a 5.x-1.3 release that includes this fix.

Cheers,
-Derek