Skip to content

Fixes download filtered submissions: don't reset state on modal hide#1309

Merged
sadiqkhoja merged 2 commits intogetodk:masterfrom
sadiqkhoja:fixes/1168-filter-subs-on-download
Jul 15, 2025
Merged

Fixes download filtered submissions: don't reset state on modal hide#1309
sadiqkhoja merged 2 commits intogetodk:masterfrom
sadiqkhoja:fixes/1168-filter-subs-on-download

Conversation

@sadiqkhoja
Copy link
Contributor

@sadiqkhoja sadiqkhoja commented Jul 8, 2025

Closes getodk/central#1168

This was kinda hard bug to fix.

What's happening: When you hover on the hyperlink, you see the correct link but when you click on it, another function runs and hides the modal resulting in resetting of modal state/data, so when default event handler of browser is run it sees no odatafilter query parameter.

Before submitting this PR, please make sure you have:

  • run npm run test and npm run lint and confirmed all checks still pass OR confirm CircleCI build passes
  • verified that any code or assets from external sources are properly credited in comments or that everything is internally sourced

@sadiqkhoja sadiqkhoja force-pushed the fixes/1168-filter-subs-on-download branch from aa3f0dc to 2bfa4ac Compare July 8, 2025 18:47
@sadiqkhoja sadiqkhoja marked this pull request as ready for review July 8, 2025 18:48
@sadiqkhoja sadiqkhoja requested a review from matthew-white July 8, 2025 18:53
@matthew-white
Copy link
Member

When the SubmissionDownload modal is hidden, it doesn't reset the form fields in the modal other than the passphrase. But you're totally right that it ends up resetting the odataFilter prop that's passed to SubmissionDownload. In addition to the default event handler, that would be an issue for the "Try again" CTA in the toast. The CTA needs the odataFilter prop to continue to be set.

Copy link
Member

@matthew-white matthew-white left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, surprisingly tricky bug!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that it'd be nice if we could add a test to prevent future regression. I know it's a bit hard to hook into the default event handler though. What do you think of something like the following? It fails with the code today, but passes with your change.

it('does not remove filter from link after modal is hidden', async () => {
  testData.extendedForms.createPast(1);
  const component = await load('/projects/1/forms/f/submissions?reviewState=null');

  let clickCount = 0;
  let failure = false;
  component.element.addEventListener('click', (event) => {
    if (event.target.tagName !== 'A') return;
    event.preventDefault(); // Don't actually download the file.
    clickCount += 1;
    const url = relativeUrl(event.target.getAttribute('href'));
    const filter = url.searchParams.get('$filter');
    if (filter == null || !filter.includes('reviewState')) failure = true;
  });

  // Click the link in the modal.
  await component.find('#submission-download-button .btn-primary').trigger('click');
  await component.find('#submission-download-button li:nth-of-type(1) button').trigger('click');
  await component.getComponent(SubmissionDownload).get('a').trigger('click');

  // Click the link via the CTA in the toast.
  component.vm.$container.toast.cta.handler();
  await nextTick();

  clickCount.should.equal(2);
  failure.should.be.false;
});

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the event listener to component.element so that it would run after the Vue handler that hides the modal.

@sadiqkhoja sadiqkhoja force-pushed the fixes/1168-filter-subs-on-download branch from 8f51a11 to c1b0de8 Compare July 15, 2025 02:08
@sadiqkhoja sadiqkhoja merged commit 19438df into getodk:master Jul 15, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Download button is not filtering Submissions

2 participants