256004 - new fields added after runing checksetup.pl will have wrong sort key in table fielddefs

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect, P2)

Description

[Alexandre Michetti Manduca]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8a2) Gecko/20040612 Firefox/0.8.0+
Build Identifier: 

Bug 190432 put $headernum++ inside if (!$fieldid) in AddFDef.
This cause new fields (added after running checksetup with some patch for
example)  to have a wrong sortkey.

Reproducible: Always
Steps to Reproduce:

Comment 1

[Joel Peshkin]

This results in duplicate sortkeys.  It is unclear how much of a problem that
makes, but it should be fixed before it leaves too much of a mess.

Comment 2

[Christian Reis]

Ouch, nice catch.

Comment 3

[Dave Miller [:justdave]]

Attachment: Patch v1

This restores the previous behavior.  As a side effect, it'll also clean up any
damage done by anyone already bit by it.

Comment 4

[Vlad Dascalu]

Joel, comment #1, can you explain how does this create duplicate sortkeys?

Comment 5

[Dave Miller [:justdave]]

(In reply to comment #4)
> Joel, comment #1, can you explain how does this create duplicate sortkeys?

The original behavior was when checksetup.pl got run, new fields would be
inserted, and existing fields would have their sortkeys adjusted so that all of
the fields were still in the order defined by checksetup.pl.

Bug 109432 accidently changed this so the "current sortkey" (aka $headernum)
only got incremented if a NEW field was added, and it didn't update it when an
existing field was already there.

This would cause any new fields getting added to start with a sortkey of 1, no
matter what was already in the database, so if fields get added over time one at
a time, you'd wind up with several fields with a sortkey of 1.

This patch restores the original behavior, so the entire table's sortkeys are
updated every time checksetup runs.

Comment 6

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 158533 [details] [diff] [review]
Patch v1

Yup, this works. r=myk

Comment 7

[Dave Miller [:justdave]]

Trunk:

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.305; previous revision: 1.304
done

2.18 branch:

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.289.2.7; previous revision: 1.289.2.6
done