Skip to content

feat(discover2): Discover v2 table and friends #14403

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

Merged
merged 57 commits into from
Aug 29, 2019
Merged

feat(discover2): Discover v2 table and friends #14403

merged 57 commits into from
Aug 29, 2019

Conversation

dashed
Copy link
Member

@dashed dashed commented Aug 15, 2019

WORK IN PROGRESS

Extends #14345

TODO

  • revamped data view schema EDIT: EventView class abstraction
  • utilities to convert data view to query strings
  • encode field name/aggr(name), col name, col size to query strings
  • push url from data view
  • unmarshal query strings to data view
  • revamped table component
  • auto-expanding table column
  • remove experimental auto expanding column
  • remove any and all references of snuba.

Unresolved

  • are there illegal characters for field name/aggr(name)? EDIT: addressed with JSON stringify
  • are there illegal characters for col name? EDIT: addressed with JSON stringify

Deferred

  • add columns
  • remove columns
  • resize columns

Closes SEN-806
Closes SEN-907

@dashed dashed added the WIP label Aug 15, 2019
@dashed dashed force-pushed the discover2/typescript-all-the-things branch from 58d3443 to d130c69 Compare August 15, 2019 15:35
@dashed dashed force-pushed the discover2/typescript-all-the-things branch from f7ed133 to a276ec7 Compare August 15, 2019 17:10
@dashed dashed force-pushed the discover2/typescript-all-the-things branch from a276ec7 to 4581ac8 Compare August 15, 2019 18:06
@dashed dashed self-assigned this Aug 15, 2019
@dashed dashed changed the base branch from discover2/typescript-all-the-things to master August 15, 2019 19:29
@dashed dashed changed the base branch from master to discover2/typescript-all-the-things August 16, 2019 21:48
@dashed dashed force-pushed the discover2/typescript-all-the-things branch 17 times, most recently from 8c3bb26 to 727d7e1 Compare August 21, 2019 21:19
@dashed dashed requested a review from markstory August 27, 2019 23:58
@@ -152,8 +152,11 @@ def test_modal_from_errors_view(self, mock_now):
event_ids.append(event.event_id)

with self.feature(FEATURE_NAME):

error_view = 'field=%5B"title"%2C"error"%5D&field=%5B"count%28id%29"%2C"events"%5D&field=%5B"count_unique%28user%29"%2C"users"%5D&field=%5B"project"%2C"project"%5D&field=%5B"last_seen"%2C"last+seen"%5D&name=Errors&query=event.type%3Aerror&sort=-last_seen&sort=-title&tag=error.type&tag=project.name'
Copy link
Member Author

Choose a reason for hiding this comment

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

@markstory I hardcoded this for now. We may need a utility function to encode some dict into query strings.

@dashed dashed requested a review from leedongwei August 28, 2019 00:01
@dashed
Copy link
Member Author

dashed commented Aug 28, 2019

@leedongwei @markstory I've addressed the acceptance/js tests for this PR, and either of you can take over. Mark has some remaining feedback that are yet to be addressed (please see unresolved comments in the PR).

const {defaultSort, location} = this.props;
return location.query.sort ? location.query.sort : defaultSort;
return typeof location.query.sort === 'string' ? location.query.sort : defaultSort;
Copy link
Member Author

Choose a reason for hiding this comment

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

location.query.sort can also be Array<string>; if it is and non-empty, we can use the first item.


if (typeof name === 'string' && String(name).trim().length > 0) {
return [t('Events'), String(name).trim()];
// return `${} \u2014 ${}`;
Copy link
Member Author

Choose a reason for hiding this comment

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

stray comment

@markstory markstory merged commit be8f194 into master Aug 29, 2019
@markstory markstory deleted the SEN-806 branch August 29, 2019 15:05
@github-actions github-actions bot locked and limited conversation to collaborators Dec 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants