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

chore(query Builder): fix query builder state #18649

Merged
merged 7 commits into from
Jun 25, 2020

Conversation

rbose22
Copy link
Contributor

@rbose22 rbose22 commented Jun 22, 2020

Closes #18120

Originally, when a user begins writing in the script editor in the explore page and then navigates to the data page and clicks on one of the buckets, the user is taken to the explore page, but the script editor is still visible instead of the query builder. This solution updates the redux state of the explore page, so that when navigating from the data page to the explore page, the query builder is always displayed.

This change does not affect navigation from any other pages. For example, if the user was using the script editor on the explore page and then navigated to the tasks page, when they return to the explore page, the script editor will still be visible instead of the query builder.

@rbose22 rbose22 force-pushed the rashi-switching-to-query-builder branch from 476a113 to 6c9d4e4 Compare June 22, 2020 23:32
Copy link
Contributor

@hoorayimhelping hoorayimhelping left a comment

Choose a reason for hiding this comment

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

solid work!

a whitespace nit, and a suggestion about test syntax, but mostly nitpicks. talk to deniz about how to handle them :)

cy.getByTestID('nav-item-data-explorer').click()

cy.get('.flux-editor').within(() => {
cy.get('.view-lines').contains(fluxCode).should('exist')
Copy link
Contributor

Choose a reason for hiding this comment

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

you can safely drop the should('exist') if it follows a contains function. https://docs.cypress.io/guides/core-concepts/introduction-to-cypress.html#Default-Assertions

would

  cy.contains(fluxCode)

work? or does this need to be scoped to .view-lines?

if you can just find fluxCode anywhere, this can be simplified to

Suggested change
cy.get('.view-lines').contains(fluxCode).should('exist')
cy..contains(fluxCode)

@@ -155,7 +159,7 @@ export const loadBuckets = () => async (
const orgID = getOrg(getState()).id

dispatch(setBuilderBucketsStatus(RemoteDataState.Loading))

Copy link
Contributor

Choose a reason for hiding this comment

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

@rbose22 rbose22 merged commit 449d475 into influxdata:master Jun 25, 2020
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.

navigating to DE from bucket selection doesn't overwrite script editor state of DE.
3 participants