currently drupal.org is using a hack to get around the fact that there's a release node that's been created that we don't want people using as a valid release.

attached is an attempt to turn this hack into a simple conf setting.

Comments

dww’s picture

Status: Needs review » Needs work

Cool, thanks. No UI for it? Otherwise, looks good to my eyes, though I should probably test it a little before I call it RTBC. ;)

hunmonk’s picture

Status: Needs work » Needs review
StatusFileSize
new2.38 KB

i honestly think a UI for this particular setting is overkill, since it can be useful, but most likely will be rarely used. i've added some code comments to clarify what's happening when the variable is used.

IMO we should leave this as a conf only setting for now -- the UI for it would be unneeded noise for most admins.

aclight’s picture

StatusFileSize
new2.85 KB

This patch doesn't change anything in hunmonk's patch but fixes a line right above the relevant part that currently uses a not-so-obvious syntax:

-          empty($rid) and form_set_error('project_info][rid', t('You have to specify a valid version.'));
+          if (empty($rid)) {
+            form_set_error('project_info][rid', t('You have to specify a valid version.'));
+          }
aclight’s picture

Oh, right, I also reviewed hunmonk's patch and it looks good to me. Can we put this up on project.drupal.org and remove the current hack (if it's on there right now) for testing? Testing this on a local test install isn't real easy.

dww’s picture

Status: Needs review » Needs work

Hrm, just tried this locally and we're never hitting the error message in this patch ("%version is not a valid version, please select a different value."). I tried it on d.o and the same is true there. :( This seems to have all changed with IFAC and I just never noticed. ;) Basically, once you rip the invalid value out of the list of available version drop down, when you load an issue with an invalid version, the version dropdown is pre-filled to just "none", and the only error you see if you try to post without changing it is "You have to specify a valid version.".

So, either:

A) we should get this working so that if we hit the no-version case, but the current value of the rid in the issue is invalid, we trigger the warning

or

B) the error message part of the patch is dead code and should be ripped out entirely.

I prefer (A) from a usability standpoint, but (B) is probably easier for the code. I'm torn.

hunmonk’s picture

Status: Needs work » Needs review
StatusFileSize
new1.4 KB

here's B. i'm not movitated to work on A, and i think this solution is fine given the issue.

dww’s picture

Status: Needs review » Fixed
StatusFileSize
new2.96 KB

I ended up implementing (A), committing, and deploying on d.o. See attached patch if you care.

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.