fix(ui): Only save on explicit user actions (#18521)

This only saves last selected projects/environments on explicit user actions on global selection header and not when dates are changed.

src/sentry/static/sentry/app/actionCreators/globalSelection.tsx

@@ -20,6 +20,7 @@ type Options = {
    * List of parameters to remove when changing URL params
    */
   resetParams?: string[];
+  save?: boolean;
 };
 
 /**
@@ -106,7 +107,7 @@ export function updateDateTime(
   if (!router) {
     GlobalSelectionActions.updateDateTime(datetime);
   }
-  updateParams(datetime, router, options);
+  updateParams(datetime, router, {...options, save: false});
 }
 
 /**
@@ -149,6 +150,10 @@ export function updateParams(obj: UrlParams, router?: Router, options?: Options)
     return;
   }
 
+  if (options?.save) {
+    GlobalSelectionActions.save(newQuery);
+  }
+
   router.push({
     pathname: router.location.pathname,
     query: newQuery,

src/sentry/static/sentry/app/actions/globalSelectionActions.tsx

@@ -5,4 +5,5 @@ export default Reflux.createActions([
   'updateProjects',
   'updateDateTime',
   'updateEnvironments',
+  'save',
 ]);

src/sentry/static/sentry/app/components/organizations/globalSelectionHeader/globalSelectionHeader.tsx

@@ -478,6 +478,7 @@ class GlobalSelectionHeader extends React.Component<Props, State> {
     !this.props.hasCustomRouting
       ? {
           resetParams: this.props.resetParamsOnChange,
+          save: true,
         }
       : {};
 

src/sentry/static/sentry/app/stores/globalSelectionStore.jsx

@@ -70,6 +70,7 @@ const GlobalSelectionStore = Reflux.createStore({
   init() {
     this.reset(this.selection);
     this.listenTo(GlobalSelectionActions.reset, this.onReset);
+    this.listenTo(GlobalSelectionActions.save, this.onSave);
     this.listenTo(GlobalSelectionActions.updateProjects, this.updateProjects);
     this.listenTo(GlobalSelectionActions.updateDateTime, this.updateDateTime);
     this.listenTo(GlobalSelectionActions.updateEnvironments, this.updateEnvironments);
@@ -148,7 +149,6 @@ const GlobalSelectionStore = Reflux.createStore({
       ...this.selection,
       projects,
     };
-    this.updateLocalStorage();
     this.trigger(this.selection);
   },
 
@@ -161,7 +161,6 @@ const GlobalSelectionStore = Reflux.createStore({
       ...this.selection,
       datetime,
     };
-    this.updateLocalStorage();
     this.trigger(this.selection);
   },
 
@@ -174,11 +173,20 @@ const GlobalSelectionStore = Reflux.createStore({
       ...this.selection,
       environments,
     };
-    this.updateLocalStorage();
     this.trigger(this.selection);
   },
 
-  updateLocalStorage() {
+  /**
+   * Save to local storage when user explicitly changes header values.
+   *
+   * e.g. if localstorage is empty, user loads issue details for project "foo"
+   * this should not consider "foo" as last used and should not save to local storage.
+   *
+   * However, if user then changes environment, it should...? Currently it will
+   * save the current project alongside environment to local storage. It's debatable if
+   * this is the desired behavior.
+   */
+  onSave(updateObj) {
     // Do nothing if no org is loaded or user is not an org member. Only
     // organizations that a user has membership in will be available via the
     // organizations store
@@ -186,11 +194,16 @@ const GlobalSelectionStore = Reflux.createStore({
       return;
     }
 
+    const {project, environment} = updateObj;
+    const validatedProject = typeof project === 'string' ? [Number(project)] : project;
+    const validatedEnvironment =
+      typeof environment === 'string' ? [environment] : environment;
+
     try {
       const localStorageKey = `${LOCAL_STORAGE_KEY}:${this.organization.slug}`;
       const dataToSave = {
-        projects: this.selection.projects,
-        environments: this.selection.environments,
+        projects: validatedProject || this.selection.projects,
+        environments: validatedEnvironment || this.selection.environments,
       };
       localStorage.setItem(localStorageKey, JSON.stringify(dataToSave));
     } catch (ex) {

tests/acceptance/page_objects/global_selection.py

@@ -14,6 +14,9 @@ class GlobalSelectionPage(BasePage):
     def get_selected_environment(self):
         return self.browser.element('[data-test-id="global-header-environment-selector"]').text
 
+    def get_selected_date(self):
+        return self.browser.element('[data-test-id="global-header-timerange-selector"]').text
+
     def go_back_to_issues(self):
         self.browser.click('[data-test-id="back-to-issues"]')
 
@@ -45,6 +48,6 @@ class GlobalSelectionPage(BasePage):
     def select_date(self, date):
         date_path = u'//*[text()="{}"]'.format(date)
 
-        self.open_project_selector()
+        self.open_date_selector()
         self.browser.wait_until(xpath=date_path)
         self.browser.click(xpath=date_path)

tests/acceptance/test_organization_global_selection_header.py

@@ -153,9 +153,11 @@ class OrganizationGlobalHeaderTest(AcceptanceTestCase, SnubaTestCase):
                 self.issues_list.global_selection.get_selected_project_slug() == self.project_2.slug
             )
 
-            # FIXME(billy): This is a bug, should not be in local storage
             # should not be in local storage
-            # assert self.browser.get_local_storage_item(u"global-selection:{}".format(self.org.slug)) is None
+            assert (
+                self.browser.get_local_storage_item(u"global-selection:{}".format(self.org.slug))
+                is None
+            )
 
             # reloads page with no project id in URL, remains "My Projects" because
             # there has been no explicit project selection via UI
@@ -172,6 +174,12 @@ class OrganizationGlobalHeaderTest(AcceptanceTestCase, SnubaTestCase):
                 self.issues_list.global_selection.get_selected_project_slug() == self.project_3.slug
             )
 
+            self.issues_list.global_selection.select_date("Last 24 hours")
+            self.issues_list.wait_until_loaded()
+            assert u"statsPeriod=24h" in self.browser.current_url
+            # This doesn't work because we treat as dynamic data in CI
+            # assert self.issues_list.global_selection.get_selected_date() == "Last 24 hours"
+
             # reloading page with no project id in URL after previously
             # selecting an explicit project should load previously selected project
             # from local storage