diff --git a/src/composables/useUrlParamWithStore.js b/src/composables/useUrlParamWithStore.js index f2f479cdd1..76893ad52a 100644 --- a/src/composables/useUrlParamWithStore.js +++ b/src/composables/useUrlParamWithStore.js @@ -67,14 +67,14 @@ export function useUrlParamWithStore(queryParam, options = {}) { watch( getRouteValue, whenIsRoute(to, (newValue) => { - if (newValue && newValue !== getStoreValue()) { + if (newValue !== null && newValue !== getStoreValue()) { setStoreValue(newValue) } }) ) // Initialize the store value with the URL value if they are different - if (getRouteValue() && !isEqual(getRouteValue(), getStoreValue())) { + if (getRouteValue() !== null && !isEqual(getRouteValue(), getStoreValue())) { setStoreValue(getRouteValue()) } diff --git a/src/composables/useUrlParamsWithStore.js b/src/composables/useUrlParamsWithStore.js index dadb328683..4b8bf28ae4 100644 --- a/src/composables/useUrlParamsWithStore.js +++ b/src/composables/useUrlParamsWithStore.js @@ -1,5 +1,6 @@ import noop from 'lodash/noop' import identity from 'lodash/identity' +import isEqual from 'lodash/isEqual' import isUndefined from 'lodash/isUndefined' import { computed, watch } from 'vue' import { useRoute, useRouter } from 'vue-router' @@ -59,7 +60,7 @@ export function useUrlParamsWithStore(queryParams, options = {}) { ) // Initialize the store value with the URL value if they are different - if (getRouteValues() && getRouteValues() !== getValue()) { + if (getRouteValues() && !isEqual(getRouteValues(), getValue())) { setValue(...getRouteValues()) } diff --git a/src/router/guards/checkSearchOrder.js b/src/router/guards/checkSearchOrder.js index dacdc9db0e..b8c61cb613 100644 --- a/src/router/guards/checkSearchOrder.js +++ b/src/router/guards/checkSearchOrder.js @@ -1,5 +1,8 @@ +import { useSearchStore } from '@/store/modules' + /** * This navigation guard checks the 'order' query parameter in the route. + * A missing or invalid order falls back to the one from the search settings. * * @param {Object} to - The target route object. * @returns {Object|null} - Returns a new route object with the 'order' query @@ -9,7 +12,7 @@ export const checkSearchOrder = (to) => { const { order } = to.query if (!['asc', 'desc'].includes(order)) { - const query = { ...to.query, order: 'asc' } + const query = { ...to.query, order: useSearchStore().orderBy === 'asc' ? 'asc' : 'desc' } return { name, params, query } } } diff --git a/src/router/guards/fillSearchRouteQuery.js b/src/router/guards/fillSearchRouteQuery.js new file mode 100644 index 0000000000..3899dbff0a --- /dev/null +++ b/src/router/guards/fillSearchRouteQuery.js @@ -0,0 +1,21 @@ +import isUndefined from 'lodash/isUndefined' +import omitBy from 'lodash/omitBy' + +import { useSearchStore } from '@/store/modules' + +/** + * This navigation guard adds the search params missing from the route query (sort, perPage...) + * with their value from the store. Normalizing the URL here, with a redirect, adds no history + * entry: if the view mirrored its state into the URL after landing, the back button would lead + * back to the very same page. + * + * @param {Object} to - The target route object. + * @returns {Object|null} - Returns a new route object with the missing query parameters. + */ +export const fillSearchRouteQuery = (to) => { + const { name, params } = to + const query = omitBy({ ...useSearchStore().toRouteQuery, ...to.query }, isUndefined) + if (Object.keys(query).some(key => !(key in to.query))) { + return { name, params, query } + } +} diff --git a/src/router/index.js b/src/router/index.js index 6cf7144e99..f0d0f83120 100644 --- a/src/router/index.js +++ b/src/router/index.js @@ -22,6 +22,7 @@ import IPhKeyboard from '~icons/ph/keyboard' import { MODE_NAME } from '@/mode' import { checkSearchOrder } from '@/router/guards/checkSearchOrder' import { checkSearchSort } from '@/router/guards/checkSearchSort' +import { fillSearchRouteQuery } from '@/router/guards/fillSearchRouteQuery' import { prefillSearchStore } from '@/router/guards/prefillSearchStore' import { replaceSizeToPerPage } from '@/router/guards/replaceSizeToPerPage' import { ROLE } from '@/enums/roles.js' @@ -71,7 +72,7 @@ export const routes = [ filters: () => import('@/views/Search/SearchFilters'), settings: () => import('@/views/Search/SearchSettings') }, - beforeEnter: [checkSearchSort, checkSearchOrder, prefillSearchStore, replaceSizeToPerPage], + beforeEnter: [checkSearchSort, checkSearchOrder, prefillSearchStore, replaceSizeToPerPage, fillSearchRouteQuery], children: [ { name: 'document', diff --git a/tests/unit/specs/composables/useUrlParam.spec.js b/tests/unit/specs/composables/useUrlParam.spec.js index 28b0f9b927..e8a5cf5e79 100644 --- a/tests/unit/specs/composables/useUrlParam.spec.js +++ b/tests/unit/specs/composables/useUrlParam.spec.js @@ -1,4 +1,5 @@ -import { createApp } from 'vue' +/* eslint-disable vue/one-component-per-file -- withSetup and a test that needs a resolved route each build an app */ +import { createApp, ref } from 'vue' import { createRouter, createMemoryHistory } from 'vue-router' import { flushPromises } from '@vue/test-utils' import { createPinia } from 'pinia' @@ -281,6 +282,20 @@ describe('useUrlParamWithStore', () => { expect(router.currentRoute.value.query.perPage).toBe('66') }) + + it('should update the store when the query parameter changes to zero', async () => { + const from = ref(null) + const [, router] = withSetup({ + composable: () => useUrlParamWithStore('from', { transform: Number, get: () => from.value, set: value => (from.value = value) }) + }) + + await router.push({ query: { from: '25' } }) + await flushPromises() + await router.push({ query: { from: '0' } }) + await flushPromises() + + expect(from.value).toBe(0) + }) }) describe('useUrlParamsWithStore', () => { @@ -339,6 +354,20 @@ describe('useUrlParamsWithStore', () => { expect(router.currentRoute.value.query.sort).toBe('creationDate') expect(router.currentRoute.value.query.order).toBe('desc') }) + + it('should not set the store on mount when the query parameters already match it', async () => { + const set = vi.fn() + const router = createRouter({ history: createMemoryHistory(), routes: [{ path: '/', component: {} }] }) + await router.replace({ query: { sort: 'name', order: 'asc' } }) + // withSetup mounts before the initial route resolves, so this test builds its own app + const app = createApp({ + setup: () => useUrlParamsWithStore(['sort', 'order'], { get: () => ['name', 'asc'], set }) + }) + app.use(createPinia()).use(router).mount(document.createElement('div')) + + expect(set).not.toHaveBeenCalled() + app.unmount() + }) }) describe('replaceUrlParam', () => { diff --git a/tests/unit/specs/views/Search/SearchSettings.spec.js b/tests/unit/specs/views/Search/SearchSettings.spec.js new file mode 100644 index 0000000000..634bb74f85 --- /dev/null +++ b/tests/unit/specs/views/Search/SearchSettings.spec.js @@ -0,0 +1,54 @@ +import { shallowMount, flushPromises } from '@vue/test-utils' + +import CoreSetup from '~tests/unit/CoreSetup' +import Search from '@/views/Search/Search' +import SearchSettings from '@/views/Search/SearchSettings' + +vi.mock('@/api/apiInstance', () => ({ + apiInstance: { + updateProject: vi.fn(), + removeProject: vi.fn() + } +})) + +// `batchQueryParamUpdate` debounces its router navigation by 50ms. +const flushDebouncedRouterUpdate = async () => { + await new Promise(resolve => setTimeout(resolve, 60)) + await flushPromises() +} + +describe('SearchSettings.vue', () => { + let core, wrappers + + beforeEach(() => { + core = CoreSetup.init().useAll().useRouterWithoutGuards() + wrappers = [] + }) + + // Unmount even when an assertion fails, or Search stays subscribed to the shared hash history. + afterEach(() => { + wrappers.forEach(wrapper => wrapper.unmount()) + }) + + it('lands on a bare query with a URL the view has nothing to add to', async () => { + // The minimal query the magnifying glass of Insights > Paths links to. + const query = { 'f[path]': '/vault/luxleaks/v1', 'indices': 'luxleaks' } + await core.router.push({ name: 'search', query }) + await flushPromises() + + const { length } = window.history + + // Search.vue hydrates the stores from the route query, SearchSettings mirrors the + // resulting settings back into the URL: any write would push a history entry. + const global = { plugins: core.plugins, renderStubDefaultSlot: true } + wrappers.push(shallowMount(Search, { global }), shallowMount(SearchSettings, { global })) + await flushPromises() + await flushDebouncedRouterUpdate() + + expect(window.history.length).toBe(length) + expect(core.router.currentRoute.value.query).toHaveProperty('f[path]', '/vault/luxleaks/v1') + expect(core.router.currentRoute.value.query).toHaveProperty('sort', '_score') + expect(core.router.currentRoute.value.query).toHaveProperty('order', 'desc') + expect(core.router.currentRoute.value.query).toHaveProperty('perPage', '25') + }) +})