This allows installs to essentially set a "moderator" for Ponder, who can clean up answers. Fixes T9098
Details
- Reviewers
epriestley - Maniphest Tasks
- T9098: Implement Default Edit Answer capability
- Commits
- Restricted Diffusion Commit
rPf98f5a081e41: Add a default moderation policy to Ponder
Edit an answer I don't own.
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
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. |
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)
- Build out PonderAnswer transactions more
- fix brace alignment
- Add PonderDefaultModerateCapability
- Remove contentSource from Answers
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. |
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. |
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. |
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. |