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

fix: proceed to checkout button blinking #1341

Merged
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
1d35546
feat: navigate line-items by keypress
ivan-kalachikov Sep 26, 2024
e88cd0a
Merge branch 'dev' into feat/VCST-1887-navigate-line-items-by-keypress
ivan-kalachikov Sep 26, 2024
5c674df
feat: navigate line-items by keypress
ivan-kalachikov Sep 26, 2024
92efe0a
fix: remove unnesessary code
ivan-kalachikov Sep 26, 2024
1b0f4d0
fix: proceed to chechout button blinking
ivan-kalachikov Sep 26, 2024
e889149
fix: proceed to chechout button blinking
ivan-kalachikov Sep 26, 2024
1e5c506
fix: remove console.log and extend aborted request catch
ivan-kalachikov Sep 26, 2024
34f42b1
fix: simplify solution
ivan-kalachikov Sep 27, 2024
49dd61a
fix: update example
ivan-kalachikov Sep 27, 2024
e923686
fix: add test cases for loading value, minor fixes
ivan-kalachikov Sep 27, 2024
c1b2970
Merge branch 'dev' into fix/VCST-1861-proceed-to-checkout-button-blin…
ivan-kalachikov Sep 30, 2024
be2f0b6
fix: add batcher synchronizer
ivan-kalachikov Oct 1, 2024
f1e259f
fix: refactor - replace computed setter with function
ivan-kalachikov Oct 1, 2024
e2b108f
fix: remove console.log
ivan-kalachikov Oct 1, 2024
91ed137
fix: add unit tests and minor refactor
ivan-kalachikov Oct 1, 2024
e4bea4a
fix: add unit tests and minor refactor
ivan-kalachikov Oct 1, 2024
d7de3be
fix: minor refactor
ivan-kalachikov Oct 1, 2024
86376b2
fix: upgrade types
ivan-kalachikov Oct 1, 2024
535cbab
fix: add test cases and minor refactor
ivan-kalachikov Oct 1, 2024
3b4a8a4
fix: pull changes from another branch
ivan-kalachikov Oct 1, 2024
9322988
Merge branch 'dev' into fix/VCST-1861-proceed-to-checkout-button-blin…
ivan-kalachikov Oct 1, 2024
9eb1c45
fix: sonar issue
ivan-kalachikov Oct 1, 2024
b4dcfe1
fix: minor refactor
ivan-kalachikov Oct 2, 2024
731ce09
fix: minor refactor
ivan-kalachikov Oct 2, 2024
42c6fb0
fix: resolve comments - add more explicit option for resetting count …
ivan-kalachikov Oct 3, 2024
1c936cf
fix: resolve comments
ivan-kalachikov Oct 3, 2024
6839499
fix: add optimistic updates
ivan-kalachikov Oct 9, 2024
ce5adfc
fix: add optimistic updates
ivan-kalachikov Oct 9, 2024
63467d3
fix: types
ivan-kalachikov Oct 9, 2024
9f4cfea
fix: types
ivan-kalachikov Oct 9, 2024
868b2e3
fix: add comment
ivan-kalachikov Oct 9, 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
2 changes: 2 additions & 0 deletions client-app/core/composables/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ export * from "./useErrorsTranslator";
export * from "./useGoogleAnalytics";
export * from "./useHistoricalEvents";
export * from "./useImpersonate";
export * from "./useMutationBatcher";
export * from "./useNavigations";
export * from "./usePageHead";
export * from "./useProductsRoutes";
export * from "./useRouteQueryParam";
export * from "./useSyncMutationBatchers";
export * from "./useThemeContext";
export * from "./useWhiteLabeling";
108 changes: 108 additions & 0 deletions client-app/core/composables/useMutationBatcher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,16 @@ describe("useMutationBatcher", () => {
vi.restoreAllMocks();
});

it("should generate a unique id for each instance", () => {
const mutation = vi.fn().mockImplementation(mutationMock);
const { id: id1 } = useMutationBatcher(mutation);
const { id: id2 } = useMutationBatcher(mutation);

expect(id1).not.toEqual(id2);
expect(typeof id1).toBe("string");
expect(typeof id2).toBe("string");
});

it("should call one mutation after delay", () => {
const mutation = vi.fn().mockImplementation(mutationMock);
const { add } = useMutationBatcher(mutation);
Expand Down Expand Up @@ -151,6 +161,104 @@ describe("useMutationBatcher", () => {
vi.advanceTimersByTime(INITIAL_DELAY_MS);
expect(mutation2).toBeCalled();
});

it("should set loading to true when mutation starts and to false after mutation is done", async () => {
const mutation = vi.fn().mockImplementation(mutationMock);
const { add, loading } = useMutationBatcher(mutation);

expect(loading.value).toBe(false);

const promise = add(DUMMY_TEXT_ARGUMENTS);
vi.advanceTimersByTime(DEFAULT_DEBOUNCE_IN_MS);

expect(loading.value).toBe(true);

vi.advanceTimersByTime(INITIAL_DELAY_MS);
await promise;

expect(loading.value).toBe(false);
});

it("should set loading to false if mutation throws an error", async () => {
const mutation = vi.fn().mockRejectedValue(new Error("Test Error"));
const { add, loading } = useMutationBatcher(mutation);

expect(loading.value).toBe(false);

const promise = add(DUMMY_TEXT_ARGUMENTS);

vi.advanceTimersByTime(DEFAULT_DEBOUNCE_IN_MS);
expect(loading.value).toBe(true);

vi.advanceTimersByTime(INITIAL_DELAY_MS);

await expect(promise).rejects.toThrow("Test Error");

expect(loading.value).toBe(false);
});

it("should handle multiple instances with independent loading states", async () => {
const mutation1 = vi.fn().mockImplementation(mutationMock);
const mutation2 = vi.fn().mockImplementation(mutationMock);

const { add: add1, loading: loading1 } = useMutationBatcher(mutation1, { debounce: SHORT_INITIAL_DELAY_MS });
const { add: add2, loading: loading2 } = useMutationBatcher(mutation2, { debounce: SHORT_INITIAL_DELAY_MS });

expect(loading1.value).toBe(false);
expect(loading2.value).toBe(false);

add1(DUMMY_TEXT_ARGUMENTS);
add2(DUMMY_TEXT_ARGUMENTS);

vi.advanceTimersByTime(DEFAULT_DEBOUNCE_IN_MS);
vi.advanceTimersByTime(DEFAULT_DEBOUNCE_IN_MS);

expect(loading1.value).toBe(true);
expect(loading2.value).toBe(true);

vi.advanceTimersByTime(SHORT_INITIAL_DELAY_MS);

await vi.runAllTimersAsync();

expect(loading1.value).toBe(false);
expect(loading2.value).toBe(false);
});

it("should abort the mutation when abort is called", () => {
const abortSpy = vi.spyOn(AbortController.prototype, "abort");
const mutation = vi.fn().mockImplementation(mutationMock);
const { add, abort } = useMutationBatcher(mutation);

void add(DUMMY_TEXT_ARGUMENTS);
vi.advanceTimersByTime(SIMULATED_REQUEST_DURATION_MS);

abort();

expect(abortSpy).toBeCalled();
});

it("should update the arguments ref after adding a mutation", () => {
const mutation = vi.fn().mockImplementation(mutationMock);
const { add, arguments: batchArguments } = useMutationBatcher(mutation);

const testArgs = getTestArguments("product_id_1");
void add(testArgs);

expect(batchArguments.value).toEqual(testArgs);
});

it("should call the onAddHandler when a new mutation is added", () => {
const mutation = vi.fn().mockImplementation(mutationMock);
const onAddHandlerMock = vi.fn();
const { add, registerOnAddHandler } = useMutationBatcher(mutation);

registerOnAddHandler(onAddHandlerMock);

const testArgs = getTestArguments("product_id_1");
void add(testArgs);

expect(onAddHandlerMock).toBeCalledWith(expect.any(String), testArgs);
});
});

describe("getMergeStrategyUniqueBy", () => {
Expand Down
82 changes: 66 additions & 16 deletions client-app/core/composables/useMutationBatcher.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import { ApolloError } from "@apollo/client/core";
import cloneDeep from "lodash/cloneDeep";
import mergeWith from "lodash/mergeWith";
import noop from "lodash/noop";
import uniqueId from "lodash/uniqueId";
import { ref } from "vue";
import { AbortReason } from "@/core/api/common/enums";
import { uniqByLast } from "@/core/utilities/common";
import { DEFAULT_DEBOUNCE_IN_MS } from "@/shared/cart/constants";
import type { UniqByLastIterateeType } from "@/core/utilities/common";
import type { FetchResult } from "@apollo/client/core";
import type { MutateFunction } from "@vue/apollo-composable";
import type { MutateFunction, MutateOverrideOptions } from "@vue/apollo-composable";
import type { Ref } from "vue";

const DEFAULT_MAX_LENGTH = 10;

Expand Down Expand Up @@ -41,15 +45,22 @@ export function getMergeStrategyUniqueBy(keyOrFn: string | ((item: unknown) => u
/**
* @description Vue composable to batch Apollo Client mutations.
* @param mutation - Apollo Client mutation function.
* @param options - Options object with `debounce`, `maxLength`, and `merge` properties.
* @param options - Options object with `debounce`, `maxLength`, and `mergeStrategy` properties.
* @param options.debounce - Debounce time in milliseconds. Default is {@link DEFAULT_DEBOUNCE_IN_MS}.
* @param options.length - Maximum number of mutations to batch. Default is {@link DEFAULT_MAX_LENGTH}. After reaching this number, `overflowed` ref will be set to `true`.
* @param options.merge - Function to merge two mutation parameters objects. See {@link DEFAULT_MERGE_STRATEGY} and {@link getMergeStrategyUniqueBy}
* @returns Object with `overflowed` boolean ref and `add` function to add a new mutation to the batch.
* @param options.maxLength - Maximum number of mutations to batch. Default is {@link DEFAULT_MAX_LENGTH}. After reaching this number, `overflowed` ref will be set to `true`.
* @param options.mergeStrategy - Function to merge two mutation parameters objects. See {@link DEFAULT_MERGE_STRATEGY} and {@link getMergeStrategyUniqueBy}
* @returns {object} An object with the following properties:
* - `id`: A unique identifier for the batcher instance.
* - `overflowed`: A boolean ref indicating whether the batch has exceeded the maximum length.
* - `add`: A function to add a new mutation to the batch.
* - `loading`: A boolean ref indicating whether the batch is currently being processed.
* - `abort`: A function to abort the current batch.
* - `arguments`: A ref containing the current batch arguments.
* - `registerOnAddHandler`: A function to register a handler that will be called when a new mutation is added to the batch.
* @example ```ts
* const { mutate: changeCartItemsQuantity, loading } = useChangeCartItemsQuantity();
* const { overflowed, add } = useMutationBatcher(changeCartItemsQuantity);
* const result = await add({ command { cartItems: [{ productId: "1", quantity: 1}] }});
* const { mutate: changeCartItemsQuantity } = useChangeCartItemsQuantity();
* const { overflowed, add, loading } = useMutationBatcher(changeCartItemsQuantity);
* const result = await add({ command: { cartItems: [{ productId: "1", quantity: 1}] }});
* ```
*/
export function useMutationBatcher<TData, TVariables extends object>(
Expand All @@ -66,15 +77,28 @@ export function useMutationBatcher<TData, TVariables extends object>(
mergeStrategy: merge = DEFAULT_MERGE_STRATEGY,
} = options;

const id = uniqueId();
const overflowed = ref(false);
const loading = ref(false);
let abortController: AbortController | null = null;
let batch: TVariables = {} as TVariables;
const batch = ref({}) as Ref<TVariables>;
let calledCount = 0;
let debounceTimeoutId: ReturnType<typeof setTimeout> | null = null;
let mutationOptions: MutateOverrideOptions<TData> | undefined;
let onAddHandler: (id: string, args: TVariables) => void = noop;

async function add(args: TVariables): Promise<FetchResult<TData> | null> {
async function add(
args: TVariables,
overrideOptions?: MutateOverrideOptions<TData> | undefined,
fireAddHandler = true,
): Promise<FetchResult<TData> | null> {
loading.value = true;
if (fireAddHandler) {
onAddHandler(id, args);
}
clearPreviousDebounce();
batch = merge(batch, args);
batch.value = merge(batch.value, args);
mutationOptions = overrideOptions;
calledCount += 1;

if (calledCount >= maxLength) {
Expand All @@ -87,8 +111,13 @@ export function useMutationBatcher<TData, TVariables extends object>(
resolve(result);
resetBatchState();
} catch (error) {
if (error instanceof Error && error.toString() !== (AbortReason.Explicit as string)) {
reject(error);
const explicitError = AbortReason.Explicit as string;
if (
!(error instanceof Error && error.toString() === explicitError) &&
!(error instanceof ApolloError && error.networkError?.toString() === explicitError)
) {
loading.value = false;
reject(error instanceof Error ? error : new Error(error as string));
}
}
}, debounce);
Expand All @@ -104,17 +133,38 @@ export function useMutationBatcher<TData, TVariables extends object>(

async function executeBatch(): Promise<FetchResult<TData> | null> {
abortController = new AbortController();
return await mutation(batch, {
return await mutation(batch.value, {
context: { fetchOptions: { signal: abortController.signal } },
...mutationOptions,
});
}

function resetBatchState() {
overflowed.value = false;
abortController = null;
batch = {} as TVariables;
batch.value = {} as TVariables;
loading.value = false;
calledCount = 0;
}

return { overflowed, add };
function registerOnAddHandler(handler: (id: string, args: TVariables) => void) {
onAddHandler = onAddHandler.name === noop.name ? handler : onAddHandler;
}

function abort() {
clearPreviousDebounce();
abortController = null;
batch.value = {} as TVariables;
loading.value = false;
}

return {
id,
overflowed,
add,
loading,
abort,
arguments: batch,
registerOnAddHandler,
};
}
76 changes: 76 additions & 0 deletions client-app/core/composables/useSyncMutationBatchers.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { useSyncMutationBatchers } from "./useSyncMutationBatchers";
import type { useMutationBatcher } from "./useMutationBatcher";
import type { Mock } from "vitest";

type MutationBatcherType = ReturnType<typeof useMutationBatcher>;

describe("useSyncMutationBatchers", () => {
let mockBatcher1: MutationBatcherType;
let mockBatcher2: MutationBatcherType;
let callback: ReturnType<typeof vi.fn>;

beforeEach(() => {
mockBatcher1 = {
id: "batcher1",
registerOnAddHandler: vi.fn(),
} as unknown as MutationBatcherType;

mockBatcher2 = {
id: "batcher2",
registerOnAddHandler: vi.fn(),
} as unknown as MutationBatcherType;

callback = vi.fn();
});

it("should register onAddHandler for both batchers", () => {
useSyncMutationBatchers(mockBatcher1, mockBatcher2, callback);
expect(mockBatcher1.registerOnAddHandler).toHaveBeenCalledTimes(1);
expect(mockBatcher2.registerOnAddHandler).toHaveBeenCalledTimes(1);
});

it("should call callback with batcher1 as currentBatcher when mutation is added to batcher1", () => {
useSyncMutationBatchers(mockBatcher1, mockBatcher2, callback);
const handler = (mockBatcher1.registerOnAddHandler as Mock).mock.calls[0][0];
handler("batcher1", { test: "data" });
expect(callback).toHaveBeenCalledWith({
args: { test: "data" },
currentBatcher: mockBatcher1,
anotherBatcher: mockBatcher2,
});
});

it("should call callback with batcher2 as currentBatcher when mutation is added to batcher2", () => {
useSyncMutationBatchers(mockBatcher1, mockBatcher2, callback);
const handler = (mockBatcher2.registerOnAddHandler as Mock).mock.calls[0][0];
handler("batcher2", { test: "data2" });
expect(callback).toHaveBeenCalledWith({
args: { test: "data2" },
currentBatcher: mockBatcher2,
anotherBatcher: mockBatcher1,
});
});

it("should handle multiple calls to the add handler", () => {
useSyncMutationBatchers(mockBatcher1, mockBatcher2, callback);
const handler1 = (mockBatcher1.registerOnAddHandler as Mock).mock.calls[0][0];
const handler2 = (mockBatcher2.registerOnAddHandler as Mock).mock.calls[0][0];

handler1("batcher1", { test: "data1" });
expect(callback).toHaveBeenCalledWith({
args: { test: "data1" },
currentBatcher: mockBatcher1,
anotherBatcher: mockBatcher2,
});

handler2("batcher2", { test: "data2" });
expect(callback).toHaveBeenCalledWith({
args: { test: "data2" },
currentBatcher: mockBatcher2,
anotherBatcher: mockBatcher1,
});

expect(callback).toHaveBeenCalledTimes(2);
});
});
Loading