Skip to content

Commit

Permalink
Fixed duplicated usernames in and-distinct error object (#75)
Browse files Browse the repository at this point in the history
Found a bug where usernames where duplicated in the error object.

This fixes #73
  • Loading branch information
Bullrich authored Sep 8, 2023
1 parent 1658036 commit feb3619
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ export class ActionRunner {
// Utility method used to generate error
const generateErrorReport = (): ReviewReport => {
const filterMissingUsers = (reviewData: { users?: string[] }[]): string[] =>
reviewData.flatMap((r) => r.users ?? []).filter((u) => approvals.indexOf(u) < 0);
Array.from(new Set(reviewData.flatMap((r) => r.users ?? []).filter((u) => approvals.indexOf(u) < 0)));

// Calculating all the possible combinations to see the missing reviewers is very complicated
// Instead we request everyone who hasn't reviewed yet
Expand Down
17 changes: 17 additions & 0 deletions src/test/runner/validation/andDistinct.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,23 @@ describe("'And distinct' rule validation", () => {
expect(logger.warn).toHaveBeenCalledWith("Not enough positive reviews to match a subcondition");
});

test("should not have duplicates in missingUsers", async () => {
const rule: AndDistinctRule = {
type: RuleTypes.AndDistinct,
reviewers: [
{ users: [users[0], "def"], min_approvals: 2 },
{ teams: ["team-abc"], min_approvals: 1 },
],
name: "test",
condition: { include: [] },
};

const [result, error] = await runner.andDistinctEvaluation(rule);
expect(result).toBe(false);
const hasDuplicates = <T>(arr: T[]) => arr.some((item, index) => arr.indexOf(item) !== index);
expect(hasDuplicates(error?.missingUsers as string[])).toBeFalsy();
});

test("should not consider author in evaluation", async () => {
const rule: AndDistinctRule = {
type: RuleTypes.AndDistinct,
Expand Down

0 comments on commit feb3619

Please sign in to comment.