doc/patchqueue/move_page.mdwn

   1 This is my first cut at a feature like that requested in [[todo/Moving_Pages]]. In case it gets mangled you can find it on [my site](http://www.betacantrips.com//attic/move.patch).
   2
   3 A bunch of obvious shortcomings exist:
   4
   5 * I'm not sure all the untaints are safe.
   6
   7   > I've looked them over.
   8   >
   9   > * one is safe because it only allows existing page names through.
  10   > * one is safe because it follows the example of editpage in calling
  11   > titlepage to sanitize.
  12   > * the last one follows the example of rcs_commit in svn.pm, so I
  13   > hope it's safe..
  14   >
  15   > --Ethan
  16 * No precautions whatsoever are made to protect against race conditions or failures
  17   in the rcs\_move function.
  18 * movepage.tmpl doesn't exist yet.
  19 * Some code is duplicated between cgi\_movepage and cgi\_editpage, as well
  20   as rcs\_commit and rcs\_move.
  21 * The user interface is pretty lame -- there's no handy select list full
  22   of possible places to move it or anything.
  23 * I don't think I implemented cancelling.
  24 * from is redundant with page.
  25 * I don't think I called the right hook functions.
  26 * No redirect pages like those mentioned on [[todo/Moving_Pages]] exist yet,
  27   so none are created.
  28 * It's not possible to get there through the actions listed on the wiki page.
  29   Instead you can select "Edit" and then change "edit" to "move" in the
  30   location bar.
  31
  32 Anyhow, here's the patch, for whatever good it does.
  33
  34 > Looks like a good start, although I agree about many of the points above,
  35 > and also feel that something needs to be done about rcses that don't
  36 > implement a move operation -- falling back to an add and delete.
  37 > --[[Joey]]
  38
  39 Hmm. Shouldn't that be done on a by-RCS basis, though? (i.e. implemented
  40 by backends in the `rcs_move` function)
  41
  42 > Probably, yes, but maybe there's a way to avoid duplicating code for that
  43 > in several of them.
  44
  45 Also, how should ikiwiki react if a page is edited (say, by another user)
  46 before it is moved? Bail, or shrug and proceed?
  47
  48 > The important thing is to keep in mind that the page could be edited,
  49 > moved, deleted, etc in between the user starting the move and the move
  50 > happening. So, the code really needs to deal with all of these cases in
  51 > some way. It seems fine to me to go ahead with the move even if the page
  52 > was edited. If the page was deleted or moved, it seems reasonable to exit
  53 > with an error.
  54 >
  55 > Another set of issues exists if a separate web user is trying to edit the
  56 > page at the same time. We'll have to make sure that something sane
  57 > happens there; will ikiwiki re-add the moved page under the old name if
  58 > the user saves their edit after the move? Could be confusing.. I think it
  59 > probably errors out instead, but I'm not sure. Of course, this is an
  60 > issue that occurs if the page is moved using a regular svn commit too, so
  61 > it's not really your concern in a way. :-)
  62
  63 >> I wrote a patch to address this -- it's in [[disappearing_pages]]. --Ethan
  64
  65 Could you elaborate on [[commit-internals]]? Can I assume that ikiwiki's
  66 working copy W will always reflect a revision of the master copy M?
  67 (That is, nobody changes W and leaves it uncommitted.) I would guess
  68 probably not; a user probably expects that if he starts editing W it
  69 won't get randomly committed by web user actions. But then looking at
  70 the svn backend, it looks like if I edit foo.mdwn, don't commit, and then
  71 a web user makes different changes, my changes get wiped out. So does
  72 W "belong" to ikiwiki? --Ethan
  73
  74 > The working copy used by ikiwiki belongs to ikiwiki; it should not be
  75 > edited directly.
  76
  77     diff -urx .svn ikiwiki/IkiWiki/CGI.pm ikiwiki-new/IkiWiki/CGI.pm
  78     --- ikiwiki/IkiWiki/CGI.pm  2007-01-04 03:52:47.000000000 -0800
  79     +++ ikiwiki-new/IkiWiki/CGI.pm      2007-01-11 18:49:37.000000000 -0800
  80     @@ -523,6 +523,97 @@
  81         }
  82      } #}}}
  83
  84     +sub cgi_movepage($$) {
  85     +   my $q = shift;
  86     +   my $session = shift;
  87     +   eval q{use CGI::FormBuilder};
  88     +   error($@) if $@;
  89     +   my @fields=qw(do from rcsinfo subpage page newname message); # subpage ignored so far
  90     +   my @buttons=("Rename Page", "Cancel");
  91     +
  92     +   my $form = CGI::FormBuilder->new(
  93     +           fields => \@fields,
  94     +                header => 1,
  95     +                charset => "utf-8",
  96     +                method => 'POST',
  97     +           action => $config{cgiurl},
  98     +                template => (-e "$config{templatedir}/movepage.tmpl" ?
  99     +                        {template_params("movepage.tpml")} : ""),
 100     +   );
 101     +   run_hooks(formbuilder_setup => sub {
 102     +           shift->(form => $form, cgi => $q, session => $session);
 103     +   });
 104     +
 105     +   decode_form_utf8($form);
 106     +
 107     +   # This untaint is safe because if the page doesn't exist, bail.
 108     +   my $page = $form->field('page');
 109     +   $page = possibly_foolish_untaint($page);
 110     +   if (! exists $pagesources{$page}) {
 111     +           error("page does not exist");
 112     +   }
 113     +   my $file=$pagesources{$page};
 114     +   my $type=pagetype($file);
 115     +
 116     +   my $from;
 117     +   if (defined $form->field('from')) {
 118     +           ($from)=$form->field('from')=~/$config{wiki_file_regexp}/;
 119     +   }
 120     +
 121     +   $form->field(name => "do", type => 'hidden');
 122     +   $form->field(name => "from", type => 'hidden');
 123     +   $form->field(name => "rcsinfo", type => 'hidden');
 124     +   $form->field(name => "subpage", type => 'hidden');
 125     +   $form->field(name => "page", value => $page, force => 1);
 126     +   $form->field(name => "newname", type => "text", size => 80);
 127     +   $form->field(name => "message", type => "text", size => 80);
 128     +
 129     +   if (! $form->submitted) {
 130     +           $form->field(name => "rcsinfo", value => rcs_prepedit($file),
 131     +                        force => 1);
 132     +   }
 133     +
 134     +   if ($form->submitted eq "Cancel") {
 135     +           redirect($q, "$config{url}/".htmlpage($from));
 136     +           return;
 137     +   }
 138     +
 139     +   if (! $form->submitted || $form->submitted eq "Preview" ||
 140     +       ! $form->validate) {
 141     +           if ($form->field("do") eq "move"){
 142     +                   page_locked($page, $session);
 143     +                   $form->tmpl_param("page_select", 0);
 144     +                   $form->field(name => "page", type => 'hidden');
 145     +                   $form->field(name => "type", type => 'hidden');
 146     +                   $form->title(sprintf(gettext("moving %s"), pagetitle($page)));
 147     +                   if (! defined $form->field('newname') ||
 148     +                       ! length $form->field('newname')) {
 149     +                           $form->field(name => "newname",
 150     +                                        value => pagetitle($page), force => 1);
 151     +                   }
 152     +
 153     +           }
 154     +           print $form->render(submit => \@buttons);
 155     +   }
 156     +   else{
 157     +           # This untaint is safe because titlepage removes any problematic
 158     +           # characters.
 159     +           my ($newname)=$form->field('newname');
 160     +           $newname=titlepage(possibly_foolish_untaint($newname));
 161     +           if (! defined $newname || ! length $newname || file_pruned($newname, $config{srcdir}) || $newname=~/^\//) {
 162     +                   error("bad page name");
 163     +           }
 164     +           page_locked($page, $session);
 165     +
 166     +           my $newfile = $newname . ".$type";
 167     +           my $message = $form->field('message');
 168     +           unlockwiki();
 169     +           rcs_move($file, $newfile, $message, $form->field("rcsinfo"),
 170     +                    $session->param("name"), $ENV{REMOTE_ADDR});
 171     +           redirect($q, "$config{url}/".htmlpage($newname));
 172     +   }
 173     +}
 174     +
 175      sub cgi_getsession ($) { #{{{
 176         my $q=shift;
 177
 178     @@ -631,6 +722,9 @@
 179         if ($do eq 'create' || $do eq 'edit') {
 180                 cgi_editpage($q, $session);
 181         }
 182     +   elsif ($do eq 'move') {
 183     +           cgi_movepage($q, $session);
 184     +   }
 185         elsif ($do eq 'prefs') {
 186                 cgi_prefs($q, $session);
 187         }
 188     diff -urx .svn ikiwiki/IkiWiki/Rcs/svn.pm ikiwiki-new/IkiWiki/Rcs/svn.pm
 189     --- ikiwiki/IkiWiki/Rcs/svn.pm      2006-12-28 17:50:46.000000000 -0800
 190     +++ ikiwiki-new/IkiWiki/Rcs/svn.pm  2007-01-11 18:14:30.000000000 -0800
 191     @@ -60,6 +60,34 @@
 192         }
 193      } #}}}
 194
 195     +sub rcs_move ($$$$;$$) {
 196     +   my $file=shift;
 197     +   my $newname=shift;
 198     +   my $message=shift;
 199     +   my $rcstoken=shift;
 200     +   my $user=shift;
 201     +   my $ipaddr=shift;
 202     +   if (defined $user) {
 203     +           $message="web commit by $user".(length $message ? ": $message" : "");
 204     +   }
 205     +   elsif (defined $ipaddr) {
 206     +           $message="web commit from $ipaddr".(length $message ? ": $message" : "");
 207     +   }
 208     +
 209     +   chdir($config{srcdir}); # svn merge wants to be here
 210     +
 211     +   if (system("svn", "move", "--quiet",
 212     +              "$file", "$newname") != 0) {
 213     +           return 1;
 214     +   }
 215     +   if (system("svn", "commit", "--quiet",
 216     +              "--encoding", "UTF-8", "-m",
 217     +              possibly_foolish_untaint($message)) != 0) {
 218     +           return 1;
 219     +   }
 220     +   return undef # success
 221     +}
 222     +
 223      sub rcs_commit ($$$;$$) { #{{{
 224         # Tries to commit the page; returns undef on _success_ and
 225         # a version of the page with the rcs's conflict markers on failure.