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

feat: Add ability to group in Query Builder #16226

Merged
merged 1 commit into from
Dec 27, 2019

Conversation

hoorayimhelping
Copy link
Contributor

@hoorayimhelping hoorayimhelping commented Dec 13, 2019

Closes #15984

Adds grouping to query builder

in

@hoorayimhelping hoorayimhelping force-pushed the bs_query_builder_grouping branch from 03c6ed1 to 50fcaf7 Compare December 13, 2019 22:15
@hoorayimhelping hoorayimhelping changed the title Bs query builder grouping feat: Add ability to group in Query Builder Dec 13, 2019
@hoorayimhelping hoorayimhelping force-pushed the bs_query_builder_grouping branch 7 times, most recently from b8c26a0 to e46a170 Compare December 17, 2019 00:31
Copy link
Contributor

@121watts 121watts left a comment

Choose a reason for hiding this comment

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

I love that this is getting put into the builder. Really awesome stuff.

ui/cypress/e2e/explorer.test.ts Outdated Show resolved Hide resolved
ui/src/timeMachine/components/TagSelector.tsx Show resolved Hide resolved
/>
)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

This could probably be its own component.

Copy link
Contributor Author

@hoorayimhelping hoorayimhelping Dec 18, 2019

Choose a reason for hiding this comment

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

@121watts it could, but I don't think it adds a ton of value. Once this feature flag goes away, the tag selector will have a BuilderCard.DropdownHeader header until we change the interaction, and the BuilderCard.Header will be applied to other parts of the query builder (like the aggregate functions window)

draftState.queryBuilder.tags.splice(index, 1)
draftQuery.builderConfig.tags.splice(index, 1)
draftState.queryBuilder.tags.splice(index, 1)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

A reducer test for this added logic would be 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@121watts this change i made only adds a check that draftState exists. I'll add tests for this, but in a different ticket since this is kind of big and needs to get out.

#16294

Copy link
Contributor

@ebb-tide ebb-tide left a comment

Choose a reason for hiding this comment

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

❤️❤️❤️

ui/src/timeMachine/components/TagSelector.tsx Show resolved Hide resolved
ui/src/timeMachine/utils/queryBuilder.ts Outdated Show resolved Hide resolved
ui/src/timeMachine/utils/queryBuilder.ts Show resolved Hide resolved
@hoorayimhelping hoorayimhelping force-pushed the bs_query_builder_grouping branch 13 times, most recently from fe3a44d to d13c0bd Compare December 19, 2019 19:00
@hoorayimhelping hoorayimhelping force-pushed the bs_query_builder_grouping branch 13 times, most recently from c0ed96c to b4924ad Compare December 23, 2019 22:27
@hoorayimhelping hoorayimhelping force-pushed the bs_query_builder_grouping branch 3 times, most recently from b91d01e to 0e7e82b Compare December 26, 2019 20:09
@clifforddw
Copy link

clifforddw commented Jan 9, 2020

I just installed the beta and I don't see this functionality anything specific I need to do ?

@mark-rushakoff mark-rushakoff deleted the bs_query_builder_grouping branch April 16, 2020 20:30
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.

Query Builder: Add ability to group by columns
4 participants