Page MenuHomePhabricator

Correct some policy/membership issues in milestone creation
Closed, ResolvedPublic

Description

See PHI1552. There appear to be three bugs related to creating milestones:

  • We check the wrong edit policy when testing if you can create a milestone: we check the default application policy, but should check the parent project policy?
  • We predict the wrong set of members for the milestone when testing policies: we predict "no members", but should predict "exactly the same as the members of the parent project"?
  • We materialize some members into the milestone? This causes no real problems, but we shouldn't materialize members into milestones.

I haven't isolated these yet so I may be slightly wrong about what's going on here in some cases. These all seem like straightforward bugs rather than foundational issues, however.

Event Timeline

epriestley triaged this task as Normal priority.Nov 19 2019, 6:09 AM
epriestley created this task.

We check the wrong edit policy when testing if you can create a milestone: we check the default application policy, but should check the parent project policy?

To reproduce this:

  • Set the "Projects" application default edit policy to "No One".
  • Try to create a milestone which you should have permission to create.

Expected behavior:

  • You can create the milestone.

Actual behavior:

  • Milestone creation fails with message:

The selected edit policy excludes you. Choose a edit policy which allows you to edit the object.

We predict the wrong set of members for the milestone when testing policies: we predict "no members", but should predict "exactly the same as the members of the parent project"?

To reproduce this:

  • Create a project with "Editable By: Project Members".
  • As a member of the project, try to create a milestone.

Expected behavior:

  • You can create the milestone.

Actual behavior:

  • Milestone creation fails with message:

The selected edit policy excludes you. Choose a edit policy which allows you to edit the object.


There are likely several issues here. One is that project milestones directly inherit policies from their parents with $this->getParentProject()->getPolicy(...). This is generally incorrect, because object policies will evaluate on the milestone instead of on the parent project. That is, the object policy "Project Members" will evaluate "Project Members <of Milestone>" when the real policy is "Project Members <of Parent>".

Since milestones and parents always have the same members -- and there is no other upstream object policy which ever evaluates differently for parents and milestones -- this doesn't actually matter: both evaluations always produce the same result. Some day, an object policy which cares about this distinction may be introduced and this may need to be fixed properly.

The second issue is that it seems like we predict milestone members incorrectly. We should predict that the post-edit milestone members will be exactly the same as the parent members. It's desirable to correct this prediction whether the policy-inheritance issue is resolved or not, since it's a bug in its own right.

We materialize some members into the milestone? This causes no real problems, but we shouldn't materialize members into milestones.

After applying the other fixes, I can no longer reproduce this. Since it didn't cause any user-facing effects I'm not going to go searching for it; it's realistic to imagine that attaching the parent project earlier (in D20919) may have fixed it.