Revert "Overhaul CICD label management for presubmits (#5028)" (#5029)

This reverts commit 7cbf0ea.

This change caused issues where we were double-scheduling presubmits on roller bots, which caused failures. Rolling this back until we get this problem resolved.

Author: eyebrowsoffire

app_dart/docs/cicd_flowchart.md

@@ -173,8 +173,6 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
   /// retried with exponential backoff within that time period. The GoB mirror
   /// should be caught up within that time frame via either the internal
   /// mirroring service or [VacuumGithubCommits].
-  ///
-  /// See docs/cicd_flowchart.md for the flowchart of CICD label management.
   Future<Response> _handlePullRequest(String rawRequest) async {
     final pullRequestEvent = _getPullRequestEvent(rawRequest);
     if (pullRequestEvent == null) {
@@ -201,20 +199,16 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
         final result = await _processPullRequestClosed(pullRequestEvent);
         return result.toResponse();
       case 'edited':
+        if (pullRequestEvent.changes != null &&
+            pullRequestEvent.changes!.base != null) {
+          await _addCICDForRollersAndMembers(pullRequestEvent);
+        }
+        await scheduler.createAwaitingCicdLabelCheckRun(slug, pr.head!.sha!);
         await _checkForTests(pullRequestEvent);
         break;
       case 'opened':
-        final isPrivileged = await _isPrivilegedUser(pr, slug);
-        if (!pr.draft! && isPrivileged) {
-          final gitHubClient = await config.createGitHubClient(pullRequest: pr);
-          await gitHubClient.issues.addLabelsToIssue(slug, pr.number!, [
-            'CICD',
-          ]);
-          log.debug('Added CICD label to PR ${pr.number}');
-          await _scheduleIfMergeable(pullRequestEvent);
-        } else {
-          await scheduler.createAwaitingCicdLabelCheckRun(slug, pr.head!.sha!);
-        }
+        await _addCICDForRollersAndMembers(pullRequestEvent);
+        await scheduler.createAwaitingCicdLabelCheckRun(slug, pr.head!.sha!);
         await _checkForTests(pullRequestEvent);
         await _tryReleaseApproval(pullRequestEvent);
         break;
@@ -248,31 +242,7 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
         await _respondToPullRequestDequeued(pullRequestEvent);
         break;
       case 'synchronize':
-        final isPrivileged = await _isPrivilegedUser(pr, slug);
-        final hasLabel =
-            pr.labels?.any((l) => l.name == Config.kCicdLabel) ?? false;
-        final githubService = await config.createGithubService(slug);
-
-        if (isPrivileged) {
-          if (hasLabel) {
-            await _scheduleIfMergeable(pullRequestEvent);
-          } else {
-            await scheduler.createAwaitingCicdLabelCheckRun(
-              slug,
-              pr.head!.sha!,
-            );
-          }
-        } else {
-          if (hasLabel) {
-            await githubService.removeLabel(
-              slug,
-              pr.number!,
-              Config.kCicdLabel,
-            );
-            log.debug('Removed CICD label from PR ${pr.number}');
-          }
-          await scheduler.createAwaitingCicdLabelCheckRun(slug, pr.head!.sha!);
-        }
+        await _addCICDForRollersAndMembers(pullRequestEvent);
         break;
       // Ignore the rest of the events.
       case 'ready_for_review':
@@ -610,16 +580,27 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
     );
   }
 
-  /// Returns true if the user is a roller or a member of flutter-hackers.
-  Future<bool> _isPrivilegedUser(PullRequest pr, RepositorySlug slug) async {
-    final isRoller = config.rollerAccounts.contains(pr.user!.login);
+  Future<void> _addCICDForRollersAndMembers(
+    PullRequestEvent pullRequestEvent,
+  ) async {
+    final pr = pullRequestEvent.pullRequest!;
+    final slug = pr.base!.repo!.slug();
+    final author = pr.user!.login!;
     final githubService = await config.createGithubService(slug);
+
+    final isRoller = config.rollerAccounts.contains(pr.user!.login);
     final isFlutterHacker = await githubService.isTeamMember(
       'flutter-hackers',
-      pr.user!.login!,
-      'flutter',
+      author,
+      slug.owner,
     );
-    return isRoller || isFlutterHacker;
+    log.debug('isRoller=$isRoller, isFlutterHacker=$isFlutterHacker');
+
+    if (config.supportedRepos.contains(slug) && (isRoller || isFlutterHacker)) {
+      final gitHubClient = await config.createGitHubClient(pullRequest: pr);
+      await gitHubClient.issues.addLabelsToIssue(slug, pr.number!, ['CICD']);
+      log.debug('Added CICD label to PR $pr.number');
+    }
   }
 
   Future<void> _checkForTests(PullRequestEvent pullRequestEvent) async {

app_dart/lib/src/request_handlers/github/webhook_subscription.dart

@@ -3755,7 +3755,7 @@ void foo() {
     });
 
     test(
-      'opened PR with adds CICD label and schedules tests if author is member of flutter-hackers',
+      'opened PR with adds CICD label if author is member of flutter-hackers',
       () async {
         tester.message = generateGithubWebhookMessage(
           action: 'opened',
@@ -3766,8 +3766,7 @@ void foo() {
 
         verify(
           issuesService.addLabelsToIssue(Config.flutterSlug, 123, ['CICD']),
-        ).called(1);
-        expect(scheduler.triggerPresubmitTargetsCnt, 1);
+        );
       },
     );
     test(
@@ -3862,87 +3861,50 @@ void foo() {
     );
 
     test(
-      'synchronize event by non-privileged user removes CICD label and creates awaiting checkrun',
+      'synchronize event with CICD label does not schedules tests',
       () async {
-        githubService.checkRunsMock = '{"total_count": 0, "check_runs": [{}]}';
         tester.message = generateGithubWebhookMessage(
           action: 'synchronize',
           withCicdLabel: true,
         );
 
         await tester.post(webhook);
-
-        expect(githubService.removedLabels, [
-          (Config.flutterSlug, 123, Config.kCicdLabel),
-        ]);
-        verify(
-          mockGithubChecksUtil.createCheckRun(
-            any,
-            any,
-            any,
-            'Awaiting CICD label',
-            output: anyNamed('output'),
-          ),
-        ).called(1);
         expect(scheduler.triggerPresubmitTargetsCnt, 0);
       },
     );
 
     test(
-      'synchronize event by non-privileged user without label creates awaiting checkrun',
+      'synchronize event adds CICD label if author is member of flutter-hackers',
       () async {
-        githubService.checkRunsMock = '{"total_count": 0, "check_runs": [{}]}';
         tester.message = generateGithubWebhookMessage(
           action: 'synchronize',
-          withCicdLabel: false,
+          login: 'test-flutter-hacker',
         );
 
         await tester.post(webhook);
-
         verify(
-          mockGithubChecksUtil.createCheckRun(
-            any,
-            any,
-            any,
-            'Awaiting CICD label',
-            output: anyNamed('output'),
-          ),
-        ).called(1);
-        expect(scheduler.triggerPresubmitTargetsCnt, 0);
+          issuesService.addLabelsToIssue(Config.flutterSlug, 123, ['CICD']),
+        );
       },
     );
-
     test(
-      'synchronize event by privileged user with label schedules tests',
+      'synchronize event does not add CICD label if author is not member of flutter-hackers',
       () async {
-        tester.message = generateGithubWebhookMessage(
-          action: 'synchronize',
-          login: 'test-flutter-hacker',
-          withCicdLabel: true,
-        );
+        tester.message = generateGithubWebhookMessage(action: 'synchronize');
 
         await tester.post(webhook);
-
-        expect(scheduler.triggerPresubmitTargetsCnt, 1);
         verifyNever(
-          mockGithubChecksUtil.createCheckRun(
-            any,
-            any,
-            any,
-            'Awaiting CICD label',
-            output: anyNamed('output'),
-          ),
+          issuesService.addLabelsToIssue(Config.flutterSlug, 123, any),
         );
       },
     );
 
     test(
-      'synchronize event by privileged user without label creates awaiting checkrun',
+      'opened PR without CICD label creates "Awaiting CICD label" checkrun',
       () async {
         githubService.checkRunsMock = '{"total_count": 0, "check_runs": [{}]}';
         tester.message = generateGithubWebhookMessage(
-          action: 'synchronize',
-          login: 'test-flutter-hacker',
+          action: 'opened',
           withCicdLabel: false,
         );
 
@@ -3957,17 +3919,16 @@ void foo() {
             output: anyNamed('output'),
           ),
         ).called(1);
-        expect(scheduler.triggerPresubmitTargetsCnt, 0);
       },
     );
 
     test(
-      'opened PR without CICD label creates "Awaiting CICD label" checkrun',
+      'opened PR with CICD label STILL creates "Awaiting CICD label" checkrun if not exists',
       () async {
         githubService.checkRunsMock = '{"total_count": 0, "check_runs": [{}]}';
         tester.message = generateGithubWebhookMessage(
           action: 'opened',
-          withCicdLabel: false,
+          withCicdLabel: true,
         );
 
         await tester.post(webhook);
@@ -3985,12 +3946,12 @@ void foo() {
     );
 
     test(
-      'opened PR with CICD label STILL creates "Awaiting CICD label" checkrun if not exists',
+      'edited PR without CICD label creates "Awaiting CICD label" checkrun if not exists',
       () async {
         githubService.checkRunsMock = '{"total_count": 0, "check_runs": [{}]}';
         tester.message = generateGithubWebhookMessage(
-          action: 'opened',
-          withCicdLabel: true,
+          action: 'edited',
+          withCicdLabel: false,
         );
 
         await tester.post(webhook);