From 6ba4ef1183d2081e1da6f9921806c03df8483f97 Mon Sep 17 00:00:00 2001 From: Sadiq Khoja Date: Tue, 17 Dec 2024 13:59:33 -0500 Subject: [PATCH] fixes #756: don't make the odata request when route is being changed (#1042) Root cause: when user clicks on another page link, query parameter changes, which causes odataFilter watcher in SubmissionList component to send another odata request. --------- Co-authored-by: Matthew White --- src/composables/query-ref.js | 11 ++++++++++- test/components/submission/filters.spec.js | 12 ++++++++++++ test/composables/query-ref.spec.js | 16 ++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/composables/query-ref.js b/src/composables/query-ref.js index c96cc9624..4b7c78358 100644 --- a/src/composables/query-ref.js +++ b/src/composables/query-ref.js @@ -65,12 +65,21 @@ export default ({ fromQuery, toQuery }) => { the same -- something we want to avoid. */ let ignoreNextRouteChange = false; - watch(router.currentRoute, ({ query: newQuery }, { query: oldQuery }) => { + watch(router.currentRoute, (newRoute, oldRoute) => { + // If the route has changed, then the component that called this composable + // is probably about to be unmounted, so there's no need to set the value of + // the ref. If a new component is mounted that also uses useQueryRef(), that + // will set up a different watcher. See: + // https://github.com/getodk/central/issues/756 + if (newRoute.path !== oldRoute.path) return; + if (ignoreNextRouteChange) { ignoreNextRouteChange = false; return; } + const newQuery = newRoute.query; + const oldQuery = oldRoute.query; for (const name of params) { // Using equals() rather than === to account for array values. if (!equals(newQuery[name], oldQuery[name])) { diff --git a/test/components/submission/filters.spec.js b/test/components/submission/filters.spec.js index 411b19c7c..e9ba546c9 100644 --- a/test/components/submission/filters.spec.js +++ b/test/components/submission/filters.spec.js @@ -475,4 +475,16 @@ describe('SubmissionFilters', () => { multiselects[1].attributes('aria-disabled').should.equal('true'); }); }); + + // https://github.com/getodk/central/issues/756 + it('does not send an extra OData request after filtering, then navigating away', () => { + testData.extendedForms.createPast(1); + testData.extendedFormVersions.createPast(1, { draft: true }); + return load('/projects/1/forms/f/submissions?reviewState=%27approved%27') + .complete() + // If an extra request is sent, then .load() will fail. + .load('/projects/1/forms/f/draft/testing', { + project: false, form: false, formDraft: false, attachments: false + }); + }); }); diff --git a/test/composables/query-ref.spec.js b/test/composables/query-ref.spec.js index 9472a3596..8e1e4c9c1 100644 --- a/test/composables/query-ref.spec.js +++ b/test/composables/query-ref.spec.js @@ -44,6 +44,22 @@ describe('useQueryRef()', () => { result.value.should.equal('c,d'); }); + // https://github.com/getodk/central/issues/756 + it('does not change value of ref after route path changes', async () => { + let result; + return load('/?x=a&y=b') + .afterResponses(app => { + result = withSetup(() => useQueryRef(options.comma), { + container: app.vm.$container + }); + result.value.should.equal('a,b'); + }) + .load('/users?x=c&y=d') + .afterResponses(() => { + result.value.should.equal('a,b'); + }); + }); + describe('change to an irrelevant query parameter', () => { it('does not change the value of the ref', async () => { // `z` is the irrelevant query parameter that the ref doesn't use.