Bad field order numbers in trac.ini cause problems for admin form

[Jenn Drummond]

I had three custom fields in trac.ini, with orders of 2, 3, and 4. While that's not really ideal, it does work without any trouble (I think I did it when I was trying to get things to appear in one column or the other). This causes several problems, but hopefully not unfixable ones.

1. When I created a new custom field, it was assigned the number 4. Presumably the plugin counted my custom fields and assigned count + 1 as the new order number. But then if I try to edit anything with the custom fields !admin form, it complains that order numbers must be unique. Suggestion: take the maximum of the list of order numbers instead of the length, and add one to that.

2. The options in the dropdowns only contain the set of preexisting order numbers; in my case, 2, 3, and 4 (or, after adding one, 2, 3, 4 and 4!). This prevents the user from fixing the problem through the plugin form by changing them to 1, 2, and 3. Suggestion: take the maximum order number and generate the list of select options as the range between 1 and that number. Or if you're worried that someone will come in with a single custom field numbered 500, just make sure that all the numbers from 1 to (number-of-fields) are represented in addition to the actual numbers coming in from trac.ini.

[osimons]

I could change, and that is of course quite trivial - likely a change from len() to max() or something. Then again, I didn't want the plugin to do anything but the straight-forward creation, deletion and reordering that it handles ok. Anyone needing special can just edit the values in trac.ini directly anyway. Or, if the order is somehow wonky, just deleting the fields and re-creating them will also fix the order.

The same issue kept appearing all over for all the other Trac enum values such as priority, severity, etc. Recently these were stamped out to close all the gaps to keep a 'straight' order. See: ​ticket:2876 and ​ticket:4982.

I'll keep it open for the day I look at that code again, but won't make any promises on this one...

[Jenn Drummond]

S'alright; as you say, plenty of workarounds. Just wanted to mention it.

[Cameron Junge]

Oops, I'd say that the bug I just logged (#3193) is related or a duplicate of this one.

I think this is quite important, as the behaviour is all wonky!

[osimons]

Replying to cjunge@author-it.com:

Oops, I'd say that the bug I just logged (#3193) is related or a duplicate of this one.

Yes indeed. That ticket is now closed as duplicate.

[anonymous]

This is something I would love to see fixed also.

The reason being, in my case, those that can admin most of the things on our server, can't actually access the server directly. So, without fail, I always get a "can you add filed x to project y" requests in my mail box. This would be the final enabling feature. "Teach a man to fish..." My numbers are a little wonky for formatting, as well as half of them are contained in an include file.

[osimons]

Patch to fix generating and editing custom field order.

[osimons]

I've made a stab at fixing this. t2363_cf-order_011.diff​ essentially...

• fixes correct order for newly created field
• enumerates the count of fields as drop-down alternatives + possibly the actual value if that doesn't fall in regular range
• removes the check for uniqueness as it isn't really a 'critical error' - and now easily fixed anyway

The patch is against latest 0.11, and please test it and give me feedback on how it works.

[Jenn Drummond]

Behavior still seems a bit odd -- though definitely better!

If I set orders of 1, 3, 4, and 5 in trac.ini, I get the following dropdowns. Asterisks indicate the initially-set value.

Field A:  0  1* 2  3
Field B:  0  1  2* 3
Field C:  0  1  2  3* 4 
Field D:  0* 1  2  3  5

Fields B and C are set to 3 and 4 in trac.ini, and are being reset to 2 and 3 on the form. This makes some sense. But Field D is set to 5 in trac.ini, is showing up as 0 on the form, and in fact cannot be set to the reasonable value of 4!

I would expect the dropdowns each to have the numbers 1, 2, 3, 4, and 5 (and not 0), and for the initial values of the form to read 1, 3, 4, 5 like they do in trac.ini. My proposal was to leave the reordering to the user, not to do it automatically, since they're the ones who presumably messed up trac.ini in the first place -- though I know you're trying to bring this code in line with gap-closing behavior in other areas.

The zeroes are also a little surprising, especially since it's the last item that's defaulting to zero...maybe the presence of the zeroes is just an off-by-one error, and you really meant the last field to have 12345 instead of 01235?

Thanks for tackling the problem, though; seems to be getting better!

[osimons]

Thanks for testing!

First, the basic code (and Trac layout rendering) will have 0 as first, and in your example 3 as last. So, by creating and editing fields with the plugin only, you will have your range of 0 1 2 3 as your options.

The reason for 4 and 5 (pre-existing) is that whatever number is not in the 'basic' order count gets added to display existing field correctly. So order 4 won't be an option, nor is adding whatever numbers are inbetween - as someone using order 10 50 100 300 would create a stupid list.

As for the 0 you noticed, and the sense of automatic reorder, that is strange. I can't replicate that. The plugin subtracts -1 on higher-than-deleted options, but it does not auto-change anything. Nor does the -1 impact the actual order at all, other than trying to bring/keep the number used in range. Could you try to figure out why/how Field D gets 0 selected and 5 available in drop-down?

Anyway, I've made a minor change to the patch to visually improve the confusing range of numbers. Could you look at it and see if adding a divider in front of the not-in-range number makes it more understandable?

[osimons]

Same patch, but with minor drop-down visual improvement.

[andrew.small@…]

Also, if the hand edited trac.ini fields didn't state the order, then the fields are listed with a droplist of 0 0 0. While new fields are 'max' numbered, e.g. 0 0 0 4, the unique order warning is shown on committing the settings. It's not currently possible to correct the order within the plugin UI.

[osimons]

Replying to andrew.small@renesas.com:

Also, if the hand edited trac.ini fields didn't state the order, then the fields are listed with a droplist of 0 0 0. While new fields are 'max' numbered, e.g. 0 0 0 4, the unique order warning is shown on committing the settings. It's not currently possible to correct the order within the plugin UI.

Did you try the patch for 0.11 that attempts to correct this?

[osimons]

Patch committed as part of major cleanup of code in [4964].