Просмотр исходного кода

fix(discover): Add environment during drilldown (#19894)

During a drilldown, the environment does not get added to the query params.
Once added, the global headers do no update to reflect this. This change
properly adds the environment to the query params and ensures that the global
headers update accordingly.
Tony 4 лет назад
Родитель
Сommit
1428019090

+ 6 - 4
src/sentry/static/sentry/app/components/organizations/globalSelectionHeader/initializeGlobalSelectionHeader.tsx

@@ -82,6 +82,7 @@ class InitializeGlobalSelectionHeader extends React.Component<Props> {
      * This happens e.g. using browser's navigation button, in which case
      * This happens e.g. using browser's navigation button, in which case
      * we need to update our store to reflect URL changes
      * we need to update our store to reflect URL changes
      */
      */
+
     if (prevProps.location.query !== this.props.location.query) {
     if (prevProps.location.query !== this.props.location.query) {
       const oldQuery = getStateFromQuery(prevProps.location.query, {
       const oldQuery = getStateFromQuery(prevProps.location.query, {
         allowEmptyPeriod: true,
         allowEmptyPeriod: true,
@@ -100,10 +101,11 @@ class InitializeGlobalSelectionHeader extends React.Component<Props> {
        */
        */
       if (!isEqual(oldQuery.project, newQuery.project)) {
       if (!isEqual(oldQuery.project, newQuery.project)) {
         updateProjects(newQuery.project || [], null, {environments: newEnvironments});
         updateProjects(newQuery.project || [], null, {environments: newEnvironments});
-      }
-      if (!isEqual(oldQuery.environment, newQuery.project)) {
-        // Projects changing will also change environments, so only update environments
-        // by itself if projects is unchanged
+      } else if (!isEqual(oldQuery.environment, newQuery.environment)) {
+        /**
+         * When the project stays the same, it's still possible that the environment
+         * changed, so explictly update the enviornment
+         */
         updateEnvironments(newEnvironments);
         updateEnvironments(newEnvironments);
       }
       }
 
 

+ 5 - 1
src/sentry/static/sentry/app/components/organizations/multipleEnvironmentSelector.jsx

@@ -322,7 +322,11 @@ class EnvironmentSelectorItem extends React.PureComponent {
   render() {
   render() {
     const {environment, inputValue, isChecked} = this.props;
     const {environment, inputValue, isChecked} = this.props;
     return (
     return (
-      <GlobalSelectionHeaderRow checked={isChecked} onCheckClick={this.handleClick}>
+      <GlobalSelectionHeaderRow
+        data-test-id={`environment-${environment}`}
+        checked={isChecked}
+        onCheckClick={this.handleClick}
+      >
         <Highlight text={inputValue}>{environment}</Highlight>
         <Highlight text={inputValue}>{environment}</Highlight>
       </GlobalSelectionHeaderRow>
       </GlobalSelectionHeaderRow>
     );
     );

+ 4 - 2
src/sentry/static/sentry/app/views/eventsV2/utils.tsx

@@ -406,13 +406,15 @@ function generateExpandedConditions(
 
 
   // Add additional conditions provided and generated.
   // Add additional conditions provided and generated.
   for (const key in conditions) {
   for (const key in conditions) {
-    const value = additionalConditions[key];
+    const value = conditions[key];
     if (key === 'project.id') {
     if (key === 'project.id') {
       eventView.project = [...eventView.project, parseInt(value, 10)];
       eventView.project = [...eventView.project, parseInt(value, 10)];
       continue;
       continue;
     }
     }
     if (key === 'environment') {
     if (key === 'environment') {
-      eventView.environment = [...eventView.environment, value];
+      if (!eventView.environment.includes(value)) {
+        eventView.environment = [...eventView.environment, value];
+      }
       continue;
       continue;
     }
     }
     if (key === 'user' && typeof value === 'string') {
     if (key === 'user' && typeof value === 'string') {

+ 70 - 0
tests/acceptance/test_organization_global_selection_header.py

@@ -152,6 +152,76 @@ class OrganizationGlobalHeaderTest(AcceptanceTestCase, SnubaTestCase):
         assert u"project={}".format(self.project_1.id) in self.browser.current_url
         assert u"project={}".format(self.project_1.id) in self.browser.current_url
         assert self.issues_list.global_selection.get_selected_project_slug() == self.project_1.slug
         assert self.issues_list.global_selection.get_selected_project_slug() == self.project_1.slug
 
 
+    def test_global_selection_header_updates_environment_with_browser_navigation_buttons(self):
+        """
+        Global Selection Header should:
+        1) load project from URL if it exists
+        2) clear the current environment if the user clicks clear
+        3) reload the environment from URL if it exists on browser navigation
+        """
+        with self.feature("organizations:global-views"):
+            self.create_issues()
+
+            """
+            set up workflow:
+            1) environment=All environments
+            2) environment=prod
+            3) environment=All environments
+            """
+            self.issues_list.visit_issue_list(self.org.slug)
+            self.issues_list.wait_until_loaded()
+            assert u"environment=" not in self.browser.current_url
+            assert (
+                self.issue_details.global_selection.get_selected_environment() == "All Environments"
+            )
+
+            self.browser.click('[data-test-id="global-header-environment-selector"]')
+            self.browser.click('[data-test-id="environment-prod"]')
+            self.issues_list.wait_until_loaded()
+            assert u"environment=prod" in self.browser.current_url
+            assert self.issue_details.global_selection.get_selected_environment() == "prod"
+
+            self.browser.click('[data-test-id="global-header-environment-selector"] > svg')
+            self.issues_list.wait_until_loaded()
+            assert u"environment=" not in self.browser.current_url
+            assert (
+                self.issue_details.global_selection.get_selected_environment() == "All Environments"
+            )
+
+            """
+            navigate back through history to the beginning
+            1) environment=All Environments -> environment=prod
+            2) environment=prod -> environment=All Environments
+            """
+            self.browser.back()
+            self.issues_list.wait_until_loaded()
+            assert u"environment=prod" in self.browser.current_url
+            assert self.issue_details.global_selection.get_selected_environment() == "prod"
+
+            self.browser.back()
+            self.issues_list.wait_until_loaded()
+            assert u"environment=" not in self.browser.current_url
+            assert (
+                self.issue_details.global_selection.get_selected_environment() == "All Environments"
+            )
+
+            """
+            navigate foward through history to the end
+            1) environment=All Environments -> environment=prod
+            2) environment=prod -> environment=All Environments
+            """
+            self.browser.forward()
+            self.issues_list.wait_until_loaded()
+            assert u"environment=prod" in self.browser.current_url
+            assert self.issue_details.global_selection.get_selected_environment() == "prod"
+
+            self.browser.forward()
+            self.issues_list.wait_until_loaded()
+            assert u"environment=" not in self.browser.current_url
+            assert (
+                self.issue_details.global_selection.get_selected_environment() == "All Environments"
+            )
+
     def test_global_selection_header_loads_with_correct_project_with_multi_project(self):
     def test_global_selection_header_loads_with_correct_project_with_multi_project(self):
         """
         """
         Global Selection Header should:
         Global Selection Header should:

+ 25 - 0
tests/js/spec/views/eventsV2/utils.spec.jsx

@@ -447,6 +447,31 @@ describe('getExpandedResults()', function() {
     const result = getExpandedResults(view, {}, event);
     const result = getExpandedResults(view, {}, event);
     expect(result.query).toEqual('title:"hello there "');
     expect(result.query).toEqual('title:"hello there "');
   });
   });
+
+  it('should add environment from the data row', () => {
+    const view = new EventView({
+      ...state,
+      environment: [],
+      query: '',
+      fields: [{field: 'environment'}],
+    });
+    expect(view.environment).toEqual([]);
+    const event = {environment: 'staging'};
+    const result = getExpandedResults(view, {}, event);
+    expect(result.environment).toEqual(['staging']);
+  });
+
+  it('should not add duplicate environment', () => {
+    const view = new EventView({
+      ...state,
+      query: '',
+      fields: [{field: 'environment'}],
+    });
+    expect(view.environment).toEqual(['staging']);
+    const event = {environment: 'staging'};
+    const result = getExpandedResults(view, {}, event);
+    expect(result.environment).toEqual(['staging']);
+  });
 });
 });
 
 
 describe('downloadAsCsv', function() {
 describe('downloadAsCsv', function() {