Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query Build Card Save modal does not properly show errors from API #21597

Closed
camsaul opened this issue Apr 11, 2022 · 1 comment · Fixed by #21668
Closed

Query Build Card Save modal does not properly show errors from API #21597

camsaul opened this issue Apr 11, 2022 · 1 comment · Fixed by #21668
Assignees
Labels
.Frontend Priority:P3 Cosmetic bugs, minor bugs with a clear workaround Querying/Native The SQL/native query editor Querying/Parameters & Variables Filter widgets, field filters, variables etc. .Reproduced Issues reproduced in test (usually Cypress) Type:Bug Product defects
Milestone

Comments

@camsaul
Copy link
Member

camsaul commented Apr 11, 2022

I ran into this while fixing #14145.

After PR #21598, the API returns a 400 error response (actually 202 with _status since the endpoint is async, but still effectively a 400):

{"message":"Invalid Field Filter: Field 164574 \"PRODUCTS\".\"CATEGORY\" belongs to Database 2276 \"sample-dataset\", but the query is against Database 2275 \"test-data\"","_status":400}

But the Save Card modal just shows a Type Error:

image

Console logs:

Form.jsx:216 Form submission error TypeError: Cannot read properties of undefined (reading 'collection')
    at reducer (schemas.js:56:1)
    at redux.js:163:1
    at combination (combineReducers.js:120:1)
    at combination (combineReducers.js:120:1)
    at dispatch (createStore.js:165:1)
    at middleware.js:22:1
    at index.js:28:1
    at store.js:25:1
    at dispatch (applyMiddleware.js:35:1)
    at _callee3$ (redux.js:210:1)

It should show the actual error message here.

Note that #14145 is a custom success escalation so this should probably be considered one as well

@camsaul camsaul added Type:Bug Product defects .Needs Triage .Frontend Querying/GUI Query builder catch-all, including simple mode labels Apr 11, 2022
@flamber flamber added Priority:P3 Cosmetic bugs, minor bugs with a clear workaround Querying/Parameters & Variables Filter widgets, field filters, variables etc. Querying/Native The SQL/native query editor and removed .Needs Triage Querying/GUI Query builder catch-all, including simple mode labels Apr 12, 2022
@losrebellos losrebellos self-assigned this Apr 12, 2022
@flamber
Copy link
Contributor

flamber commented Apr 13, 2022

Reproduce with PR #21598:

  1. Admin > Databases > create another H2 database pointing to the sample dataset
  2. Native > Sample Database > SELECT COUNT(*) FROM PRODUCTS WHERE {{FILTER}}
  3. Change variable to Field Filter and connect to Products.Category
  4. Run query - works
  5. Change database (above the editor) to the second H2
  6. Run query - fails with kinda understandable error
  7. Save question - fails without understandable error
Screencast-2022-04-13_11.06.53.mp4

@flamber flamber added this to the 0.43 milestone Apr 20, 2022
@nemanjaglumac nemanjaglumac added the .Reproduced Issues reproduced in test (usually Cypress) label Aug 8, 2022
nemanjaglumac added a commit that referenced this issue Sep 1, 2022
* Fix native query

* Replace deprecated `cy.server` and `cy.route`

* Add database through API rather than using UI

* Remove superfluous `visit`

`openNativeEditor` already contains `cy.visit("/")`

* Use Postgres instead of H2 for a second database

* Speed typing up a bit

* Do not use hard coded IDs

* Update test title

* Move spec to `native` reproductions
nemanjaglumac added a commit that referenced this issue Sep 1, 2022
* Fix native query

* Replace deprecated `cy.server` and `cy.route`

* Add database through API rather than using UI

* Remove superfluous `visit`

`openNativeEditor` already contains `cy.visit("/")`

* Use Postgres instead of H2 for a second database

* Speed typing up a bit

* Do not use hard coded IDs

* Update test title

* Move spec to `native` reproductions
nemanjaglumac added a commit that referenced this issue Sep 2, 2022
* Fix native query

* Replace deprecated `cy.server` and `cy.route`

* Add database through API rather than using UI

* Remove superfluous `visit`

`openNativeEditor` already contains `cy.visit("/")`

* Use Postgres instead of H2 for a second database

* Speed typing up a bit

* Do not use hard coded IDs

* Update test title

* Move spec to `native` reproductions
snoe pushed a commit that referenced this issue Sep 6, 2022
* Fix native query

* Replace deprecated `cy.server` and `cy.route`

* Add database through API rather than using UI

* Remove superfluous `visit`

`openNativeEditor` already contains `cy.visit("/")`

* Use Postgres instead of H2 for a second database

* Speed typing up a bit

* Do not use hard coded IDs

* Update test title

* Move spec to `native` reproductions
This was referenced Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.Frontend Priority:P3 Cosmetic bugs, minor bugs with a clear workaround Querying/Native The SQL/native query editor Querying/Parameters & Variables Filter widgets, field filters, variables etc. .Reproduced Issues reproduced in test (usually Cypress) Type:Bug Product defects
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants