Fix "collection items sorting" E2E flake (#44023)

* Fix "collection items sorting" E2E flake

* Add comments

* Remove the `@flaky` tag

e2e/test/scenarios/collections/collection-items-listing.cy.spec.js

@@ -87,127 +87,124 @@ describe("scenarios > collection items listing", () => {
       archiveAll();
     });
 
-    it(
-      "should allow to sort unpinned items by columns asc and desc",
-      { tags: "@flaky" },
-      () => {
-        ["A", "B", "C"].forEach((letter, i) => {
-          cy.createDashboard({
-            name: `${letter} Dashboard`,
-            collection_position: null,
-          });
-
-          // Signing in as a different users, so we have different names in "Last edited by"
-          // In that way we can test sorting by this column correctly
-          cy.signIn("normal");
-
-          cy.createQuestion({
-            name: `${letter} Question`,
-            collection_position: null,
-            query: TEST_QUESTION_QUERY,
-          });
+    it("should allow to sort unpinned items by columns asc and desc", () => {
+      ["A", "B", "C"].forEach((letter, i) => {
+        cy.createDashboard({
+          name: `${letter} Dashboard`,
+          collection_position: null,
         });
 
-        visitRootCollection();
+        // Signing in as a different users, so we have different names in "Last edited by"
+        // In that way we can test sorting by this column correctly
+        cy.signIn("normal");
 
-        getAllCollectionItemNames().then(({ actualNames, sortedNames }) => {
-          expect(actualNames, "sorted alphabetically by default").to.deep.equal(
-            sortedNames,
-          );
+        cy.createQuestion({
+          name: `${letter} Question`,
+          collection_position: null,
+          query: TEST_QUESTION_QUERY,
         });
+      });
 
-        toggleSortingFor(/Name/i);
-        cy.wait("@getCollectionItems");
+      visitRootCollection();
+      // We're waiting for the loading spinner to disappear from the main sidebar.
+      // Otherwise, this causes the page re-render and the flaky test.
+      cy.findByTestId("main-navbar-root").get("circle").should("not.exist");
 
-        getAllCollectionItemNames().then(({ actualNames, sortedNames }) => {
-          expect(actualNames, "sorted alphabetically reversed").to.deep.equal(
-            sortedNames.reverse(),
-          );
-        });
+      getAllCollectionItemNames().then(({ actualNames, sortedNames }) => {
+        expect(actualNames, "sorted alphabetically by default").to.deep.equal(
+          sortedNames,
+        );
+      });
 
-        toggleSortingFor(/Name/i);
-        // Not sure why the same XHR doesn't happen after we click the "Name" sorting again?
-        getAllCollectionItemNames().then(({ actualNames, sortedNames }) => {
-          expect(actualNames, "sorted alphabetically").to.deep.equal(
-            sortedNames,
-          );
-        });
+      toggleSortingFor(/Name/i);
+      cy.wait("@getCollectionItems");
 
-        toggleSortingFor(/Type/i);
-        cy.wait("@getCollectionItems");
-        getAllCollectionItemNames().then(({ actualNames, sortedNames }) => {
-          const dashboardsFirst = _.chain(sortedNames)
-            .sortBy(name => name.toLowerCase().includes("question"))
-            .sortBy(name => name.toLowerCase().includes("collection"))
-            .sortBy(name => name.toLowerCase().includes("metabase analytics"))
-            .value();
-          expect(actualNames, "sorted dashboards first").to.deep.equal(
-            dashboardsFirst,
-          );
-        });
+      getAllCollectionItemNames().then(({ actualNames, sortedNames }) => {
+        expect(actualNames, "sorted alphabetically reversed").to.deep.equal(
+          sortedNames.reverse(),
+        );
+      });
 
-        toggleSortingFor(/Type/i);
-        cy.wait("@getCollectionItems");
-        getAllCollectionItemNames().then(({ actualNames, sortedNames }) => {
-          const questionsFirst = _.chain(sortedNames)
-            .sortBy(name => name.toLowerCase().includes("question"))
-            .sortBy(name => name.toLowerCase().includes("dashboard"))
-            .value();
-          expect(actualNames, "sorted questions first").to.deep.equal(
-            questionsFirst,
-          );
-        });
+      toggleSortingFor(/Name/i);
+      // Not sure why the same XHR doesn't happen after we click the "Name" sorting again?
+      getAllCollectionItemNames().then(({ actualNames, sortedNames }) => {
+        expect(actualNames, "sorted alphabetically").to.deep.equal(sortedNames);
+      });
 
-        const lastEditedByColumnTestId = "collection-entry-last-edited-by";
-
-        toggleSortingFor(/Last edited by/i);
-        cy.wait("@getCollectionItems");
-
-        cy.findAllByTestId(lastEditedByColumnTestId).then(nodes => {
-          const actualNames = _.map(nodes, "innerText");
-          const sortedNames = _.chain(actualNames)
-            .sortBy(actualNames)
-            .sortBy(name => !name)
-            .value();
-          expect(
-            actualNames,
-            "sorted by last editor name alphabetically",
-          ).to.deep.equal(sortedNames);
-        });
+      toggleSortingFor(/Type/i);
+      cy.wait("@getCollectionItems");
+      getAllCollectionItemNames().then(({ actualNames, sortedNames }) => {
+        const dashboardsFirst = _.chain(sortedNames)
+          .sortBy(name => name.toLowerCase().includes("question"))
+          .sortBy(name => name.toLowerCase().includes("collection"))
+          .sortBy(name => name.toLowerCase().includes("metabase analytics"))
+          .value();
+        expect(actualNames, "sorted dashboards first").to.deep.equal(
+          dashboardsFirst,
+        );
+      });
+
+      toggleSortingFor(/Type/i);
+      cy.wait("@getCollectionItems");
+      getAllCollectionItemNames().then(({ actualNames, sortedNames }) => {
+        const questionsFirst = _.chain(sortedNames)
+          .sortBy(name => name.toLowerCase().includes("question"))
+          .sortBy(name => name.toLowerCase().includes("dashboard"))
+          .value();
+        expect(actualNames, "sorted questions first").to.deep.equal(
+          questionsFirst,
+        );
+      });
 
-        toggleSortingFor(/Last edited by/i);
-        cy.wait("@getCollectionItems");
+      const lastEditedByColumnTestId = "collection-entry-last-edited-by";
 
-        cy.findAllByTestId(lastEditedByColumnTestId).then(nodes => {
-          const actualNames = _.map(nodes, "innerText");
-          const sortedNames = _.sortBy(actualNames);
-          expect(
-            actualNames,
-            "sorted by last editor name alphabetically reversed",
-          ).to.deep.equal(sortedNames.reverse());
-        });
+      toggleSortingFor(/Last edited by/i);
+      cy.wait("@getCollectionItems");
 
-        toggleSortingFor(/Last edited at/i);
-        cy.wait("@getCollectionItems");
+      cy.findAllByTestId(lastEditedByColumnTestId).then(nodes => {
+        const actualNames = _.map(nodes, "innerText");
+        const sortedNames = _.chain(actualNames)
+          .sortBy(actualNames)
+          .sortBy(name => !name)
+          .value();
+        expect(
+          actualNames,
+          "sorted by last editor name alphabetically",
+        ).to.deep.equal(sortedNames);
+      });
+
+      toggleSortingFor(/Last edited by/i);
+      cy.wait("@getCollectionItems");
 
-        getAllCollectionItemNames().then(({ actualNames, sortedNames }) => {
-          expect(actualNames, "sorted newest last").to.deep.equal(sortedNames);
-        });
+      cy.findAllByTestId(lastEditedByColumnTestId).then(nodes => {
+        const actualNames = _.map(nodes, "innerText");
+        const sortedNames = _.sortBy(actualNames);
+        expect(
+          actualNames,
+          "sorted by last editor name alphabetically reversed",
+        ).to.deep.equal(sortedNames.reverse());
+      });
 
-        toggleSortingFor(/Last edited at/i);
-        cy.wait("@getCollectionItems");
-
-        getAllCollectionItemNames().then(({ actualNames, sortedNames }) => {
-          const newestFirst = _.chain(sortedNames)
-            .reverse()
-            .sortBy(name => name.toLowerCase().includes("collection"))
-            .sortBy(name => name.toLowerCase().includes("personal"))
-            .sortBy(name => name.toLowerCase().includes("metabase analytics"))
-            .value();
-          expect(actualNames, "sorted newest first").to.deep.equal(newestFirst);
-        });
-      },
-    );
+      toggleSortingFor(/Last edited at/i);
+      cy.wait("@getCollectionItems");
+
+      getAllCollectionItemNames().then(({ actualNames, sortedNames }) => {
+        expect(actualNames, "sorted newest last").to.deep.equal(sortedNames);
+      });
+
+      toggleSortingFor(/Last edited at/i);
+      cy.wait("@getCollectionItems");
+
+      getAllCollectionItemNames().then(({ actualNames, sortedNames }) => {
+        const newestFirst = _.chain(sortedNames)
+          .reverse()
+          .sortBy(name => name.toLowerCase().includes("collection"))
+          .sortBy(name => name.toLowerCase().includes("personal"))
+          .sortBy(name => name.toLowerCase().includes("metabase analytics"))
+          .value();
+        expect(actualNames, "sorted newest first").to.deep.equal(newestFirst);
+      });
+    });
 
     it("should reset pagination if sorting applied on not first page", () => {
       _.times(15, i => cy.createDashboard(`dashboard ${i}`));
@@ -238,6 +235,9 @@ describe("scenarios > collection items listing", () => {
 function toggleSortingFor(columnName) {
   const testId = "items-table-head";
   cy.findByTestId(testId).findByText(columnName).click();
+  // These random waits are really discouraged, but I couldn't find a reliable
+  // way to prevent the flaky sorting test without it.
+  cy.wait(50);
 }
 
 function getAllCollectionItemNames() {