Fix Part Of N/A: Ensures that bot adds 'PR:Affects Datastore Label' w…

…hen a new cron job is added (#268) * updated * added label for cron job * updated * index.js reverted
oppia · May 18, 2021 · 2053701 · 2053701
1 parent c6ba512
commit 2053701
Show file tree

Hide file tree

Showing 3 changed files with 100 additions and 16 deletions.
diff --git a/lib/checkNewCronJobs.js b/lib/checkNewCronJobs.js
@@ -16,11 +16,12 @@ const {
   serverJobAdmins
 } = require('../userWhitelist.json');
 const {
+  DATASTORE_LABEL,
   JOBS_AND_FETURES_TESTING_WIKI_LINK,
   getAllChangedFiles,
+  hasDatastoreLabel,
   pingAndAssignUsers
 } = require('./utils');
-const constants = require('../constants');
 const CRON_REGISTRY_FILENAME = 'main_cron.py';
 const CRON_TEST_FILENAME = 'core/controllers/cron_test.py';
 const CRON_JOB_FILE = 'core/controllers/cron.py';
@@ -105,24 +106,34 @@ const checkForNewCronJob = async (context) => {
    * @type {import('probot').Octokit.PullsGetResponse} pullRequest
    */
   const pullRequest = context.payload.pull_request;
-  const changedFiles = await getAllChangedFiles(context);
-  const cronFileIsChanged = isCronFileChanged(changedFiles);
-  const testsAreAdded = hasAddedTests(changedFiles);
-  if (cronFileIsChanged || testsAreAdded) {
-    const commentBody = getCommentBody(
-      changedFiles,
-      pullRequest.user.login
-    );
+  if (!hasDatastoreLabel(pullRequest)) {
+    const changedFiles = await getAllChangedFiles(context);
+    const cronFileIsChanged = isCronFileChanged(changedFiles);
+    const testsAreAdded = hasAddedTests(changedFiles);
+    if (cronFileIsChanged || testsAreAdded) {
+      const commentBody = getCommentBody(
+        changedFiles,
+        pullRequest.user.login
+      );
 
-    await pingAndAssignUsers(
-      context,
-      pullRequest,
-      serverJobAdmins,
-      commentBody
-    );
+      await pingAndAssignUsers(
+        context,
+        pullRequest,
+        serverJobAdmins,
+        commentBody
+      );
+
+      const labelParams = context.repo({
+        issue_number: pullRequest.number,
+        labels: [DATASTORE_LABEL]
+      });
+      await context.github.issues.addLabels(labelParams);
+    }
   }
 };
 
 module.exports = {
   checkForNewCronJob,
+  hasDatastoreLabel,
+  DATASTORE_LABEL
 };
diff --git a/lib/checkPullRequestJob.js b/lib/checkPullRequestJob.js
@@ -24,7 +24,6 @@ const {
   hasDatastoreLabel,
   pingAndAssignUsers
 } = require('./utils');
-const constants = require('../constants');
 const REGISTRY_FILENAME = 'jobs_registry.py';
 const TEST_DIR_PREFIX = 'core/tests/';
 const JOB_REGEX = new RegExp(

diff --git a/spec/checkNewCronJobSpec.js b/spec/checkNewCronJobSpec.js
@@ -273,6 +273,16 @@ describe('Cron Job Spec', () => {
         assignees: ['vojtechjelinek']
       });
     });
+
+    it('should add datastore label', () => {
+      expect(github.issues.addLabels).toHaveBeenCalled();
+      expect(github.issues.addLabels).toHaveBeenCalledWith({
+        issue_number: payloadData.payload.pull_request.number,
+        repo: payloadData.payload.repository.name,
+        owner: payloadData.payload.repository.owner.login,
+        labels: ['PR: Affects datastore layer']
+      });
+    });
   });
 
   describe('When a new cron job is added in an existing cron job file', () => {
@@ -330,6 +340,16 @@ describe('Cron Job Spec', () => {
         assignees: ['vojtechjelinek']
       });
     });
+
+    it('should add datastore label', () => {
+      expect(github.issues.addLabels).toHaveBeenCalled();
+      expect(github.issues.addLabels).toHaveBeenCalledWith({
+        issue_number: payloadData.payload.pull_request.number,
+        repo: payloadData.payload.repository.name,
+        owner: payloadData.payload.repository.owner.login,
+        labels: ['PR: Affects datastore layer']
+      });
+    });
   });
 
   describe('When no job file is modified in a pull request', () => {
@@ -357,6 +377,10 @@ describe('Cron Job Spec', () => {
     it('should not ping server job admin', () => {
       expect(github.issues.createComment).not.toHaveBeenCalled();
     });
+
+    it('should not add datastore label', () => {
+      expect(github.issues.addLabels).not.toHaveBeenCalled();
+    });
   });
 
   describe('When test and URL redirects are added for a  cron job', () => {
@@ -404,6 +428,16 @@ describe('Cron Job Spec', () => {
         owner: payloadData.payload.repository.owner.login,
       });
     });
+
+    it('should add datastore label', () => {
+      expect(github.issues.addLabels).toHaveBeenCalled();
+      expect(github.issues.addLabels).toHaveBeenCalledWith({
+        issue_number: payloadData.payload.pull_request.number,
+        repo: payloadData.payload.repository.name,
+        owner: payloadData.payload.repository.owner.login,
+        labels: ['PR: Affects datastore layer']
+      });
+    });
   });
 
   describe('When only tests are added for a  cron job', () => {
@@ -451,5 +485,45 @@ describe('Cron Job Spec', () => {
         owner: payloadData.payload.repository.owner.login,
       });
     });
+
+    it('should add datastore label', () => {
+      expect(github.issues.addLabels).toHaveBeenCalled();
+      expect(github.issues.addLabels).toHaveBeenCalledWith({
+        issue_number: payloadData.payload.pull_request.number,
+        repo: payloadData.payload.repository.name,
+        owner: payloadData.payload.repository.owner.login,
+        labels: ['PR: Affects datastore layer']
+      });
+    });
+  });
+
+  describe('When pull request has datastore label', () => {
+    beforeEach(async () => {
+      payloadData.payload.pull_request.labels = [{
+        name: 'PR: Affects datastore layer'
+      }];
+      github.pulls = {
+        listFiles: jasmine.createSpy('listFiles').and.resolveTo({
+          data: [
+            nonJobFile, firstNewJobFileObj
+          ],
+        }),
+      };
+
+      payloadData.payload.pull_request.changed_files = 2;
+      await robot.receive(payloadData);
+    });
+
+    it('should check for cron jobs', () => {
+      expect(checkCronJobModule.checkForNewCronJob).toHaveBeenCalled();
+    });
+
+    it('should not get modified files', () => {
+      expect(github.pulls.listFiles).not.toHaveBeenCalled();
+    });
+
+    it('should not ping server job admin', () => {
+      expect(github.issues.createComment).not.toHaveBeenCalled();
+    });
   });
 });