Skip to content

Commit b0b43c3

Browse files
fix(web): convert pop up error as cancellation (#1516)
* fix(web): handle google popup-close oauth cancellations Co-authored-by: Tyler Dane <tyler-dane@users.noreply.github.com> * fix(web): wait for elements in DayView route tests Replace getByRole/getByText with findByRole/findByText to wait for components to render before assertions. This fixes timing-related test failures in CI where elements weren't immediately available after render. Co-authored-by: Tyler Dane <tyler-dane@users.noreply.github.com> * refactor(middleware): streamline promise middleware and update imports - Removed unused import from promise.middleware.test.ts to improve clarity. - Simplified condition in sendResponse function of promise.middleware.ts for better readability. - Updated import path for Google OAuth error utility in useGoogleLogin.ts to reflect new directory structure. * fix(tests): update import path for Google OAuth error utility in test file - Adjusted the import statement in `google-oauth-error.util.test.ts` to reflect the new directory structure for better organization and clarity. * refactor(migrations): remove console logs and enhance migration logic - Eliminated console log statements from data and external migration functions to reduce clutter and improve performance. - Updated the demo data seed migration to skip seeding when existing data is present, enhancing efficiency. - Added console method spies in test files to control logging behavior during tests, improving test clarity and reducing noise. * chore(assets): remove unused Google OAuth and logo images - Deleted the `google-oauth-preview.png` and `logo.png` files from the assets directory as they are no longer needed, streamlining the asset management. * refactor(tests): update rendering methods and improve test setup - Replaced synchronous rendering methods with asynchronous counterparts in various test files to ensure proper initialization and avoid timing issues. - Updated utility functions to prepare storage for tests, enhancing the clarity and reliability of the test environment. - Removed unused imports and streamlined test setups across multiple components, improving overall code organization and readability. * fix(tests): update test command in package.json for consistency - Changed the test command in package.json from "cross-env-shell" to "cross-env" for improved compatibility and consistency across environments. * refactor(Task): improve drag handle visibility and floating behavior - Updated the DraggableTask component to conditionally enable the drag handle based on the number of tasks, enhancing user experience. - Refactored the floating behavior to only activate when the drag handle is visible and not in test environments, improving performance and reliability. - Simplified the reference setting logic for the floating element, ensuring it only attaches when necessary. * test(floating-ui): add mock setup for floating-ui in tests - Introduced a new setup file to mock @floating-ui/react, allowing tests to run without real layout calculations. - Updated TaskContextMenu and TaskList test files to include the new floating-ui mock, ensuring consistent behavior across tests. - Simplified the DraggableTask component by removing environment checks for floating behavior, enhancing test reliability. --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Tyler Dane <tyler-dane@users.noreply.github.com>
1 parent afd5eca commit b0b43c3

28 files changed

Lines changed: 349 additions & 132 deletions

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
"dev:update": "git checkout main && git pull && yarn",
2828
"dev:web": "cd packages/web && yarn webpack serve --mode=development --node-env=local",
2929
"postinstall": "husky install | chmod ug+x .husky/*",
30-
"test": "cross-env-shell TZ=Etc/UTC yarn jest",
30+
"test": "cross-env TZ=Etc/UTC yarn jest",
3131
"test:e2e": "playwright test",
3232
"test:backend": "yarn test backend",
3333
"test:core": "yarn test core",

packages/backend/src/common/middleware/promise.middleware.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { type NextFunction, type Request, type Response } from "express";
1+
import { type NextFunction, type Request } from "express";
22
import { Status } from "@core/errors/status.codes";
33
import { requestMiddleware } from "@backend/common/middleware/promise.middleware";
44
import { type Res_Promise } from "@backend/common/types/express.types";

packages/backend/src/common/middleware/promise.middleware.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,7 @@ const sendResponse = (res: express.Response, data: unknown) => {
2727
return;
2828
}
2929

30-
if (
31-
isStatusOnlyPayload(data) &&
32-
(data.statusCode as Status) === Status.NO_CONTENT
33-
) {
30+
if (isStatusOnlyPayload(data) && data.statusCode === Status.NO_CONTENT) {
3431
res.status(Status.NO_CONTENT).send();
3532
return;
3633
}

packages/web/src/__tests__/__mocks__/mock.render.tsx

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import {
22
type ComponentType,
33
type PropsWithChildren,
44
type ReactElement,
5-
isValidElement,
65
} from "react";
76
import { RouterProvider, type RouterProviderProps } from "react-router-dom";
87
import { configureStore } from "@reduxjs/toolkit";
@@ -50,12 +49,6 @@ const TestProviders = (props?: {
5049
);
5150
}
5251

53-
if (isValidElement(children)) {
54-
console.warn(
55-
"When providing a router, children is not a valid option and will be ignored.",
56-
);
57-
}
58-
5952
return (
6053
<div id={ID_ROOT} data-testid={ID_ROOT}>
6154
<CompassRequiredProviders store={props?.store}>
@@ -91,6 +84,7 @@ const customRender = (
9184

9285
const options: RenderOptions = { ...renderOptions };
9386
const BaseProviders = TestProviders({ store, router });
87+
const renderUi = router ? <></> : ui;
9488

9589
const Wrapper = ({ children }: PropsWithChildren) => {
9690
if (!CustomWrapper) return <BaseProviders>{children}</BaseProviders>;
@@ -103,7 +97,7 @@ const customRender = (
10397
};
10498

10599
// wraps test component with providers
106-
return render(ui, {
100+
return render(renderUi, {
107101
wrapper: Wrapper,
108102
...options,
109103
});
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/**
2+
* Mocks @floating-ui/react so components like DraggableTask run the same
3+
* production code paths (refs.setReference, refs.setFloating, floatingStyles)
4+
* in tests without requiring real layout. We call the real useFloating and
5+
* only override refs and floatingStyles so other components (SelectView,
6+
* Tooltip) still receive a valid context. Individual test files may override
7+
* this mock with their own jest.mock("@floating-ui/react", ...).
8+
*/
9+
jest.mock("@floating-ui/react", () => {
10+
const actual =
11+
jest.requireActual<typeof import("@floating-ui/react")>(
12+
"@floating-ui/react",
13+
);
14+
return {
15+
...actual,
16+
useFloating: jest.fn(
17+
(options: Parameters<typeof actual.useFloating>[0]) => {
18+
const result = actual.useFloating({
19+
...options,
20+
whileElementsMounted: options?.whileElementsMounted
21+
? jest.fn(() => jest.fn())
22+
: undefined,
23+
});
24+
return {
25+
...result,
26+
refs: {
27+
setReference: jest.fn(),
28+
setFloating: jest.fn(),
29+
},
30+
floatingStyles: {},
31+
};
32+
},
33+
),
34+
autoUpdate: jest.fn(() => jest.fn()),
35+
inline: jest.fn(() => () => {}),
36+
offset: jest.fn(() => () => {}),
37+
};
38+
});

packages/web/src/__tests__/utils/storage/indexeddb.test.util.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1+
import { resetStorageAsync } from "@web/common/storage/adapter/adapter";
12
import { DEMO_DATA_SEED_FLAG_KEY } from "@web/common/storage/migrations/external/demo-data-seed";
23

34
const COMPASS_LOCAL_DB_NAME = "compass-local";
45

56
export async function clearCompassLocalDb(): Promise<void> {
7+
await resetStorageAsync();
68
await new Promise<void>((resolve) => {
79
const request = indexedDB.deleteDatabase(COMPASS_LOCAL_DB_NAME);
810
request.onsuccess = () => resolve();

packages/web/src/__tests__/utils/tasks/task.test.util.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { screen, waitFor } from "@testing-library/react";
1+
import { act, screen, waitFor } from "@testing-library/react";
22
import type userEvent from "@testing-library/user-event";
33

44
type User = ReturnType<typeof userEvent.setup>;
@@ -30,9 +30,11 @@ export const addTasks = async (user: User, taskTitles: string[]) => {
3030
screen.getByPlaceholderText("Enter task title..."),
3131
)) as HTMLInputElement;
3232

33-
await user.clear(input);
34-
await user.type(input, title);
35-
await user.keyboard("{Enter}");
33+
await act(async () => {
34+
await user.clear(input);
35+
await user.type(input, title);
36+
await user.keyboard("{Enter}");
37+
});
3638

3739
// Wait for the task to be created and appear in the DOM
3840
await waitFor(
@@ -58,7 +60,9 @@ export const clickCreateTaskButton = async (user: User) => {
5860
{ timeout: 5000 },
5961
);
6062

61-
await user.click(addButton);
63+
await act(async () => {
64+
await user.click(addButton);
65+
});
6266

6367
return waitFor(() => screen.getByRole("textbox", { name: /task title/i }), {
6468
timeout: 5000,
-36.1 KB
Binary file not shown.
-199 KB
Binary file not shown.
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { isGooglePopupClosedError } from "@web/auth/google/google-oauth-error.util";
2+
3+
describe("isGooglePopupClosedError", () => {
4+
it("returns true for non-oauth popup_closed type", () => {
5+
expect(isGooglePopupClosedError({ type: "popup_closed" })).toBe(true);
6+
});
7+
8+
it("returns true for popup-closed error messages", () => {
9+
expect(
10+
isGooglePopupClosedError({ message: "Popup window closed by user" }),
11+
).toBe(true);
12+
});
13+
14+
it("returns false for non-popup auth failures", () => {
15+
expect(isGooglePopupClosedError({ error: "access_denied" })).toBe(false);
16+
expect(isGooglePopupClosedError(new Error("network down"))).toBe(false);
17+
});
18+
});

0 commit comments

Comments
 (0)