Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,18 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
case 'edited':
await _addCICDForRollers(pullRequestEvent);
await _checkForTests(pullRequestEvent);
if (pullRequestEvent.changes != null &&
pullRequestEvent.changes!.base != null &&
_isOrgMember(pr)) {
await _scheduleIfMergeable(pullRequestEvent);
}
break;
case 'opened':
await _addCICDForRollers(pullRequestEvent);
await _checkForTests(pullRequestEvent);
if (_isOrgMember(pr)) {
await _scheduleIfMergeable(pullRequestEvent);
}
await _tryReleaseApproval(pullRequestEvent);
break;
case 'reopened':
Expand Down Expand Up @@ -232,8 +240,12 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
case 'dequeued':
await _respondToPullRequestDequeued(pullRequestEvent);
break;
// Ignore the rest of the events.
case 'synchronize':
if (_isOrgMember(pr)) {
await _scheduleIfMergeable(pullRequestEvent);
}
break;
// Ignore the rest of the events.
case 'ready_for_review':
case 'unlabeled':
case 'assigned':
Expand All @@ -247,6 +259,17 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
return Response.emptyOk;
}

/// Returns `true` if the PR author is a member or owner of the org that
/// owns the repository.
///
/// This uses the `author_association` field from the GitHub webhook payload,
/// which avoids the need for an additional API call. The values `MEMBER` and
/// `OWNER` indicate the PR author belongs to the organization.
static bool _isOrgMember(PullRequest pr) {
final association = pr.authorAssociation;
return association == 'MEMBER' || association == 'OWNER';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are any of these valid:

COLLABORATOR, CONTRIBUTOR, FIRST_TIMER, FIRST_TIME_CONTRIBUTOR, MANNEQUIN, MEMBER, NONE, OWNER

I'm not sure what COLLABORATOR vs CONTRIBUTOR

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want MEMBER or OWNER only here, as that maps to the flutter hackers group, as I understand

}

Future<void> _processLabels(PullRequest pullRequest) async {
final slug = pullRequest.base!.repo!.slug();
final githubService = await config.createGithubService(slug);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,7 @@ void main() {
number: issueNumber,
baseRef: 'master',
slug: Config.packagesSlug,
isOrgMember: false,
);

tester.message = pushMessage;
Expand Down Expand Up @@ -459,6 +460,7 @@ void main() {
baseRef: 'master',
slug: Config.packagesSlug,
includeChanges: true,
isOrgMember: false,
);

tester.message = pushMessage;
Expand Down Expand Up @@ -531,6 +533,7 @@ void main() {
action: 'opened',
number: issueNumber,
isDraft: true,
isOrgMember: false,
);

var batchRequestCalled = false;
Expand Down Expand Up @@ -2667,6 +2670,7 @@ void foo() {
tester.message = generateGithubWebhookMessage(
action: 'synchronize',
number: issueNumber,
isOrgMember: false,
);

final mockRepositoriesService = MockRepositoriesService();
Expand Down Expand Up @@ -3691,26 +3695,61 @@ void foo() {
});

group('CICD label', () {
test('opened PR without CICD label does not schedule tests', () async {
tester.message = generateGithubWebhookMessage(
action: 'opened',
withCicdLabel: false,
);
test(
'opened PR without CICD label does schedule tests if author is MEMBER',
() async {
tester.message = generateGithubWebhookMessage(
action: 'opened',
withCicdLabel: false,
isOrgMember: true,
);

await tester.post(webhook);
expect(scheduler.triggerPresubmitTargetsCnt, 0);
});
await tester.post(webhook);
expect(scheduler.triggerPresubmitTargetsCnt, 1);
},
);

test('opened PR with CICD label does not schedules tests', () async {
tester.message = generateGithubWebhookMessage(
action: 'opened',
withCicdLabel: true,
);
test(
'opened PR without CICD label does not schedule tests if author is not MEMBER',
() async {
tester.message = generateGithubWebhookMessage(
action: 'opened',
withCicdLabel: false,
isOrgMember: false,
);

await tester.post(webhook);
await tester.post(webhook);
expect(scheduler.triggerPresubmitTargetsCnt, 0);
},
);

expect(scheduler.triggerPresubmitTargetsCnt, 0);
});
test(
'opened PR with CICD label does schedules tests if author is MEMBER',
() async {
tester.message = generateGithubWebhookMessage(
action: 'opened',
withCicdLabel: true,
);

await tester.post(webhook);

expect(scheduler.triggerPresubmitTargetsCnt, 1);
},
);
test(
'opened PR with CICD label does not schedules tests if author is not MEMBER',
() async {
tester.message = generateGithubWebhookMessage(
action: 'opened',
withCicdLabel: true,
isOrgMember: false,
);

await tester.post(webhook);

expect(scheduler.triggerPresubmitTargetsCnt, 0);
},
);

test(
'labeled event with CICD label schedules tests on flutter/flutter',
Expand Down Expand Up @@ -3796,6 +3835,7 @@ void foo() {
tester.message = generateGithubWebhookMessage(
action: 'synchronize',
withCicdLabel: true,
isOrgMember: false,
);

await tester.post(webhook);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ PushMessage generateGithubWebhookMessage({
bool withAutosubmit = false,
bool withRevertOf = false,
bool withCicdLabel = false,
bool isOrgMember = true,
DateTime? closedAt,
Iterable<String> additionalLabels = const [],
Map<String, Object?>? labeledLabel,
Expand All @@ -54,6 +55,7 @@ PushMessage generateGithubWebhookMessage({
withAutosubmit: withAutosubmit,
withRevertOf: withRevertOf,
withCicdLabel: withCicdLabel,
isOrgMember: isOrgMember,
closedAt: closedAt,
additionalLabels: additionalLabels,
labeledLabel: labeledLabel,
Expand Down Expand Up @@ -81,6 +83,7 @@ String _generatePullRequestEvent(
required bool withAutosubmit,
required bool withRevertOf,
bool withCicdLabel = false,
bool isOrgMember = true,
Iterable<String> additionalLabels = const [],
Map<String, Object?>? labeledLabel,
}) {
Expand Down Expand Up @@ -463,7 +466,7 @@ String _generatePullRequestEvent(
"href": "https://api.github.com/repos/${slug.fullName}/statuses/deadbeef"
}
},
"author_association": "MEMBER",
"author_association": "${isOrgMember ? 'MEMBER' : 'CONTRIBUTOR'}",
"draft" : $isDraft,
"merged": $merged,
"mergeable": $isMergeable,
Expand Down
Loading