Fix DQT filter counts when no fields selected#10229
Conversation
github-actions
Bot
added
the
Module: dataquery
There was a problem hiding this comment.
Pull request overview
This PR fixes a UI crash in the Data Query Tool that occurred when users removed the PSCID field from the "Define Fields" tab, causing the "Define Filters" tab to become unresponsive. The fix adds proper guards and error handling for the candidate count fetch operation.
Key Changes:
- Added a guard condition to prevent fetching candidate counts when no fields are selected
- Refactored the count-fetching logic from promise chains to async/await with proper abort handling
- Implemented cleanup mechanism using AbortController to cancel in-flight requests when the component unmounts
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| throw new Error('Error creating query.'); | ||
| } | ||
| return resp.json(); | ||
| } | ||
| ).then( | ||
| (data) => { | ||
| fetch( | ||
| '/dataquery/queries/' | ||
| + data.QueryID + '/count', | ||
|
|
||
| const data = await createResp.json(); | ||
| const countResp = await fetch( | ||
| `/dataquery/queries/${data.QueryID}/count`, | ||
| { | ||
| method: 'GET', | ||
| credentials: 'same-origin', | ||
| } | ||
| ).then((resp) => { | ||
| if (!resp.ok) { | ||
| throw new Error('Could not get count.'); | ||
| } | ||
| return resp.json(); | ||
| }).then((result) => { | ||
| signal: controller.signal, | ||
| }, | ||
| ); | ||
|
|
||
| if (!countResp.ok) { | ||
| throw new Error('Could not get count.'); |
There was a problem hiding this comment.
The error messages "Error creating query." and "Could not get count." are generic and don't provide helpful context for debugging. Consider including the HTTP status code or response details to help identify the root cause when these errors occur. For example: throw new Error(\Error creating query: ${createResp.status} ${createResp.statusText}`);`
|
@driusan, could you please assign someone to review this PR? I’d really appreciate it. |
|
This is great: I had noticed this bug ! |
Thanks! I'm glad I could catch that one while working on this. If everything else looks good to you, could you please approve the PR so we can get this merged? |
|
@skarya22 does this need my review ? |
|
@adamdaudrich looks like it is assigned to @jeffersoncasimir |
| method: 'POST', | ||
| credentials: 'same-origin', | ||
| body: JSON.stringify(payload), | ||
| signal: controller.signal, |
There was a problem hiding this comment.
signal is added to the payload but isn't expected by the back-end
CamilleBeau
added
Module: dqt
|
thanks @Mohit-Lakra for your contribution, we'll update, rebase and include this in our next release. |
|
Rebased onto 28.0-release as a replacement PR because of permission denial on Mohit-Lakra's fork. 10508 |
|
Thanks everyone for reviewing and helping move this forward. I appreciate the feedback and the follow-up rebase work. Glad the fix will be included in the upcoming release. Thanks again! |
Brief summary of changes
tab.
Testing instructions
Descriptions”, open the Data Query Tool.
Filters” tab opens normally.
npm run lint:js(passes; existing repo warnings only).Link(s) to related issue(s)
Before Fix
After Fix
