Edgewall Software
Modify

Opened 12 years ago

Closed 12 years ago

#10858 closed defect (fixed)

delete ticket change fails if it contains a modification of a custom field no longer defined

Reported by: Christian Boos Owned by: Christian Boos
Priority: normal Milestone: 0.12.5
Component: ticket system Version: 1.1.1dev
Severity: major Keywords: delete comment customfield
Cc: Branch:
Release Notes:

Fix deletion of ticket changes triggering an error if they contained a change for a custom field which is no longer in use.

API Changes:
Internal Changes:

Description

How to Reproduce

While doing a POST operation on /ticket/255, Trac issued an internal error (OperationalError: no such column: test_five)

Request parameters:

{'__FORM_TOKEN': u'f146ad994605e1310e6110f1',
 'action': u'delete-comment',
 'cdate': u'1340278350970000',
 'cnum': u'14',
 'id': u'255'}

User agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.4 (KHTML, like Gecko) Chrome/22.0.1229.56 Safari/537.4

System Information

Trac 1.1.1dev-r0
Babel 0.9.6dev-r0
Docutils 0.9.1
Genshi 0.6.1dev-r0 (without speedups)
GIT 1.7.7.msysgit.0
Mercurial 2.3.1+1-4b25077ae7ee
Pygments 1.4
pysqlite 2.6.3
Python 2.7.2 (default, Jun 12 2011, 15:08:59) [MSC v.1500 32 bit (Intel)]
pytz 2011h
setuptools 0.6
SQLite 3.7.6.2
Subversion 1.7.6 (r1370777)
jQuery 1.7.2

Enabled Plugins

graphviz 0.13.0.6
MilestoneOperation Rev
MyPage 0.13.0.0dev
new-ticket-in-milestone r6326
TracMercurial 1.0.0.1dev
TracWikiExtras 0.13.2dev-r10635
vulnerability-tickets r6669

Python Traceback

Traceback (most recent call last):
  File "c:\Trac\repos\trunk\trac\web\main.py", line 497, in _dispatch_request
    dispatcher.dispatch(req)
  File "c:\Trac\repos\trunk\trac\web\main.py", line 214, in dispatch
    resp = chosen_handler.process_request(req)
  File "c:\Trac\repos\trunk\tracopt\ticket\deleter.py", line 146, in process_request
    ticket.delete_change(cdate=cdate)
  File "c:\Trac\repos\trunk\trac\ticket\model.py", line 531, in delete_change
    % field, (oldvalue, self.id))
  File "c:\Trac\repos\trunk\trac\db\util.py", line 121, in execute
    cursor.execute(query, params)
  File "c:\Trac\repos\trunk\trac\db\util.py", line 65, in execute
    return self.cursor.execute(sql_escape_percent(sql), args)
  File "c:\Trac\repos\trunk\trac\db\sqlite_backend.py", line 78, in execute
    result = PyFormatCursor.execute(self, *args)
  File "c:\Trac\repos\trunk\trac\db\sqlite_backend.py", line 56, in execute
    args or [])
  File "c:\Trac\repos\trunk\trac\db\sqlite_backend.py", line 48, in _rollback_on_error
    return function(self, *args, **kwargs)
OperationalError: no such column: test_five

Attachments (0)

Change History (8)

comment:1 by Remy Blank, 12 years ago

I wonder if this could be related to the recent changes related to #1942. A field named test_five should not end up at line 531 of model.py, but rather at line 526. Is our detection of custom fields somehow messed up?

comment:2 by Christian Boos, 12 years ago

No, same thing with 1.0-stable:

File "c:/Trac/repos/1.0-stable/trac/ticket/model.py", line 492, in delete_change
  % field, (oldvalue, self.id))

And looking at the code (sorry had no time this morning besides posting the bug ;-) ) it's not surprising, the test is if field in self.custom_fields:, but that field is no longer part of the custom fields…

comment:3 by Remy Blank, 12 years ago

Ah, right. So basically, we should revert the logic and check if field not in self.std_fields. And check all other uses of self.custom_fields for similar issues.

comment:4 by Christian Boos, 12 years ago

Milestone: next-stable-1.0.x1.0.1
Owner: set to Christian Boos
Status: newassigned

See test, fix and a few code clean-ups in repos:cboos.git:ticket10847-deleted-custom-fields.

All the other uses of custom_fields seemed fine.

comment:5 by Remy Blank, 12 years ago

Looks good. Is it intentional that the ticket10847-deleted-custom-fields branch name is set for all changesets in the revision log of your repository, even changesets that are on 1.0-stable? (Or in other words, am I misunderstanding git branches?)

comment:6 by Christian Boos, 12 years ago

git branches are like hg bookmarks (well, it's even the other way round, historically), so it's just an external pointer to a given commit (in git terms, this pointer is a "ref"). Now the fact that, in Trac, we propagate that information to the ancestors is quite arbitrary. It is convenient when you follow a topic branch like here, but only up to a point, i.e. it gets really annoying when you go deep in the past, as then you have basically the list of all branches for each changeset (see … well I thought I created a ticket for that, can't find it). There must be something smarter to do.

Last edited 12 years ago by Christian Boos (previous) (diff)

comment:7 by Christian Boos, 12 years ago

Milestone: 1.0.10.12.5

Btw, we need that in 0.12-stable as well.

comment:8 by Christian Boos, 12 years ago

Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

Applied in r11355 on 0.12-stable, will merge in the next changesets.

Modify Ticket

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