New custom ticket fields are set incorrectly on old tickets

[nikolaus.wittenstein@…]

If I create a new checkbox field in trac.ini like so:

test_checkbox = checkbox
test_checkbox.label = Test checkbox
test_checkbox.value = 1
test_checkbox.order = -1

then for all new tickets the value is correctly set to 1 (checked). However, on old tickets when the page loads (i.e. /ticket/#) the fields are only populated if they're already set on the ticket. This means that the test checkbox doesn't get set at all, so it stays unchecked. Then when the ticket it saved, unchecked isn't equal to no value, so the custom checkbox gets updated to unchecked even though I didn't change its value at all. I'm running Trac 0.12dev-r7331, if it matters.

[Remy Blank <remy.blank@…>]

This issue is tricky. I was going to set unset custom fields to the default value when getting a ticket from the database (in model.py). But this is wrong, as it gives an inconsistent view of the ticket data:

• In the ticket view, the custom field looks as if it was set to the default value
• In the database, the field is unset
• As queries work directly on the database, they see that the custom field is unset, and therefore display it as unset
• Changing the default value in trac.ini would instantaneously change the value of the custom field in the views of all tickets where the field is unset.

Instead, what should probably be done is to detect an unset field in the ticket view, and use the default value to set the form control only.

[nikolaus.wittenstein@…]

I was going to set unset custom fields to the default value when getting a ticket from the database (in model.py).

Is there some reason one wouldn't want it to change the ticket in the database on loading it?
That is, if model.py reads the ticket from the database and notices that some fields are unset, could it set those fields to their defaults in the ticket object, write it back to the database as a change, and then serve the ticket to whomever is asking for it? It could even just write it straight to ticket_custom, bypassing the whole ticket_change thing so that setting the field to the default doesn't show up as a change but rather happens invisibly—just as for a new ticket.
This would also have the effect that if I then save a ticket page where only a form control got set there's no potential for confusion when the page refreshes and it says I changed something that I really didn't change.

[Remy Blank <remy.blank@…>]

Patch against 0.11-stable setting unset custom field controls to the default value

[Remy Blank <remy.blank@…>]

This patch should fix the issue:

• Every control in the ticket properties is set to the field value if it is set, or to the default value if it isn't.
• The fields at the top of the ticket (yellow box) show the real values as stored in the database.

I also thought about changing the values in the DB at the next read, but I find it confusing that displaying a ticket would change its properties. Another option would be to add an admin panel that allows setting all unset custom fields in the database in one operation. This could be done in addition to this patch.

Oh wait, I see you are running 0.12. Let me see... yes, the patch applies cleanly to [7389/trunk] as well. Could you please test the patch on your installation?

[nikolaus.wittenstein@…]

I just tried it by making a new checkbox field with a default value of 1 and it worked great. When I submitted, the change was saved, as intended. So yes, the patch does in fact work on 0.12.

I'm not sure exactly what to do regarding closing tickets here (so I'll be leaving it open), but I'd consider this matter resolved. Thanks very much.

[Remy Blank <remy.blank@…>]

Yes, it should be left open. I am not a core developer, so I will need one of the devs to review the patch and either apply it or reject it. They normally monitor the changes to tickets, but if there is no reaction for a few days, I'll ask on the trac-dev mailing list.

[osimons]

Patch looks good - untested, but it 'reads well'. It shouldn't do more than fix diplay and update on next save - changing DB when model fetches a ticket would indeed be a no-no.

[rblank]

Patch applied in [7463].