Skip to content
Snippets Groups Projects
Unverified Commit 3a3785d2 authored by Romeo Van Snick's avatar Romeo Van Snick Committed by GitHub
Browse files

Disable value column on Browse metrics page (#48217) 

* Remove value column from Browse metrics page

* Remove tests for metric values

* Fix markdown test
parent 670ec100
Branches
Tags
No related merge requests found
Hide whitespace changes
Inline Side-by-side
e2e/test/scenarios/metrics/browse.cy.spec.ts
+ 1
− 188
View file @ 3a3785d2
import { SAMPLE_DB_ID } from "e2e/support/cypress_data";
import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database";
import {
ALL_USERS_GROUP_ID,
FIRST_COLLECTION_ID,
ORDERS_MODEL_ID,
} from "e2e/support/cypress_sample_instance_data";
import {
type StructuredQuestionDetails,
assertIsEllipsified,
createNativeQuestion,
createQuestion,
describeEE,
getSidebarSectionTitle,
......@@ -18,9 +15,7 @@ import {
popover,
restore,
setTokenFeatures,
tooltip,
} from "e2e/support/helpers";
import { DataPermissionValue } from "metabase/admin/permissions/types";
const { ORDERS_ID, ORDERS, PRODUCTS_ID, PRODUCTS } = SAMPLE_DATABASE;
......@@ -91,53 +86,6 @@ const NON_NUMERIC_METRIC: StructuredQuestionDetailsWithName = {
display: "scalar",
};
const TEMPORAL_METRIC_WITH_SORT: StructuredQuestionDetailsWithName = {
name: "Count of orders over time",
type: "metric",
description: "A metric",
query: {
"source-table": ORDERS_ID,
aggregation: [["count"]],
breakout: [
[
"field",
ORDERS.CREATED_AT,
{ "base-type": "type/DateTime", "temporal-unit": "month" },
],
],
"order-by": [["asc", ["aggregation", 0]]],
},
};
const SCALAR_METRIC_WITH_NO_VALUE: StructuredQuestionDetailsWithName = {
name: "Scalar metric with no value",
type: "metric",
description: "A metric",
query: {
"source-table": ORDERS_ID,
aggregation: [["max", ["field", ORDERS.TOTAL, {}]]],
filter: ["=", ["field", ORDERS.TOTAL, {}], 3.333],
},
};
const TIMESERIES_METRIC_WITH_NO_VALUE: StructuredQuestionDetailsWithName = {
name: "Timeseries metric with no value",
type: "metric",
description: "A metric",
query: {
"source-table": ORDERS_ID,
aggregation: [["max", ["field", ORDERS.TOTAL, {}]]],
filter: ["=", ["field", ORDERS.TOTAL, {}], 3.333],
breakout: [
[
"field",
ORDERS.CREATED_AT,
{ "base-type": "type/DateTime", "temporal-unit": "month" },
],
],
},
};
const ALL_METRICS = [
ORDERS_SCALAR_METRIC,
ORDERS_SCALAR_MODEL_METRIC,
......@@ -227,7 +175,7 @@ describe("scenarios > browse > metrics", () => {
it("should render truncated markdown in the table", () => {
const description =
"This is a _very_ **long description** that should be truncated";
"This is a _very_ **long description** that should be truncated by the metrics table because it is really very long.";
createMetrics([
{
......@@ -393,47 +341,6 @@ describe("scenarios > browse > metrics", () => {
});
});
describe("scalar metric value", () => {
it("should render a scalar metric's value in the table", () => {
restore();
cy.signInAsAdmin();
createMetrics([ORDERS_SCALAR_METRIC]);
cy.visit("/browse/metrics");
checkMetricValueAndTooltipExist("18,760", "Overall");
});
it("should render a scalar metric's value in the table even when it's not a number", () => {
restore();
cy.signInAsAdmin();
createMetrics([NON_NUMERIC_METRIC]);
cy.visit("/browse/metrics");
checkMetricValueAndTooltipExist("Widget", "Overall");
});
});
describeEE("scalar metric value", () => {
it("should not render a scalar metric's value when the user does not have permissions to see it", () => {
cy.signInAsAdmin();
createMetrics([ORDERS_SCALAR_METRIC]);
setTokenFeatures("all");
cy.updatePermissionsGraph({
[ALL_USERS_GROUP_ID]: {
[SAMPLE_DB_ID]: {
"view-data": DataPermissionValue.BLOCKED,
},
},
});
cy.signInAsNormalUser();
cy.visit("/browse/metrics");
metricsTable().findByText("18,760").should("not.exist");
});
});
describe("verified metrics", () => {
describeEE("on enterprise", () => {
beforeEach(() => {
......@@ -520,94 +427,6 @@ describe("scenarios > browse > metrics", () => {
});
});
});
describe("temporal metric value", () => {
it("should show the last value of a temporal metric", () => {
cy.signInAsAdmin();
createMetrics([ORDERS_TIMESERIES_METRIC]);
cy.visit("/browse/metrics");
checkMetricValueAndTooltipExist("344", "April 2026");
});
it("should show the last value of a temporal metric with a sort clause", () => {
cy.signInAsAdmin();
createMetrics([TEMPORAL_METRIC_WITH_SORT]);
cy.visit("/browse/metrics");
checkMetricValueAndTooltipExist("584", "January 2025");
});
it("should render an empty value for a scalar metric with no value", () => {
cy.signInAsAdmin();
createMetrics([SCALAR_METRIC_WITH_NO_VALUE]);
cy.visit("/browse/metrics");
findMetric(SCALAR_METRIC_WITH_NO_VALUE.name).should("be.visible");
cy.findByTestId("metric-value").should("be.empty");
});
it("should render an empty value for a timeseries metric with no value", () => {
cy.signInAsAdmin();
createMetrics([TIMESERIES_METRIC_WITH_NO_VALUE]);
cy.visit("/browse/metrics");
findMetric(TIMESERIES_METRIC_WITH_NO_VALUE.name).should("be.visible");
cy.findByTestId("metric-value").should("be.empty");
});
it("should render an empty value for metric with errors", () => {
cy.signInAsAdmin();
createNativeQuestion(
{
name: "Question with error",
native: {
query: "SELECT __syntax_error__;",
},
},
{ wrapId: true },
).then(id => {
createMetrics([
{
name: "Metric with error",
type: "metric",
description: "A metric",
query: {
"source-table": `card__${id}`,
aggregation: [["count"]],
},
},
]);
});
cy.visit("/browse/metrics");
findMetric("Metric with error").should("be.visible");
cy.findByTestId("metric-value").should("be.empty");
});
});
describeEE("temporal metric value", () => {
it("should not render a temporal metric's value when the user does not have permissions to see it", () => {
cy.signInAsAdmin();
createMetrics([ORDERS_TIMESERIES_METRIC]);
setTokenFeatures("all");
cy.updatePermissionsGraph({
[ALL_USERS_GROUP_ID]: {
[SAMPLE_DB_ID]: {
"view-data": DataPermissionValue.BLOCKED,
},
},
});
cy.signInAsNormalUser();
cy.visit("/browse/metrics");
metricsTable().findByText("344").should("not.exist");
});
});
});
function createMetrics(
......@@ -638,12 +457,6 @@ function shouldNotHaveBookmark(name: string) {
navigationSidebar().findByText(name).should("not.exist");
}
function checkMetricValueAndTooltipExist(value: string, label: string) {
metricsTable().findByText(value).should("be.visible");
metricsTable().findByText(value).realHover();
tooltip().should("contain", label);
}
function verifyMetric(metric: StructuredQuestionDetailsWithName) {
metricsTable().findByText(metric.name).should("be.visible").click();
......
frontend/src/metabase/browse/metrics/MetricsTable.tsx
+ 2
− 68
View file @ 3a3785d2
......@@ -3,10 +3,8 @@ import { push } from "react-router-redux";
import { c, t } from "ttag";
import {
skipToken,
useCreateBookmarkMutation,
useDeleteBookmarkMutation,
useGetCardQueryQuery,
} from "metabase/api";
import { getCollectionName } from "metabase/collections/utils";
import { EllipsifiedCollectionPath } from "metabase/common/components/EllipsifiedPath/EllipsifiedCollectionPath";
......@@ -36,8 +34,6 @@ import {
type IconName,
Menu,
Skeleton,
Text,
Tooltip,
} from "metabase/ui";
import { Repeat } from "metabase/ui/components/feedback/Skeleton/Repeat";
import { SortDirection, type SortingOptions } from "metabase-types/api/sorting";
......@@ -48,17 +44,10 @@ import {
CollectionTableCell,
NameColumn,
TableRow,
Value,
ValueTableCell,
ValueWrapper,
} from "../components/BrowseTable.styled";
import type { MetricResult } from "./types";
import {
getDatasetValueForMetric,
getMetricDescription,
sortMetrics,
} from "./utils";
import { getMetricDescription, sortMetrics } from "./utils";
type MetricsTableProps = {
metrics?: MetricResult[];
......@@ -90,10 +79,6 @@ const collectionProps: ResponsiveProps = {
hideAtContainerBreakpoint: "sm",
};
const valueProps = {
...sharedProps,
};
const menuProps = {
...sharedProps,
};
......@@ -114,16 +99,14 @@ export function MetricsTable({
const handleSortingOptionsChange = skeleton ? undefined : setSortingOptions;
/** The name column has an explicitly set width. The remaining columns divide the remaining width. This is the percentage allocated to the collection column */
const valueWidth = 25;
const collectionWidth = 30;
const descriptionWidth = 100 - collectionWidth - valueWidth;
const descriptionWidth = 100 - collectionWidth;
return (
<Table aria-label={skeleton ? undefined : t`Table of metrics`}>
<colgroup>
{/* <col> for Name column */}
<NameColumn {...nameProps} />
<TableColumn {...valueProps} width={`${valueWidth}%`} />
<TableColumn {...collectionProps} width={`${collectionWidth}%`} />
<TableColumn {...descriptionProps} width={`${descriptionWidth}%`} />
<TableColumn {...menuProps} width={DOTMENU_WIDTH} />
......@@ -143,11 +126,6 @@ export function MetricsTable({
>
{t`Name`}
</SortableColumnHeader>
<ColumnHeader
style={{
textAlign: "right",
}}
>{t`Value`}</ColumnHeader>
<SortableColumnHeader
name="collection"
sortingOptions={sortingOptions}
......@@ -235,7 +213,6 @@ function MetricRow({ metric }: { metric?: MetricResult }) {
return (
<TableRow onClick={handleClick}>
<NameCell metric={metric} />
<ValueCell metric={metric} />
<CollectionCell metric={metric} />
<DescriptionCell metric={metric} />
<MenuCell metric={metric} />
......@@ -450,46 +427,3 @@ function MenuCell({ metric }: { metric?: MetricResult }) {
</Cell>
);
}
function ValueCell({ metric }: { metric?: MetricResult }) {
const { data, isLoading, error } = useGetCardQueryQuery(
metric ? { cardId: metric.id } : skipToken,
);
const emptyCell = (
<ValueTableCell>
<ValueWrapper>
<Value data-testid="metric-value" />
</ValueWrapper>
</ValueTableCell>
);
if (error) {
return emptyCell;
}
if (!metric || isLoading || !data) {
return (
<ValueTableCell>
<ValueWrapper data-testid="metric-value">
<SkeletonText />
</ValueWrapper>
</ValueTableCell>
);
}
const value = getDatasetValueForMetric(data);
if (!value) {
return emptyCell;
}
return (
<ValueTableCell>
<ValueWrapper>
<Tooltip label={<Text>{value.label}</Text>}>
<Value data-testid="metric-value">{value.value}</Value>
</Tooltip>
</ValueWrapper>
</ValueTableCell>
);
}
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment