fix(ui): Clear environments when changing projects [SEN-661] (#13334)

If you are on project A that has envs 1, 2, 3, and you select env 3. If you switch to a project B, there is no guarantee project B will have the same envs 1, 2, 3.

We should clear environments when switching projects since environments are scoped to a project.

Fixes SEN-661

src/sentry/static/sentry/app/components/organizations/globalSelectionHeader/index.jsx

@@ -351,8 +351,23 @@ class GlobalSelectionHeader extends React.Component {
 
   handleUpdateProjects = () => {
     const {projects} = this.state;
-    updateProjects(projects, this.getRouter(), this.getUpdateOptions());
-    this.setState({projects: null});
+
+    // Clear environments when switching projects
+    //
+    // Update both params at once, otherwise:
+    // - if you update projects first, we could get a flicker
+    //   because you'll have projects & environments before we update
+    // - if you update environments first, there could be race conditions
+    //   with value of router.location.query
+    updateParams(
+      {
+        environment: null,
+        project: projects,
+      },
+      this.getRouter(),
+      this.getUpdateOptions()
+    );
+    this.setState({projects: null, environments: null});
     callIfFunction(this.props.onUpdateProjects, projects);
   };
 

src/sentry/static/sentry/app/components/organizations/multipleEnvironmentSelector.jsx

@@ -1,7 +1,7 @@
 import PropTypes from 'prop-types';
 import React from 'react';
 import styled, {css} from 'react-emotion';
-import {uniq, intersection} from 'lodash';
+import {uniq} from 'lodash';
 
 import {analytics} from 'app/utils/analytics';
 import getRouteStringFromRoutes from 'app/utils/getRouteStringFromRoutes';
@@ -60,27 +60,14 @@ class MultipleEnvironmentSelector extends React.PureComponent {
   }
 
   componentDidUpdate(prevProps, prevState) {
-    // When projects change we may need to update the selected value if the current values
-    // become invalid.
-    const sharedProjects = intersection(
-      prevProps.selectedProjects,
-      this.props.selectedProjects
-    );
-    if (sharedProjects.length !== prevProps.selectedProjects.length) {
-      const selectedEnvs = Array.from(this.state.selectedEnvs.values());
-      const validEnvironments = this.getEnvironments();
-      const newSelection = validEnvironments.filter(env => selectedEnvs.includes(env));
-
-      if (newSelection.length !== selectedEnvs.length) {
-        this.replaceSelected(newSelection);
-      }
+    // Need to sync state
+    if (this.props.value !== prevProps.value) {
+      this.syncSelectedStateFromProps();
     }
   }
 
-  replaceSelected(newSelection) {
-    this.setState({selectedEnvs: new Set(newSelection)});
-    this.props.onChange(newSelection);
-  }
+  syncSelectedStateFromProps = () =>
+    this.setState({selectedEnvs: new Set(this.props.value)});
 
   /**
    * If value in state is different than value from props, propagate changes

tests/js/spec/components/organizations/globalSelectionHeader.spec.jsx

@@ -1,6 +1,7 @@
 import React from 'react';
 
 import {initializeOrg} from 'app-test/helpers/initializeOrg';
+import {mockRouterPush} from 'app-test/helpers/mockRouterPush';
 import {mount} from 'enzyme';
 import ConfigStore from 'app/stores/configStore';
 import GlobalSelectionHeader from 'app/components/organizations/globalSelectionHeader';
@@ -23,7 +24,7 @@ const changeQuery = (routerContext, query) => ({
 
 jest.mock('app/utils/localStorage', () => {
   return {
-    getItem: () => JSON.stringify({projects: [5], environments: ['staging']}),
+    getItem: () => JSON.stringify({projects: [3], environments: ['staging']}),
     setItem: jest.fn(),
   };
 });
@@ -33,14 +34,19 @@ describe('GlobalSelectionHeader', function() {
     organization: {features: ['global-views']},
     projects: [
       {
-        id: 5,
-        isMember: true,
+        id: 2,
+      },
+      {
+        id: 3,
+        slug: 'project-3',
+        environments: ['prod', 'staging'],
       },
     ],
     router: {
       location: {query: {}},
     },
   });
+  jest.spyOn(ProjectsStore, 'getAll').mockImplementation(() => organization.projects);
 
   beforeAll(function() {
     jest.spyOn(globalActions, 'updateDateTime');
@@ -124,13 +130,22 @@ describe('GlobalSelectionHeader', function() {
       })
     );
 
-    // component will initially try to sync router + stores
+    wrapper.setContext(
+      changeQuery(routerContext, {
+        statsPeriod: '21d',
+      }).context
+    );
+
+    await tick();
+    wrapper.update();
+
     expect(globalActions.updateDateTime).toHaveBeenCalledWith({
-      period: '7d',
+      period: '21d',
       utc: null,
       start: null,
       end: null,
     });
+    // These should not be called because they have not changed, only date has changed
     expect(globalActions.updateProjects).toHaveBeenCalledWith([]);
     expect(globalActions.updateEnvironments).toHaveBeenCalledWith([]);
 
@@ -138,35 +153,101 @@ describe('GlobalSelectionHeader', function() {
     globalActions.updateProjects.mockClear();
     globalActions.updateEnvironments.mockClear();
 
-    wrapper.setContext(
-      changeQuery(routerContext, {
-        statsPeriod: '21d',
-      }).context
+    expect(GlobalSelectionStore.get()).toEqual({
+      datetime: {
+        period: '21d',
+        utc: null,
+        start: null,
+        end: null,
+      },
+      environments: [],
+      projects: [],
+    });
+  });
+
+  it('updates environments when switching projects', async function() {
+    const wrapper = mount(
+      <GlobalSelectionHeader
+        organization={organization}
+        projects={organization.projects}
+      />,
+      routerContext
     );
+
+    mockRouterPush(wrapper, router);
+
+    // Open dropdown and select both projects
+    wrapper.find('MultipleProjectSelector HeaderItem').simulate('click');
+    wrapper
+      .find('MultipleProjectSelector CheckboxFancy')
+      .at(0)
+      .simulate('click');
+    wrapper
+      .find('MultipleProjectSelector CheckboxFancy')
+      .at(1)
+      .simulate('click');
+    wrapper.find('MultipleProjectSelector HeaderItem').simulate('click');
+
     await tick();
     wrapper.update();
+    expect(wrapper.find('MultipleProjectSelector Content').text()).toBe(
+      'project-slug, project-3'
+    );
 
-    expect(globalActions.updateDateTime).toHaveBeenCalledWith({
-      period: '21d',
-      utc: null,
-      start: null,
-      end: null,
+    // Select environment
+    wrapper.find('MultipleEnvironmentSelector HeaderItem').simulate('click');
+    wrapper
+      .find('MultipleEnvironmentSelector CheckboxFancy')
+      .at(1)
+      .simulate('click');
+    wrapper.find('MultipleEnvironmentSelector HeaderItem').simulate('click');
+    await tick();
+
+    expect(wrapper.find('MultipleEnvironmentSelector Content').text()).toBe('staging');
+
+    expect(GlobalSelectionStore.get()).toEqual({
+      datetime: {
+        period: null,
+        utc: null,
+        start: null,
+        end: null,
+      },
+      environments: ['staging'],
+      projects: [2, 3],
+    });
+    const query = wrapper.prop('location').query;
+    expect(query).toEqual({
+      environment: 'staging',
+      project: ['2', '3'],
     });
 
-    // These should not be called because they have not changed, only date has changed
-    expect(globalActions.updateProjects).not.toHaveBeenCalled();
-    expect(globalActions.updateEnvironments).not.toHaveBeenCalled();
+    // Now change projects, first project has no enviroments
+    wrapper.find('MultipleProjectSelector HeaderItem').simulate('click');
+    wrapper
+      .find('MultipleProjectSelector CheckboxFancy')
+      .at(1)
+      .simulate('click');
 
+    wrapper.find('MultipleProjectSelector HeaderItem').simulate('click');
+
+    await tick();
+    wrapper.update();
+
+    // Store should not have any environments selected
     expect(GlobalSelectionStore.get()).toEqual({
       datetime: {
-        period: '21d',
+        period: null,
         utc: null,
         start: null,
         end: null,
       },
       environments: [],
-      projects: [],
+      projects: [2],
     });
+    expect(wrapper.prop('location').query).toEqual({project: '2'});
+    expect(wrapper.find('MultipleEnvironmentSelector Content').text()).toBe(
+      'All Environments'
+    );
   });
 
   it('updates URL to match GlobalSelection store when re-rendered with `forceUrlSync` prop', async function() {
@@ -188,7 +269,7 @@ describe('GlobalSelectionHeader', function() {
       expect.objectContaining({
         query: {
           environment: ['staging'],
-          project: [5],
+          project: [3],
         },
       })
     );
@@ -367,7 +448,7 @@ describe('GlobalSelectionHeader', function() {
     it('selects first project if none (i.e. all) is requested', function() {
       const project = TestStubs.Project({id: '3'});
       const org = TestStubs.Organization({projects: [project]});
-      ProjectsStore.loadInitialData(org.projects);
+      jest.spyOn(ProjectsStore, 'getAll').mockImplementation(() => org.projects);
 
       const initializationObj = initializeOrg({
         organization: org,
@@ -427,7 +508,7 @@ describe('GlobalSelectionHeader', function() {
       memberProject = TestStubs.Project({id: '3', isMember: true});
       nonMemberProject = TestStubs.Project({id: '4', isMember: false});
       const org = TestStubs.Organization({projects: [memberProject, nonMemberProject]});
-      ProjectsStore.loadInitialData(org.projects);
+      jest.spyOn(ProjectsStore, 'getAll').mockImplementation(() => org.projects);
 
       initialData = initializeOrg({
         organization: org,

tests/js/spec/components/organizations/multipleEnvironmentSelector.spec.jsx

@@ -58,32 +58,10 @@ describe('MultipleEnvironmentSelector', function() {
     expect(onChange).toHaveBeenCalledTimes(3);
     expect(onChange).toHaveBeenLastCalledWith(envs, expect.anything());
 
-    wrapper.setProps({value: envs});
-    wrapper.update();
     wrapper
-      .find('MultipleEnvironmentSelector')
-      .instance()
-      .doUpdate();
-    expect(onUpdate).toHaveBeenCalledWith();
-  });
-
-  it('will call onUpdate when project selection change causes invalid values', async function() {
-    await wrapper.find('MultipleEnvironmentSelector HeaderItem').simulate('click');
-
-    // Select 'production'
-    await wrapper
-      .find('MultipleEnvironmentSelector AutoCompleteItem CheckboxHitbox')
-      .at(0)
+      .find('MultipleSelectorSubmitRow button[aria-label="Apply"]')
       .simulate('click');
-    await wrapper.update();
-
-    // Update project selection so that 'production' is no longer an option.
-    wrapper.setProps({selectedProjects: [2]});
-    await wrapper.update();
-
-    expect(onChange).toHaveBeenCalled();
-    const selector = wrapper.find('MultipleEnvironmentSelector').instance();
-    expect(selector.state.selectedEnvs).toEqual(new Set([]));
+    expect(onUpdate).toHaveBeenCalledWith();
   });
 
   it('selects multiple environments and uses chevron to update', async function() {