#3920 closed defect (fixed)
[Patch] Error in code causing diff crash on tickets with changes in description only (web_ui.py)
Reported by: | Owned by: | Noah Kantrowitz | |
---|---|---|---|
Priority: | normal | Component: | MasterTicketsPlugin |
Severity: | major | Keywords: | |
Cc: | martin@…, dale.miller@…, Jason Winnebeck, Marcus Lindblom, Mitar | Trac Release: | 0.11 |
Description
source:masterticketsplugin/0.11/mastertickets/web_ui.py@4178:63-64#L62
NOW:
63 for change in data.get('changes', []): # next line get key error, if data.get use default: 64 for field, field_data in change['fields'].iteritems():
CORRECTED:
63 for change in data.get('changes', {'fields':[]}):
With this correction, diff of tickets with description changed only passes again.
rgds, Paul
Attachments (1)
Change History (23)
comment:1 Changed 16 years ago by
Cc: | martin@… added; anonymous removed |
---|
comment:2 Changed 16 years ago by
comment:3 Changed 16 years ago by
Replace only line 63 with this
for change in data.get('changes', {'fields':[]}):
nothing else
comment:4 Changed 16 years ago by
The above fix doesn't help for me: I still get the KeyError:
File "build/bdist.linux-i686/egg/mastertickets/web_ui.py", line 65, in post_process_request for field, field_data in change['fields'].iteritems(): KeyError: 'fields'
after debugging a bit, I found that the call to data.get('changes', []) was indeed returning an array with data and not returning the default value []. The problem is that the array changes contains several dictionaries with keys such as diff, but without a key fields. This is why the following line 64 chokes when trying to access changesfields?.
my fix follows:
for change in data.get('changes', []): if not change.has_key('fields'): continue for field, field_data in change['fields'].iteritems():
comment:5 Changed 16 years ago by
Thanks! I can confirm the latest patch fixes the problem for me, too.
comment:6 Changed 16 years ago by
This solution is not complete - if
data.get('changes', [])
return default value (= [] = list), next line will crash:
change.has_key('fields')
because list([]) has no attribute has_key
Correct solution is to combine both patches:
for change in data.get('changes', {}): if not change.has_key('fields'): continue
comment:7 Changed 16 years ago by
Yes, that make sense - data.get('changes', {}) is what is should be.
Changed 16 years ago by
Attachment: | fix_ticket_diff.patch added |
---|
Patch for the fix discussed in the ticket
comment:8 Changed 16 years ago by
I've attached a diff which implements the fix discussed in this ticket. It should be applied with -p0 from the masterticketsplugin/0.11-directory.
comment:9 Changed 16 years ago by
Cc: | dale.miller@… added |
---|
comment:10 Changed 16 years ago by
Cc: | Jason Winnebeck added |
---|
I'm using the latest version of the plugin and I think this is happening to me too. I haven't tried dagvl's patch yet. I assume because this ticket is not closed that it has not been merged into the upstream yet?
How to Reproduce
While doing a GET operation on /ticket/598
, Trac issued an internal error.
Click on "diff" next to the description on any ticket that I've tried so far in my Trac.
Request parameters:
{'action': u'diff', 'id': u'598', 'version': u'2'}
User Agent was: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.5) Gecko/2008120122 Firefox/3.0.5 (.NET CLR 3.5.30729)
System Information
Trac | 0.11.2
|
Python | 2.5.2 (r252:60911, Jul 31 2008, 17:44:49) [GCC 4.2.3 (Ubuntu 4.2.3-2ubuntu7)]
|
setuptools | 0.6c8
|
SQLite | 3.4.2
|
pysqlite | 2.5.0
|
Genshi | 0.5.1
|
mod_python | 3.3.1
|
Pygments | 0.11.1
|
Subversion | 1.5.1 (r32289)
|
jQuery: | 1.2.6
|
Python Traceback
Traceback (most recent call last): File "/usr/lib/python2.5/site-packages/Trac-0.11.2-py2.5.egg/trac/web/main.py", line 432, in _dispatch_request dispatcher.dispatch(req) File "/usr/lib/python2.5/site-packages/Trac-0.11.2-py2.5.egg/trac/web/main.py", line 216, in dispatch self._post_process_request(req, *resp) File "/usr/lib/python2.5/site-packages/Trac-0.11.2-py2.5.egg/trac/web/main.py", line 308, in _post_process_request resp = f.post_process_request(req, *resp) File "build/bdist.linux-i686/egg/mastertickets/web_ui.py", line 64, in post_process_request for field, field_data in change['fields'].iteritems(): KeyError: 'fields'
comment:11 Changed 16 years ago by
The patch attached to this ticket works for me, by patching the latest source (as of r4179).
comment:12 Changed 16 years ago by
When I installed the MasterTicketsPlugin 4 weeks ago I applied the patches listed for #3920 and #4167. After reviewing the open tickets I decided both of these looked like they should be included so I applied the patches prior to building the plugin.
#3920 - Error in code causing diff crash on tickets with changes in description only (web_ui.py). The patch.
#4167 - Concurrent edits of tickets can overwrite changes to blocking / blocked_by fields. The patch.
comment:13 Changed 15 years ago by
Cc: | Marcus Lindblom added |
---|
I got bitten by this too. What's holding the patch from being folded into svn?
comment:14 Changed 15 years ago by
I presume the only reason is because the maintainer (coderanger) has not been responsive to commit. Open-source is great because people can make patches, but the unfortunate part is if there's no one to integrate the patch, the only choice is to fork.
comment:15 Changed 15 years ago by
Yeah. Moving it to github or bitbucket (yay! :) would make it tons easier to fork and fix in the meantime, with an easy opton to merge changes back when/if the original author gets some time to spend.
CVCS:es aren't really good for handling open source projects that fall out of active maintenance.
comment:16 Changed 15 years ago by
I pushed my local git repos for this plugin into http://github.com/dagvl/masterticketsplugin/tree/master
It contains the two patches mentioned here. Additionally it has a modification which allows tickets with blockers in the state "resolved" to be closed. This pertains to our custom workflow at my workplace, but shouldn't interfere with anything.
The version numbering I've used is to suffix the original version number it with a -viz1, -viz2 etc as I've done modifications.
comment:17 Changed 15 years ago by
Summary: | Error in code causing diff crash on tickets with changes in description only (web_ui.py) → [Patch] Error in code causing diff crash on tickets with changes in description only (web_ui.py) |
---|
comment:18 Changed 14 years ago by
Cc: | Mitar added |
---|
comment:19 Changed 14 years ago by
Fixed in MasterTickets 3.0 (for Trac 0.12):
http://github.com/coderanger/trac-mastertickets/commit/b46dc7e8614bf02a9c35ca14aa17962bc1ee3156
comment:20 Changed 14 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
this correction doesn't help for me. Could someone clarify this patch? Does only line 63 have to be changed, or does line 64 also have to be removed? How about the rest of the block (which accesses field) - shouldn't it need modifications as well?