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

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