Page MenuHomePhabricator

Projects - add "lock membership", which prevents people from leaving
ClosedPublic

Authored by btrahan on Sep 17 2014, 8:27 PM.
Tags
None
Referenced Files
F13085220: D10508.diff
Wed, Apr 24, 11:44 PM
Unknown Object (File)
Fri, Apr 12, 3:06 AM
Unknown Object (File)
Thu, Apr 11, 8:00 AM
Unknown Object (File)
Tue, Apr 9, 9:01 PM
Unknown Object (File)
Wed, Apr 3, 2:40 PM
Unknown Object (File)
Wed, Apr 3, 2:40 PM
Unknown Object (File)
Wed, Apr 3, 2:40 PM
Unknown Object (File)
Wed, Apr 3, 2:40 PM
Subscribers
Tokens
"Evil Spooky Haunted Tree" token, awarded by epriestley.

Details

Summary

Fixes T5603. Puts the toggling of locking membership into the editor so we get exceptions and all that.

I think the dialogue when you try to leave a project that is locked could be a little better maybe? Right now it just says "You can't leave" and "The membership is locked" more or less; should I surface a link to the policy stuff there too?

Test Plan
  • made a project, toggled the "lock" setting, observed stickiness and good transactions being made
  • locked a project and tried to leave as a non-editor - got a dialogue letting me know i couldn't
  • locked a project and tried to leave as an editor - left successfully

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

btrahan retitled this revision from to Projects - add "lock membership", which prevents people from leaving.
btrahan updated this object.
btrahan edited the test plan for this revision. (Show Details)
btrahan added a reviewer: epriestley.
epriestley edited edge metadata.

Sweet.

src/applications/project/controller/PhabricatorProjectUpdateController.php
55–57

At some point (maybe now) we should move this to ProjectTransactionEditor->validateTransaction(), so we get the check for free on, e.g., some future project.leave Conduit method.

This revision is now accepted and ready to land.Sep 18 2014, 12:39 AM

I think the dialogue when you try to leave a project that is locked could be a little better maybe? Right now it just says "You can't leave" and "The membership is locked" more or less; should I surface a link to the policy stuff there too?

Oh, I think this is fine as-is. Since this is basically an administrative option, hopefully the reasoning will be clear from context. I think we can wait for users to get confused, at least.

If they do get confused, I think they're probably most likely to be confused about why a project is locked, which we can't easily answer anyway. Like, the information that's most helpful would be "who locked it (which we do know) and why (which we don't)", not "who is allowed to lock projects". So we could add some kind of "lock reason" prompt or something, but this seems like overkill unless users are actually confused.

btrahan edited edge metadata.
  • enforce "not being able to leave a locked project" at the editor level in requireCapabilities
    • keep the userCannotLeave project so we can render the better dialogue for users who hit this case
      • comment it though

re-jigger logic in requireCapabilities so it requires the right capabilities. in more detail, break apart the if / else if / else into three distinct if statements.

back out that last bit and restore the if / else if / else

This revision was automatically updated to reflect the committed changes.