342998 - AMO app-version management does not permit versions such as "2.0a1"

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Administration, defect)

Description

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Firefox has releases in the wild with such version strings.  AMO won't let me put them in, though, so we have problems.  (We had "2.0" as a legal version until quite recently, which was an even bigger problem.)

This is a pretty serious issue for us right now, as people update extensions to work with Bon Echo, and will only become more so when beta1 ships and we really start to push on extension authors to update their stuff.

I didn't want to reopen bug 313605, though the case in the summary indeed still present, and I don't know if we need the full Monty from 283803, so I'm filing here, because I care a lot about this specific problem, and a lot less about the unified, standardized, one-true-versioning system being in place.

Comment 1

[Michael Morgan [:morgamic]]

Attachment: Diff for v1 admin portion -- removing dependencies on minor/major/release/build/subver fields.

Comment 2

[Michael Morgan [:morgamic]]

Attachment: v2 diff - only faq were affected, fairly simple adjustments.

Comment 3

[Michael Morgan [:morgamic]]

I was able to:
* upload a 2.0a1 (or any arbitrary version string that matches something in the applications table) from the dev cp, then approve it
* view the faq in the public site

Note - I've got mixed patches here (see bug 302287).  I'm fairly certain the crypto hashes patches are fine -- so I'd like to just get an approval on all of these.

Nonetheless I'm placing a dependency on the crypto hashes for now so we can get that out the door as well.

Comment 4

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 227503 [details] [diff] [review]
v2 diff - only faq were affected, fairly simple adjustments.

>Index: public/tpl/faq.tpl
>===================================================================
>RCS file: /cvsroot/mozilla/webtools/addons/public/tpl/faq.tpl,v
>retrieving revision 1.4
>diff -u -p -8 -r1.4 faq.tpl
>--- public/tpl/faq.tpl	6 May 2006 10:10:40 -0000	1.4
>+++ public/tpl/faq.tpl	29 Jun 2006 04:38:33 -0000
>@@ -53,18 +53,17 @@ Software.  Browse plug-ins for:
> {/section}
> </dl>
> 
> <h2>Valid App Versions for Addon Developers</h2>
> 
> <table class="appversions">
> <tr>
>     <th>Application Name</th>
>-    <th>Display Version</th>
>-    <th>install.rdf Version</th>
>+    <th>Version</th>
>     <th>GUID</th>
> </tr>
> 
> {foreach item=app from=$appVersions}
> <tr>
> <td>{$app.appName}</td>
> <td>{$app.displayVersion}</td>
> <td>{$app.versionNumber}</td>
>Index: public/htdocs/faq.php

You net removed a column from the header, but didn't change the template that generates the rows of the table -- is that right?

If so, r=shaver.  If not, fix that and r=shaver. :)

Comment 5

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 227502 [details] [diff] [review]
Diff for v1 admin portion -- removing dependencies on minor/major/release/build/subver fields.

I'll do a real review when I get back from dinner, one question so far:

Does "ORDER BY Version DESC" sort how we want?  I thought we determined that it didn't, but I'd love to be wrong.

Comment 6

[Michael Morgan [:morgamic]]

(In reply to comment #5)
> Does "ORDER BY Version DESC" sort how we want?  I thought we determined that it
> didn't, but I'd love to be wrong.
The sort should be okay after all -- there isn't something that will break if the char sorting messes things up, the worst-case is a couple extra iterations until the additem script finds a version string match. 

Re comment#4, yes, I missed a column.  Fixed that.

Comment 7

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 227502 [details] [diff] [review]
Diff for v1 admin portion -- removing dependencies on minor/major/release/build/subver fields.

Thanks, r=shaver.

Comment 8

[Michael Morgan [:morgamic]]

Code is checked in, waiting for update in bug 343225.