format links on the table visualization (#15964)

* fix question level link display settings

* add cypress spec for the question-level link settings on the table visualization

* Address review comments: add a spec for clicn behavior with view_as = link, fix admin column settings

* revert a straightforward fix for hiding irrelevant settings

* fixup! Address review comments: add a spec for clicn behavior with view_as = link, fix admin column settings

* prettier

* handle link_text null value

frontend/src/metabase/lib/formatting.js

@@ -38,7 +38,10 @@ import {
   getTimeFormatFromStyle,
   hasHour,
 } from "metabase/lib/formatting/date";
-import { renderLinkTextForClick } from "metabase/lib/formatting/link";
+import {
+  renderLinkTextForClick,
+  renderLinkURLForClick,
+} from "metabase/lib/formatting/link";
 import { NULL_NUMERIC_VALUE, NULL_DISPLAY_VALUE } from "metabase/lib/constants";
 
 import type Field from "metabase-lib/lib/metadata/Field";
@@ -547,9 +550,14 @@ const EMAIL_ALLOW_LIST_REGEX = /^(?=.{1,254}$)(?=.{1,64}@)[-!#$%&'*+/0-9=?A-Z^_`
 
 export function formatEmail(
   value: Value,
-  { jsx, rich, view_as = "auto", link_text }: FormattingOptions = {},
+  { jsx, rich, view_as = "auto", link_text, clicked }: FormattingOptions = {},
 ) {
   const email = String(value);
+  const label =
+    clicked && link_text
+      ? renderLinkTextForClick(link_text, getDataFromClicked(clicked))
+      : null;
+
   if (
     jsx &&
     rich &&
@@ -557,7 +565,7 @@ export function formatEmail(
     EMAIL_ALLOW_LIST_REGEX.test(email)
   ) {
     return (
-      <ExternalLink href={"mailto:" + email}>{link_text || email}</ExternalLink>
+      <ExternalLink href={"mailto:" + email}>{label || email}</ExternalLink>
     );
   } else {
     return email;
@@ -585,35 +593,69 @@ function isDefaultLinkProtocol(protocol) {
   );
 }
 
-export function formatUrl(value: Value, options: FormattingOptions = {}) {
-  const { jsx, rich, view_as, column, link_text } = options;
-  const url = value;
+function getLinkUrl(value, { view_as, link_url, clicked, column }) {
+  const isExplicitLink = view_as === "link";
+  const hasCustomizedUrl = link_url && clicked;
 
-  const protocol = getUrlProtocol(url);
-  if (
-    jsx &&
-    rich &&
-    protocol &&
-    isSafeProtocol(protocol) &&
-    (view_as === undefined
-      ? isURL(column) || isDefaultLinkProtocol(protocol)
-      : view_as === "link"
-      ? true
-      : view_as === "auto"
-      ? isDefaultLinkProtocol(protocol)
-      : false)
-  ) {
-    const urlText =
-      link_text ||
-      getRemappedValue(value, options) ||
-      formatValue(value, { ...options, view_as: null });
+  if (isExplicitLink && hasCustomizedUrl) {
+    return renderLinkURLForClick(link_url, getDataFromClicked(clicked));
+  }
+
+  const protocol = getUrlProtocol(value);
+  const isValueSafeLink = protocol && isSafeProtocol(protocol);
+
+  if (!isValueSafeLink) {
+    return null;
+  }
+
+  if (isExplicitLink) {
+    return value;
+  }
+
+  const isDefaultProtocol = protocol && isDefaultLinkProtocol(protocol);
+  const isMaybeLink = view_as === "auto";
+
+  if (isMaybeLink && isDefaultProtocol) {
+    return value;
+  }
+
+  if (view_as === undefined && (isURL(column) || isDefaultProtocol)) {
+    return value;
+  }
+
+  return null;
+}
+
+function getLinkText(value, options) {
+  const { view_as, link_text, clicked } = options;
+
+  const isExplicitLink = view_as === "link";
+  const hasCustomizedText = link_text && clicked;
+
+  if (isExplicitLink && hasCustomizedText) {
+    return renderLinkTextForClick(link_text, getDataFromClicked(clicked));
+  }
+
+  return (
+    getRemappedValue(value, options) ||
+    formatValue(value, { ...options, view_as: null })
+  );
+}
+
+export function formatUrl(value, options = {}) {
+  const { jsx, rich } = options;
+
+  const url = getLinkUrl(value, options);
+
+  if (jsx && rich && url) {
+    const text = getLinkText(value, options);
     return (
       <ExternalLink className="link link--wrappable" href={url}>
-        {urlText}
+        {text}
       </ExternalLink>
     );
   } else {
-    return url;
+    return value;
   }
 }
 

frontend/src/metabase/visualizations/components/ChartSettingsWidget.jsx

@@ -2,10 +2,12 @@
 import React from "react";
 
 import cx from "classnames";
+import Icon from "metabase/components/Icon";
 
 const ChartSettingsWidget = ({
   title,
   description,
+  hint,
   hidden,
   disabled,
   widget: Widget,
@@ -28,7 +30,16 @@ const ChartSettingsWidget = ({
         disable: disabled,
       })}
     >
-      {title && <h4 className="mb1 flex align-center">{title}</h4>}
+      {title && (
+        <h4 className="mb1 flex align-center">
+          {title}
+          {hint && (
+            <span className="flex ml1">
+              <Icon name="info" size={14} tooltip={hint} />
+            </span>
+          )}
+        </h4>
+      )}
       {description && <div className="mb1">{description}</div>}
       {Widget && (
         <Widget

frontend/src/metabase/visualizations/components/settings/ChartSettingInput.jsx

@@ -6,6 +6,7 @@ import InputBlurChange from "metabase/components/InputBlurChange";
 const ChartSettingInput = ({ value, onChange, ...props }) => (
   <InputBlurChange
     {...props}
+    data-testid={props.id}
     className="input block full"
     value={value}
     onBlurChange={e => onChange(e.target.value)}

frontend/src/metabase/visualizations/visualizations/Table.jsx

@@ -14,7 +14,6 @@ import {
   isMetric,
   isDimension,
   isNumber,
-  isString,
   isURL,
   isEmail,
   isImageURL,
@@ -240,49 +239,59 @@ export default class Table extends Component {
         widget: "toggle",
       };
     }
-    if (isString(column)) {
-      let defaultValue = null;
-      const options: { name: string, value: null | string }[] = [
-        { name: t`Off`, value: null },
-      ];
-      if (!column.semantic_type || isURL(column)) {
-        defaultValue = "link";
-        options.push({ name: t`Link`, value: "link" });
-      }
-      if (!column.semantic_type || isEmail(column)) {
-        defaultValue = "email_link";
-        options.push({ name: t`Email link`, value: "email_link" });
-      }
-      if (!column.semantic_type || isImageURL(column) || isAvatarURL(column)) {
-        defaultValue = isAvatarURL(column) ? "image" : "link";
-        options.push({ name: t`Image`, value: "image" });
-      }
-      if (!column.semantic_type) {
-        defaultValue = "auto";
-        options.push({ name: t`Automatic`, value: "auto" });
-      }
-
-      if (options.length > 1) {
-        settings["view_as"] = {
-          title: t`View as link or image`,
-          widget: "select",
-          default: defaultValue,
-          props: {
-            options,
-          },
-        };
-      }
 
-      settings["link_text"] = {
-        title: t`Link text`,
-        widget: "input",
-        default: null,
-        getHidden: (column, settings) =>
-          settings["view_as"] !== "link" &&
-          settings["view_as"] !== "email_link",
+    let defaultValue = !column.semantic_type || isURL(column) ? "link" : null;
+
+    const options = [
+      { name: t`Off`, value: null },
+      { name: t`Link`, value: "link" },
+    ];
+
+    if (!column.semantic_type || isEmail(column)) {
+      defaultValue = "email_link";
+      options.push({ name: t`Email link`, value: "email_link" });
+    }
+    if (!column.semantic_type || isImageURL(column) || isAvatarURL(column)) {
+      defaultValue = isAvatarURL(column) ? "image" : "link";
+      options.push({ name: t`Image`, value: "image" });
+    }
+    if (!column.semantic_type) {
+      defaultValue = "auto";
+      options.push({ name: t`Automatic`, value: "auto" });
+    }
+
+    if (options.length > 1) {
+      settings["view_as"] = {
+        title: t`View as link or image`,
+        widget: "select",
+        default: defaultValue,
+        props: {
+          options,
+        },
       };
     }
 
+    const linkFieldsHint = t`You can use the value of any column here like this: {{COLUMN}}`;
+
+    settings["link_text"] = {
+      title: t`Link text`,
+      widget: "input",
+      hint: linkFieldsHint,
+      default: null,
+      getHidden: (_, settings) =>
+        settings["view_as"] !== "link" && settings["view_as"] !== "email_link",
+      readDependencies: ["view_as"],
+    };
+
+    settings["link_url"] = {
+      title: t`Link URL`,
+      widget: "input",
+      hint: linkFieldsHint,
+      default: null,
+      getHidden: (_, settings) => settings["view_as"] !== "link",
+      readDependencies: ["view_as"],
+    };
+
     return settings;
   };
 

frontend/test/metabase/lib/formatting.unit.spec.js

@@ -337,14 +337,84 @@ describe("formatting", () => {
         }),
       ).toEqual("data:text/plain;charset=utf-8,hello%20world");
     });
-    it("should return link component for type/URL and  view_as = link", () => {
-      const formatted = formatUrl("http://whatever", {
-        jsx: true,
-        rich: true,
-        column: { semantic_type: TYPE.URL },
-        view_as: "link",
+
+    describe("when view_as = link", () => {
+      it("should return link component for type/URL and  view_as = link", () => {
+        const formatted = formatUrl("http://whatever", {
+          jsx: true,
+          rich: true,
+          column: { semantic_type: TYPE.URL },
+          view_as: "link",
+        });
+        expect(isElementOfType(formatted, ExternalLink)).toEqual(true);
+      });
+
+      it("should return link component using link_url and link_text when specified", () => {
+        const formatted = formatUrl("http://not.metabase.com", {
+          jsx: true,
+          rich: true,
+          link_text: "metabase link",
+          link_url: "http://metabase.com",
+          view_as: "link",
+          clicked: {},
+        });
+
+        expect(isElementOfType(formatted, ExternalLink)).toEqual(true);
+        expect(formatted.props.children).toEqual("metabase link");
+        expect(formatted.props.href).toEqual("http://metabase.com");
+      });
+
+      it("should return link component using link_text and the value as url when link_url is empty", () => {
+        const formatted = formatUrl("http://metabase.com", {
+          jsx: true,
+          rich: true,
+          link_text: "metabase link",
+          link_url: "",
+          view_as: "link",
+          clicked: {},
+        });
+
+        expect(isElementOfType(formatted, ExternalLink)).toEqual(true);
+        expect(formatted.props.children).toEqual("metabase link");
+        expect(formatted.props.href).toEqual("http://metabase.com");
+      });
+
+      it("should return link component using link_url and the value as text when link_text is empty", () => {
+        const formatted = formatUrl("metabase link", {
+          jsx: true,
+          rich: true,
+          link_text: "",
+          link_url: "http://metabase.com",
+          view_as: "link",
+          clicked: {},
+        });
+
+        expect(isElementOfType(formatted, ExternalLink)).toEqual(true);
+        expect(formatted.props.children).toEqual("metabase link");
+        expect(formatted.props.href).toEqual("http://metabase.com");
+      });
+
+      it("should not return an ExternalLink in jsx + rich mode if there's click behavior", () => {
+        const formatted = formatValue("http://metabase.com/", {
+          jsx: true,
+          rich: true,
+          click_behavior: {
+            linkTemplate: "foo",
+            linkTextTemplate: "bar",
+            linkType: "url",
+            type: "link",
+          },
+          link_text: "metabase link",
+          link_url: "http://metabase.com",
+          view_as: "link",
+          clicked: {},
+        });
+
+        // it is not a link set on the question level
+        expect(isElementOfType(formatted, ExternalLink)).toEqual(false);
+        // it is formatted as a link cell for the dashboard level click behavior
+        expect(formatted.props.className).toEqual("link link--wrappable");
       });
-      expect(isElementOfType(formatted, ExternalLink)).toEqual(true);
     });
 
     it("should not crash if column is null", () => {

frontend/test/metabase/scenarios/visualizations/table.cy.spec.js

+import { restore, visitQuestionAdhoc, popover } from "__support__/e2e/cypress";
+import { SAMPLE_DATASET } from "__support__/e2e/cypress_sample_dataset";
+
+const { PEOPLE_ID } = SAMPLE_DATASET;
+
+const testQuery = {
+  database: 1,
+  query: {
+    "source-table": PEOPLE_ID,
+  },
+  type: "query",
+};
+
+describe("scenarios > visualizations > table", () => {
+  beforeEach(() => {
+    restore();
+    cy.signInAsNormalUser();
+  });
+
+  it("should allow to display any column as link with extrapolated url and text", () => {
+    visitQuestionAdhoc({
+      dataset_query: testQuery,
+      display: "table",
+    });
+
+    cy.findByText("City").click();
+
+    popover().within(() => {
+      cy.icon("gear").click();
+    });
+
+    cy.findByText("Off").click();
+
+    popover().within(() => {
+      cy.findByText("Link").click();
+    });
+
+    cy.findByTestId("link_text").type("{{CITY}} {{ID}} fixed text", {
+      parseSpecialCharSequences: false,
+    });
+    cy.findByTestId("link_url").type("http://metabase.com/people/{{ID}}", {
+      parseSpecialCharSequences: false,
+    });
+
+    cy.findByText("Done").click();
+
+    cy.findByText("Wood River 1 fixed text").should(
+      "have.attr",
+      "href",
+      "http://metabase.com/people/1",
+    );
+  });
+});