• Do not register here on develop.twiki.org, login with your twiki.org account.
• Use View topic Item7848 for generic doc work for TWiki-6.1.1. Use View topic Item7851 for doc work on extensions that are not part of a release. More... Close
• Anything you create or change in standard webs (Main, TWiki, Sandbox etc) will be automatically reverted on every SVN update.
Does this site look broken?. Use the LitterTray web for test cases.

Item6813: TWiki::UI::Save::save ignores topic name changes when redirecting on save

Item Form Data

AppliesTo: Component: Priority: CurrentState: WaitingFor: TargetRelease ReleasedIn
Engine   Normal Closed   patch 5.1.1

Edit Form Data

Summary:
Reported By:
Codebase:
Applies To:
Component:
Priority:
Current State:
Waiting For:
Target Release:
Released In:
 

Detail

I use a customized version of TWiki:Plugins.WorkflowPlugin which, in afterSaveHandler, renames a new topic (created from a template topic and form) using the values of two of the form fields. We're currently running TWiki 4.1.1, and I've been working on migrating/upgrading to 5.0.1 on a new server. Since I started the upgrade, TWiki 5.1.0 has come out, so I will switch to that before I go live, but for simplicity's sake, I wanted to finish figuring this out without potentially complicating my troubleshooting picture.

The problem I had was, in 5.0.1, when I created a new topic using the workflow, my plugin would create the topic with the correctly constructed name, but instead of redirecting to the new topic as it does in 4.1.1, the redirect tried to go to the old topic name, so I'd wind up at the 'topic does not exist' page.

After weeks of work (I am not conversant with the TWiki core), I discovered that TWiki::UI::Save::save sets the redirect URL before it runs $store->saveTopic, and does not check for an updated topic name before doing the redirect at the end. So if any plugin (like mine) renames the topic in beforeSaveHandler or afterSaveHandler, the redirect operates on the previous topic name, which no longer exists.

So, on line 534, the redirect URL is set using the topic name set previously from the session.:

$redirecturl ||= $session->getScriptUrl( 1, 'view', $web, $topic );

Then on line 588, the actual saving takes place, which runs before/afterSaveHandler:

$store->saveTopic( $user, $web, $topic, $newText, $newMeta, $saveOpts );

Lastly, at the end on line 613, the redirect to the saved page takes place:

$session->redirect( $redirecturl, undef, ( $saveaction ne 'checkpoint' ) );

Here is the fix I used; it adds an update to the topicName and the redirect URL after saveTopic has run.

@@ -596,6 +598,10 @@
             params => [ shift->{-text} ] );
     };

+    # update $topic because it may have changed with saveTopic
+    $topic = $session->{topicName};
+    $redirecturl = $session->getScriptUrl( 1, 'view', $web, $topic );
+
     my $lease = $store->getLease( $web, $topic );
     # clear the lease, if (and only if) we own it
     if( $lease && $lease->{user} eq $user ) {

-- TWiki:Main/JohnWorsley - 2011-09-22

Thanks for the report and proposed fix. Not sure if we can hardcode the redirecturl to view. It might break stuff, for example a checkpoint save that redirects back to edit instead of view. Could you test this version here that does a graceful replace of the topic in the existing redirecturl?

--- TWiki/UI/Save.pm   (revision 22298)
+++ TWiki/UI/Save.pm   (working copy)
@@ -602,6 +602,12 @@
         $store->clearLease( $web, $topic );
     }
 
+    if( $session->{topicName} ne $topic ) {
+        # TWikibug:Item6813: update $topic because it has changed in an plugin afterSaveHandler
+        $topic = $session->{topicName};
+        $redirecturl =~ s/([\/\\])[^\/\\]+([?#]|$)/$1$topic$2/; # replace topic
+    }
+
     if( $merged ) {
         throw TWiki::OopsException(
             'attention',

-- TWiki:Main.PeterThoeny - 2011-09-24

Yes, your version is much better. I tested it, and it works just like my version.

-- TWiki:Main.JohnWorsley - 2011-09-26

Thanks, marking this item as waiting for release.

-- TWiki:Main.PeterThoeny - 2011-09-27

Hmm, looks like this was marked Closed instead.

-- TWiki:Main.JohnWorsley - 2012-08-29

This is correct. It was a core bug that was fixed and released in TWiki-5.1.1. After a release, all "waiting for release" are toggled to "closed". This is part of our TWiki:Codev.TWikiReleaseProcess.

-- TWiki:Main.PeterThoeny - 2012-08-29

ItemTemplate
Summary TWiki::UI::Save::save ignores topic name changes when redirecting on save
ReportedBy TWiki:Main.JohnWorsley
Codebase ~twiki4, 5.0.1, 5.0.0
SVN Range

AppliesTo Engine
Component

Priority Normal
CurrentState Closed
WaitingFor

Checkins TWikirev:22348 TWikirev:22349
TargetRelease patch
ReleasedIn 5.1.1
Edit | Attach | Watch | Print version | History: r10 < r9 < r8 < r7 < r6 | Backlinks | Raw View |  Raw edit | More topic actions
Topic revision: r10 - 2012-08-30 - PeterThoeny
 
This site is powered by the TWiki collaboration platform Powered by PerlCopyright © 2008-2024 by the contributing authors. All material on this collaboration platform is the property of the contributing authors.
Ideas, requests, problems regarding TWiki? Send feedback