Modify

Opened 3 years ago

Closed 20 months ago

#11902 closed defect (fixed)

Got an error on the Timeline Page

Reported by: sdegrande Owned by: Steffen Hoffmann
Priority: high Component: DiscussionPlugin
Severity: critical Keywords:
Cc: Ryan J Ollos, Steffen Hoffmann Trac Release: 1.0

Description

When displaying the Timeline with DiscussionPlugin enabled, I got the following Trac error :

Event provider DiscussionTimeline failed for filters "Forums changes": TypeError: tuple indices must be integers, not str

I use Trac 1.1.1 + Python 2.7.3 + TracDiscussion 0.9dev-r13945

Please find attached a patch that fixes it for me.

Attachments (2)

fix_error_on_timeline.diff (707 bytes) - added by sdegrande 3 years ago.
discussionplugin-t11902-r14133.diff (3.0 KB) - added by Jun Omae 3 years ago.

Download all attachments as: .zip

Change History (11)

Changed 3 years ago by sdegrande

Attachment: fix_error_on_timeline.diff added

comment:1 Changed 3 years ago by Ryan J Ollos

Cc: Steffen Hoffmann added

comment:2 Changed 3 years ago by Steffen Hoffmann

Would you explain the proposed change a bit more, please?

AFAIKS the patch is badly formatted and patched Python code will not run due to indentation issues. So I doubt that you tested with the patched code. Furthermore the patch just decomposes the list comprehension, what should have zero effect on the reported issue with tuple indices. But maybe I'm overlooking something here?

comment:3 Changed 3 years ago by Jun Omae

#11957 was closed as duplicate.

Changed 3 years ago by Jun Omae

comment:4 Changed 3 years ago by Jun Omae

We should add unit tests for that, see discussionplugin-t11902-r14133.diff.

The row variable is a tuple in discussionplugin/0.11/tracdiscussion/model.py@13944:240-242#L216.

        return [dict(zip(columns, row),
                     status=topic_status_to_list(row['status']))
                for row in cursor]

comment:5 Changed 3 years ago by sdegrande

Sorry, I did not see that you commented this ticket 2 weeks ago, hasienda.

Sorry for the bad formatted patch (I use it and it runs, so something went wrong when I uploaded the diff).

As far as I understand it, the error comes from the execution of row['status'], because row is not 'yet' a dict when used in the call to topic_status_to_list() (I can be wrong, I'm not a python expert at all).

That's why, in my patch, I first retrieve the dict(zip(...)) before to call topic_status_to_list(row['status'])

jun66j5's patch is indeed much better, in my humble opinion.

Last edited 3 years ago by Steffen Hoffmann (previous) (diff)

comment:6 Changed 3 years ago by Steffen Hoffmann

Thank you Jun for helping with the explanation and handling the other ticket. I'll apply your smaller fix, happily extending the unit test harness.

comment:7 Changed 3 years ago by Steffen Hoffmann

In 14139:

DiscussionPlugin: Prevent TypeError: tuple indices must be integers, not str, refs #11902.

Thanks to Samuel Degrande for the original report and to Jun Omae for
explaining the issue and providing both, an elegant fix and improved unit tests.

comment:8 Changed 3 years ago by Steffen Hoffmann

Owner: changed from anonymous to Steffen Hoffmann
Status: newassigned

comment:9 Changed 20 months ago by Ryan J Ollos

Resolution: fixed
Status: assignedclosed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Steffen Hoffmann.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.