Page MenuHomePhabricator

Add a default moderation policy to Ponder
ClosedPublic

Authored by chad on Aug 7 2015, 3:22 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Dec 24, 8:28 AM
Unknown Object (File)
Tue, Dec 17, 7:43 AM
Unknown Object (File)
Tue, Dec 17, 6:16 AM
Unknown Object (File)
Sun, Dec 15, 12:10 AM
Unknown Object (File)
Fri, Dec 13, 11:17 AM
Unknown Object (File)
Mon, Dec 9, 10:08 AM
Unknown Object (File)
Sat, Dec 7, 12:58 AM
Unknown Object (File)
Wed, Dec 4, 4:29 PM
Subscribers

Details

Summary

This allows installs to essentially set a "moderator" for Ponder, who can clean up answers. Fixes T9098

Test Plan

Edit an answer I don't own.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

chad retitled this revision from to Add a default edit answer policy to Ponder.
chad updated this object.
chad edited the test plan for this revision. (Show Details)
chad added a reviewer: epriestley.
  • Build out PonderAnswer transactions more

Some discussion in T9098, but I think we probably don't want to save this value to the database. Specifically, it will cause this bad behavior:

  • Global policy is set wrong (say, to "No One") by accident.
  • Someone writes a bad answer: "chad is a jerk butt lol".
  • You try to hide the answer, but get a policy error.
  • You go fix the global policy and set it to "Administrators".

At this point, the answer still won't be editable (since "No One" was saved to the database). It seems like it should be, unless you have plans for this moving forward?

See inline for how to fix this.


I would probably also get rid of the contentSource edit stuff and plan to remove it eventually. No other application stores contentSource on the actual object, and I think it doesn't make sense: individual edits have content sources, but an answer as a whole may have many content sources (posted from web, updated by Herald, modified by Conduit, updated from mobile later, etc). The individual transactions already record a content source.

Even if we want to keep track of "content source of last edit", I think we can just look on the most recent transaction.

Do you think this information is valuable enough to keep as a separate property, or have plans for it? If not, I think we should just ignore/remove it.

src/applications/ponder/storage/PonderAnswer.php
204

To fix the persitence issue, just look up the application policy here and return it directly:

$app = PhabricatorApplication::getByClass(...);
return $app->getPolicy(...)

Then changes to the global config will be effective immediately and retroactively.

And maybe call the policy CanModerateAnswersCapability or something, instead of DefaultEditCapability.

Yeah, I'd like to drop content source.

In thinking about this a little bit, should the "CanModerate" Policy also apply to Question? (I assume we should have one Policy to rule them both)

chad edited edge metadata.
  • Build out PonderAnswer transactions more
  • fix brace alignment
  • Add PonderDefaultModerateCapability
  • Remove contentSource from Answers
chad retitled this revision from Add a default edit answer policy to Ponder to Add a default moderation policy to Ponder.Aug 7 2015, 4:22 PM
src/applications/ponder/controller/PonderQuestionEditController.php
127–133

I feel like this could mean someone can create a question that moderator's can't see.

epriestley edited edge metadata.
epriestley added inline comments.
src/applications/ponder/capability/PonderDefaultModerateCapability.php
6 ↗(On Diff #33378)

Or just ponder.moderate, since this isn't really a default anymore.

src/applications/ponder/controller/PonderQuestionEditController.php
127–133

It does, yeah.

You can do this check in hasAutomaticCapability() in Question and Answer:

if ($capability is view) {
  if (PhabricatorPolicyFilter::hasCapability($viewer, $this, BlahBlah::CAN_EDIT)) {
    return true;
  }
}

That will always let anyone who can edit a question view it, and mean that the moderator power implies both view and edit.

src/applications/ponder/storage/PonderAnswer.php
224

We should add a line here about moderators, and to Question.

This revision is now accepted and ready to land.Aug 8 2015, 4:41 PM
src/applications/ponder/storage/PonderAnswer.php
213–215

Actually, since this falls back to checking for an automatic capability on the question, you could just put the "has moderator" check there (instead of in both places), since if it's in Question it will apply here too.

That is, the current rule is "if you can see the question, you can always see all the answers", so if we just make sure moderators can always see all questions that's good enough to also let them see all answers.

src/applications/ponder/storage/PonderAnswer.php
213–215

I presume Spaces still trumps here? That is, if I can moderate all questions, but it's asked in a Space I can't see... then I can't moderate it?

src/applications/ponder/storage/PonderAnswer.php
213–215

Yeah, Spaces beats everything here. I think that's reasonable/correct.

chad marked 5 inline comments as done.
chad edited edge metadata.
  • Update policies
src/applications/ponder/storage/PonderAnswer.php
220–222

We might not want to do this -- this lets the user who asked the question edit all the answers.

This revision was automatically updated to reflect the committed changes.