Fixed user OR teams to users AND teams (#30)

Fixed the `XOR` condition for a `OR` condition.

Updated tests.

Closes #28

src/file/validator.ts

@@ -5,7 +5,7 @@ import { ActionLogger } from "../github/types";
 import { BasicRule, ConfigurationFile, Rule } from "./types";
 
 /** For the users or team schema. Will be recycled A LOT
- * Remember to add `.xor("users", "teams")` to force one of the two to be picked up
+ * Remember to add `.or("users", "teams")` to force at least one of the two to be defined
  */
 const reviewersObj = {
   users: Joi.array().items(Joi.string()).optional().empty(null),
@@ -39,7 +39,7 @@ export const generalSchema = Joi.object<ConfigurationFile>().keys({
  */
 export const basicRuleSchema = Joi.object<BasicRule>()
   .keys({ min_approvals: Joi.number().empty(1), ...reviewersObj })
-  .xor("users", "teams");
+  .or("users", "teams");
 
 /**
  * Evaluates a config thoroughly. If there is a problem with it, it will throw.

src/runner.ts

@@ -6,6 +6,7 @@ import { validateConfig, validateRegularExpressions } from "./file/validator";
 import { PullRequestApi } from "./github/pullRequest";
 import { TeamApi } from "./github/teams";
 import { ActionLogger } from "./github/types";
+import { concatArraysUniquely } from "./util";
 
 type ReviewReport = {
   /** The amount of missing reviews to fulfill the requirements */
@@ -88,7 +89,7 @@ export class ActionRunner {
    */
   async evaluateCondition(rule: { min_approvals: number } & Reviewers): Promise<ReviewState> {
     // This is a list of all the users that need to approve a PR
-    const requiredUsers: string[] = [];
+    let requiredUsers: string[] = [];
     // If team is set, we fetch the members of such team
     if (rule.teams) {
       for (const team of rule.teams) {
@@ -101,11 +102,12 @@ export class ActionRunner {
         }
       }
       // If, instead, users are set, we simply push them to the array as we don't need to scan a team
-    } else if (rule.users) {
-      requiredUsers.push(...rule.users);
-    } else {
-      // This should be captured before by the validation
-      throw new Error("Teams and Users field are not set for rule.");
+    }
+    if (rule.users) {
+      requiredUsers = concatArraysUniquely(requiredUsers, rule.users);
+    }
+    if (requiredUsers.length === 0) {
+      throw new Error("No users have been found in the required reviewers");
     }
 
     if (requiredUsers.length < rule.min_approvals) {

src/test/runner/conditions.test.ts

@@ -19,7 +19,7 @@ describe("evaluateCondition tests", () => {
 
   test("should throw if no teams or users were set", async () => {
     await expect(runner.evaluateCondition({ min_approvals: 99 })).rejects.toThrowError(
-      "Teams and Users field are not set for rule.",
+      "No users have been found in the required reviewers",
     );
   });
 
@@ -131,6 +131,23 @@ describe("evaluateCondition tests", () => {
         expect(report?.missingUsers).toEqual([...team1.users, team2.users[0]]);
         expect(report?.teamsToRequest).toEqual([team1.name, team2.name]);
       });
+
+      describe("teams and users combined", () => {
+        test("should not duplicate users if they belong to team and user list", async () => {
+          teamsApi.getTeamMembers.calledWith(team).mockResolvedValue(users);
+          api.listApprovedReviewsAuthors.mockResolvedValue([]);
+          const [result, report] = await runner.evaluateCondition({
+            min_approvals: 1,
+            teams: [team],
+            users: [users[0]],
+          });
+          expect(result).toBeFalsy();
+          // Should not send required users more than once
+          expect(report?.missingUsers).toEqual(users);
+          expect(report?.teamsToRequest).toEqual([team]);
+          expect(report?.usersToRequest).toEqual([users[0]]);
+        });
+      });
     });
   });
 });

src/util.ts

@@ -5,3 +5,11 @@ import { ActionLogger } from "./github/types";
 export function generateCoreLogger(): ActionLogger {
   return { info, debug, warn: warning, error };
 }
+
+/** Concats two arrays and remove the duplicates */
+export function concatArraysUniquely<T>(arr1?: T[], arr2?: T[]): T[] {
+  // We concat the two arrays
+  const concatedArray = (arr1 ?? []).concat(arr2 ?? []);
+  // We remove the duplicated values and return the array
+  return concatedArray.filter((item, pos) => concatedArray.indexOf(item) === pos);
+}