Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
27 changes: 21 additions & 6 deletions app_dart/lib/src/request_handlers/github/webhook_subscription.dart
Original file line number Diff line number Diff line change
Expand Up @@ -199,11 +199,14 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
final result = await _processPullRequestClosed(pullRequestEvent);
return result.toResponse();
case 'edited':
await _addCICDForRollers(pullRequestEvent);
if (pullRequestEvent.changes != null &&
pullRequestEvent.changes!.base != null) {
await _addCICDForRollersAndMembers(pullRequestEvent);
Copy link
Copy Markdown
Member Author

@gmackall gmackall Mar 25, 2026

Choose a reason for hiding this comment

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

This is a slight behavior change, I made it this way because before the path for scheduling tests checked these conditions:
363e9cd#diff-4d5f5e21f5b7eda8e7775b9a678fb60bb6cfba3321799948fb2b9a55cefbe439L202-L206
And I figured it was also fine to apply those checks for the roller case. Let me know if not

}
await _checkForTests(pullRequestEvent);
break;
case 'opened':
await _addCICDForRollers(pullRequestEvent);
await _addCICDForRollersAndMembers(pullRequestEvent);
await _checkForTests(pullRequestEvent);
await _tryReleaseApproval(pullRequestEvent);
break;
Expand Down Expand Up @@ -232,8 +235,10 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
case 'dequeued':
await _respondToPullRequestDequeued(pullRequestEvent);
break;
// Ignore the rest of the events.
case 'synchronize':
await _addCICDForRollersAndMembers(pullRequestEvent);
break;
// Ignore the rest of the events.
case 'ready_for_review':
case 'unlabeled':
case 'assigned':
Expand Down Expand Up @@ -569,12 +574,22 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
);
}

Future<void> _addCICDForRollers(PullRequestEvent pullRequestEvent) async {
Future<void> _addCICDForRollersAndMembers(
PullRequestEvent pullRequestEvent,
) async {
final pr = pullRequestEvent.pullRequest!;
final slug = pr.base!.repo!.slug();
final author = pr.user!.login!;
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 double checked that this returns the author's user, and not the user of the person interacting with the PR:
https://pub.dev/documentation/github/latest/github/PullRequest/user.html

final githubService = await config.createGithubService(slug);

final isRoller = config.rollerAccounts.contains(pr.user!.login);
final isFlutterHacker = await githubService.isTeamMember(
'flutter-hackers',
author,
slug.owner,
);

if (config.rollerAccounts.contains(pr.user!.login) &&
config.supportedRepos.contains(slug)) {
if (config.supportedRepos.contains(slug) && (isRoller || isFlutterHacker)) {
final gitHubClient = await config.createGitHubClient(pullRequest: pr);
await gitHubClient.issues.addLabelsToIssue(slug, pr.number!, ['CICD']);
}
Expand Down
14 changes: 14 additions & 0 deletions app_dart/lib/src/service/github_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,20 @@ class GithubService {
.toList();
}

/// Check to see if user is a member of team in org.
///
/// Note that we catch here as the api returns a 404 if the user has no
/// membership in general or is not a member of the team.
Future<bool> isTeamMember(String team, String user, String org) async {
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 copied this from

Future<bool> isTeamMember(String team, String user, String org) async {

as there are duplicate github_service.darts and this one did not have the method. It may be good to de-dupe these at some point

try {
final teamMembershipState = await github.organizations
.getTeamMembershipByName(org, team, user);
return teamMembershipState.isActive;
} on GitHubError {
return false;
}
}

/// Creates a pull request against the `baseRef` in the `slug` repository.
///
/// The `entries` contains the file changes in the created pull request. This
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3715,6 +3715,34 @@ void foo() {
expect(scheduler.triggerPresubmitTargetsCnt, 0);
});

test(
'opened PR with adds CICD label if author is member of flutter-hackers',
() async {
tester.message = generateGithubWebhookMessage(
action: 'opened',
login: 'test-flutter-hacker',
);

await tester.post(webhook);

verify(
issuesService.addLabelsToIssue(Config.flutterSlug, 123, ['CICD']),
);
},
);
test(
'opened PR does not add CICD label if author is not member of flutter-hackers',
() async {
tester.message = generateGithubWebhookMessage(action: 'opened');

await tester.post(webhook);

verifyNever(
issuesService.addLabelsToIssue(Config.flutterSlug, 123, any),
);
},
);

test(
'labeled event with CICD label schedules tests on flutter/flutter',
() async {
Expand Down Expand Up @@ -3805,5 +3833,31 @@ void foo() {
expect(scheduler.triggerPresubmitTargetsCnt, 0);
},
);

test(
'synchronize event adds CICD label if author is member of flutter-hackers',
() async {
tester.message = generateGithubWebhookMessage(
action: 'synchronize',
login: 'test-flutter-hacker',
);

await tester.post(webhook);
verify(
issuesService.addLabelsToIssue(Config.flutterSlug, 123, ['CICD']),
);
},
);
test(
'synchronize event does not add CICD label if author is not member of flutter-hackers',
() async {
tester.message = generateGithubWebhookMessage(action: 'synchronize');

await tester.post(webhook);
verifyNever(
issuesService.addLabelsToIssue(Config.flutterSlug, 123, any),
);
},
);
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,14 @@ class FakeGithubService implements GithubService {
return true;
}

@override
Future<bool> isTeamMember(String team, String user, String org) async {
if (user == 'test-flutter-hacker') {
return true;
}
return false;
}

@override
Future<void> assignReviewer(
RepositorySlug slug, {
Expand Down
Loading