Browse Source

fix(starfish): Revamp the Starfish project selector (#53628)

Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
Co-authored-by: Abhijeet Prasad <aprasad@sentry.io>
Ash Anand 1 year ago
parent
commit
291defc170

+ 39 - 0
static/app/actionCreators/pageFilters.spec.tsx

@@ -359,6 +359,45 @@ describe('PageFilters ActionCreators', function () {
 
       pageFilterStorageMock.mockRestore();
     });
+
+    it('retrieves filters from a separate key when storageNamespace is provided', function () {
+      const starfishKey = `global-selection:starfish:${organization.slug}`;
+      localStorage.setItem(
+        starfishKey,
+        JSON.stringify({
+          environments: [],
+          projects: [1],
+          pinnedFilters: ['projects', 'environments'],
+        })
+      );
+
+      initializeUrlState({
+        organization,
+        queryParams: {},
+        router,
+        memberProjects: [],
+        shouldEnforceSingleProject: false,
+        storageNamespace: 'starfish',
+      });
+
+      expect(localStorage.getItem).toHaveBeenCalledWith(starfishKey);
+      expect(PageFiltersStore.onInitializeUrlState).toHaveBeenCalledWith(
+        expect.objectContaining({
+          environments: [],
+          projects: [1],
+        }),
+        new Set(['projects', 'environments']),
+        true
+      );
+      expect(router.replace).toHaveBeenCalledWith(
+        expect.objectContaining({
+          query: {
+            environment: [],
+            project: ['1'],
+          },
+        })
+      );
+    });
   });
 
   describe('updateProjects()', function () {

+ 18 - 2
static/app/actionCreators/pageFilters.tsx

@@ -50,6 +50,10 @@ type Options = {
    * Persist changes to the page filter selection into local storage
    */
   save?: boolean;
+  /**
+   * Optional prefix for the storage key, for areas of the app that need separate pagefilters (i.e Starfish)
+   */
+  storageNamespace?: string;
 };
 
 /**
@@ -145,6 +149,10 @@ export type InitializeUrlStateParams = {
    * An example is Starfish, which doesn't support environments.
    */
   skipLoadLastUsedEnvironment?: boolean;
+  /**
+   * Optional prefix for the storage key, for areas of the app that need separate pagefilters (i.e Starfish)
+   */
+  storageNamespace?: string;
 };
 
 export function initializeUrlState({
@@ -161,6 +169,7 @@ export function initializeUrlState({
   forceProject,
   showAbsolute = true,
   skipInitializeUrlParams = false,
+  storageNamespace,
 }: InitializeUrlStateParams) {
   const orgSlug = organization.slug;
 
@@ -195,7 +204,9 @@ export function initializeUrlState({
     pageFilters.environments = parsed.environment || [];
   }
 
-  const storedPageFilters = skipLoadLastUsed ? null : getPageFilterStorage(orgSlug);
+  const storedPageFilters = skipLoadLastUsed
+    ? null
+    : getPageFilterStorage(orgSlug, storageNamespace);
   let shouldUsePinnedDatetime = false;
 
   // We may want to restore some page filters from local storage. In the new
@@ -324,6 +335,7 @@ export function updateProjects(
   PageFiltersStore.updateProjects(projects, options?.environments ?? null);
   updateParams({project: projects, environment: options?.environments}, router, options);
   persistPageFilters('projects', options);
+
   if (options?.environments) {
     persistPageFilters('environments', options);
   }
@@ -430,7 +442,11 @@ async function persistPageFilters(filter: PinnedPageFilter | null, options?: Opt
   }
 
   const targetFilter = filter !== null ? [filter] : [];
-  setPageFiltersStorage(orgSlug, new Set<PinnedPageFilter>(targetFilter));
+  setPageFiltersStorage(
+    orgSlug,
+    new Set<PinnedPageFilter>(targetFilter),
+    options.storageNamespace
+  );
 }
 
 /**

+ 8 - 1
static/app/components/organizations/pageFilters/container.tsx

@@ -49,6 +49,10 @@ interface Props extends InitializeUrlStateProps {
    * Slugs of projects to display in project selector
    */
   specificProjectSlugs?: string[];
+  /**
+   * If provided, will store page filters separately from the rest of Sentry
+   */
+  storageNamespace?: string;
 }
 
 /**
@@ -72,6 +76,7 @@ function Container({
     disablePersistence,
     desyncedAlertMessage,
     hideDesyncRevertButton,
+    storageNamespace,
   } = props;
   const router = useRouter();
   const location = useLocation();
@@ -91,7 +96,7 @@ function Container({
     ? specifiedProjects
     : specifiedProjects.filter(project => project.isMember);
 
-  const doInitialization = () =>
+  const doInitialization = () => {
     initializeUrlState({
       organization,
       queryParams: location.query,
@@ -106,7 +111,9 @@ function Container({
       shouldPersist: !disablePersistence,
       showAbsolute,
       skipInitializeUrlParams,
+      storageNamespace,
     });
+  };
 
   // Initializes GlobalSelectionHeader
   //

+ 10 - 4
static/app/components/organizations/pageFilters/persistence.tsx

@@ -34,7 +34,8 @@ type StoredObject = {
  */
 export function setPageFiltersStorage(
   orgSlug: string,
-  updateFilters: Set<PinnedPageFilter>
+  updateFilters: Set<PinnedPageFilter>,
+  storageNamespace: string = ''
 ) {
   const {selection, pinnedFilters} = PageFiltersStore.getState();
 
@@ -84,7 +85,9 @@ export function setPageFiltersStorage(
     pinnedFilters: Array.from(pinnedFilters),
   };
 
-  const localStorageKey = makeLocalStorageKey(orgSlug);
+  const localStorageKey = makeLocalStorageKey(
+    storageNamespace.length > 0 ? `${storageNamespace}:${orgSlug}` : orgSlug
+  );
 
   try {
     localStorage.setItem(localStorageKey, JSON.stringify(dataToSave));
@@ -96,8 +99,11 @@ export function setPageFiltersStorage(
 /**
  * Retrieves the page filters from local storage
  */
-export function getPageFilterStorage(orgSlug: string) {
-  const localStorageKey = makeLocalStorageKey(orgSlug);
+export function getPageFilterStorage(orgSlug: string, storageNamespace: string = '') {
+  const localStorageKey = makeLocalStorageKey(
+    storageNamespace.length > 0 ? `${storageNamespace}:${orgSlug}` : orgSlug
+  );
+
   const value = localStorage.getItem(localStorageKey);
 
   if (!value) {

+ 3 - 1
static/app/views/starfish/components/starfishPageFiltersContainer.tsx

@@ -6,6 +6,8 @@ type Props = {
 
 export function StarfishPageFiltersContainer({children}: Props) {
   return (
-    <PageFiltersContainer skipLoadLastUsedEnvironment>{children}</PageFiltersContainer>
+    <PageFiltersContainer storageNamespace="starfish" skipLoadLastUsedEnvironment>
+      {children}
+    </PageFiltersContainer>
   );
 }

+ 25 - 20
static/app/views/starfish/components/starfishProjectSelector.tsx

@@ -1,3 +1,5 @@
+import {useMemo, useState} from 'react';
+
 import {updateProjects} from 'sentry/actionCreators/pageFilters';
 import {CompactSelect} from 'sentry/components/compactSelect';
 import ProjectBadge from 'sentry/components/idBadge/projectBadge';
@@ -12,10 +14,24 @@ import {ALLOWED_PROJECT_IDS_FOR_ORG_SLUG} from 'sentry/views/starfish/allowedPro
 export function StarfishProjectSelector() {
   const {projects, initiallyLoaded: projectsLoaded, fetchError} = useProjects();
   const organization = useOrganization();
-  const {selection, isReady} = usePageFilters();
   const router = useRouter();
+  const {selection} = usePageFilters();
+
+  const allowedProjectIDs: string[] = useMemo(
+    () => ALLOWED_PROJECT_IDS_FOR_ORG_SLUG[organization.slug] ?? [],
+    [organization.slug]
+  );
 
-  if (!projectsLoaded || !isReady) {
+  const [selectedProjectId, setSelectedProjectId] = useState(
+    selection.projects[0] ?? allowedProjectIDs[0]
+  );
+
+  const currentProject = selection.projects[0] ?? allowedProjectIDs[0];
+  if (selectedProjectId !== currentProject) {
+    setSelectedProjectId(currentProject);
+  }
+
+  if (!projectsLoaded) {
     return (
       <CompactSelect
         disabled
@@ -29,36 +45,25 @@ export function StarfishProjectSelector() {
     throw new Error('Failed to fetch projects');
   }
 
-  const allowedProjectIDs: string[] =
-    ALLOWED_PROJECT_IDS_FOR_ORG_SLUG[organization.slug] ?? [];
-
   const projectOptions = projects
     .filter(project => allowedProjectIDs.includes(project.id))
     .map(project => ({
       label: <ProjectOptionLabel project={project} />,
       value: project.id,
-    }));
-
-  const selectedOption =
-    projectOptions.find(option =>
-      selection.projects.includes(parseInt(option.value, 10))
-    ) ?? projectOptions[0];
+    }))
+    .sort((projectA, projectB) => Number(projectA.value) - Number(projectB.value));
 
   const handleProjectChange = option =>
-    updateProjects([parseInt(option.value, 10)], router, {save: true});
-
-  if (
-    selection.projects.length > 1 ||
-    !allowedProjectIDs.includes(`${selection.projects[0]}`)
-  ) {
-    handleProjectChange(projectOptions[0]);
-  }
+    updateProjects([parseInt(option.value, 10)], router, {
+      storageNamespace: 'starfish',
+      save: true,
+    });
 
   return (
     <CompactSelect
       menuWidth={250}
       options={projectOptions}
-      defaultValue={selectedOption?.value}
+      value={String(selectedProjectId)}
       onChange={handleProjectChange}
     />
   );