Edgewall Software
Modify

Opened 11 years ago

Last modified 4 years ago

#7850 new enhancement

[patch] Update default_<enum> options in trac.ini when enum value is updated/deleted

Reported by: ebray Owned by:
Priority: normal Milestone: next-major-releases
Component: ticket system Version: devel
Severity: normal Keywords: patch
Cc: macke@… Branch:
Release Notes:
API Changes:

Description

I had a user complained that he deleted a component, but that the component was still showing up as the default when creating new tickets.

I explained that this is the expected behavior, since the default_component is still set to that component, causing it to be displayed. But he pointed it that it's somewhat user-hostile to keep a default_component setting for a component that no longer exists, and I'm inclined to agree.

Likewise the default_ setting should be updated when an enum value is updated.

Attachments (1)

update_default_enum-r7733.patch (3.8 KB ) - added by ebray 11 years ago.
Patch to delete default_ options in trac.ini for enum fields when the default's value is deleted from the enum. Likewise updates the default if the default's value was updated in the enum. (I realize that sounds confusing.)

Download all attachments as: .zip

Change History (11)

by ebray, 11 years ago

Patch to delete default_ options in trac.ini for enum fields when the default's value is deleted from the enum. Likewise updates the default if the default's value was updated in the enum. (I realize that sounds confusing.)

comment:1 by Christian Boos, 11 years ago

The problem comes from the fact that those defaults are managed in the .ini file but also in the DB. IIRC, Remy has already raised the point that we should rather rationalize this and have those defaults maintained in one place, preferably the DB (Remy correct me if I got you wrong).

in reply to:  1 comment:2 by ebray, 11 years ago

Replying to cboos:

The problem comes from the fact that those defaults are managed in the .ini file but also in the DB. IIRC.

Not so. The default settings are only in trac.ini. But regardless of where they're stored, they aren't updated/deleted if they're set to an enum value that's updated/deleted.

comment:3 by Christian Boos, 11 years ago

Ok, but nevertheless, having the base data in the DB, then some metadata coming from the trac.ini is error prone. Furthermore, the way you tried to address the deletion won't work in the presence of an inherited setting.

It's not only about the default values, we're going to have the same update issue with i18n of those enum names. I was hesitating whether we should take the translations from the .ini files or from a dedicated table, now this ticket makes me more inclined to store them in the db and extend the admin panel so that translations for the terms could be added.

comment:4 by ebray, 11 years ago

True, the deletion won't work with an inherited setting, but that shouldn't matter. Users (with TICKET_ADMIN) can set the defaults through the Admin interface, and this adds a default_ option to the environment's local trac.ini. With this patch, deleting an enum value that causes the default_ option to be deleted from the local trac.ini will mean going back to whatever the global default is. This is still error prone, sure. But less so than having a default_ setting in the local trac.ini set to an enum value that's definitely no longer valid.

If you wanted to add a 'default' column to the enum table that would work too I suppose. I use a plugin that allows the creation of additional custom enums in the environment, and it stores the default in the database. Much easier to deal with it this way. Of course, then you lose the convenience of being able to set it in the trac.ini, but I never allow users to touch that directly anyways. And if they can do it through the Admin UI, then it's no problem.

comment:5 by Marcus Lindblom <macke@…>, 11 years ago

See #8023, which is a similar bug caused by the same behaviour. (for severities and milestones.)

At the very least, a big warning saying 'the current default in trac.ini does not exist' would've been nice. :)

comment:6 by Marcus Lindblom <macke@…>, 11 years ago

Cc: macke@… added

comment:7 by Christian Boos, 11 years ago

Milestone: 0.13

comment:8 by Ryan J Ollos, 6 years ago

Milestone: next-major-releasesnext-dev-1.1.x

The current behavior is that the new ticket form select elements default to the first entry in the list when the default_ value does not exist.

I agree that the ticket property default value should be updated when it's corresponding object is renamed or deleted, and that the default value should be stored in the database. A warning (comment:5) would otherwise be a good idea, but if the default is a column in the database, it won't be possible to have an invalid value. If we implement this as a column in the database, the default value will also be preserved when the item is renamed.

  • An upgrade script will be needed to get the default value from trac.ini and add it to the database.
  • A trac-admin command should allow setting this from the command line, which address the concern about not being able to set the value in trac.ini, assuming the concern is about having some way to easily set the value from an installation script or from the command line (comment:4).

comment:9 by Ryan J Ollos, 5 years ago

Milestone: next-dev-1.1.xnext-major-releases

Retargetting tickets to narrow focus for milestone:1.2. Please move the ticket back to milestone:next-dev-1.1.x if you intend to resolve it by milestone:1.2.

comment:10 by figaro, 4 years ago

Keywords: patch added

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The ticket will remain with no owner.
The ticket will be disowned. Next status will be 'new'.
as The resolution will be set. Next status will be 'closed'.
The owner will be changed from (none) to anonymous. Next status will be 'assigned'.

Add Comment


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