Skip to content

Commit

Permalink
fixes #756: don't make the odata request when route is being changed (#…
Browse files Browse the repository at this point in the history
…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 <[email protected]>
  • Loading branch information
sadiqkhoja and matthew-white authored Dec 17, 2024
1 parent 54d0cad commit 6ba4ef1
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 1 deletion.
11 changes: 10 additions & 1 deletion src/composables/query-ref.js
Original file line number Diff line number Diff line change
Expand Up @@ -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])) {
Expand Down
12 changes: 12 additions & 0 deletions test/components/submission/filters.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
});
});
});
16 changes: 16 additions & 0 deletions test/composables/query-ref.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 6ba4ef1

Please sign in to comment.