Verification in search (#17329)

* Include and bump moderation status in search results

* Include limit 1 and order by for moderation review latests

backend does work to ensure there is an invariant that there is 1
most_recent true entry and all the rest are most_recent false. But if
there's some bug this invariant is violated we don't want to blow up
with multiple rows returned in these queries.

* Verification scoring is enterprise now

* wip moving moderation review to enterprise

still need to delete the old non-moved file

* add moderation icon to search results

* update names of icon props in SavedQuestionHeaderButton

* move moderation cypress tests, add test for search results

* Move moderation review api and test to enterprise

* fix props for getVerifiedIcon call

* add moderation status icon to collections

* add/improve moderation cypress tests

Co-authored-by: Dalton Johnson <daltojohnso@users.noreply.github.com>

src/metabase/api/moderation_review.clj → enterprise/backend/src/metabase_enterprise/moderation/api/review.clj

-(ns metabase.api.moderation-review
+(ns metabase-enterprise.moderation.api.review
   (:require [compojure.core :refer [POST]]
             [metabase.api.common :as api]
             [metabase.models.moderation-review :as moderation-review]

enterprise/backend/src/metabase_enterprise/search/scoring.clj

@@ -10,6 +10,13 @@
     1
     0))
 
+(defn- verified-score
+  "A scorer for verified items."
+  [{:keys [moderated_status]}]
+  (if (contains? #{"verified"} moderated_status)
+    1
+    0))
+
 (def scoring-impl
   "Scoring implementation that adds score for items in official collections."
   (reify scoring/ResultScore
@@ -17,7 +24,10 @@
       (conj (scoring/score-result scoring/oss-score-impl result)
             {:weight 2
              :score  (official-collection-score result)
-             :name   "official collection score"}))))
+             :name   "official collection score"}
+            {:weight 2
+             :score  (verified-score result)
+             :name   "verified"}))))
 
 (def ee-scoring
   "Enterprise scoring of results, falling back to the open source version if enterprise is not enabled."

test/metabase/api/moderation_review_test.clj → enterprise/backend/test/metabase_enterprise/moderation/api/review_test.clj

-(ns metabase.api.moderation-review-test
+(ns metabase-enterprise.moderation.api.review-test
   (:require [clojure.test :refer :all]
             [metabase.models.card :refer [Card]]
             [metabase.models.moderation-review :as mod-review :refer [ModerationReview]]

enterprise/backend/test/metabase_enterprise/search/scoring_test.clj

@@ -4,6 +4,18 @@
             [metabase-enterprise.search.scoring :as ee-scoring]
             [metabase.search.scoring :as scoring]))
 
+(deftest verified-score-test
+  (let [score #'ee-scoring/verified-score
+        item  (fn [id status] {:moderated_status status
+                               :id               id
+                               :model            "card"})
+        score (fn [items] (into [] (map :id) (reverse (sort-by score items))))]
+    (testing "verification bumps result"
+      ;; stable sort all with score 0 and then reverse to get descending rather than ascending
+      (is (= [3 2 1] (score [(item 1 nil) (item 2 nil) (item 3 nil)])))
+      ;; verified item is promoted
+      (is (= [1 3 2] (score [(item 1 "verified") (item 2 nil) (item 3 nil)]))))))
+
 (deftest official-collection-tests
   (testing "it should bump up the value of items in official collections"
     ;; using the ee implementation that isn't wrapped by enable-enhancements? check
@@ -36,4 +48,28 @@
                 "examples of custom expressions"
                 "custom expression examples"]
                (map :name (sort-by ee-score [a b c
-                                             (assoc d :collection_authority_level "official")]))))))))
+                                             (assoc d :collection_authority_level "official")])))))))
+  (testing "It should bump up the value of verified items"
+    (let [search-string     "foo"
+          dashboard-count   #(assoc % :dashboardcard_count 0)
+          ee-score          (comp :score
+                                  (partial scoring/score-and-result ee-scoring/scoring-impl search-string)
+                                  dashboard-count)
+          os-score          (comp :score
+                                  (partial scoring/score-and-result scoring/oss-score-impl search-string)
+                                  dashboard-count)
+          labeled-results   {:a {:name "foobar" :model "card" :id :a}
+                             :b {:name "foo foo" :model "card" :id :b}
+                             :c {:name "foo foo foo" :model "card" :id :c}}
+          {:keys [a b c]} labeled-results]
+      (doseq [item [a b c]]
+        (is (> (ee-score (assoc item :moderated_status "verified")) (ee-score item))
+            (str "Item not greater for model: " (:model item))))
+      (let [items (shuffle [a b c])]
+        (is (= (sort-by os-score items) (sort-by ee-score items))))
+      ;; a is sorted lowest here (sort-by is ascending)
+      (is (= [:a :c :b] (map :id (sort-by ee-score [a b c]))))
+      ;; a is verified and is now last or highest score
+      (is (= [:c :b :a]
+             (map :id
+                  (sort-by ee-score [(assoc a :moderated_status "verified") b c])))))))

enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationActions/ModerationActions.styled.jsx

@@ -3,7 +3,7 @@ import styled from "styled-components";
 import { color } from "metabase/lib/colors";
 import { getVerifiedIcon } from "metabase-enterprise/moderation/service";
 
-const { icon: verifiedIcon, iconColor: verifiedIconColor } = getVerifiedIcon();
+const { name: verifiedIconName, color: verifiedIconColor } = getVerifiedIcon();
 
 import Button from "metabase/components/Button";
 
@@ -20,7 +20,7 @@ export const Label = styled.h5`
 `;
 
 export const VerifyButton = styled(Button).attrs({
-  icon: verifiedIcon,
+  icon: verifiedIconName,
   iconSize: 20,
 })`
   border: none;

enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationReviewBanner/ModerationReviewBanner.jsx

@@ -60,7 +60,9 @@ export function ModerationReviewBanner({
   const relativeCreationTime = getRelativeTimeAbbreviated(
     moderationReview.created_at,
   );
-  const { icon, iconColor } = getIconForReview(moderationReview);
+  const { name: iconName, color: iconColor } = getIconForReview(
+    moderationReview,
+  );
   const showClose = isHovering || isActive;
 
   return (
@@ -78,14 +80,14 @@ export function ModerationReviewBanner({
             data-testid="moderation-remove-review-action"
             onFocus={() => setIsActive(true)}
             onBlur={() => setIsActive(false)}
-            icon={showClose ? "close" : icon}
+            icon={showClose ? "close" : iconName}
             color={color(showClose ? "text-medium" : iconColor)}
             onClick={onRemove}
             iconSize={ICON_BUTTON_SIZE}
           />
         ) : (
           <StatusIcon
-            name={icon}
+            name={iconName}
             color={color(iconColor)}
             size={ICON_BUTTON_SIZE}
           />

enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationStatusIcon/ModerationStatusIcon.jsx

+import React from "react";
+import PropTypes from "prop-types";
+
+import { color } from "metabase/lib/colors";
+import { getStatusIcon } from "metabase-enterprise/moderation/service";
+
+import Icon from "metabase/components/Icon";
+
+ModerationStatusIcon.propTypes = {
+  status: PropTypes.string,
+};
+
+function ModerationStatusIcon({ status, ...iconProps }) {
+  const { name: iconName, color: iconColor } = getStatusIcon(status);
+  return iconName ? (
+    <Icon name={iconName} color={color(iconColor)} {...iconProps} />
+  ) : null;
+}
+
+export default ModerationStatusIcon;

enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationStatusIcon/ModerationStatusIcon.unit.spec.js

+import React from "react";
+import ModerationStatusIcon from "./ModerationStatusIcon";
+import { render } from "@testing-library/react";
+
+const VERIFIED_ICON_SELECTOR = ".Icon-verified";
+
+describe("ModerationReviewBanner", () => {
+  it("should show an icon when given a real moderation status", () => {
+    render(<ModerationStatusIcon status="verified" />);
+
+    expect(document.querySelector(VERIFIED_ICON_SELECTOR)).toBeTruthy();
+  });
+
+  it("should not show an icon when given an undefined status", () => {
+    render(<ModerationStatusIcon status={undefined} />);
+
+    expect(document.querySelector(VERIFIED_ICON_SELECTOR)).toBeNull();
+  });
+
+  it("should not show an icon when given a status that does not match any existing moderation status", () => {
+    render(<ModerationStatusIcon status="foo" />);
+
+    expect(document.querySelector(VERIFIED_ICON_SELECTOR)).toBeNull();
+  });
+});

enterprise/frontend/src/metabase-enterprise/moderation/index.js

 import { PLUGIN_MODERATION } from "metabase/plugins";
 import QuestionModerationSection from "./components/QuestionModerationSection/QuestionModerationSection";
+import ModerationStatusIcon from "./components/ModerationStatusIcon/ModerationStatusIcon";
 
-import { getStatusIconForReviews } from "./service";
+import { getStatusIconForQuestion } from "./service";
 
 Object.assign(PLUGIN_MODERATION, {
   QuestionModerationSection,
-  getStatusIconForReviews,
+  ModerationStatusIcon,
+  getStatusIconForQuestion,
 });

enterprise/frontend/src/metabase-enterprise/moderation/service.js

@@ -21,19 +21,17 @@ export function removeReview({ itemId, itemType }) {
   });
 }
 
+export function getStatusIcon(status) {
+  const { icon, color } = ACTIONS[status] || {};
+  return { name: icon, color };
+}
+
 export function getVerifiedIcon() {
-  const { icon, color } = ACTIONS["verified"];
-  return { icon, iconColor: color };
+  return getStatusIcon("verified");
 }
 
 export function getIconForReview(review) {
-  if (review && review.status !== null) {
-    const { status } = review;
-    const { icon, color } = ACTIONS[status] || {};
-    return { icon, iconColor: color };
-  }
-
-  return {};
+  return getStatusIcon(review?.status);
 }
 
 export function getTextForReviewBanner(
@@ -81,7 +79,8 @@ export function getLatestModerationReview(reviews) {
   }
 }
 
-export function getStatusIconForReviews(reviews) {
+export function getStatusIconForQuestion(question) {
+  const reviews = question.getModerationReviews();
   const review = getLatestModerationReview(reviews);
   return getIconForReview(review);
 }

enterprise/frontend/src/metabase-enterprise/moderation/service.unit.spec.js

@@ -6,7 +6,7 @@ import {
   getTextForReviewBanner,
   isItemVerified,
   getLatestModerationReview,
-  getStatusIconForReviews,
+  getStatusIconForQuestion,
 } from "./service";
 
 jest.mock("metabase/services", () => ({
@@ -61,8 +61,8 @@ describe("moderation/service", () => {
   describe("getVerifiedIcon", () => {
     it("should return verified icon name/color", () => {
       expect(getVerifiedIcon()).toEqual({
-        icon: "verified",
-        iconColor: "brand",
+        name: "verified",
+        color: "brand",
       });
     });
   });
@@ -172,25 +172,46 @@ describe("moderation/service", () => {
   });
 });
 
-describe("getStatusIconForReviews", () => {
+describe("getStatusIconForQuestion", () => {
   it('should return the status icon for the most recent "real" review', () => {
-    const reviews = [
-      { id: 1, status: "verified" },
-      { id: 2, status: "verified", most_recent: true },
-      { id: 3, status: null },
-    ];
+    const questionWithReviews = {
+      getModerationReviews: () => [
+        { id: 1, status: "verified" },
+        { id: 2, status: "verified", most_recent: true },
+        { id: 3, status: null },
+      ],
+    };
 
-    expect(getStatusIconForReviews(reviews)).toEqual(getVerifiedIcon());
+    expect(getStatusIconForQuestion(questionWithReviews)).toEqual(
+      getVerifiedIcon(),
+    );
   });
 
-  it("should return undefined for no review", () => {
-    const reviews = [
-      { id: 1, status: "verified" },
-      { id: 2, status: "verified" },
-      { id: 3, status: null, most_recent: true },
-    ];
-
-    expect(getLatestModerationReview(reviews)).toEqual(undefined);
-    expect(getLatestModerationReview([])).toEqual(undefined);
+  it("should return undefined vals for no review", () => {
+    const questionWithNoMostRecentReview = {
+      getModerationReviews: () => [
+        { id: 1, status: "verified" },
+        { id: 2, status: "verified" },
+        { id: 3, status: null, most_recent: true },
+      ],
+    };
+
+    const questionWithNoReviews = {
+      getModerationReviews: () => [],
+    };
+
+    const questionWithUndefinedReviews = {
+      getModerationReviews: () => undefined,
+    };
+
+    const noIcon = { name: undefined, color: undefined };
+
+    expect(getStatusIconForQuestion(questionWithNoMostRecentReview)).toEqual(
+      noIcon,
+    );
+    expect(getStatusIconForQuestion(questionWithNoReviews)).toEqual(noIcon);
+    expect(getStatusIconForQuestion(questionWithUndefinedReviews)).toEqual(
+      noIcon,
+    );
   });
 });

frontend/src/metabase/collections/components/BaseItemsTable.styled.jsx

@@ -36,6 +36,10 @@ export const EntityIconCheckBox = styled(EntityItem.IconCheckBox)`
 `;
 
 export const ItemLink = styled(Link)`
+  display: flex;
+  grid-gap: 0.5rem;
+  align-items: center;
+
   &:hover {
     color: ${color("brand")};
   }

frontend/src/metabase/collections/components/BaseTableItem.jsx

@@ -2,6 +2,7 @@ import React, { useCallback } from "react";
 import PropTypes from "prop-types";
 import moment from "moment";
 
+import { PLUGIN_MODERATION } from "metabase/plugins";
 import { color } from "metabase/lib/colors";
 
 import ItemDragSource from "metabase/containers/dnd/ItemDragSource";
@@ -18,6 +19,8 @@ import {
   TableItemSecondaryField,
 } from "./BaseItemsTable.styled";
 
+const { ModerationStatusIcon } = PLUGIN_MODERATION;
+
 BaseTableItem.propTypes = {
   item: PropTypes.object,
   draggable: PropTypes.bool,
@@ -99,6 +102,7 @@ export function BaseTableItem({
         <td data-testid={`${testId}-name`}>
           <ItemLink {...linkProps} to={item.getUrl()}>
             <EntityItem.Name name={item.name} />
+            <ModerationStatusIcon status={item.moderated_status} />
           </ItemLink>
         </td>
         <td data-testid={`${testId}-last-edited-by`}>

frontend/src/metabase/plugins/index.js

@@ -81,5 +81,6 @@ export const PLUGIN_COLLECTION_COMPONENTS = {
 
 export const PLUGIN_MODERATION = {
   QuestionModerationSection: PluginPlaceholder,
-  getStatusIconForReviews: object,
+  ModerationStatusIcon: PluginPlaceholder,
+  getStatusIconForQuestion: object,
 };

frontend/src/metabase/query_builder/components/SavedQuestionHeaderButton/SavedQuestionHeaderButton.jsx

@@ -4,7 +4,7 @@ import PropTypes from "prop-types";
 import { PLUGIN_MODERATION } from "metabase/plugins";
 import { HeaderButton } from "./SavedQuestionHeaderButton.styled";
 
-const { getStatusIconForReviews } = PLUGIN_MODERATION;
+const { getStatusIconForQuestion } = PLUGIN_MODERATION;
 
 export default SavedQuestionHeaderButton;
 
@@ -17,16 +17,16 @@ SavedQuestionHeaderButton.propTypes = {
 
 function SavedQuestionHeaderButton({ className, question, onClick, isActive }) {
   const {
-    icon: reviewIcon,
-    iconColor: reviewIconColor,
-  } = getStatusIconForReviews(question.getModerationReviews());
+    name: reviewIconName,
+    color: reviewIconColor,
+  } = getStatusIconForQuestion(question);
 
   return (
     <HeaderButton
       className={className}
       onClick={onClick}
       iconRight="chevrondown"
-      icon={reviewIcon}
+      icon={reviewIconName}
       leftIconColor={reviewIconColor}
       isActive={isActive}
       iconSize={20}

frontend/src/metabase/query_builder/components/view/sidebars/QuestionDetailsSidebarPanel.jsx

@@ -17,14 +17,12 @@ export default QuestionDetailsSidebarPanel;
 QuestionDetailsSidebarPanel.propTypes = {
   question: PropTypes.object.isRequired,
   onOpenModal: PropTypes.func.isRequired,
-  moderatorVerifyCard: PropTypes.func.isRequired,
   removeModerationReview: PropTypes.func.isRequired,
 };
 
 function QuestionDetailsSidebarPanel({
   question,
   onOpenModal,
-  moderatorVerifyCard,
   removeModerationReview,
 }) {
   const canWrite = question.canWrite();

frontend/src/metabase/search/components/SearchResult.jsx

@@ -12,13 +12,17 @@ import Icon from "metabase/components/Icon";
 import Link from "metabase/components/Link";
 import Text from "metabase/components/type/Text";
 
-import { PLUGIN_COLLECTION_COMPONENTS } from "metabase/plugins";
+import {
+  PLUGIN_COLLECTION_COMPONENTS,
+  PLUGIN_MODERATION,
+} from "metabase/plugins";
 
 import Schema from "metabase/entities/schemas";
 import Database from "metabase/entities/databases";
 import Table from "metabase/entities/tables";
 
 const { CollectionAuthorityLevelIcon } = PLUGIN_COLLECTION_COMPONENTS;
+const { ModerationStatusIcon } = PLUGIN_MODERATION;
 
 function getColorForIconWrapper(props) {
   if (props.item.collection_position) {
@@ -89,6 +93,12 @@ const ResultLink = styled(Link)`
   }
 `;
 
+const TitleWrapper = styled.div`
+  display: flex;
+  grid-gap: 0.25rem;
+  align-items: center;
+`;
+
 function ItemIcon({ item, type }) {
   return (
     <IconWrapper item={item} type={type}>
@@ -253,7 +263,10 @@ export default function SearchResult({ result, compact }) {
       <Flex align="start">
         <ItemIcon item={result} type={result.model} />
         <Box>
-          <Title>{result.name}</Title>
+          <TitleWrapper>
+            <Title>{result.name}</Title>
+            <ModerationStatusIcon status={result.moderated_status} size={12} />
+          </TitleWrapper>
           <Text>
             <InfoText result={result} />
           </Text>

frontend/test/__support__/e2e/commands.js

@@ -5,6 +5,7 @@ import "./commands/api/question";
 import "./commands/api/dashboard";
 import "./commands/api/dashboardFilters";
 import "./commands/api/collection";
+import "./commands/api/moderation";
 
 import "./commands/api/composite/createQuestionAndDashboard";
 import "./commands/api/composite/createNativeQuestionAndDashboard";

frontend/test/__support__/e2e/commands/api/moderation.js

+Cypress.Commands.add(
+  "createModerationReview",
+  ({ status, moderated_item_type, moderated_item_id }) => {
+    cy.log(
+      `Create a moderation review, status: ${status}, item type: ${moderated_item_type}, item id: ${moderated_item_id}`,
+    );
+    cy.request("POST", "/api/moderation-review", {
+      status,
+      moderated_item_id,
+      moderated_item_type,
+    });
+  },
+);

frontend/test/metabase/scenarios/question/moderation.cy.spec.js → frontend/test/metabase/scenarios/moderation/question-moderation.cy.spec.js

@@ -6,7 +6,7 @@ describeWithToken("scenarios > saved question moderation", () => {
       restore();
       cy.signInAsAdmin();
 
-      cy.visit("/question/1");
+      cy.visit("/question/2");
     });
 
     it("should be able to verify a saved question", () => {
@@ -16,8 +16,16 @@ describeWithToken("scenarios > saved question moderation", () => {
 
       cy.findByText("You verified this").should("be.visible");
 
-      cy.findByTestId("saved-question-header-button").click();
-      cy.icon("verified").should("be.visible");
+      cy.findByPlaceholderText("Search…").type("orders{enter}");
+      cy.findByText("Orders, Count")
+        .icon("verified")
+        .should("exist");
+
+      cy.visit("/collection/root");
+
+      cy.findByText("Orders, Count")
+        .icon("verified")
+        .should("exist");
     });
 
     it("should be able to unverify a verified saved question", () => {
@@ -30,41 +38,72 @@ describeWithToken("scenarios > saved question moderation", () => {
       cy.findByTestId("saved-question-header-button").click();
 
       cy.icon("verified").should("not.exist");
+
+      cy.findByPlaceholderText("Search…").type("orders{enter}");
+      cy.findByText("Orders, Count")
+        .find(".Icon-verified")
+        .should("not.exist");
+
+      cy.visit("/collection/root");
+
+      cy.findByText("Orders, Count")
+        .find(".Icon-verified")
+        .should("not.exist");
     });
   });
 
   describe("as a non-admin user", () => {
     beforeEach(() => {
       restore();
+      cy.signInAsAdmin();
+
+      cy.createModerationReview({
+        status: "verified",
+        moderated_item_type: "card",
+        moderated_item_id: 2,
+      });
+
       cy.signInAsNormalUser();
+    });
+
+    it("should be able to see that a question has not been verified", () => {
+      cy.visit("/question/3");
 
-      cy.intercept("GET", "/api/card/1", req => {
-        req.reply(res => {
-          res.body.moderation_reviews = [
-            {
-              status: "verified",
-              most_recent: true,
-              moderator_id: 999,
-              id: 1,
-              moderated_item_type: "card",
-              moderated_item_id: 4,
-              updated_at: "2021-07-23T09:56:46.276-07:00",
-              created_at: "2021-07-23T09:56:46.276-07:00",
-            },
-          ];
-        });
-      }).as("cardGet");
+      cy.icon("verified").should("not.exist");
+
+      cy.findByTestId("saved-question-header-button").click();
+      cy.findByText("A moderator verified this").should("not.exist");
+
+      cy.findByPlaceholderText("Search…").type("orders{enter}");
+      cy.findByText("Orders, Count, Grouped by Created At (year)")
+        .find(".Icon-verified")
+        .should("not.exist");
+
+      cy.visit("/collection/root");
+
+      cy.findByText("Orders, Count, Grouped by Created At (year)")
+        .find(".Icon-verified")
+        .should("not.exist");
     });
 
     it("should be able to see that a question has been verified", () => {
-      cy.visit(`/question/1`);
-
-      cy.wait("@cardGet");
+      cy.visit("/question/2");
 
       cy.icon("verified").should("exist");
 
       cy.findByTestId("saved-question-header-button").click();
       cy.findByText("A moderator verified this").should("exist");
+
+      cy.findByPlaceholderText("Search…").type("orders{enter}");
+      cy.findByText("Orders, Count")
+        .icon("verified")
+        .should("exist");
+
+      cy.visit("/collection/root");
+
+      cy.findByText("Orders, Count")
+        .icon("verified")
+        .should("exist");
     });
   });
 });