test: fixing test failure post merging main

Author: kriswest

.vscode/settings.json

@@ -8,5 +8,6 @@
     "source.fixAll.eslint": "explicit"
   },
   "editor.defaultFormatter": "esbenp.prettier-vscode",
-  "editor.formatOnSave": true
+  "editor.formatOnSave": true,
+  "cSpell.words": ["Deltafied"]
 }

src/proxy/processors/push-action/checkUserPushPermission.ts

@@ -5,27 +5,30 @@ import { trimTrailingDotGit } from '../../../db/helper';
 // Execute if the repo is approved
 const exec = async (req: any, action: Action): Promise<Action> => {
   const step = new Step('checkUserPushPermission');
-  const user = action.user;
+  const userEmail = action.userEmail;
 
-  if (!user) {
-    console.log('Action has no user set. This may be due to a fast-forward ref update. Deferring to getMissingData action.');
+  if (!userEmail) {
+    console.log(
+      'Action has no userEmail set. This may be due to a fast-forward ref update. Deferring to getMissingData action.',
+    );
     return action;
   }
 
-  return await validateUser(user, action, step);
+  return await validateUser(userEmail, action, step);
 };
 
 /**
  * Helper that validates the user's push permission.
  * This can be used by other actions that need it. For example, when the user is missing from the commit data,
  * validation is deferred to getMissingData, but the logic is the same.
- * @param {string} user The user to validate
+ * @param {string} userEmail The user email to validate
  * @param {Action} action The action object
  * @param {Step} step The step object
  * @return {Promise<Action>} The action object
  */
-const validateUser = async (user: string, action: Action, step: Step): Promise<Action> => { 
+const validateUser = async (userEmail: string, action: Action, step: Step): Promise<Action> => {
   const repoSplit = trimTrailingDotGit(action.repo.toLowerCase()).split('/');
+
   // we expect there to be exactly one / separating org/repoName
   if (repoSplit.length != 2) {
     step.setError('Server-side issue extracting repoName');
@@ -36,13 +39,8 @@ const validateUser = async (user: string, action: Action, step: Step): Promise<A
   const repoName = repoSplit[1];
   let isUserAllowed = false;
 
-  // n.b. action.user will be set to whatever the user had set in their user.name config in their git client.
-  // it is not necessarily the GitHub username. GitHub looks users by email address as should we
-  const userEmail = action.userEmail;
-  let username = 'unknown';
-
   // Find the user associated with this email address
-  const list = await getUsers({ email: action.userEmail });
+  const list = await getUsers({ email: userEmail });
 
   if (list.length > 1) {
     console.error(`Multiple users found with email address ${userEmail}, ending`);
@@ -59,18 +57,15 @@ const validateUser = async (user: string, action: Action, step: Step): Promise<A
   } else if (list.length == 0) {
     console.error(`No user with email address ${userEmail} found`);
   } else {
-    username = list[0].username;
-    isUserAllowed = await isUserPushAllowed(repoName, username);
+    isUserAllowed = await isUserPushAllowed(repoName, list[0].username);
   }
 
-  console.log(`User ${username} <${userEmail}> permission on Repo ${repoName} : ${isUserAllowed}`);
+  console.log(`User ${userEmail} permission on Repo ${repoName} : ${isUserAllowed}`);
 
   if (!isUserAllowed) {
     console.log('User not allowed to Push');
     step.error = true;
-    step.log(
-      `User ${username} <${userEmail}> is not allowed to push on repo ${action.repo}, ending`,
-    );
+    step.log(`User ${userEmail} is not allowed to push on repo ${action.repo}, ending`);
 
     step.setError(
       `Your push has been blocked (${action.userEmail} ` +
@@ -81,7 +76,7 @@ const validateUser = async (user: string, action: Action, step: Step): Promise<A
     return action;
   }
 
-  step.log(`User ${username} <${userEmail}> is allowed to push on repo ${action.repo}`);
+  step.log(`User ${userEmail} is allowed to push on repo ${action.repo}`);
   action.addStep(step);
   return action;
 };

src/proxy/processors/types.ts

@@ -17,33 +17,34 @@ export type CommitContent = {
   deflatedSize: number;
   objectRef: any;
   content: string;
-}
+};
 
 export type PersonLine = {
   name: string;
   email: string;
   timestamp: string;
-}
+};
 
 export type CommitHeader = {
   tree: string;
   parents: string[];
   author: PersonLine;
   committer: PersonLine;
-}
+};
 
 export type CommitData = {
   tree: string;
   parent: string;
   author: string;
   committer: string;
   authorEmail: string;
+  committerEmail: string;
   commitTimestamp: string;
   message: string;
-}
+};
 
 export type PackMeta = {
   sig: string;
   version: number;
   entries: number;
-}
+};

src/ui/utils.tsx

@@ -5,13 +5,13 @@
  */
 export const getCookie = (name: string): string | null => {
   if (!document.cookie) return null;
-  
+
   const cookies = document.cookie
     .split(';')
     .map((c) => c.trim())
     .filter((c) => c.startsWith(name + '='));
-    
+
   if (!cookies.length) return null;
-  
+
   return decodeURIComponent(cookies[0].split('=')[1]);
-};
+};

test/processors/checkUserPushPermission.test.js

@@ -59,10 +59,10 @@ describe('checkUserPushPermission', () => {
       expect(result.steps).to.have.lengthOf(1);
       expect(result.steps[0].error).to.be.false;
       expect(stepSpy.lastCall.args[0]).to.equal(
-        'User db-user <db-user@test.com> is allowed to push on repo test/repo.git',
+        'User db-user@test.com is allowed to push on repo test/repo.git',
       );
       expect(logStub.lastCall.args[0]).to.equal(
-        'User db-user <db-user@test.com> permission on Repo repo : true',
+        'User db-user@test.com permission on Repo repo : true',
       );
     });
 

test/processors/getDiff.test.js

@@ -73,7 +73,9 @@ describe('getDiff', () => {
 
     const result = await exec({}, action);
     expect(result.steps[0].error).to.be.true;
-    expect(result.steps[0].errorMessage).to.contain('Your push has been blocked because no commit data was found');
+    expect(result.steps[0].errorMessage).to.contain(
+      'Your push has been blocked because no commit data was found',
+    );
   });
 
   it('should throw an error if no commit data is provided', async () => {
@@ -86,7 +88,9 @@ describe('getDiff', () => {
 
     const result = await exec({}, action);
     expect(result.steps[0].error).to.be.true;
-    expect(result.steps[0].errorMessage).to.contain('Your push has been blocked because no commit data was found');
+    expect(result.steps[0].errorMessage).to.contain(
+      'Your push has been blocked because no commit data was found',
+    );
   });
 
   it('should handle empty commit hash in commitFrom', async () => {

test/testCheckUserPushPermission.test.js

@@ -18,15 +18,14 @@ describe('CheckUserPushPermissions...', async () => {
   before(async function () {
     await db.deleteRepo(TEST_REPO);
     await db.deleteUser(TEST_USERNAME_1);
-
+    await db.deleteUser(TEST_USERNAME_2);
     await db.createRepo({
       project: TEST_ORG,
       name: TEST_REPO,
       url: TEST_URL,
     });
     await db.createUser(TEST_USERNAME_1, 'abc', TEST_EMAIL_1, TEST_USERNAME_1, false);
     await db.addUserCanPush(TEST_REPO, TEST_USERNAME_1);
-
     await db.createUser(TEST_USERNAME_2, 'abc', TEST_EMAIL_2, TEST_USERNAME_2, false);
   });
 

test/testParsePush.test.js

@@ -110,7 +110,7 @@ describe('parsePackFile', () => {
       const step = action.steps[0];
       expect(step.stepName).to.equal('parsePackFile');
       expect(step.error).to.be.true;
-      expect(step.errorMessage).to.include('No data received');
+      expect(step.errorMessage).to.include('No body found in request');
     });
 
     it('should add error step if req.body is empty', async () => {
@@ -121,7 +121,7 @@ describe('parsePackFile', () => {
       const step = action.steps[0];
       expect(step.stepName).to.equal('parsePackFile');
       expect(step.error).to.be.true;
-      expect(step.errorMessage).to.include('No data received');
+      expect(step.errorMessage).to.include('No body found in request');
     });
 
     it('should add error step if no ref updates found', async () => {
@@ -536,6 +536,7 @@ describe('parsePackFile', () => {
         parent: '456',
         author: 'Au Thor',
         committer: 'Com Itter',
+        committerEmail: 'c@e.com',
         commitTimestamp: '222',
         message: 'Commit message here',
         authorEmail: 'a@e.com',