Mailing List Archive

Patch to fix ticket #301
Folks,

I just started to play with Trac and I really like it. However, the ticket
#301 is a blocker for me for implementation. I really need an ability to
hand-off the bug to QA instead of just letting developers mark it as closed.

I have written a patch to 'trac-0.7.1' tag of the Trac source. Two caveats.
First, I have never seen/used Python before (I like it now - I will learn it
shortly). Second, I have not worked with Trac outside of few hours in my
"sandbox" and reading the source. So if something looks wrong in my code, it
probably is! [And I would appreciate the feedback.]

Also, I have done only limited testing on this (since I do not have a complex
enough set-up to really test this out). I also have not done any notification
testing (I presume it should work just the same from the brief look at the
code, but I have not verified that). It would be great if someone with good
knowledge of Trac internals code-reviews this.

VERY IMPORTANT! DO NOT APPLY THIS PATCH TO YOUR PRODUCTION SYSTEM! To work,
this patch requires a slightly different db schema (few extra fields). I have
modified 'initenv' to reflect that, but I do not have any skill with SQLite
to implement the migration script. I am hoping that someone else will get to
that.

Here is what I did.
1. Two new fields for 'component' - dev_owner and qa_owner (instead of a
single 'owner'). The 'dev_owner' is a default owner for development purposes
and 'qa_owner' is the default owner once the bug has been resolved.
Incidently, I did not find any web-based way to manage components, only
through trac-admin (is that right?) I have modified the 'component chown'
command to have syntax 'component chown <component> [dev|qa] <user>'.

2. Added 'dev_owner' and 'qa_owner' to the 'ticket'. The 'owner' is still
there - that reflects the current owner (depending on the state of the bug,
it would be either 'dev_owner' or 'qa_owner'.

3. Added new 'status' - 'resolved' and 'resolved-new'. When the bug is marked
as resolved, it now becomes status 'resolved-new' (instead of 'closed'). Once
the qa person 'accepts' it, it becomes 'resolved'. From there they can either
close the bug or re-open it. (The need for 'acceptance' is to indicate stuck
bugs - the workflow that I envision is component qa_owner will be the QA
manager, who will immediately re-assign the bug to an appropriate QA
engineer. They can then check for bugs that have not been accepted (e.g.
"resolved-new") to see if anything has been dropped on the floor.)

4. I removed the status 'reopened' since semantically it doesn't seem right -
after the bug is reopened, it should not be marked as 'assigned' right away,
but rather go through the 'assignment' process. Besides, 'reopened' is not
really a status, it is more of a life-cycle event. [.This is where my lack of
experience with Trac may be deadly.] Instead the bug now gets marked as 'new'
and has to get 'accepted' again. I still record 'reopened' in the
'ticket_changes' table, so that it is possible to see reopen rate for bugs.

5. Disabled setting resolution for the bug untill it is 'accepted'. Strictly
speaking, this has nothing to do with bug #301. It just seems really odd that
someone can mark the resolution of the bug w/o actually 'accepting' it. In my
experience this can cause conflicts where multiple people work on the same
bug in parallel w/o realising it. Now, only one person gets to 'accept' the
bug and play with it to their heart's content.

Finally, I do not want to have a private fork of Trac installed on my system,
so if this patch will not be accepted, then I don't want to deploy it on my
system either. So, I have three questions - 1. Will this be accepted? 2. If
yes, will it make it into version 0.8 (and when is 0.8 planned)? 3. If no,
when is 0.9 planned for (I see that the ticket #301 is assigned to 0.9)?

Thank you,

Alik



__________________________________
Do you Yahoo!?
New and Improved Yahoo! Mail - Send 10MB messages!
http://promotions.yahoo.com/new_mail -------------- next part --------------
diff -Naur trac-0.7.1/scripts/trac-admin trac-0.7.1/scripts/trac-admin
--- trac-0.7.1/scripts/trac-admin 2004-08-15 06:10:21.000000000 +0000
+++ trac-0.7.1/scripts/trac-admin 2004-08-15 06:14:37.000000000 +0000
@@ -291,7 +292,7 @@
('component add <name> <owner>', 'Add a new component'),
('component rename <name> <newname>', 'Rename a component'),
('component remove <name>', 'Remove/uninstall component'),
- ('component chown <name> <owner>', 'Change component ownership')]
+ ('component chown <name> [dev|qa] <owner>', 'Change component ownership')]

def complete_component (self, text, line, begidx, endidx):
if begidx in [16,17]:
@@ -318,10 +319,15 @@
elif arg[0] == 'remove' and len(arg)==2:
name = arg[1]
self._do_component_remove(name)
- elif arg[0] == 'chown' and len(arg)==3:
+ elif arg[0] == 'chown' and len(arg)==4:
name = arg[1]
- owner = arg[2]
- self._do_component_set_owner(name, owner)
+ owner = arg[3]
+ if arg[2]=='dev':
+ self._do_component_set_dev_owner(name, arg[2], owner)
+ elif arg[2]=='qa':
+ self._do_component_set_qa_owner(name, arg[2], owner)
+ else:
+ self.do_help ('component')
else:
self.do_help ('component')
except Exception, e:
@@ -348,8 +354,12 @@
data = self.db_execsql("DELETE FROM component WHERE name='%s'"
% (name))

- def _do_component_set_owner(self, name, owner):
- data = self.db_execsql("UPDATE component SET owner='%s' WHERE name='%s'"
+ def _do_component_set_dev_owner(self, name, owner):
+ data = self.db_execsql("UPDATE component SET dev_owner='%s' WHERE name='%s'"
+ % (owner,name))
+
+ def _do_component_set_qa_owner(self, name, owner):
+ data = self.db_execsql("UPDATE component SET qa_owner='%s' WHERE name='%s'"
% (owner,name))


diff -Naur trac-0.7.1/templates/ticket.cs trac-0.7.1/templates/ticket.cs
--- trac-0.7.1/templates/ticket.cs 2004-08-15 06:10:21.000000000 +0000
+++ trac-0.7.1/templates/ticket.cs 2004-08-15 06:16:00.000000000 +0000
@@ -10,9 +10,10 @@
<div id="main-content">
<div id="searchable">

-<?cs if:ticket.status == 'closed' ?>
+<?cs if ticket.status == 'closed' ?>
<?cs set:status = ' (Closed: ' + $ticket.resolution + ')' ?>
-<?cs elif:ticket.status == 'new' ?>
+<?cs elif ticket.status == 'resolved' || ticket.status == 'resolved-new' ?>
+ <?cs set:status = ' (Resolved: ' + $ticket.resolution + ')' ?>
<?cs else ?>
<?cs set:status = ' (' + ticket.status + ')' ?>
<?cs /if ?>
@@ -41,7 +42,7 @@
<?cs call:ticketprop("Priority", ticket.priority) ?>
<?cs call:ticketprop("Reporter", ticket.reporter) ?>
<?cs call:ticketprop("Severity", ticket.severity) ?>
- <?cs if ticket.status == "assigned"?>
+ <?cs if ticket.status == "assigned" || ticket.status == "resolved" ?>
<?cs call:ticketprop("Assigned to", ticket.owner + " (accepted)") ?>
<?cs else ?>
<?cs call:ticketprop("Assigned to", ticket.owner) ?>
@@ -212,15 +213,20 @@
<legend>Action</legend>
<input type="radio" id="leave" name="action" value="leave" checked="checked" />
&nbsp;<label for="leave">leave as <?cs var:ticket.status ?></label><br />
- <?cs if $ticket.status == "new" ?>
+ <?cs if $ticket.status == "new" || $ticket.status == "resolved-new" ?>
<input type="radio" id="accept" name="action" value="accept" />
&nbsp;<label for="accept">accept ticket</label><br />
<?cs /if ?>
- <?cs if $ticket.status == "closed" ?>
+ <?cs if $ticket.status == "closed" || $ticket.status == "resolved" ?>
<input type="radio" id="reopen" name="action" value="reopen" />
&nbsp;<label for="reopen">reopen ticket</label><br />
<?cs /if ?>
- <?cs if $ticket.status == "new" || $ticket.status == "assigned" || $ticket.status == "reopened" ?>
+ <?cs if $ticket.status == "resolved" ?>
+ <input type="radio" id="close" name="action" value="close" />
+ &nbsp;<label for="close">close ticket</label><br />
+ <?cs /if ?>
+
+ <?cs if $ticket.status == "assigned" ?>
<input type="radio" id="resolve" name="action" value="resolve" />
&nbsp;<label for="resolve">resolve as:</label>
<select name="resolve_resolution">
diff -Naur trac-0.7.1/trac/Ticket.py trac-0.7.1/trac/Ticket.py
--- trac-0.7.1/trac/Ticket.py 2004-08-15 06:10:21.000000000 +0000
+++ trac-0.7.1/trac/Ticket.py 2004-08-15 06:16:42.000000000 +0000
@@ -30,8 +30,8 @@
from Notify import TicketNotifyEmail

fields = [.'time', 'component', 'severity', 'priority', 'milestone', 'reporter',
- 'owner', 'cc', 'url', 'version', 'status', 'resolution',
- 'keywords', 'summary', 'description']
+ 'owner', 'dev_owner', 'qa_owner', 'cc', 'url', 'version', 'status',
+ 'resolution', 'keywords', 'summary', 'description']

def get_ticket (db, id, escape_values=1):
global fields
@@ -123,17 +123,33 @@

action = new.get('action', None)
if action == 'accept':
- new['status'] = 'assigned'
- new['owner'] = self.req.authname
- if action == 'resolve':
- new['status'] = 'closed'
+ if old['status'] == 'new':
+ new['status'] = 'assigned'
+ new['owner'] = self.req.authname
+ new['dev_owner'] = self.req.authname
+ elif old['status'] == 'resolved-new':
+ new['status'] = 'resolved'
+ new['owner'] = self.req.authname
+ new['qa_owner'] = self.req.authname
+ elif action == 'resolve':
+ new['status'] = 'resolved-new'
new['resolution'] = new['resolve_resolution']
+ new['owner'] = old['qa_owner']
elif action == 'reassign':
new['owner'] = new['reassign_owner']
- new['status'] = 'assigned'
+ if old['status'] == 'resolved' or old['status'] == 'resolved-new':
+ new['qa_owner'] = new['owner']
+ new['status'] = 'resolved'
+ else:
+ new['dev_owner'] = new['owner']
+ new['status'] = 'new'
elif action == 'reopen':
- new['status'] = 'reopened'
+ new['status'] = 'new'
+ if old['status'] == 'resolved':
+ new['owner'] = old['dev_owner']
new['resolution'] = ''
+ elif action == 'close':
+ new['status'] = 'closed'

changed = 0
change = ''
@@ -152,10 +168,16 @@
(old[name] and not new[name]) or \
(old[name] and new[name] and old[name] != new[name])):

- cursor.execute ('INSERT INTO ticket_change '
- '(ticket, time, author, field, oldvalue, newvalue) '
- 'VALUES (%s, %s, %s, %s, %s, %s)',
- id, now, author, name, old[name], new[name])
+ if name == 'status' and new[name] == 'new':
+ cursor.execute ('INSERT INTO ticket_change '
+ '(ticket, time, author, field, oldvalue, newvalue) '
+ 'VALUES (%s, %s, %s, %s, %s, %s)',
+ id, now, author, name, old[name], "reopened")
+ else:
+ cursor.execute ('INSERT INTO ticket_change '
+ '(ticket, time, author, field, oldvalue, newvalue) '
+ 'VALUES (%s, %s, %s, %s, %s, %s)',
+ id, now, author, name, old[name], new[name])
cursor.execute ('UPDATE ticket SET %s=%s WHERE id=%s',
name, new[name], id)
changed = 1
@@ -194,14 +216,19 @@
cursor = self.db.cursor()

# The owner field defaults to the component owner
- if data.has_key('component') and \
- (not data.has_key('owner') or data['owner'] == ''):
- # Assign it to the default owner
- cursor.execute('SELECT owner FROM component '
+ if data.has_key('component'):
+ cursor.execute('SELECT dev_owner,qa_owner FROM component '
'WHERE name=%s', data['component'])
- owner = cursor.fetchone()[0]
- data['owner'] = owner
+ result = cursor.fetchone()
+ # Assign it to the default QA owner
+ dev_owner = result[0]
+ qa_owner = result[1]
+ data['qa_owner'] = qa_owner
+ if (not data.has_key('owner') or data['owner'] == ''):
+ # Assign it to the default owner
+ data['owner'] = dev_owner

+ data['dev_owner'] = data['owner']
nstr = string.join(data.keys(), ',')
vstr = ('%s,' * len(data.keys()))[:-1]

@@ -267,7 +294,7 @@

id = int(self.args.get('id'))

- if action in ['leave', 'accept', 'reopen', 'resolve', 'reassign']:
+ if action in ['leave', 'accept', 'reopen', 'resolve', 'reassign', 'close']:
# save changes and redirect to avoid the POST request
self.perm.assert_permission (perm.TICKET_MODIFY)
old = self.get_ticket(id, 0)
diff -Naur trac-0.7.1/trac/db_default.py trac-0.7.1/trac/db_default.py
--- trac-0.7.1/trac/db_default.py 2004-08-15 06:10:21.000000000 +0000
+++ trac-0.7.1/trac/db_default.py 2004-08-15 06:16:42.000000000 +0000
@@ -85,6 +85,8 @@
severity text,
priority text,
owner text, -- who is this ticket assigned to
+ dev_owner text, -- who is the developer
+ qa_owner text, -- who is the QA
reporter text,
cc text, -- email addresses to notify
url text, -- url related to this ticket
@@ -119,7 +121,8 @@
);
CREATE TABLE component (
name text PRIMARY KEY,
- owner text
+ dev_owner text,
+ qa_owner text
);
CREATE TABLE milestone (
name text PRIMARY KEY,
@@ -328,9 +331,9 @@

# (table, (column1, column2), ((row1col1, row1col2), (row2col1, row2col2)))
data = (('component',
- ('name', 'owner'),
- (('component1', 'somebody'),
- ('component2', 'somebody'))),
+ ('name', 'dev_owner', 'qa_owner'),
+ (('component1', 'somebody', 'somebody'),
+ ('component2', 'somebody', 'somebody'))),
('milestone',
('name', 'time'),
(('', 0),