Edgewall Software
Modify

Opened 16 years ago

Closed 16 years ago

Last modified 16 years ago

#7465 closed defect (fixed)

New custom ticket fields are set incorrectly on old tickets

Reported by: nikolaus.wittenstein@… Owned by: Remy Blank
Priority: low Milestone: 0.11.2
Component: ticket system Version: 0.12dev
Severity: minor Keywords: ticket custom patch
Cc: remy.blank@… Branch:
Release Notes:
API Changes:
Internal Changes:

Description

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.

Attachments (1)

7465-custom-field-default-r7388.patch (4.5 KB ) - added by Remy Blank <remy.blank@…> 16 years ago.
Patch against 0.11-stable setting unset custom field controls to the default value

Download all attachments as: .zip

Change History (13)

comment:1 by Remy Blank <remy.blank@…>, 16 years ago

Cc: remy.blank@… added

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.

comment:2 by nikolaus.wittenstein@…, 16 years ago

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.

by Remy Blank <remy.blank@…>, 16 years ago

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

comment:3 by Remy Blank <remy.blank@…>, 16 years ago

Keywords: patch added; field removed

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?

comment:4 by nikolaus.wittenstein@…, 16 years ago

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.

comment:5 by Remy Blank <remy.blank@…>, 16 years ago

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.

comment:6 by osimons, 16 years ago

Milestone: 0.11.2

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.

comment:7 by Remy Blank, 16 years ago

Resolution: fixed
Status: newclosed

Patch applied in [7463].

comment:8 by Christian Boos, 16 years ago

Resolution: fixed
Status: closedreopened

comment:9 by Christian Boos, 16 years ago

Owner: set to Remy Blank
Status: reopenednew

comment:10 by Christian Boos, 16 years ago

Resolution: fixed
Status: newclosed

fixing owner

in reply to:  10 comment:11 by Remy Blank, 16 years ago

Replying to cboos:

fixing owner

I have always wondered why the owner shouldn't be directly editable if a user has TICKET_ADMIN. Would this be a desirable feature? (Yes, I know, it interferes with the workflow, that's why I suggest this be only available to TICKET_ADMIN)

comment:12 by Christian Boos, 16 years ago

Well, that could be done by customizing the workflow:

20	; Allow correcting the ownership of a closed ticket.
21	change_owner = closed -> closed
22	change_owner.name = change ownership
23	change_owner.operations = set_owner
24	change_owner.permissions = TICKET_MODIFY

(taken from source:contrib/workflow/opensource-workflow.ini)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Remy Blank.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Remy Blank to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.