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

feat(frontend): Utilize TanStack Query #5096

Merged
merged 44 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
c3c0300
setup tanstack query
amanape Nov 18, 2024
df09646
Setup infinity query for github repos
amanape Nov 18, 2024
dc9f48c
Move more requests to tanstack query
amanape Nov 18, 2024
fa879dd
Merge branch 'main' into ALL-726/replace-loaders-with-query
amanape Nov 19, 2024
cbf980a
More
amanape Nov 19, 2024
aa7862f
Quick fix
amanape Nov 19, 2024
aee10ed
Query for file explorer
amanape Nov 19, 2024
36414f5
Setup tests for existing action and loader behavior
amanape Nov 19, 2024
d78973d
Merge branch 'main' into ALL-726/replace-loaders-with-query
amanape Nov 19, 2024
0d786d3
Begin removing main route client loader
amanape Nov 19, 2024
0870944
Merge branch 'main' into ALL-726/replace-loaders-with-query
amanape Nov 20, 2024
8d50d99
Remove root loader
amanape Nov 20, 2024
7717704
Handle nits
amanape Nov 20, 2024
8a7bec4
Complete root layout migration
amanape Nov 20, 2024
73458c6
Complete root index route migration
amanape Nov 20, 2024
3732ad7
Remove loaders and actions from remaining routes
amanape Nov 20, 2024
7e0320d
Merge branch 'main' into ALL-726/replace-loaders-with-query
amanape Nov 20, 2024
e807107
Retire resource routes
amanape Nov 20, 2024
d8c17fc
Simplify settings form handling
amanape Nov 20, 2024
26e7a87
Remove final resource route
amanape Nov 20, 2024
e5502d3
Introduce mutations
amanape Nov 20, 2024
b581a16
Refactoring
amanape Nov 20, 2024
2cc17c8
Merge branch 'main' into ALL-726/replace-loaders-with-query
amanape Nov 21, 2024
6f48512
Refactor and improve hook usage
amanape Nov 21, 2024
ea292dd
Minor adjustments
amanape Nov 21, 2024
1aa1ac0
Refactor request handlers
amanape Nov 21, 2024
d2a9d97
More refactors and setup auth context
amanape Nov 21, 2024
de66d69
Merge branch 'main' into ALL-726/replace-loaders-with-query
amanape Nov 21, 2024
02558c3
Refactor and improvements
amanape Nov 21, 2024
261f0ad
Address build errors
amanape Nov 21, 2024
603bdd5
Setup user prefs context
amanape Nov 21, 2024
cfada41
Use prefs context
amanape Nov 21, 2024
493f685
Persist auth state and fix bug
amanape Nov 21, 2024
97a6411
Use auth token s directly
amanape Nov 21, 2024
3630c36
Fetch all repos
amanape Nov 21, 2024
a43a0f3
Merge branch 'main' into ALL-726/replace-loaders-with-query
amanape Nov 22, 2024
f1097d3
Capture consent in account settings
amanape Nov 22, 2024
0a31482
Minor update
amanape Nov 22, 2024
04b1edc
Fix tests
amanape Nov 22, 2024
dad647e
Lint
amanape Nov 22, 2024
5e2a9a4
Fix gh auth
amanape Nov 22, 2024
f0135ec
Improve auth handling
amanape Nov 22, 2024
a1aa5ab
Merge branch 'main' into ALL-726/replace-loaders-with-query
amanape Nov 22, 2024
cbb6a27
Better naming
amanape Nov 22, 2024
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
3 changes: 2 additions & 1 deletion frontend/.eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
"plugin:@typescript-eslint/eslint-recommended",
"plugin:@typescript-eslint/recommended",
"plugin:react/recommended",
"plugin:react-hooks/recommended"
"plugin:react-hooks/recommended",
"plugin:@tanstack/query/recommended"
],
"plugins": [
"prettier"
Expand Down
40 changes: 0 additions & 40 deletions frontend/__tests__/clear-session.test.ts

This file was deleted.

14 changes: 10 additions & 4 deletions frontend/__tests__/components/feedback-form.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { render, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { afterEach, describe, expect, it, vi } from "vitest";
import { renderWithProviders } from "test-utils";
import { FeedbackForm } from "#/components/feedback-form";

describe("FeedbackForm", () => {
Expand All @@ -12,7 +13,9 @@ describe("FeedbackForm", () => {
});

it("should render correctly", () => {
render(<FeedbackForm polarity="positive" onClose={onCloseMock} />);
renderWithProviders(
<FeedbackForm polarity="positive" onClose={onCloseMock} />,
);

screen.getByLabelText("Email");
screen.getByLabelText("Private");
Expand All @@ -23,7 +26,9 @@ describe("FeedbackForm", () => {
});

it("should switch between private and public permissions", async () => {
render(<FeedbackForm polarity="positive" onClose={onCloseMock} />);
renderWithProviders(
<FeedbackForm polarity="positive" onClose={onCloseMock} />,
);
const privateRadio = screen.getByLabelText("Private");
const publicRadio = screen.getByLabelText("Public");

Expand All @@ -40,10 +45,11 @@ describe("FeedbackForm", () => {
});

it("should call onClose when the close button is clicked", async () => {
render(<FeedbackForm polarity="positive" onClose={onCloseMock} />);
renderWithProviders(
<FeedbackForm polarity="positive" onClose={onCloseMock} />,
);
await user.click(screen.getByRole("button", { name: "Cancel" }));

expect(onCloseMock).toHaveBeenCalled();
});

});
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,13 @@ vi.mock("../../services/fileService", async () => ({
}));

const renderFileExplorerWithRunningAgentState = () =>
renderWithProviders(
<FileExplorer error={null} isOpen onToggle={() => {}} />,
{
preloadedState: {
agent: {
curAgentState: AgentState.RUNNING,
},
renderWithProviders(<FileExplorer isOpen onToggle={() => {}} />, {
preloadedState: {
agent: {
curAgentState: AgentState.RUNNING,
},
},
);
});

describe.skip("FileExplorer", () => {
afterEach(() => {
Expand Down
12 changes: 2 additions & 10 deletions frontend/__tests__/components/user-actions.test.tsx
Original file line number Diff line number Diff line change
@@ -1,22 +1,16 @@
import { render, screen } from "@testing-library/react";
import { describe, expect, it, test, vi, afterEach } from "vitest";
import userEvent from "@testing-library/user-event";
import * as Remix from "@remix-run/react";
import { UserActions } from "#/components/user-actions";

describe("UserActions", () => {
const user = userEvent.setup();
const onClickAccountSettingsMock = vi.fn();
const onLogoutMock = vi.fn();

const useFetcherSpy = vi.spyOn(Remix, "useFetcher");
// @ts-expect-error - Only returning the relevant properties for the test
useFetcherSpy.mockReturnValue({ state: "idle" });

afterEach(() => {
onClickAccountSettingsMock.mockClear();
onLogoutMock.mockClear();
useFetcherSpy.mockClear();
});

it("should render", () => {
Expand Down Expand Up @@ -111,10 +105,8 @@ describe("UserActions", () => {
expect(onLogoutMock).not.toHaveBeenCalled();
});

it("should display the loading spinner", () => {
// @ts-expect-error - Only returning the relevant properties for the test
useFetcherSpy.mockReturnValue({ state: "loading" });

// FIXME: Spinner now provided through useQuery
it.skip("should display the loading spinner", () => {
render(
<UserActions
onClickAccountSettings={onClickAccountSettingsMock}
Expand Down
162 changes: 140 additions & 22 deletions frontend/__tests__/routes/_oh.test.tsx
Original file line number Diff line number Diff line change
@@ -1,35 +1,153 @@
import { describe, it, test } from "vitest";
import { afterEach, beforeAll, describe, expect, it, vi } from "vitest";
import { createRemixStub } from "@remix-run/testing";
import { screen, waitFor, within } from "@testing-library/react";
import { renderWithProviders } from "test-utils";
import userEvent from "@testing-library/user-event";
import MainApp from "#/routes/_oh";
import * as CaptureConsent from "#/utils/handle-capture-consent";
import i18n from "#/i18n";

describe("frontend/routes/_oh", () => {
describe("brand logo", () => {
it.todo("should not do anything if the user is in the main screen");
it.todo(
"should be clickable and redirect to the main screen if the user is not in the main screen",
const RemixStub = createRemixStub([{ Component: MainApp, path: "/" }]);

const { userIsAuthenticatedMock, settingsAreUpToDateMock } = vi.hoisted(
() => ({
userIsAuthenticatedMock: vi.fn(),
settingsAreUpToDateMock: vi.fn(),
}),
);

beforeAll(() => {
vi.mock("#/utils/user-is-authenticated", () => ({
userIsAuthenticated: userIsAuthenticatedMock.mockReturnValue(true),
}));

vi.mock("#/services/settings", async (importOriginal) => ({
...(await importOriginal<typeof import("#/services/settings")>()),
settingsAreUpToDate: settingsAreUpToDateMock,
}));
});

afterEach(() => {
vi.clearAllMocks();
localStorage.clear();
});

it("should render", async () => {
renderWithProviders(<RemixStub />);
await screen.findByTestId("root-layout");
});

it("should render the AI config modal if the user is authed", async () => {
// Our mock return value is true by default
renderWithProviders(<RemixStub />);
await screen.findByTestId("ai-config-modal");
});

it("should render the AI config modal if settings are not up-to-date", async () => {
settingsAreUpToDateMock.mockReturnValue(false);
renderWithProviders(<RemixStub />);

await screen.findByTestId("ai-config-modal");
});

it("should not render the AI config modal if the settings are up-to-date", async () => {
settingsAreUpToDateMock.mockReturnValue(true);
renderWithProviders(<RemixStub />);

await waitFor(() => {
expect(screen.queryByTestId("ai-config-modal")).not.toBeInTheDocument();
});
});

it("should capture the user's consent", async () => {
const user = userEvent.setup();
const handleCaptureConsentSpy = vi.spyOn(
CaptureConsent,
"handleCaptureConsent",
);

renderWithProviders(<RemixStub />);

// The user has not consented to tracking
const consentForm = await screen.findByTestId("user-capture-consent-form");
expect(handleCaptureConsentSpy).not.toHaveBeenCalled();
expect(localStorage.getItem("analytics-consent")).toBeNull();

const submitButton = within(consentForm).getByRole("button", {
name: /confirm preferences/i,
});
await user.click(submitButton);

// The user has now consented to tracking
expect(handleCaptureConsentSpy).toHaveBeenCalledWith(true);
expect(localStorage.getItem("analytics-consent")).toBe("true");
expect(
screen.queryByTestId("user-capture-consent-form"),
).not.toBeInTheDocument();
});

describe("user menu", () => {
it.todo("should open the user menu when clicked");
it("should not render the user consent form if the user has already made a decision", async () => {
localStorage.setItem("analytics-consent", "true");
renderWithProviders(<RemixStub />);

describe("logged out", () => {
it.todo("should display a placeholder");
test.todo("the logout option in the user menu should be disabled");
await waitFor(() => {
expect(
screen.queryByTestId("user-capture-consent-form"),
).not.toBeInTheDocument();
});
});

it("should render a new project button if a token is set", async () => {
localStorage.setItem("token", "test-token");
const { rerender } = renderWithProviders(<RemixStub />);

describe("logged in", () => {
it.todo("should display the user's avatar");
it.todo("should log the user out when the logout option is clicked");
await screen.findByTestId("new-project-button");

localStorage.removeItem("token");
rerender(<RemixStub />);

await waitFor(() => {
expect(
screen.queryByTestId("new-project-button"),
).not.toBeInTheDocument();
});
});

describe("config", () => {
it.todo("should open the config modal when clicked");
it.todo(
"should not save the config and close the config modal when the close button is clicked",
);
it.todo(
"should save the config when the save button is clicked and close the modal",
);
it.todo("should warn the user about saving the config when in /app");
// TODO: Move to e2e tests
it.skip("should update the i18n language when the language settings change", async () => {
const changeLanguageSpy = vi.spyOn(i18n, "changeLanguage");
const { rerender } = renderWithProviders(<RemixStub />);

// The default language is English
expect(changeLanguageSpy).toHaveBeenCalledWith("en");

localStorage.setItem("LANGUAGE", "es");

rerender(<RemixStub />);
expect(changeLanguageSpy).toHaveBeenCalledWith("es");

rerender(<RemixStub />);
// The language has not changed, so the spy should not have been called again
expect(changeLanguageSpy).toHaveBeenCalledTimes(2);
});

// FIXME: logoutCleanup has been replaced with a hook
it.skip("should call logoutCleanup after a logout", async () => {
const user = userEvent.setup();
localStorage.setItem("ghToken", "test-token");

// const logoutCleanupSpy = vi.spyOn(LogoutCleanup, "logoutCleanup");
renderWithProviders(<RemixStub />);

const userActions = await screen.findByTestId("user-actions");
const userAvatar = within(userActions).getByTestId("user-avatar");
await user.click(userAvatar);

const logout = within(userActions).getByRole("button", { name: /logout/i });
await user.click(logout);

// expect(logoutCleanupSpy).toHaveBeenCalled();
expect(localStorage.getItem("ghToken")).toBeNull();
});
});
53 changes: 0 additions & 53 deletions frontend/__tests__/utils/cache.test.ts

This file was deleted.

13 changes: 13 additions & 0 deletions frontend/__tests__/utils/extract-next-page-from-link.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { expect, test } from "vitest";
import { extractNextPageFromLink } from "#/utils/extract-next-page-from-link";

test("extractNextPageFromLink", () => {
const link = `<https://api.github.com/repositories/1300192/issues?page=2>; rel="prev", <https://api.github.com/repositories/1300192/issues?page=4>; rel="next", <https://api.github.com/repositories/1300192/issues?page=515>; rel="last", <https://api.github.com/repositories/1300192/issues?page=1>; rel="first"`;
expect(extractNextPageFromLink(link)).toBe(4);

const noNextLink = `<https://api.github.com/repositories/1300192/issues?page=2>; rel="prev", <https://api.github.com/repositories/1300192/issues?page=1>; rel="first"`;
expect(extractNextPageFromLink(noNextLink)).toBeNull();

const extra = `<https://api.github.com/user/repos?sort=pushed&page=2&per_page=3>; rel="next", <https://api.github.com/user/repos?sort=pushed&page=22&per_page=3>; rel="last"`;
expect(extractNextPageFromLink(extra)).toBe(2);
});
Loading
Loading