Merge branch 'master' into async-load-heavy-libs-for-exports

metabase · Apr 4, 2024 · 4fe624c · 4fe624c
2 parents 988f42d + 7d16a87
commit 4fe624c
Show file tree

Hide file tree

Showing 44 changed files with 1,534 additions and 1,259 deletions.
diff --git a/docs/questions/sharing/exporting-results.md b/docs/questions/sharing/exporting-results.md
@@ -29,7 +29,7 @@ Metabase will export the raw results of a question without applying any of the [
 
 ## Exporting data via a public link
 
-You can also create a [public link](../sharing/public-links.md) that people can use to download data in a specific format.
+You can also create a [public link](../sharing/public-links.md#public-link-to-export-question-results-in-csv-xlsx-json) that people can use to download data in a specific format, as well as [raw, unformatted question results](public-links.md#exporting-raw-unformatted-question-results).
 
 ## Exporting question data via alerts
 

diff --git a/docs/questions/sharing/public-links.md b/docs/questions/sharing/public-links.md
@@ -47,6 +47,18 @@ To create a public link that people can use to download the results of a questio
 
 ![Public export](../images/public-export.png)
 
+### Exporting raw, unformatted question results
+
+To export the raw, unformatted rows, you'll need to append `?format_rows=false` to the URL Metabase generates. For example, if you create a public link for a CSV download, the URL would look like:
+
+```html
+https://www.example.com/public/question/cf347ce0-90bb-4669-b73b-56c73edd10cb.csv?format_rows=false
+```
+
+By default, Metabase will export the results of a question that include any formatting you added (for example, if you formatted a column with floats to display as a percentage (o.42 -> 42%)).
+
+See docs for the [export format endpoint](https://www.metabase.com/docs/latest/api/public#get-apipubliccarduuidqueryexport-format).
+
 ## Simulating drill-through with public links
 
 Metabase's automatic [drill-through](https://www.metabase.com/learn/questions/drill-through) won't work on public dashboards because public links don't give people access to your raw data.

diff --git a/e2e/support/helpers/e2e-ui-elements-helpers.js b/e2e/support/helpers/e2e-ui-elements-helpers.js
@@ -1,17 +1,12 @@
 // various Metabase-specific "scoping" functions like inside popover/modal/navbar/main/sidebar content area
 export const POPOVER_ELEMENT =
-  ".popover[data-state~='visible'],[data-position]:not(.emotion-HoverCard-dropdown)";
+  ".popover[data-state~='visible'],[data-element-id=mantine-popover]";
 
 export function popover() {
   cy.get(POPOVER_ELEMENT).should("be.visible");
   return cy.get(POPOVER_ELEMENT);
 }
 
-export function mantinePopover() {
-  const MANTINE_POPOVER = "[data-element-id=mantine-popover]";
-  return cy.get(MANTINE_POPOVER).should("be.visible");
-}
-
 const HOVERCARD_ELEMENT = ".emotion-HoverCard-dropdown[role='dialog']:visible";
 
 export function hovercard() {

diff --git a/e2e/test/scenarios/admin-2/whitelabel.cy.spec.js b/e2e/test/scenarios/admin-2/whitelabel.cy.spec.js
@@ -4,7 +4,6 @@ import {
   describeEE,
   entityPickerModal,
   main,
-  mantinePopover,
   modal,
   popover,
   restore,
@@ -156,7 +155,7 @@ describeEE("formatting > whitelabel", () => {
           cy.findByRole("searchbox", {
             name: "Login and unsubscribe pages",
           }).click();
-          mantinePopover().findByText("Custom").click();
+          popover().findByText("Custom").click();
           /**
            * Clicking "Choose File" doesn't actually open the file browser on Cypress,
            * so I need to use `selectFile` with the file input instead.
@@ -178,7 +177,7 @@ describeEE("formatting > whitelabel", () => {
           cy.findByRole("searchbox", {
             name: "Login and unsubscribe pages",
           }).click();
-          mantinePopover().findByText("Custom").click();
+          popover().findByText("Custom").click();
           cy.log("test uploading a corrupted file");
           cy.findByTestId("login-page-illustration-setting").within(() => {
             cy.findByTestId("file-input").selectFile(
@@ -230,7 +229,7 @@ describeEE("formatting > whitelabel", () => {
           cy.findByTestId("login-page-illustration-setting")
             .findByRole("searchbox", { name: "Login and unsubscribe pages" })
             .click();
-          mantinePopover().findByText("Custom").click();
+          popover().findByText("Custom").click();
           cy.findByTestId("login-page-illustration-setting").within(() => {
             cy.findByTestId("file-input").selectFile(
               {
@@ -270,7 +269,7 @@ describeEE("formatting > whitelabel", () => {
           cy.findByRole("searchbox", {
             name: "Login and unsubscribe pages",
           }).click();
-          mantinePopover().findByText("No illustration").click();
+          popover().findByText("No illustration").click();
 
           cy.signOut();
           cy.visit("/");
@@ -291,7 +290,7 @@ describeEE("formatting > whitelabel", () => {
           );
 
           cy.findByRole("searchbox", { name: "Landing page" }).click();
-          mantinePopover().findByText("Custom").click();
+          popover().findByText("Custom").click();
 
           cy.findByTestId("landing-page-illustration-setting").within(() => {
             cy.findByTestId("file-input").selectFile(
@@ -321,7 +320,7 @@ describeEE("formatting > whitelabel", () => {
           cy.visit("/admin/settings/whitelabel/conceal-metabase");
 
           cy.findByLabelText("Landing page").click();
-          mantinePopover().findByText("No illustration").click();
+          popover().findByText("No illustration").click();
 
           cy.visit("/");
           cy.findByTestId("landing-page-illustration").should("not.exist");
@@ -339,7 +338,7 @@ describeEE("formatting > whitelabel", () => {
           cy.findByRole("searchbox", {
             name: "When calculations return no results",
           }).click();
-          mantinePopover().findByText("Custom").click();
+          popover().findByText("Custom").click();
 
           cy.findByTestId("no-data-illustration-setting").within(() => {
             cy.findByTestId("file-input").selectFile(
@@ -398,7 +397,7 @@ describeEE("formatting > whitelabel", () => {
           cy.findByRole("searchbox", {
             name: "When calculations return no results",
           }).click();
-          mantinePopover().findByText("No illustration").click();
+          popover().findByText("No illustration").click();
 
           visitDashboard("@dashboardId");
           cy.findByAltText("No results").should("not.exist");
@@ -421,7 +420,7 @@ describeEE("formatting > whitelabel", () => {
           cy.findByRole("searchbox", {
             name: "When no objects can be found",
           }).click();
-          mantinePopover().findByText("Custom").click();
+          popover().findByText("Custom").click();
 
           cy.findByTestId("no-object-illustration-setting").within(() => {
             cy.findByTestId("file-input").selectFile(
@@ -474,7 +473,7 @@ describeEE("formatting > whitelabel", () => {
           cy.findByRole("searchbox", {
             name: "When no objects can be found",
           }).click();
-          mantinePopover().findByText("No illustration").click();
+          popover().findByText("No illustration").click();
 
           cy.findByRole("navigation").findByText("Exit admin").click();
           appBar().findByText("New").click();

diff --git a/e2e/test/scenarios/models/reproductions/23103-wrong-number-of-distinct-values.cy.spec.ts b/e2e/test/scenarios/models/reproductions/23103-wrong-number-of-distinct-values.cy.spec.ts
@@ -0,0 +1,45 @@
+import {
+  createNativeQuestion,
+  hovercard,
+  openQuestionActions,
+  popover,
+  restore,
+} from "e2e/support/helpers";
+
+describe("issue 23103", () => {
+  beforeEach(() => {
+    restore();
+    cy.signInAsAdmin();
+    cy.intercept("PUT", "/api/card/*").as("updateModel");
+  });
+
+  it("shows correct number of distinct values (metabase#23103)", () => {
+    createNativeQuestion(
+      {
+        type: "model",
+        native: {
+          query: "select * from products limit 5",
+        },
+      },
+      { visitQuestion: true },
+    );
+
+    openQuestionActions();
+    popover().findByText("Edit metadata").click();
+
+    cy.findAllByTestId("header-cell").contains("CATEGORY").click();
+    cy.findAllByTestId("select-button").contains("None").click();
+    popover().within(() => {
+      cy.findByText("Products").click();
+      cy.findByText("Category").click();
+    });
+
+    cy.button("Save changes").click();
+    cy.wait("@updateModel");
+    cy.button("Saving…").should("not.exist");
+
+    cy.findAllByTestId("header-cell").contains("Category").trigger("mouseover");
+
+    hovercard().findByText("4 distinct values").should("exist");
+  });
+});
diff --git a/e2e/test/scenarios/visualizations-tabular/drillthroughs/chart_drill.cy.spec.js b/e2e/test/scenarios/visualizations-tabular/drillthroughs/chart_drill.cy.spec.js
@@ -767,7 +767,7 @@ describe("scenarios > visualizations > drillthroughs > chart drill", () => {
       { visitQuestion: true },
     );
 
-    cy.get(".CardVisualization").get("path.cursor-pointer").first().click();
+    cy.findAllByTestId("choropleth-feature").first().click();
 
     popover().within(() => {
       cy.findByText("See these People").should("be.visible");

diff --git a/enterprise/backend/test/metabase_enterprise/upload_test.clj b/enterprise/backend/test/metabase_enterprise/upload_test.clj
@@ -10,17 +10,18 @@
 
 (use-fixtures :once (fixtures/initialize :db :test-users))
 
-(deftest uploads-disabled-for-sandboxed-user-test
+(deftest create-disabled-for-sandboxed-user-test
   (mt/test-drivers (mt/normal-drivers-with-feature :uploads)
     (met/with-gtaps-for-user! :rasta {:gtaps {:venues {}}}
       (is (thrown-with-msg? Exception #"Uploads are not permitted for sandboxed users\."
             (upload-test/upload-example-csv! {:grant-permission? false}))))))
 
-(deftest appends-disabled-for-sandboxed-user-test
+(deftest update-disabled-for-sandboxed-user-test
   (mt/test-drivers (mt/normal-drivers-with-feature :uploads)
-    (met/with-gtaps-for-user! :rasta {:gtaps {:venues {}}}
-      (is (thrown-with-msg? Exception #"Uploads are not permitted for sandboxed users\."
-            (upload-test/append-csv-with-defaults! :user-id (mt/user->id :rasta)))))))
+    (doseq [verb [:metabase.upload/append :metabase.upload/replace]]
+      (met/with-gtaps-for-user! :rasta {:gtaps {:venues {}}}
+        (is (thrown-with-msg? Exception #"Uploads are not permitted for sandboxed users\."
+              (upload-test/update-csv-with-defaults! verb :user-id (mt/user->id :rasta))))))))
 
 (deftest based-on-upload-for-sandboxed-user-test
   (mt/with-temporary-setting-values [uploads-enabled true]

diff --git a/frontend/src/metabase/admin/datamodel/components/DimensionList/DimensionList.jsx b/frontend/src/metabase/admin/datamodel/components/DimensionList/DimensionList.jsx
@@ -147,8 +147,8 @@ export class DimensionList extends Component {
             <Icon
               name="add"
               className={cx(
-                "mx1",
-                "cursor-pointer",
+                CS.mx1,
+                CS.cursorPointer,
                 CS.hoverChild,
                 "faded",
                 "fade-in-hover",
@@ -163,7 +163,7 @@ export class DimensionList extends Component {
         {isSelected && onRemoveDimension && (
           <Icon
             name="close"
-            className="mx1 cursor-pointer faded fade-in-hover"
+            className={cx(CS.mx1, CS.cursorPointer, "faded", "fade-in-hover")}
             onClick={e => {
               e.stopPropagation();
               this.handleRemove(item);

diff --git a/frontend/src/metabase/admin/datamodel/components/ObjectActionSelect.jsx b/frontend/src/metabase/admin/datamodel/components/ObjectActionSelect.jsx
@@ -72,7 +72,7 @@ export default class ObjectActionsSelect extends Component {
                   CS.block,
                   CS.p2,
                   "bg-error-hover",
-                  "text-error",
+                  CS.textError,
                   "text-white-hover",
                   CS.cursorPointer,
                 )}

diff --git a/.../src/metabase/admin/datamodel/components/filters/pickers/DatePicker/HoursMinutesInput.tsx b/.../src/metabase/admin/datamodel/components/filters/pickers/DatePicker/HoursMinutesInput.tsx
@@ -76,7 +76,12 @@ const HoursMinutesInput = ({
     )}
     {onClear && (
       <Icon
-        className="text-light cursor-pointer text-medium-hover ml-auto"
+        className={cx(
+          CS.textLight,
+          CS.cursorPointer,
+          "text-medium-hover",
+          CS.mlAuto,
+        )}
         name="close"
         onClick={onClear}
       />

diff --git a/...etabase/admin/datamodel/components/filters/pickers/LegacyDatePicker/HoursMinutesInput.jsx b/...etabase/admin/datamodel/components/filters/pickers/LegacyDatePicker/HoursMinutesInput.jsx
@@ -64,7 +64,12 @@ const HoursMinutesInput = ({
     )}
     {onClear && (
       <Icon
-        className="text-light cursor-pointer text-medium-hover ml-auto"
+        className={cx(
+          CS.textLight,
+          CS.cursorPointer,
+          "text-medium-hover",
+          CS.mlAuto,
+        )}
         name="close"
         onClick={onClear}
       />

diff --git a/frontend/src/metabase/admin/people/components/AddMemberRow.jsx b/frontend/src/metabase/admin/people/components/AddMemberRow.jsx
@@ -77,7 +77,12 @@ export default function AddMemberRow({ users, excludeIds, onCancel, onDone }) {
             >
               {user.common_name}
               <Icon
-                className="pl1 cursor-pointer text-slate text-medium-hover"
+                className={cx(
+                  CS.pl1,
+                  CS.cursorPointer,
+                  "text-slate",
+                  "text-medium-hover",
+                )}
                 name="close"
                 onClick={() => handleRemoveUser(user)}
               />

diff --git a/frontend/src/metabase/admin/people/components/GroupMembersTable/GroupMembersTable.tsx b/frontend/src/metabase/admin/people/components/GroupMembersTable/GroupMembersTable.tsx
@@ -202,7 +202,7 @@ const UserRow = ({
       <td>{user.email}</td>
       {canRemove ? (
         <td
-          className={cx(CS.textRight, "cursor-pointer")}
+          className={cx(CS.textRight, CS.cursorPointer)}
           onClick={() => onMembershipRemove(groupMembership?.membership_id)}
         >
           <Icon name="close" className="text-light" size={16} />

diff --git a/frontend/src/metabase/admin/settings/components/ApiKeys/ManageApiKeys.tsx b/frontend/src/metabase/admin/settings/components/ApiKeys/ManageApiKeys.tsx
@@ -80,15 +80,15 @@ function ApiKeysTable({
                 <Group spacing="md">
                   <Icon
                     name="pencil"
-                    className="cursor-pointer"
+                    className={CS.cursorPointer}
                     onClick={() => {
                       setActiveApiKey(apiKey);
                       setModal("edit");
                     }}
                   />
                   <Icon
                     name="trash"
-                    className="cursor-pointer"
+                    className={CS.cursorPointer}
                     onClick={() => {
                       setActiveApiKey(apiKey);
                       setModal("delete");

diff --git a/frontend/src/metabase/admin/settings/components/widgets/CustomGeoJSONWidget.jsx b/frontend/src/metabase/admin/settings/components/widgets/CustomGeoJSONWidget.jsx
@@ -226,10 +226,10 @@ const ListMaps = ({ maps, onEditMap, onDeleteMap }) => (
           .filter(map => !map.builtin)
           .map(map => (
             <tr key={map.id}>
-              <td className="cursor-pointer" onClick={() => onEditMap(map)}>
+              <td className={CS.cursorPointer} onClick={() => onEditMap(map)}>
                 {map.name}
               </td>
-              <td className="cursor-pointer" onClick={() => onEditMap(map)}>
+              <td className={CS.cursorPointer} onClick={() => onEditMap(map)}>
                 <Ellipsified style={{ maxWidth: 600 }}>{map.url}</Ellipsified>
               </td>
               <td className={AdminS.TableActions}>

diff --git a/frontend/src/metabase/components/MetadataInfo/Popover/Popover.tsx b/frontend/src/metabase/components/MetadataInfo/Popover/Popover.tsx
@@ -1,5 +1,5 @@
 import type { ReactNode } from "react";
-import { useCallback, useRef, useState } from "react";
+import { useCallback, useState } from "react";
 
 import useSequencedContentCloseHandler from "metabase/hooks/use-sequenced-content-close-handler";
 import type { HoverCardProps } from "metabase/ui";
@@ -36,12 +36,10 @@ export function Popover({
   const { setupCloseHandler, removeCloseHandler } =
     useSequencedContentCloseHandler();
 
-  const ref = useRef(null);
   const handleOpen = useCallback(() => {
-    setupCloseHandler(ref.current, () => setIsOpen(false));
     group.onOpen();
     setIsOpen(true);
-  }, [setupCloseHandler, group]);
+  }, [group]);
 
   const handleClose = useCallback(() => {
     removeCloseHandler();
@@ -60,14 +58,19 @@ export function Popover({
       transitionProps={{
         duration: group.shouldDelay ? POPOVER_TRANSITION_DURATION : 0,
       }}
-      keepMounted
     >
       <HoverCard.Target>{children}</HoverCard.Target>
       <Dropdown>
         {/* HACK: adds an element between the target and the card */}
         {/* to avoid the card from disappearing */}
         <Target />
-        <WidthBound ref={ref}>{isOpen && content}</WidthBound>
+        <WidthBound
+          ref={node => {
+            setupCloseHandler(node, () => setIsOpen(false));
+          }}
+        >
+          {isOpen && content}
+        </WidthBound>
       </Dropdown>
     </HoverCard>
   );

diff --git a/frontend/src/metabase/components/Triggerable/Triggerable.jsx b/frontend/src/metabase/components/Triggerable/Triggerable.jsx
@@ -99,6 +99,7 @@ const Triggerable = ComposedComponent =>
         }, 250);
       }
     }
+
     _stopCheckObscured() {
       if (this._offscreenTimer != null) {
         clearInterval(this._offscreenTimer);
@@ -156,7 +157,7 @@ const Triggerable = ComposedComponent =>
               !isOpen && triggerClassesClose,
               CS.noDecoration,
               {
-                "cursor-default": this.props.disabled,
+                [CS.cursorDefault]: this.props.disabled,
               },
             )}
             aria-disabled={this.props.disabled}