feat(new-widget-builder-experience): Add Metrics dataset - Part 1 - [DD-537] by priscilawebdev · Pull Request #33047 · getsentry/sentry

priscilawebdev · 2022-03-29T13:44:24Z

Enable Release DataSet
Create the hooks useMetricTags and useMetricMetags + Tests
Add Release SeachBar to the widget Builder
Add Test to verify if the release data set is rendered
Update MetricsTagStore and MetricsMetaStore to have the flag loaded

Preview:

Screen.Recording.2022-04-03.at.14.24.11.mov

narsaynorath · 2022-03-31T19:58:46Z

-export default withIssueTags(IssuesSearchBar);
+const IssuesSearchBar = withIssueTags(IssuesSearchBarContainer);
+
+export {IssuesSearchBar};


Thank you. I wasn't quite sure how to do the named export with the HoC so this is good. I realized the tests are failing elsewhere because this IssuesSearchBar is shared with the modal, we need to update the import in issueWidgetQueriesForm.tsx. I'll push a commit up

I think you can also do export const IssuesSearchBar = withIssueTags(IssuesSearchBarContainer); 😄

github-actions · 2022-04-03T12:23:15Z

size-limit report 📦

Path	Size
src/sentry/static/sentry/dist/entrypoints/app.js	61.15 KB (+0.02% 🔺)
src/sentry/static/sentry/dist/entrypoints/sentry.css	69.97 KB (0%)

…rics-data-set

shruthilayaj · 2022-04-04T16:38:25Z

+
+export function useMetricMetas() {
+  const api = useApi();
+  const organization = useOrganization();


When I go into a dashboard and try to add from the 'Add Widget' button, I get an error saying useOrganization called but organization is not set

this shouldn't happened :/ the OrganizationContainerContext is wrapping the whole app... I will take a look into it...thanks.

shruthilayaj · 2022-04-04T17:28:40Z

                    onSearch={handleSearch(queryIndex)}
                  />
+                ) : (
+                  <ReleaseSearchBar


As an aside, not related to the work here (I'll create a separate ticket to track it) we should probably only show recent searches related to the dataset you're in!

when I add stuff to the filter and click outside to onBlur, it resets the selected fields to events defaults

Order bys also are broken, they show issue fields (I guess the search bar and order by isn't in the scope of this PR?) if so, can we just remove the fields? for now?

I've one more PR on the top of this one for the sortBy #33241

when I add stuff to the filter and click outside to onBlur, it resets the selected fields to events defaults

yes this was fixed in the follow-up PRs

edwardgou-sentry

I'm not that familiar with all the release metrics context but code looks good to me. Just had a few observations but I dont think any of them are blockers

edwardgou-sentry · 2022-04-04T18:01:38Z

+        (prevState.displayType === DisplayType.TABLE &&
+          widgetToBeUpdated?.widgetType &&
+          WIDGET_TYPE_TO_DATA_SET[widgetToBeUpdated.widgetType] === DataSet.ISSUES) ||


Hmm I wonder if this part of the condition is necessary? Looks like it's going to run any time we switch to the table visualization when editing an issue widget. I notice some behaviour differences in the builder depending on the widget type due to this (switching to table display on an existing discover widget behaves differently from an existing issue widget even if you already edited the both widgets to the exact same state).

Not a blocker though, this doesn't seem like part of what this PR is addressing. We can also discuss details offline

edwardgou-sentry · 2022-04-04T19:32:21Z

+  useEffect(() => {
+    let unmounted = false;
+
+    if (!unmounted && shouldFetchMetricTags) {
+      fetchMetricsTags(api, organization.slug, selection.projects);
+    }
+
+    return () => {
+      unmounted = true;
+    };
+  }, [selection.projects, organization.slug, shouldFetchMetricTags]);


Not a blocker, but I think it's possible for fetchMetricsTags to be ran multiple times if it is not loaded and useMetricTags is used multiple times on the same page. This is because the loaded variable is only set to true after the request to metrics/tags/ is finished so multiple requests can happen if they are fired at the same time.

Maybe we can investigate reducing this to a single request by having some kind of state for loading in the store (i think useProjects does something similar)? I think this also applies for useMetricMetas. Again, not a blocker.

yes I've noticed that and was talking about it with @narsaynorath yesterday... that's why we are still keeping the fetching(tags and fields) on the top of the widget builder, but that's something we want to improve and will investigate afterward

priscilawebdev · 2022-04-05T14:24:30Z

@edwardgou-sentry I'm addressing your feedback in a follow-up PR 😉

priscilawebdev changed the title ~~feat(new-widget-builder-experience): Add Metrics dataset~~ wip: feat(new-widget-builder-experience): Add Metrics dataset Mar 29, 2022

priscilawebdev changed the title ~~wip: feat(new-widget-builder-experience): Add Metrics dataset~~ wip: feat(new-widget-builder-experience): Add Metrics dataset - Part 1 Mar 30, 2022

vercel Bot deployed to Preview – storybook March 30, 2022 11:21 View deployment

vercel Bot had a problem deploying to Preview – sentry March 30, 2022 11:21 Failure

vercel Bot deployed to Preview – storybook March 30, 2022 14:53 View deployment

vercel Bot deployed to Preview – sentry March 30, 2022 14:53 View deployment

priscilawebdev changed the title ~~wip: feat(new-widget-builder-experience): Add Metrics dataset - Part 1~~ wip: feat(new-widget-builder-experience): Add Metrics dataset - Part 1 - [DD-537] Mar 30, 2022

vercel Bot deployed to Preview – sentry March 31, 2022 13:14 View deployment

vercel Bot deployed to Preview – storybook March 31, 2022 13:14 View deployment

vercel Bot deployed to Preview – sentry March 31, 2022 14:59 View deployment

vercel Bot deployed to Preview – storybook March 31, 2022 14:59 View deployment

narsaynorath reviewed Mar 31, 2022

View reviewed changes

vercel Bot deployed to Preview – sentry March 31, 2022 19:59 View deployment

vercel Bot deployed to Preview – storybook March 31, 2022 19:59 View deployment

vercel Bot deployed to Preview – storybook March 31, 2022 20:39 View deployment

vercel Bot deployed to Preview – sentry March 31, 2022 20:39 View deployment

vercel Bot deployed to Preview – sentry March 31, 2022 22:57 View deployment

vercel Bot deployed to Preview – storybook March 31, 2022 22:58 View deployment

feat(new-widget-builder-experience): Add Metrics dataset

2b3315a

priscilawebdev force-pushed the feat/new-widget-builder-experience-add-metrics-data-set branch from b16da70 to 2b3315a Compare April 3, 2022 08:42

vercel Bot deployed to Preview – storybook April 3, 2022 08:43 View deployment

vercel Bot deployed to Preview – sentry April 3, 2022 08:43 View deployment

add tests

2bbbb27

vercel Bot deployed to Preview – storybook April 3, 2022 12:20 View deployment

vercel Bot deployed to Preview – sentry April 3, 2022 12:21 View deployment

remove not need file

f74b349

vercel Bot deployed to Preview – sentry April 3, 2022 12:23 View deployment

vercel Bot deployed to Preview – storybook April 3, 2022 12:23 View deployment

add extra comment

3dd22a3

vercel Bot deployed to Preview – sentry April 3, 2022 12:28 View deployment

vercel Bot deployed to Preview – storybook April 3, 2022 12:28 View deployment

priscilawebdev changed the title ~~wip: feat(new-widget-builder-experience): Add Metrics dataset - Part 1 - [DD-537]~~ feat(new-widget-builder-experience): Add Metrics dataset - Part 1 - [DD-537] Apr 3, 2022

move client mock

c7c334f

vercel Bot deployed to Preview – sentry April 3, 2022 13:22 View deployment

vercel Bot deployed to Preview – storybook April 3, 2022 13:22 View deployment

fix test

79a57bc

vercel Bot deployed to Preview – sentry April 3, 2022 14:44 View deployment

vercel Bot deployed to Preview – storybook April 3, 2022 14:44 View deployment

remove useProjects

41cd3aa

vercel Bot deployed to Preview – storybook April 3, 2022 16:22 View deployment

vercel Bot deployed to Preview – sentry April 3, 2022 16:22 View deployment

priscilawebdev added 2 commits April 4, 2022 09:55

Merge branch 'master' into feat/new-widget-builder-experience-add-met…

d068e51

…rics-data-set

fix test

f5de69d

vercel Bot deployed to Preview – sentry April 4, 2022 08:04 View deployment

vercel Bot deployed to Preview – storybook April 4, 2022 08:04 View deployment

priscilawebdev marked this pull request as ready for review April 4, 2022 08:27

priscilawebdev requested review from a team April 4, 2022 08:27

shruthilayaj reviewed Apr 4, 2022

View reviewed changes

edwardgou-sentry approved these changes Apr 4, 2022

View reviewed changes

priscilawebdev merged commit de17ee9 into master Apr 5, 2022

priscilawebdev deleted the feat/new-widget-builder-experience-add-metrics-data-set branch April 5, 2022 06:24

priscilawebdev mentioned this pull request Apr 5, 2022

ref(new-widget-builder-experience): Revert MetricsSearchbar logic #33303

Merged

github-actions Bot locked and limited conversation to collaborators Apr 21, 2022

Uh oh!

Conversation

priscilawebdev commented Mar 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Apr 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report 📦

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shruthilayaj Apr 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shruthilayaj Apr 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

priscilawebdev Apr 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edwardgou-sentry left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

priscilawebdev Apr 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

priscilawebdev commented Apr 5, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

priscilawebdev commented Mar 29, 2022 •

edited

Loading

github-actions Bot commented Apr 3, 2022 •

edited

Loading

shruthilayaj Apr 4, 2022 •

edited

Loading

shruthilayaj Apr 4, 2022 •

edited

Loading

priscilawebdev Apr 5, 2022 •

edited

Loading

priscilawebdev Apr 5, 2022 •

edited

Loading