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

Lightning filter fixes #4757

Merged
merged 3 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ const Checkboxes = ({
const show = useRecoilValue(showSearchSelector({ modal, path }));
const getCount = useGetCount(modal, path);

if (loading) {
// if results are null, and show is false, values are loading
if (loading || (!show && results === null)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

probably we should add a comment here to explain this

return <LoadingDots text={"Loading"} />;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export default function (
const shown =
(resultsLoadable.state !== "loading" || lightning) &&
(showSearch || length > CHECKBOX_LIMIT);

return {
results,
useSearch: useRecoilValue(hasSearchResultsSelector(path))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export const FILTERS = {
[fou.INT_FIELD]: filters.NumericFieldFilter,
[fou.OBJECT_ID_FIELD]: filters.StringFieldFilter,
[fou.STRING_FIELD]: filters.StringFieldFilter,
["_LABEL_TAGS"]: filters.LabelFieldFilter,
_LABEL_TAGS: filters.LabelFieldFilter,
};

const FilterItem = ({
Expand Down
25 changes: 25 additions & 0 deletions app/packages/state/src/recoil/lightning.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { describe, expect, it, vi } from "vitest";

vi.mock("recoil");
vi.mock("recoil-relay");

import { TestSelectorFamily, setMockAtoms } from "../../../../__mocks__/recoil";
import * as lightning from "./lightning";

describe("tests lightning selectors", () => {
it("resolves wildcard indexed fields with database path", () => {
const test = <TestSelectorFamily<typeof lightning.lightningPaths>>(
(<unknown>lightning.lightningPaths("ground_truth"))
);
setMockAtoms({
dbPath: (p) =>
p === "ground_truth.id" ? "ground_truth._id" : "ground_truth.label",
expandPath: () => "ground_truth",
fieldPaths: () => ["id", "label"],
indexesByPath: new Set(["ground_truth._id", "ground_truth.label"]),
isLabelPath: () => true,
});

expect(test()).toEqual(new Set(["ground_truth.id", "ground_truth.label"]));
});
Comment on lines +9 to +24
Copy link
Contributor

Choose a reason for hiding this comment

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

Well-implemented test case for lightningPaths selector.

The test case effectively checks the selector's ability to resolve database paths. Consider adding more test cases to cover edge cases and error handling scenarios.

Would you like me to help in writing additional test cases?

});
2 changes: 1 addition & 1 deletion app/packages/state/src/recoil/lightning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ export const lightningPaths = selectorFamily<Set<string>, string>({
})
)
.map((p) => `${expanded}.${p}`)
.filter((p) => indexes.has(p))
.filter((p) => indexes.has(get(schemaAtoms.dbPath(p))))
);
}

Expand Down
Loading