Explore: make sure panes are initialized in order (#70519)

public/app/features/explore/hooks/useStateSync/index.test.tsx

@@ -6,8 +6,8 @@ import React, { ReactNode } from 'react';
 import { TestProvider } from 'test/helpers/TestProvider';
 import { getGrafanaContextMock } from 'test/mocks/getGrafanaContextMock';
 
-import { UrlQueryMap } from '@grafana/data';
-import { HistoryWrapper, setDataSourceSrv } from '@grafana/runtime';
+import { DataSourceApi, UrlQueryMap } from '@grafana/data';
+import { HistoryWrapper, setDataSourceSrv, DataSourceSrv } from '@grafana/runtime';
 import { setLastUsedDatasourceUID } from 'app/core/utils/explore';
 import { MIXED_DATASOURCE_NAME } from 'app/plugins/datasource/mixed/MixedDataSource';
 import { configureStore } from 'app/store/configureStore';
@@ -17,11 +17,33 @@ import { splitClose, splitOpen } from '../../state/main';
 
 import { useStateSync } from './';
 
+function defaultDsGetter(datasources: Array<ReturnType<typeof makeDatasourceSetup>>): DataSourceSrv['get'] {
+  return (datasource) => {
+    let ds;
+    if (!datasource) {
+      ds = datasources[0]?.api;
+    } else {
+      ds = datasources.find((ds) =>
+        typeof datasource === 'string'
+          ? ds.api.name === datasource || ds.api.uid === datasource
+          : ds.api.uid === datasource?.uid
+      )?.api;
+    }
+
+    if (ds) {
+      return Promise.resolve(ds);
+    }
+
+    return Promise.reject();
+  };
+}
+
 interface SetupParams {
   queryParams?: UrlQueryMap;
   exploreMixedDatasource?: boolean;
+  datasourceGetter?: (datasources: Array<ReturnType<typeof makeDatasourceSetup>>) => DataSourceSrv['get'];
 }
-function setup({ queryParams = {}, exploreMixedDatasource = false }: SetupParams) {
+function setup({ queryParams = {}, exploreMixedDatasource = false, datasourceGetter = defaultDsGetter }: SetupParams) {
   const history = createMemoryHistory({
     initialEntries: [{ pathname: '/explore', search: stringify(queryParams) }],
   });
@@ -38,24 +60,7 @@ function setup({ queryParams = {}, exploreMixedDatasource = false }: SetupParams
   }
 
   setDataSourceSrv({
-    get(datasource) {
-      let ds;
-      if (!datasource) {
-        ds = datasources[0]?.api;
-      } else {
-        ds = datasources.find((ds) =>
-          typeof datasource === 'string'
-            ? ds.api.name === datasource || ds.api.uid === datasource
-            : ds.api.uid === datasource?.uid
-        )?.api;
-      }
-
-      if (ds) {
-        return Promise.resolve(ds);
-      }
-
-      return Promise.reject();
-    },
+    get: datasourceGetter(datasources),
     getInstanceSettings: jest.fn(),
     getList: jest.fn(),
     reload: jest.fn(),
@@ -125,7 +130,7 @@ describe('useStateSync', () => {
     expect(search.panes).toBeDefined();
   });
 
-  it('inits an explore pane for each key in the panes search object', async () => {
+  it('correctly inits an explore pane for each key in the panes search object', async () => {
     const { location, waitForNextUpdate, store } = setup({
       queryParams: {
         panes: JSON.stringify({
@@ -134,6 +139,32 @@ describe('useStateSync', () => {
         }),
         schemaVersion: 1,
       },
+      datasourceGetter: (datasources: Array<ReturnType<typeof makeDatasourceSetup>>): DataSourceSrv['get'] => {
+        return (datasource) => {
+          let ds: DataSourceApi | undefined;
+          if (!datasource) {
+            ds = datasources[0]?.api;
+          } else {
+            ds = datasources.find((ds) =>
+              typeof datasource === 'string'
+                ? ds.api.name === datasource || ds.api.uid === datasource
+                : ds.api.uid === datasource?.uid
+            )?.api;
+          }
+
+          return new Promise((resolve, reject) => {
+            if (ds) {
+              if (typeof datasource === 'string' && datasource === 'loki-uid') {
+                setTimeout(() => Promise.resolve(ds));
+              } else {
+                resolve(ds);
+              }
+            }
+
+            reject();
+          });
+        };
+      },
     });
 
     const initialHistoryLength = location.getHistory().length;
@@ -142,15 +173,14 @@ describe('useStateSync', () => {
 
     expect(location.getHistory().length).toBe(initialHistoryLength);
 
-    const search = location.getSearchObject();
-    expect(search.panes).toBeDefined();
-    expect(Object.keys(store.getState().explore.panes)).toHaveLength(2);
-
-    // check if the URL is properly encoded when finishing rendering the hook. (this would '1 2' otherwise)
     const panes = location.getSearch().get('panes');
     expect(panes).not.toBeNull();
-    if (panes !== null) {
+    if (panes) {
+      // check if the URL is properly encoded when finishing rendering the hook. (this would be '1 2' otherwise)
       expect(JSON.parse(panes)['one'].queries[0].refId).toBe('1+2');
+
+      // we expect panes in the state to be in the same order as the ones in the URL
+      expect(Object.keys(store.getState().explore.panes)).toStrictEqual(Object.keys(JSON.parse(panes)));
     }
   });
 

public/app/features/explore/hooks/useStateSync/index.ts

@@ -170,58 +170,56 @@ export function useStateSync(params: ExploreQueryParams) {
 
       Promise.all(
         Object.entries(urlState.panes).map(([exploreId, { datasource, queries, range, panelsState }]) => {
-          return getPaneDatasource(datasource, queries, orgId, !!exploreMixedDatasource).then(
-            async (paneDatasource) => {
-              return Promise.resolve(
-                // FIXME: In theory, given the Grafana datasource will always be present, this should always be defined.
-                paneDatasource
-                  ? queries.length
-                    ? // if we have queries in the URL, we use them
-                      withUniqueRefIds(queries)
-                        // but filter out the ones that are not compatible with the pane datasource
-                        .filter(getQueryFilter(paneDatasource))
-                        .map(
-                          isMixedDatasource(paneDatasource)
-                            ? identity<DataQuery>
-                            : (query) => ({ ...query, datasource: paneDatasource.getRef() })
-                        )
-                    : getDatasourceSrv()
-                        // otherwise we get a default query from the pane datasource or from the default datasource if the pane datasource is mixed
-                        .get(isMixedDatasource(paneDatasource) ? undefined : paneDatasource.getRef())
-                        .then((ds) => [getDefaultQuery(ds)])
-                  : []
-              )
-                .then(async (queries) => {
-                  // we remove queries that have an invalid datasources
-                  const validQueries = await removeQueriesWithInvalidDatasource(queries);
+          return getPaneDatasource(datasource, queries, orgId, !!exploreMixedDatasource).then((paneDatasource) => {
+            return Promise.resolve(
+              // Given the Grafana datasource will always be present, this should always be defined.
+              paneDatasource
+                ? queries.length
+                  ? // if we have queries in the URL, we use them
+                    withUniqueRefIds(queries)
+                      // but filter out the ones that are not compatible with the pane datasource
+                      .filter(getQueryFilter(paneDatasource))
+                      .map(
+                        isMixedDatasource(paneDatasource)
+                          ? identity<DataQuery>
+                          : (query) => ({ ...query, datasource: paneDatasource.getRef() })
+                      )
+                  : getDatasourceSrv()
+                      // otherwise we get a default query from the pane datasource or from the default datasource if the pane datasource is mixed
+                      .get(isMixedDatasource(paneDatasource) ? undefined : paneDatasource.getRef())
+                      .then((ds) => [getDefaultQuery(ds)])
+                : []
+            ).then(async (queries) => {
+              // we remove queries that have an invalid datasources
+              let validQueries = await removeQueriesWithInvalidDatasource(queries);
 
-                  if (!validQueries.length && paneDatasource) {
-                    // and in case there's no query left we add a default one.
-                    return [
-                      getDefaultQuery(
-                        isMixedDatasource(paneDatasource) ? await getDatasourceSrv().get() : paneDatasource
-                      ),
-                    ];
-                  }
+              if (!validQueries.length && paneDatasource) {
+                // and in case there's no query left we add a default one.
+                validQueries = [
+                  getDefaultQuery(isMixedDatasource(paneDatasource) ? await getDatasourceSrv().get() : paneDatasource),
+                ];
+              }
 
-                  return validQueries;
-                })
-                .then((queries) => {
-                  return dispatch(
-                    initializeExplore({
-                      exploreId,
-                      datasource: paneDatasource,
-                      queries,
-                      range,
-                      panelsState,
-                    })
-                  ).unwrap();
-                });
-            }
-          );
+              return { exploreId, range, panelsState, queries: validQueries, datasource: paneDatasource };
+            });
+          });
         })
-      ).then((panes) => {
-        const newParams = panes.reduce(
+      ).then(async (panes) => {
+        const initializedPanes = await Promise.all(
+          panes.map(({ exploreId, range, panelsState, queries, datasource }) => {
+            return dispatch(
+              initializeExplore({
+                exploreId,
+                datasource,
+                queries,
+                range,
+                panelsState,
+              })
+            ).unwrap();
+          })
+        );
+
+        const newParams = initializedPanes.reduce(
           (acc, { exploreId, state }) => {
             return {
               ...acc,
@@ -235,13 +233,11 @@ export function useStateSync(params: ExploreQueryParams) {
             panes: {},
           }
         );
-
         initState.current = 'done';
-
         // we need to use partial here beacuse replace doesn't encode the query params.
         location.partial(
           {
-            // partial doesn't remove other parameters, so we delete all the current one before adding the new ones.
+            // partial doesn't remove other parameters, so we delete (by setting them to undefined) all the current one before adding the new ones.
             ...Object.keys(location.getSearchObject()).reduce<Record<string, unknown>>((acc, key) => {
               acc[key] = undefined;
               return acc;