#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 |
||
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 189 189 Attachment.reparent_all(self.env, 'wiki', old_name, 'wiki', 190 190 new_name) 191 191 192 self. name = new_name192 self.resource.id = self.name = new_name 193 193 self.env.log.info('Renamed page %s to %s', old_name, new_name) 194 194 195 195 for listener in WikiSystem(self.env).change_listeners:
I propose to fix it in stable and old-stable too.
Attachments (1)
Change History (10)
comment:1 by , 12 years ago
comment:2 by , 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.
follow-up: 5 comment:3 by , 12 years ago
Milestone: | → 0.12.6 |
---|---|
Owner: | set to |
Status: | new → assigned |
We indeed do have a test case (test_rename_page
), so you can just add an assertion after the one checking page.name
.
by , 12 years ago
Attachment: | 20130328_add-test_11138.patch added |
---|
add assertion to ensure correct page.resource.id
after rename
comment:5 by , 12 years ago
comment:7 by , 12 years ago
Owner: | changed from | to
---|
comment:8 by , 11 years ago
Release Notes: | modified (diff) |
---|
comment:9 by , 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.
May I provide a test case too, or is it too trivial?