Edgewall Software
Modify

Opened 10 months ago

Closed 4 months ago

Last modified 4 months ago

#13133 closed defect (fixed)

Do not add ticket change entry for default value of new ticket custom field

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.3.6
Component: ticket system Version:
Severity: normal Keywords: custom fields
Cc: Branch:
Release Notes:

Ticket change entry for default value of custom field is not added to existing tickets when a new custom field is added.

API Changes:

Description (last modified by Ryan J Ollos)

Adding a new custom field, such as:

[ticket-custom]
checkbox = checkbox
checkbox.value = 0

results in a ticket change entry the next time an existing ticket is updated.

I'd like to discuss if it makes sense to skip adding the ticket_change table entry when the ticket custom field has a default value.

Issue raised in gdiscussion:trac-users:Kind9BaSR4Q.

Attachments (1)

Screen Shot 2019-02-25 at 13.54.03.jpg (22.1 KB ) - added by Ryan J Ollos 10 months ago.

Download all attachments as: .zip

Change History (15)

by Ryan J Ollos, 10 months ago

comment:1 by Ryan J Ollos, 10 months ago

Description: modified (diff)

comment:2 by Ryan J Ollos, 5 months ago

Milestone: next-stable-1.2.x1.2.5

comment:3 by Ryan J Ollos, 5 months ago

Owner: set to Ryan J Ollos
Status: newassigned

comment:4 by Ryan J Ollos, 4 months ago

Milestone: 1.2.51.3.6

comment:5 by Ryan J Ollos, 4 months ago

Release Notes: modified (diff)

comment:6 by Ryan J Ollos, 4 months ago

Resolution: fixed
Status: assignedclosed

Changes committed in r17027, r17028.

in reply to:  6 comment:7 by Jun Omae, 4 months ago

Replying to Ryan J Ollos:

Changes committed in r17027, r17028.

If a time field is updated from None to the default value, the field is not listed in changelog.

>>> from datetime import datetime
>>> from trac.util.datefmt import utc
>>> from trac.test import EnvironmentStub
>>> from trac.ticket.model import Ticket
>>> env = EnvironmentStub()
>>> env.config.set('ticket-custom', 'date1', 'time')
>>> env.config.set('ticket-custom', 'date1.value', '2019-08-07T12:34:56Z')
>>> env.config.set('ticket-custom', 'date1.format', 'datetime')
>>> env.config.save()
>>> t = Ticket(env)
>>> t['summary'] = 'Summ'
>>> t['status'] = 'new'
>>> t['date1'] = None
>>> t.insert()
1
>>> env.db_query('SELECT * FROM ticket_custom WHERE ticket=1')
[(1, u'date1', None)]
>>>
>>> t = Ticket(env, 1)
>>> t['date1'] = datetime(2019, 8, 7, 12, 34, 56, tzinfo=utc)
>>> t.save_changes(comment='Set date1')  # Saving with the default value for `date1` field
1
>>> env.db_query('SELECT * FROM ticket_custom WHERE ticket=1')
[(1, u'date1', u'001565181296000000')]
>>> list(t.get_changelog())  # `date1` field is not listed
[(datetime.datetime(2019, 8, 8, 4, 42, 35, 873743, tzinfo=<FixedOffset "UTC" 0:00:00>), None, u'comment', u'1', u'Set date1', 1)]

comment:8 by Ryan J Ollos, 4 months ago

Proposed change:

  • trac/ticket/model.py

    diff --git a/trac/ticket/model.py b/trac/ticket/model.py
    index 7af629c86..170503486 100644
    a b class Ticket(object):  
    399399                        db("""INSERT INTO ticket_custom (ticket,name,value)
    400400                              VALUES(%s,%s,%s)
    401401                              """, (self.id, name, db_val))
    402                     # Don't add ticket change entry for custom field that
    403                     # was added after ticket was created.
    404                     if old_db_val is None:
    405                         field = self.fields.by_name(name)
    406                         default = self._custom_field_default(field)
    407                         if self.values.get(name) == default:
    408                             continue
     402                        # Don't add ticket change entry for custom field that
     403                        # was added after ticket was created.
     404                        if old_db_val is None:
     405                            field = self.fields.by_name(name)
     406                            default = self._custom_field_default(field)
     407                            if self.values.get(name) == default:
     408                                continue
    409409                else:
    410410                    db("UPDATE ticket SET %s=%%s WHERE id=%%s"
    411411                       % name, (db_val, self.id))

I'll add test coverage.

comment:9 by Ryan J Ollos, 4 months ago

Proposed changes in [af0276770/rjollos.git].

comment:10 by Ryan J Ollos, 4 months ago

Regression fix committed in r17047.

comment:11 by Ryan J Ollos, 4 months ago

CI Test failure for PostgreSQL on OSX. Should we order the changelog by field?

  • trac/ticket/model.py

    diff --git a/trac/ticket/model.py b/trac/ticket/model.py
    index 170503486..33b903041 100644
    a b class Ticket(object):  
    462462                SELECT time, author, 'comment', null, description,
    463463                  0 AS permanent
    464464                FROM attachment WHERE type='ticket' AND id=%s AND time=%s
    465                 ORDER BY time,permanent,author
     465                ORDER BY time,permanent,author,field
    466466                """
    467467            args = (self.id, when_ts, sid, when_ts, sid, when_ts)
    468468        else:
    class Ticket(object):  
    477477                SELECT time, author, 'comment', null, description,
    478478                  0 AS permanent
    479479                FROM attachment WHERE type='ticket' AND id=%s
    480                 ORDER BY time,permanent,author
     480                ORDER BY time,permanent,author,field
    481481                """
    482482            args = (self.id, sid, sid)
    483483        log = []

comment:12 by Ryan J Ollos, 4 months ago

Proposed changes in [91ede4ff0/rjollos.git], targeted to 1.0-stable. In the meantime, I've applied a fix in r17052.

in reply to:  12 comment:13 by Jun Omae, 4 months ago

Proposed changes in [91ede4ff0/rjollos.git], targeted to 1.0-stable. In the meantime, I've applied a fix in r17052.

The changes look good to me.

Trivial thing, Ticket.get_changelog() is invoked twice in trunk:trac/ticket/tests/model.py.

-        changelog = sorted(ticket.get_changelog())
-        self.assertEqual(7, len(ticket.get_changelog()))
+        changelog = ticket.get_changelog()
+        self.assertEqual(7, len(changelog))

comment:14 by Ryan J Ollos, 4 months ago

Thanks for the review. Committed to 1.0-stable in r17060, merged in r17061, r17062 (including comment:13 change).

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.
to as closed The owner will be changed from Ryan J Ollos 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.