Closed Bug 256004 Opened 19 years ago Closed 19 years ago

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

Categories

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

2.18
defect

Tracking

()

RESOLVED FIXED
Bugzilla 2.18

People

(Reporter: michetti, Assigned: justdave)

Details

(Keywords: regression)

Attachments

(1 file)

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:
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking2.18?
Keywords: regression
Priority: -- → P2
Target Milestone: --- → Bugzilla 2.18
Version: unspecified → 2.18
Ouch, nice catch.
Flags: blocking2.18? → blocking2.18+
Whiteboard: not a blocker?
Attached patch Patch v1Splinter Review
This restores the previous behavior.  As a side effect, it'll also clean up any
damage done by anyone already bit by it.
Attachment #158533 - Flags: review?(zach)
Hardware: PC → All
Whiteboard: not a blocker?
Whiteboard: patch awaiting review
Joel, comment #1, can you explain how does this create duplicate sortkeys?
(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 on attachment 158533 [details] [diff] [review]
Patch v1

Yup, this works. r=myk
Attachment #158533 - Flags: review?(zach) → review+
Whiteboard: patch awaiting review → patch awaiting checkin
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
Status: NEW → RESOLVED
Closed: 19 years ago
Flags: approval2.18+
Flags: approval+
Resolution: --- → FIXED
Whiteboard: patch awaiting checkin
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.