doc/todo/web_reversion.mdwn

   1 Goal: Web interface to allow reverting of changes.
   2
   3 Interface:
   4
   5 At least at first, it will be exposed via the recentchanges
   6 page, with revert icons next to each change. We may want a dynamic
   7 per-page interface that goes back more than 100 changes later.
   8
   9 Limiting assumptions:
  10
  11 * No support for resolving conflicts in reverts; such a revert would just
  12   fail and not happen.
  13 * No support for reset-to-this-point; initially the interface would only
  14   revert a single commit, and if a bunch needed to go, the user would have
  15   to drive that one at a time.
  16
  17 Implementation plan:
  18
  19 * `rcs_revert` hook that takes a revision to revert.
  20 * CGI: `do=revert&rev=foo`
  21 * recentchanges plugin adds above to recentchanges page
  22 * prompt user to confirm (to avoid spiders doing reverts),
  23   check that user is allowed to make the change, commit reversion,
  24   and refresh site.
  25
  26 Peter Gammie has done an initial implementation of the above.
  27 [[!template id=gitbranch branch=peteg/revert author="[[peteg]]"]]
  28
  29 >> It is on a separate branch now. --[[peteg]]
  30
  31 > Review: --[[Joey]]
  32 >
  33 > The revert commit will not currently say what web user did the revert.
  34 > This could be fixed by doing a --no-commit revert first and then using
  35 > rcs_commit_staged.
  36 >> Fixed, I think. --[[peteg]]
  37 >
  38 > So I see one thing I completly forgot about is `check_canedit`. Avoiding users
  39 > using reverting to make changes they would normally not be allowed to do is
  40 > tricky. I guess that a easy first pass would be to only let admins do it.
  41 > That would be enough to get the feature out there..
  42 >
  43 > I'm thinking about having a `rcs_preprevert`. It would take a rev and look
  44 > at what changes reverting it would entail, and return the same data
  45 > structure that `rcs_recieve` does. This could be done by using `git revert
  46 > --no-commit`, and then examining the changes, and then `git reset` to drop
  47 > them.
  48 >> We can use the existing `git_commit_info` with the patch ID - no need to touch the working directory. -- [[peteg]]
  49 >
  50 > Then the code that is currently in IkiWiki/Receive.pm, that calls
  51 > `check_canedit` and `check_canremove` to test the change, can be
  52 > straightforwardly refactored out, and used for checking reverts too.
  53 >> Wow, that was easy. :-) -- [[peteg]]
  54 >
  55 > (The data from `rcs_preprevert` could also be used for a confirmation
  56 > prompt -- it doesn't currently include enough info for diffs, but at
  57 > least could have a list of changed files.)
  58 >> I added `rcs_showpatch` which simply yields the output of `git show <patch-id>`. -- [[peteg]]
  59 >
  60 > Note that it's possible for a git repo to have commits that modify wiki
  61 > files in a subdir, and code files elsewhere. `rcs_preprevert` should
  62 > detect changes outside the wiki dir, and fail, like `rcs_receive` does.
  63 >> Taken care of by refactoring `rcs_receive` in `git.pm`
  64 >> I've tested it lightly in my single-user setup. It's a little nasty that the `attachment` plugin
  65 >> gets used to check whether attachments are allowed -- there really should be a hook for that.
  66 >>
  67 >> Please look it over and tell me what else needs fixing... -- [[peteg]]
  68
  69 >>> I have made my own revert branch and put a few fixes in there
  70 >>> (and fixed all the indention..). Issues I noticed but have not gotten
  71 >>> to: --[[Joey]]
  72 >>>> Please change the git pointer above, then. I will work on your branch. -- [[peteg]]
  73 >>>
  74 >>> * `rcs_diff` already exists; why add `rcs_showpatch`?
  75 >>>> If `rcs_diff` is intended for human consumption, by all means we can use that. -- [[peteg]]
  76
  77 >>> * Would it be better for `rcs_revert` to not commit, and
  78 >>>   `rcs_commit_staged` to then be used? This would work for git, but
  79 >>>   maybe other RCSs would be problimatic. It would simplifiy the
  80 >>>   interface and allow for future mulitple-revert interfaces.
  81 >>> * I quite don't understand why one caller of `git_parse_changes`
  82 >>>   needs it to chdir, and not the other one. It's running
  83 >>>   in the same git repo either way, and git doesn't need
  84 >>>   `git show` to run in a subdir at all..
  85 >>>> I was aping (preserving) what was already there. I don't understand what you say about `git show` - it must run under $srcdir, surely? And empirically the CGI process wasn't in the right place. By all means simplify that. -- [[peteg]]
  86
  87 >>> * Probably needs to untaint the revs passed in.
  88 >>> * Seems backwards for `rcs_preprevert` to import and
  89 >>>   use `IkiWiki::Receive`.
  90 >>>> Indeed. This is saying that the checking code in IkiWiki::Receive is in the wrong place. I think it would be better to set up some general hooks and shuffle it into a plugin, for then other plugins that maintain special files in the repo can have a say about validity. -- [[peteg]]