From 6c5e15eae17a9034988e8273c1b77c871c30a2e8 Mon Sep 17 00:00:00 2001 From: Fernando Terra Date: Tue, 29 Oct 2024 10:33:46 -0300 Subject: [PATCH 1/2] feat: clear old search results when a new search starts --- frontend/src/components/DataFetcher.vue | 1 + frontend/src/pages/SearchPage.vue | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/frontend/src/components/DataFetcher.vue b/frontend/src/components/DataFetcher.vue index 6b4d0e4fac..345e6654a5 100644 --- a/frontend/src/components/DataFetcher.vue +++ b/frontend/src/components/DataFetcher.vue @@ -72,6 +72,7 @@ watch([() => props.url, () => props.disabled], () => { clearTimeout(debounceTimer); } debounceTimer = setTimeout(() => { + content.value = []; fetch().then(() => { // Discard the response from old request when a newer one was already responded. if (curRequestTime >= lastUpdateRequestTime.value) { diff --git a/frontend/src/pages/SearchPage.vue b/frontend/src/pages/SearchPage.vue index 9b136c4a38..5c716a9a69 100644 --- a/frontend/src/pages/SearchPage.vue +++ b/frontend/src/pages/SearchPage.vue @@ -136,7 +136,8 @@ const searchResultToCodeNameValue = ( return result; }; -const searchResultToCodeNameValueList = (list: ClientSearchResult[]) => list.map(searchResultToCodeNameValue); +const searchResultToCodeNameValueList = (list: ClientSearchResult[]) => + list?.map(searchResultToCodeNameValue); const searchResultToText = (searchResult: ClientSearchResult): string => { const { clientNumber, clientFullName, clientType, city } = searchResult; From ec57b720806bbfcbd8961dab6790e2dadcbea631 Mon Sep 17 00:00:00 2001 From: Fernando Terra Date: Tue, 29 Oct 2024 15:06:17 -0300 Subject: [PATCH 2/2] feat: discard the response from old request --- frontend/src/components/DataFetcher.vue | 20 +- .../unittests/components/DataFetcher.spec.ts | 184 +++++++++++++++++- 2 files changed, 187 insertions(+), 17 deletions(-) diff --git a/frontend/src/components/DataFetcher.vue b/frontend/src/components/DataFetcher.vue index 345e6654a5..e2cf904c47 100644 --- a/frontend/src/components/DataFetcher.vue +++ b/frontend/src/components/DataFetcher.vue @@ -26,12 +26,12 @@ const response = ref(); const loading = ref(); const lastUpdateRequestTime = ref(0); -let debounceTimer: number | null = null; +let debounceTimer: NodeJS.Timeout | null = null; const initialUrlValue = props.url; const searchURL = computed(() => props.url); -const { loading: fetchLoading, error, fetch } = useFetchTo(searchURL, response, { +const { error, fetch } = useFetchTo(searchURL, response, { skip: true, ...props.params, }); @@ -53,20 +53,14 @@ if (!props.disabled && props.initFetch) { }); } - -// Watch for changes in the fetch loading state -// Doing like this now due to the debounce -watch(() => fetchLoading.value, (newVal) => { - loading.value = newVal; -}); - // Watch for changes in the url, and if the difference is greater than the min length, fetch watch([() => props.url, () => props.disabled], () => { if (!props.disabled && calculateStringDifference(initialUrlValue, props.url) >= props.minLength) { - + // added a manual loading state to set the loading state when the user types loading.value = true; const curRequestTime = Date.now(); + lastUpdateRequestTime.value = curRequestTime; if (debounceTimer) { clearTimeout(debounceTimer); @@ -74,10 +68,10 @@ watch([() => props.url, () => props.disabled], () => { debounceTimer = setTimeout(() => { content.value = []; fetch().then(() => { - // Discard the response from old request when a newer one was already responded. - if (curRequestTime >= lastUpdateRequestTime.value) { + // Discard the response from old request when a newer request has been made. + if (curRequestTime === lastUpdateRequestTime.value) { + loading.value = false; content.value = response.value; - lastUpdateRequestTime.value = curRequestTime; } }); }, props.debounce); // Debounce time diff --git a/frontend/tests/unittests/components/DataFetcher.spec.ts b/frontend/tests/unittests/components/DataFetcher.spec.ts index 76b1e11cce..1b2d6113af 100644 --- a/frontend/tests/unittests/components/DataFetcher.spec.ts +++ b/frontend/tests/unittests/components/DataFetcher.spec.ts @@ -1,14 +1,16 @@ -import { describe, it, expect, vi } from "vitest"; -import { mount } from "@vue/test-utils"; -import { nextTick, ref } from "vue"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { mount, type VueWrapper } from "@vue/test-utils"; +import { nextTick, ref, type Ref } from "vue"; import * as fetcher from "@/composables/useFetch"; import DataFetcher from "@/components/DataFetcher.vue"; vi.useFakeTimers(); + + describe("DataFetcher", () => { - const mockedFetchTo = (url: string, received: any, config: any = {}) => ({ + const mockedFetchTo = (url: Ref, received: Ref, config: any = {}) => ({ response: ref({}), error: ref({}), data: received, @@ -18,6 +20,54 @@ describe("DataFetcher", () => { }, }); + const mockedFetchToFunction = + ( + fetchData: (url: string) => Promise = async () => ({ + name: "Loaded", + }), + ) => + (url: Ref, received: Ref, config: any = {}) => ({ + response: ref({}), + error: ref({}), + data: received, + loading: ref(false), + fetch: async () => { + received.value = await fetchData(url.value); + console.log(received.value); + }, + }); + + const simpleFetchData = async (url: string) => { + return new Promise((resolve) => { + setTimeout(() => { + resolve({ name: url }); + }, 1000); + }); + }; + + const mockFetchSimple = mockedFetchToFunction(simpleFetchData); + + let lastResponse: any; + + const fetchDataSleepByParam = async (url: string) => { + const regex = /.*\/(.+)/; + const regexResult = regex.exec(url); + const lastParam = regexResult[1]; + const time = parseInt(lastParam); + + return new Promise((resolve) => { + setTimeout(() => { + const response = { name: url }; + resolve(response); + + // Just for checking on the tests. + lastResponse = response; + }, time); + }); + }; + + const mockFetchSleepByParam = mockedFetchToFunction(fetchDataSleepByParam); + it("should render", () => { const wrapper = mount(DataFetcher, { props: { @@ -178,4 +228,130 @@ describe("DataFetcher", () => { expect(wrapper.html()).toBe("
slot content is Loaded
"); expect(wrapper.find("div").text()).toBe("slot content is Loaded"); }); + + it("should clear the previous content once a new fetch starts", async () => { + vi.spyOn(fetcher, "useFetchTo").mockImplementation(mockFetchSimple); + + const wrapper = mount(DataFetcher, { + props: { + url: "/api/", + minLength: 1, + initValue: { name: "test" }, + debounce: 1, + }, + slots: { + default: "
slot content is {{ content.name }}
", + }, + }); + + expect(wrapper.find("div").text()).toBe("slot content is test"); + + await wrapper.setProps({ + url: "/api/one", + }); + + await vi.advanceTimersByTimeAsync(1001); + + expect(wrapper.find("div").text()).toBe("slot content is /api/one"); + + await wrapper.setProps({ + url: "/api/two", + }); + + // Not enough time to get the response + await vi.advanceTimersByTimeAsync(10); + + // It becomes empty + expect(wrapper.find("div").text()).toBe("slot content is"); + + // More time but still not enough time to get the response + await vi.advanceTimersByTimeAsync(900); + + // It's still empty + expect(wrapper.find("div").text()).toBe("slot content is"); + + // Remaining time + await vi.advanceTimersByTimeAsync(91); + + expect(wrapper.find("div").text()).toBe("slot content is /api/two"); + }); + + describe("when there is a request in progress", () => { + let wrapper: VueWrapper; + beforeEach(async () => { + vi.spyOn(fetcher, "useFetchTo").mockImplementation(mockFetchSleepByParam); + + wrapper = mount(DataFetcher, { + props: { + url: "/api/", + minLength: 1, + initValue: { name: "test" }, + debounce: 1, + }, + slots: { + default: "
slot content is {{ content.name }}
", + }, + }); + + expect(wrapper.find("div").text()).toBe("slot content is test"); + + await wrapper.setProps({ + url: "/api/one/1000", + }); + }); + + describe("and a new request gets started before the first one gets responded", () => { + beforeEach(async () => { + // Not enough time to get the response from api/one/1000 + await vi.advanceTimersByTimeAsync(500); + + await wrapper.setProps({ + url: "/api/two/1000", + }); + }); + + it("should discard the response from the first request", async () => { + // More time, enough to get the response from /api/one/1000 + await vi.advanceTimersByTimeAsync(600); + + // It was effectively responded + expect(lastResponse?.name).toEqual("/api/one/1000"); + + // It should remain empty regardless + expect(wrapper.find("div").text()).toBe("slot content is"); + + // More time, enough to get the response from /api/two/1000 + await vi.advanceTimersByTimeAsync(410); + + expect(wrapper.find("div").text()).toBe("slot content is /api/two/1000"); + }); + }); + + describe("and a new request gets started and responded before the first one gets responded", () => { + beforeEach(async () => { + // Not enough time to get the response from api/one/1000 + await vi.advanceTimersByTimeAsync(100); + + await wrapper.setProps({ + url: "/api/two/500", + }); + }); + + it("should discard the response from the first request", async () => { + // Enough to get only the response from /api/two/500 + await vi.advanceTimersByTimeAsync(600); + + expect(wrapper.find("div").text()).toBe("slot content is /api/two/500"); + + // More time, enough to get the response from /api/one/1000 + await vi.advanceTimersByTimeAsync(310); + + // It was effectively responded + expect(lastResponse?.name).toEqual("/api/one/1000"); + + // But it should keep the current value + expect(wrapper.find("div").text()).toBe("slot content is /api/two/500"); + }); + }); + }); });