Page MenuHomePhabricator

Phame blog draft posts are visible to users who can edit the blog, but the UI states that they are visible to only authors
Closed, ResolvedPublic

Description

Reproduction steps:

  1. Create a new draft post
  2. Save it (as draft)
  3. Check the post message saying <Only you can see this draft until you publish it. Use "Publish" to publish this post.>

Expected result:

  1. The author is allowed to see the post
  2. Other people are not allowed to see the post

Actual result:

  1. Both author and other people are allowed to see the draft post ( example: https://test-t5kf335nayg5.phacility.com/phame/post/view/1/post_title/ )

Event Timeline

stevex updated the task description. (Show Details)
stevex updated the task description. (Show Details)
stevex updated the task description. (Show Details)
epriestley added a subscriber: epriestley.

The language here is inconsistent with the rule we attempt to implement. Let me make sure the rule we attempt to implement actually works and that this is just a language issue.

The rule we implement seems to work properly, but the UI tells you a different rule.

Here's the actual rule:

  • Draft posts are visible to the author and to users who have permission to edit the associated blog.

The intent of this rule is to let you share drafts with other co-editors to get feedback.

If we change the text to reflect this rule, likely to this:

Only you and users who have permission to edit <Blog Name> can see this draft until it is published. Use "Publish" to publish this post.

...would that resolve things in your case? Or does the rule we actually implement not work as well as "only the author" in your use case?

Historically, Phame originally used the claimed rule ("only the author"), introduced in or around D3694.

In response to T5371 it was initially removed in D14437 -- which fixed the string! -- and then simplified slightly in D14444 instead, which omitted the string fix.

D16104 introduced an "archived" state but copied existing strings rather than actually describing the behavior it implemented. I identified the problem with both the "archived" and "draft" text in review (D16104#187585) but it was not addressed at the time.

epriestley renamed this task from Phame blog draft posts are visible to all to Phame blog draft posts are visible to users who can edit the blog, but the UI states that they are visible to only authors.Jul 5 2017, 4:58 PM
epriestley triaged this task as Normal priority.
epriestley added a project: Policy.

You must be able to view a post's blog to view the post, so the rule is effectively "view and edit".

https://secure.phabricator.com/source/phabricator/browse/master/src/applications/phame/query/PhamePostQuery.php;e516358d54d2e7f15da2e2ddf474c8d88ae7ab55$71-76

We could say this explicitly in the text, but I think it's probably what readers would assume?

The actual way this is implemented is slightly wonky -- PhamePost should really use PhabricatorExtendedPolicyInterface now that it exists. But the bugs this might result in should be limited to extreme edge cases: blogs with custom object policies, maybe posts which get published, have users subscribe to them, then get unpublished, and the user miiiight still get mail about draft edits?

Only you and users who have permission to edit <Blog Name> can see this draft until it is published. Use "Publish" to publish this post.

...would that resolve things in your case? Or does the rule we actually implement not work as well as "only the author" in your use case?

In my particular blog (a community jokes blog) everyone is allowed to post. So no, it would not solve the problem (everyone will see the draft post, and until published this is not good).

In my actual case just the author is ok, but also a group of people as all the other Phab-objects can be ok, but this "can see draft" group should be present as a field in the edit form of the post with the other fields.

Surely language should be consistent.

That use case doesn't strike me as compelling enough to justify a separate field or a behavioral change so I don't plan to adjust our behavior here.

At HEAD after D18182, the language should be consistent.