Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a component for filtering repositories based on their results #2343

Merged
merged 13 commits into from
Apr 19, 2023
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
defaultFilterSortState,
filterAndSortRepositoriesWithResults,
filterAndSortRepositoriesWithResultsByName,
FilterKey,
matchesFilter,
SortKey,
} from "../../src/pure/variant-analysis-filter-sort";
Expand Down Expand Up @@ -42,6 +43,60 @@ describe(matchesFilter.name, () => {
).toBe(matches);
},
);

it("returns true if filterKey is all and resultCount is positive", () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: this test block is quite large, I wonder if it makes sense to chunk it up into describe blocks somehow? E.g. the first tests (not the ones you added here) are all about searching, while these new ones describe "when filterKey is all/withResults"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've split it up into two describe blocks. Let me know if that wasn't quite what you meant.

expect(
matchesFilter(
{ repository, resultCount: 1 },
{ ...defaultFilterSortState, filterKey: FilterKey.All },
),
).toBe(true);
});

it("returns true if filterKey is all and resultCount is zero", () => {
expect(
matchesFilter(
{ repository, resultCount: 0 },
{ ...defaultFilterSortState, filterKey: FilterKey.All },
),
).toBe(true);
});

it("returns true if filterKey is all and resultCount is undefined", () => {
expect(
matchesFilter(
{ repository },
{ ...defaultFilterSortState, filterKey: FilterKey.All },
),
).toBe(true);
});

it("returns true if filterKey is withResults and resultCount is positive", () => {
expect(
matchesFilter(
{ repository, resultCount: 1 },
{ ...defaultFilterSortState, filterKey: FilterKey.WithResults },
),
).toBe(true);
});

it("returns false if filterKey is withResults and resultCount is zero", () => {
expect(
matchesFilter(
{ repository, resultCount: 0 },
{ ...defaultFilterSortState, filterKey: FilterKey.WithResults },
),
).toBe(false);
});

it("returns false if filterKey is withResults and resultCount is undefined", () => {
expect(
matchesFilter(
{ repository },
{ ...defaultFilterSortState, filterKey: FilterKey.WithResults },
),
).toBe(false);
});
});

describe(compareRepository.name, () => {
Expand Down Expand Up @@ -352,7 +407,7 @@ describe(filterAndSortRepositoriesWithResultsByName.name, () => {
},
];

describe("when sort key is given without filter", () => {
describe("when sort key is given without search or filter", () => {
it("returns the correct results", () => {
expect(
filterAndSortRepositoriesWithResultsByName(repositories, {
Expand All @@ -368,7 +423,7 @@ describe(filterAndSortRepositoriesWithResultsByName.name, () => {
});
});

describe("when sort key and search filter are given", () => {
describe("when sort key and search are given without filter", () => {
it("returns the correct results", () => {
expect(
filterAndSortRepositoriesWithResultsByName(repositories, {
Expand All @@ -379,6 +434,30 @@ describe(filterAndSortRepositoriesWithResultsByName.name, () => {
).toEqual([repositories[2], repositories[0]]);
});
});

describe("when sort key and filter withResults are given without search", () => {
it("returns the correct results", () => {
expect(
filterAndSortRepositoriesWithResultsByName(repositories, {
...defaultFilterSortState,
sortKey: SortKey.ResultsCount,
filterKey: FilterKey.WithResults,
}),
).toEqual([repositories[3], repositories[2], repositories[0]]);
});
});

describe("when sort key and search and filter withResults are given", () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: I found this a bit difficult to parse 😅

Maybe something like

Suggested change
describe("when sort key and search and filter withResults are given", () => {
describe("when sort key, search, and filter withResults are given", () => {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Those definitely could be clearer 😅

I've updated the tests that have at least three items in the list to use commas.

it("returns the correct results", () => {
expect(
filterAndSortRepositoriesWithResultsByName(repositories, {
sortKey: SortKey.ResultsCount,
filterKey: FilterKey.WithResults,
searchValue: "r",
}),
).toEqual([repositories[3]]);
});
});
});

describe(filterAndSortRepositoriesWithResults.name, () => {
Expand Down Expand Up @@ -413,7 +492,7 @@ describe(filterAndSortRepositoriesWithResults.name, () => {
},
];

describe("when sort key is given without filter", () => {
describe("when sort key is given", () => {
it("returns the correct results", () => {
expect(
filterAndSortRepositoriesWithResults(repositories, {
Expand All @@ -429,7 +508,7 @@ describe(filterAndSortRepositoriesWithResults.name, () => {
});
});

describe("when sort key and search filter are given", () => {
describe("when sort key and search are given", () => {
it("returns the correct results", () => {
expect(
filterAndSortRepositoriesWithResults(repositories, {
Expand All @@ -441,12 +520,49 @@ describe(filterAndSortRepositoriesWithResults.name, () => {
});
});

describe("when sort key, search filter, and repository ids are given", () => {
describe("when sort key and filter withResults are given", () => {
it("returns the correct results", () => {
expect(
filterAndSortRepositoriesWithResults(repositories, {
...defaultFilterSortState,
sortKey: SortKey.ResultsCount,
filterKey: FilterKey.WithResults,
}),
).toEqual([repositories[3], repositories[2], repositories[0]]);
});
});

describe("when sort key and filter withResults are given", () => {
it("returns the correct results", () => {
expect(
filterAndSortRepositoriesWithResults(repositories, {
...defaultFilterSortState,
sortKey: SortKey.ResultsCount,
filterKey: FilterKey.WithResults,
}),
).toEqual([repositories[3], repositories[2], repositories[0]]);
});
});

describe("when sort key and search and filter withResults are given", () => {
robertbrignull marked this conversation as resolved.
Show resolved Hide resolved
it("returns the correct results", () => {
expect(
filterAndSortRepositoriesWithResults(repositories, {
...defaultFilterSortState,
sortKey: SortKey.ResultsCount,
filterKey: FilterKey.WithResults,
searchValue: "r",
}),
).toEqual([repositories[3]]);
});
});

describe("when sort key, search, filter withResults, and repository ids are given", () => {
it("returns the correct results", () => {
expect(
filterAndSortRepositoriesWithResults(repositories, {
sortKey: SortKey.ResultsCount,
filterKey: FilterKey.WithResults,
searchValue: "la",
repositoryIds: [
repositories[1].repository.id,
Expand Down