Edgewall Software
Modify

Opened 12 years ago

Closed 12 years ago

Last modified 10 years ago

#11138 closed defect (fixed)

Data attribute inconsitency in trac.wiki.model.WikiPage while renaming a page

Reported by: Steffen Hoffmann Owned by: Steffen Hoffmann
Priority: normal Milestone: 0.12.6
Component: wiki system Version:
Severity: minor Keywords: listener resource
Cc: ryan.j.ollos@… Branch:
Release Notes:

Update WikiPage.resource.id when renaming a wiki page.

API Changes:
Internal Changes:

Description

When implementing an IWikiChangeListener for th:wiki:VotePlugin the initial draft didn't work as expected.

By comparing to known-good code for th:wiki:TagsPlugin I noticed, that there the new name is taken from page.name, while now I relied on the Resource object page.resource alone.

I feel that the current method WikiPage.rename is flawed since the initial commit in r9362 where it is updating only page.name but leaving page.resource unchanged. The WikiPage object passed to change listeners has both, the old and new name, and this is certainly unintended. However, it is easy to fix:

  • trac/wiki/model.py

    diff -u trac/wiki/model.py trac/wiki/model.py
    a b  
    189189            Attachment.reparent_all(self.env, 'wiki', old_name, 'wiki',
    190190                                    new_name)
    191191
    192         self.name = new_name
     192        self.resource.id = self.name = new_name
    193193        self.env.log.info('Renamed page %s to %s', old_name, new_name)
    194194       
    195195        for listener in WikiSystem(self.env).change_listeners:

I propose to fix it in stable and old-stable too.

Attachments (1)

20130328_add-test_11138.patch (608 bytes ) - added by Steffen Hoffmann 12 years ago.
add assertion to ensure correct page.resource.id after rename

Download all attachments as: .zip

Change History (10)

comment:1 by Steffen Hoffmann, 12 years ago

May I provide a test case too, or is it too trivial?

comment:2 by Remy Blank, 12 years ago

Thanks for the patch. A test case would be nice, or if there's already a test case for WikiPage.rename(), add an assertion for the resource attribute there.

Last edited 12 years ago by Remy Blank (previous) (diff)

comment:3 by Remy Blank, 12 years ago

Milestone: 0.12.6
Owner: set to Remy Blank
Status: newassigned

We indeed do have a test case (test_rename_page), so you can just add an assertion after the one checking page.name.

comment:4 by Christian Boos, 12 years ago

Good catch Steffen!

by Steffen Hoffmann, 12 years ago

add assertion to ensure correct page.resource.id after rename

in reply to:  3 comment:5 by Steffen Hoffmann, 12 years ago

Replying to rblank:

We indeed do have a test case (test_rename_page), so you can just add an assertion after the one checking page.name.

Done here.

Replying to cboos:

Good catch Steffen!

You're welcome. Piece of luck, improving both, a plugin and Trac core at once.

comment:6 by Remy Blank, 12 years ago

Resolution: fixed
Status: assignedclosed

Applied in [11735].

comment:7 by Remy Blank, 12 years ago

Owner: changed from Remy Blank to Steffen Hoffmann

comment:8 by Ryan J Ollos, 11 years ago

Release Notes: modified (diff)

comment:9 by Ryan J Ollos, 10 years ago

#11648 proposes changes that would implicitly avoid defects like the one seen in this ticket. Comments on the proposed changes are appreciated.

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'.
to The owner will be changed from Steffen Hoffmann 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.