Make the new Build Plan "Runnable" behavior work
ClosedPublic
Actions

Authored by epriestley on Feb 28 2019, 7:41 PM.

Details

Reviewers

Maniphest Tasks

Commits

Summary

Ref T13258. Fixes T11415. This makes "Runnable" actually do something:

With "Runnable" set to "If Editable" (default): to manually run, pause, resume, abort, or restart a build, you must normally be able to edit the associated build plan.
If you toggle "Runnable" to "If Viewable", anyone who can view the build plan may take these actions.

This is pretty straightforward since T9614 already got us pretty close to this ruleset a while ago.

Test Plan

Created a Build Plan, set "Can Edit" to just me, toggled "Runnable" to "If Viewable"/"If Editable", tried to take actions as another user.
With "If Editable", unable to run, pause, resume, abort, or restart as another user.
With "If Viewable", those actions work.

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Owners added a subscriber: Restricted Owners Package.Feb 28 2019, 7:41 PM

epriestley requested review of this revision.Feb 28 2019, 7:43 PM

amckinley added inline comments.

src/applications/harbormaster/codex/HarbormasterBuildPlanPolicyCodex.php
21–22	"You must have edit permission on this build plan to pause, abort, resume, restart, or run it manually."

This revision is now accepted and ready to land.Mar 5 2019, 7:38 PM

src/applications/harbormaster/codex/HarbormasterBuildPlanPolicyCodex.php
21–22	My phrasing is weird, but I think this replacement phrasing allows a reading as "...to (x, y, or z) it manually". That is, it could suggest we're talking about the ability to pause the build "manually", versus pause the build in general. Although this is technically true today (pauses are manual), there's no other type of pause, while there are other types of "run". Notably, you can still write a Herald global rule to run a plan you can't run manually. So the "manual" part is an important qualifier for "run", but a possibly misleading qualifier for "pause/restart/etc". I'm just going to break this into two rules/sentences, since that's probably clearer and we might refine these rules eventually anyway.