Paths

Table of Contentst

Differential D18033

Index Project milestones to accurately reflect milestone membership
ClosedPublic
Actions

Authored by amckinley on May 26 2017, 7:56 PM.

Tags

None

Referenced Files

	F15337267: D18033.diff
	Sun, Mar 9, 5:03 AM

	Unknown Object (File)
	Thu, Feb 27, 5:09 AM

	Unknown Object (File)
	Sun, Feb 16, 12:17 PM

	Unknown Object (File)
	Feb 11 2025, 4:03 PM

	Unknown Object (File)
	Feb 8 2025, 9:41 PM

	Unknown Object (File)
	Feb 7 2025, 5:56 AM

	Unknown Object (File)
	Jan 31 2025, 1:40 AM

	Unknown Object (File)
	Jan 30 2025, 1:10 PM

Subscribers

Tokens

"Doubloon" token, awarded by epriestley.

Details

Reviewers

Maniphest Tasks

T12505: Ownership rules whose owning package is a milestone fail to show as accepted

Commits

rPb27c2ed6d1e6: Index Project milestones to accurately reflect milestone membership

Summary

Fixes T12505. PhabricatorProjectsMembershipIndexEngineExtension->materializeProject() was incorrectly bailing early for milestone objects, which prevented milestone members from being calculated correctly. This was causing problems where (for example) an Owners package owned by a milestone wasn't being satisfied when a member of the milestone approved a revision.

Test Plan

Invoked migration, observed that a user's milestones correctly showed up when searched for. Also observed that accepting a revision on behalf of a milestone now satisfies Owners rules.

Diff Detail

Repository

Branch

master

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 17283
Build 23153: Run Core Tests
Build 23152: arc lint + arc unit

Event Timeline

amckinley created this revision.May 26 2017, 7:56 PM

Herald added a subscriber: Korvin. · View Herald TranscriptMay 26 2017, 7:56 PM

Harbormaster completed remote builds in B17282: Diff 43372.May 26 2017, 7:57 PM

epriestley accepted this revision.May 26 2017, 7:57 PM

epriestley added inline comments.

src/applications/project/engineextension/PhabricatorProjectsMembershipIndexEngineExtension.php
41–61	We could do the `isMilestone()` test earlier and skip this extra query I think -- we know milestones never have subprojects or descendants.

This revision is now accepted and ready to land.May 26 2017, 7:57 PM

amckinley added inline comments.May 26 2017, 7:57 PM

src/applications/project/engineextension/PhabricatorProjectsMembershipIndexEngineExtension.php
63–66	Actually, since milestones can't have sub-milestones, this is probably better written by adding another clause to the existing `if` statement.

epriestley awarded a token.May 26 2017, 7:58 PM

amckinley added inline comments.May 26 2017, 7:58 PM

src/applications/project/engineextension/PhabricatorProjectsMembershipIndexEngineExtension.php
41–61	Jinx, I owe you a coke!

haha

dont check for subprojects on milestones

Harbormaster completed remote builds in B17283: Diff 43373.May 26 2017, 8:06 PM

(That update looks right to me.)

Closed by commit rPb27c2ed6d1e6: Index Project milestones to accurately reflect milestone membership (authored by amckinley, committed by amckinley). · Explain WhyMay 26 2017, 8:10 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

Coverage (All)

Coverage (Touched)

resources/

sql/

autopatches/

20170526.milestones.php

11 lines

-

-

src/

applications/

project/

engineextension/

PhabricatorProjectsMembershipIndexEngineExtension.php

35 lines

91%

Loading...

Diff 43373

resources/sql/autopatches/20170526.milestones.php

Loading...

src/applications/project/engineextension/PhabricatorProjectsMembershipIndexEngineExtension.php

Loading...