621255 - Params without a sortkey should sort to the end

Status: RESOLVED FIXED

(Bugzilla :: Administration, task)

Description

[Gervase Markham [:gerv]]

Currently, if you add a new param panel and don't specify a sortkey, it sorts to the top, between Index and Required Settings. I suspect that is almost never the desired behaviour. It would be better if the default were to sort to the end.

Gerv

Comment 1

[Gervase Markham [:gerv]]

Attachment: Patch v.1

Comment 2

[David Lawrence [:dkl]]

(In reply to comment #1)
> Created attachment 499617 [details] [diff] [review]
> Patch v.1

Gerv, this looks good and worked for me for the most part. The only issue I have with it is when someone has $sortkey set to 0 in their Config.pm meaning they want the menu item to display at the "top" of the list. With this patch it will still display at the bottom. Is it possible for your patch to check for defined instead of a 0 or false value?

Dave

Comment 3

[Gervase Markham [:gerv]]

Attachment: Patch v.2

Good catch. Try this instead.

Gerv

Comment 4

[David Lawrence [:dkl]]

Comment on attachment 500185 [details] [diff] [review]
Patch v.2

Works for me. r=dkl

Comment 5

[Frédéric Buclin]

Comment on attachment 500185 [details] [diff] [review]
Patch v.2

>+    defined($item->{'sortkey'}) || ($item->{'sortkey'} = 100000);

This would be much clearer as $item->{sortkey} ||= 100000 as we have no sortkey = 0.

Comment 6

[Gervase Markham [:gerv]]

LpSolit: so are you disagreeing with comment 2?

Gerv

Comment 7

[Frédéric Buclin]

(In reply to comment #2)
> Gerv, this looks good and worked for me for the most part. The only issue I
> have with it is when someone has $sortkey set to 0 in their Config.pm meaning
> they want the menu item to display at the "top" of the list.

We have no such sortkey.

Comment 8

[Frédéric Buclin]

(In reply to comment #6)
> LpSolit: so are you disagreeing with comment 2?

Yes.

Comment 9

[Gervase Markham [:gerv]]

Comment on attachment 499617 [details] [diff] [review]
Patch v.1

OK, back to plan A.

Gerv

Comment 10

[David Lawrence [:dkl]]

Comment on attachment 499617 [details] [diff] [review]
Patch v.1

Well is LpSolit says that extensions should never need to have a $sortkey equal to 0 (which I am not so sure), then this works fine for me. r=dkl

Comment 11

[Frédéric Buclin]

(In reply to comment #10)
> Well is LpSolit says that extensions should never need to have a $sortkey equal
> to 0 (which I am not so sure), then this works fine for me. r=dkl

Sortkeys are currently hardcoded and none is 0. Also, we never guaranteed that a "false" sortkey (from a Perl point of view, i.e. undef, "" or 0) would be treated correctly. So I much prefer to enforce this rule than to get unexpected behaviors.

Comment 12

[Gervase Markham [:gerv]]

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified editparams.cgi
Committed revision 7677.

Gerv