Page MenuHomePhabricator

Allow Phriction document edits to be saved as drafts
ClosedPublic

Authored by epriestley on Sep 11 2018, 5:07 PM.

Details

Summary

Depends on D19661. Ref T13077. See PHI840.

When a user edits a page normally, add a "Save as Draft" button. Much of this change is around making that button render and behave properly: it needs to be an <input type="submit" ...> so browsers submit it and we can figure out which button the user clicked.

Then there are a few minor rules:

  • If you're editing a page which is already a draft, we only give you "Save as Draft". This makes edits to update/revise a draft more natural.
  • Highlight "Publish" if it's a likely action that you might want to take.

Internally, there are two types of edits. Both types create a new version with the new content. However:

  • A "content" edit sets the version shown on the live page to the newly-created version.
  • A "draft" edit does not update the version shown on the live page.
Test Plan

Edited a published document, edited the draft. Published documents. Reverted documents.

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Sep 11 2018, 5:07 PM
epriestley requested review of this revision.Sep 11 2018, 5:09 PM
amckinley accepted this revision.Sep 12 2018, 6:47 PM

A "draft" edit change the content but does not bump the version.

Do documents in draft mode still have an edit history? I can see both answers to that question being reasonable.

Other than nits, looks good.

src/applications/phriction/controller/PhrictionDocumentController.php
450

Probably should be "Revert" for clarity.

464–466

Are we going to take this out of prototypes at the end of this set of revisions?

src/view/phui/PHUIButtonView.php
235–236

Should be "If we aren't rendering a "<button>" or "<input>" tag".

This revision is now accepted and ready to land.Sep 12 2018, 6:47 PM
epriestley edited the summary of this revision. (Show Details)Sep 12 2018, 6:56 PM
epriestley marked an inline comment as done.Sep 12 2018, 7:00 PM

Do documents in draft mode still have an edit history? I can see both answers to that question being reasonable.

Yes. Basically, the only thing this draft mechanism does is allows the default version of a page you see when you visit /w/some-page/ to be some particular older version of that page instead of always being the most recent version. The version history still works the same way, everyone can see the draft versions, the drafts are linear, there's no way to discard/undo a draft, etc. It's exactly like "Edit Document", except it just doesn't update the "what do you see when you look at /w/some-page/?" pointer.

src/applications/phriction/controller/PhrictionDocumentController.php
464–466

Oh, I'll just get rid of this now. This action behaves reasonably enough currently, I think.

Do documents in draft mode still have an edit history?

I just read through PHI840 and this explanation makes perfect sense:

"Save" is split into "Save as Draft" (which creates and saves a new version, e.g. v8, and moves the draft cursor to point at that version) and a new "Publish" operation, which moves the "published version" cursor forward or backward. Put another way, editing basically still works the same way, but the "live" version of the document may be any historical version of it, not necessarily the most recent version.

But this also seems like a confusing description of the behavior:

Internally, there are two types of edits.

  • A "content" edit changes the content and bumps the version.
  • A "draft" edit changes the content but does not bump the version.

Should be (I believe):

Internally, there are two types of edits. Both types create a new version with the new content. However:

  • A "content" edit sets the version shown on the live page to the newly-created version.
  • A "draft" edit does not update the version shown on the live page.

Yeah, your revised description is accurate and more clear than my original.

epriestley edited the summary of this revision. (Show Details)Sep 12 2018, 7:12 PM
epriestley marked an inline comment as done.
epriestley added inline comments.
webroot/rsrc/css/phui/phui-form-view.css
141–147

Just for context, I'm almost entirely certain these are unused (no grep hits for the class or substrings, and I can't think of any other pages with multiple submit buttons), so I nuked 'em.

epriestley updated this revision to Diff 47000.Sep 12 2018, 8:29 PM
  • Remove prototype check.
  • Update comment for "<input />".

I'm leaving "Publish Revert" for now -- I agree this language isn't very clear and we can probably find something better, but there are currently two "revert" operations:

  • "Revert to an older version", this action on the Document page, which makes newer versions into unpublished drafts.
  • "Revert to <specific verision>", a button on the history comparison diff page, which starts editing a new version of the document using the old content.

The "history comparison diff page" also has a couple bugs I want to clean up. I'll either revisit this language then or when I actually write documentation for this.

Another thought is that we could just remove the "Publish an older version" operation completely and only let you publish forward. However, it seems theoretically more useful than confusing, at least if I can find some clearer language for it.

This revision was automatically updated to reflect the committed changes.