From b7fa2f34d5960e30b26c995d443bbdb9831a33da Mon Sep 17 00:00:00 2001
From: Dalton <daltojohnso@users.noreply.github.com>
Date: Tue, 3 Aug 2021 09:36:18 -0700
Subject: [PATCH] Add the ability to verify/unverify questions (#17030)
* rmv old bucm icons and remove verified fill color
* add moderation action section to sidebar
* add moderation review icon to the saved question header button
* hide moderation section when is not a moderator
* add UI for ModerationReviewBanner
* Backend for moderation-review
- create table moderation_review. Same as before but also has a
"most_recent" boolean flag for the most recent moderation for easy
lookup
- POST /moderation-review/ . Status can be "verified" or nil
- must be an admin to post
- No PUT or edit route yet. Not sure if this is even
necessary. _MAYBE_ to edit the text, but certainly not for the
status, ids, etc. If there's to be history, let's build some history
- Ensure we never have more than 10 reviews. Adding a new review will
delete the older ones, mark all old ones as not `most_recent`, and
add the newest one as `most_recent true`
- Ensure the card actually exists before creating the mod review
- Since admin only at this time, don't need to check moderate
permission or view permission
- When hydrating ensure reviews are ordered by id desc. Should mimic
the created_at desc
* fix moderation review banner tooltip offset
* disable verification button when already verified
* rmv iconOnly prop because it seems to do nothing
* update getLatestModerationReview to rely on most_recent boolean
* Return 400 on invalid status to post /moderation-review
the schema was using keywords on the left hand side rather than the
symbols. Required a change to the docstring generator, when it made a
docstring for enums, it would call (sort (:vs enum)) and need to
string em.
* Add ModerationReview model to models.clj and copy infra
* hydrate moderation reviews on cards
* clean up + wire up to BE + ensure mod buttons don't show for normal users
* rmv unused moderation redux logic from QuestionDetailsSidebarPanel
* finish writing unit tests for FE
* ensure getIconForReview returns an object
* enable/disable verify button tooltip when unverified/verified
* add e2e tests
* fix tests
* styling tweaks
* more styling on moderationReviewBanner
* add function for abbreviated timestamp
* increase fontsize of timestamp back to 12
* fix tooltip offset
* ensure custom locale is separate from 'en' and not used for other languages
* Deletion moderation reviews when deleting cards
i had actually thought this was a much larger problem. But it turns
out we almost never delete cards (thanks comment!). And so we won't
really generate a lot of garbage.
I was worried that since we aren't using actual foreign keys but just
`moderated_item_type "card"` and `moderated_item_id 2` we would have
deleted cards with these moderation reviews but that is not the case
as the cards aren't deleted.
* hide verify disabled button when a question is verified
* update test to use queryByTestId
* Hydrate moderation reviews on cards on ordered cards
* Handle mysql's lack of offset functionality
mysql cannot handle just a `offset` clause, it also needs a limit
clause grammar from
https://dev.mysql.com/doc/refman/8.0/en/select.html:
[LIMIT {[offset,] row_count | row_count OFFSET offset}]
select id, name from metabase_field offset 5; -- errors
select id, name from metabase_field limit 2 offset 5; -- works
Since our numbers are so small here there is no worry and just do the
offset in memory rather than jump through hoops for different dbs.
* Batch hydrate moderation reviews
* Don't let /api/user/:userId failure conceal moderation banner
* fix moderation cy tests
* work around possible bug in toucan hydration
dashboards hydrate ordered cards
(hydrate [:ordered_cards [:card :moderation_reviews] :series])
Ordered_cards are dashboard_cards which have an optional card_id. But
toucan hydration doesn't filter out the nils as they go down. It seems
toucan returns a nil card, and then when hydrating the
moderation_review, passes the collection of all "cards" including the
nil ones into the hydration function for moderation_reviews
This feels like a bug to me
* Cleanup moderation warnings
* Docstring in moderation review
* include hoisted moderated_status on cards in collections api
* Expect unverified in test :wrench:
Co-authored-by: dan sutton <dan@dpsutton.com>
Co-authored-by: Maz Ameli <maz@metabase.com>
Co-authored-by: alxnddr <alxnddr@gmail.com>
---
.../metabase-enterprise/moderation/actions.js | 31 +++
.../ModerationActions/ModerationActions.jsx | 35 ++++
.../ModerationActions.styled.jsx | 35 ++++
.../ModerationActions.unit.spec.js | 28 +++
.../ModerationReviewBanner.jsx | 98 +++++++++
.../ModerationReviewBanner.styled.jsx | 40 ++++
.../ModerationReviewBanner.unit.spec.js | 104 ++++++++++
.../QuestionModerationSection.jsx | 69 ++++++
.../QuestionModerationSection.styled.jsx | 8 +
.../moderation/constants.js | 7 +
.../metabase-enterprise/moderation/index.js | 9 +
.../moderation/selectors.js | 5 +
.../metabase-enterprise/moderation/service.js | 87 ++++++++
.../moderation/service.unit.spec.js | 196 ++++++++++++++++++
.../src/metabase-enterprise/plugins.js | 1 +
frontend/src/metabase-lib/lib/Question.js | 6 +-
frontend/src/metabase/icon_paths.js | 10 +-
frontend/src/metabase/lib/time.js | 44 ++++
frontend/src/metabase/plugins/index.js | 7 +
.../SavedQuestionHeaderButton.jsx | 10 +
.../SavedQuestionHeaderButton.styled.jsx | 4 +
.../SavedQuestionHeaderButton.unit.spec.js | 75 +++++++
.../components/view/ViewHeader.jsx | 2 +-
.../sidebars/QuestionDetailsSidebarPanel.jsx | 39 +++-
.../QuestionDetailsSidebarPanel.styled.jsx | 13 ++
.../src/metabase/selectors/user.unit.spec.js | 23 ++
frontend/src/metabase/services.js | 5 +
.../SavedQuestionHeaderButton.unit.spec.js | 34 ---
.../scenarios/question/moderation.cy.spec.js | 70 +++++++
resources/migrations/000_migrations.yaml | 64 ++++++
src/metabase/api/card.clj | 6 +-
src/metabase/api/collection.clj | 18 +-
src/metabase/api/dashboard.clj | 5 +-
src/metabase/api/moderation_review.clj | 25 +++
src/metabase/api/routes.clj | 2 +
src/metabase/cmd/copy.clj | 5 +-
src/metabase/models.clj | 3 +
src/metabase/models/card.clj | 4 +
src/metabase/models/dashboard.clj | 3 +
src/metabase/models/moderation_review.clj | 76 +++++++
src/metabase/moderation.clj | 45 ++++
src/metabase/util/schema.clj | 2 +-
test/metabase/api/card_test.clj | 20 +-
test/metabase/api/collection_test.clj | 25 ++-
test/metabase/api/moderation_review_test.clj | 86 ++++++++
45 files changed, 1404 insertions(+), 80 deletions(-)
create mode 100644 enterprise/frontend/src/metabase-enterprise/moderation/actions.js
create mode 100644 enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationActions/ModerationActions.jsx
create mode 100644 enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationActions/ModerationActions.styled.jsx
create mode 100644 enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationActions/ModerationActions.unit.spec.js
create mode 100644 enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationReviewBanner/ModerationReviewBanner.jsx
create mode 100644 enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationReviewBanner/ModerationReviewBanner.styled.jsx
create mode 100644 enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationReviewBanner/ModerationReviewBanner.unit.spec.js
create mode 100644 enterprise/frontend/src/metabase-enterprise/moderation/components/QuestionModerationSection/QuestionModerationSection.jsx
create mode 100644 enterprise/frontend/src/metabase-enterprise/moderation/components/QuestionModerationSection/QuestionModerationSection.styled.jsx
create mode 100644 enterprise/frontend/src/metabase-enterprise/moderation/constants.js
create mode 100644 enterprise/frontend/src/metabase-enterprise/moderation/index.js
create mode 100644 enterprise/frontend/src/metabase-enterprise/moderation/selectors.js
create mode 100644 enterprise/frontend/src/metabase-enterprise/moderation/service.js
create mode 100644 enterprise/frontend/src/metabase-enterprise/moderation/service.unit.spec.js
rename frontend/src/metabase/query_builder/components/{ => SavedQuestionHeaderButton}/SavedQuestionHeaderButton.jsx (70%)
rename frontend/src/metabase/query_builder/components/{ => SavedQuestionHeaderButton}/SavedQuestionHeaderButton.styled.jsx (83%)
create mode 100644 frontend/src/metabase/query_builder/components/SavedQuestionHeaderButton/SavedQuestionHeaderButton.unit.spec.js
create mode 100644 frontend/src/metabase/selectors/user.unit.spec.js
delete mode 100644 frontend/test/metabase/query_builder/components/SavedQuestionHeaderButton.unit.spec.js
create mode 100644 frontend/test/metabase/scenarios/question/moderation.cy.spec.js
create mode 100644 src/metabase/api/moderation_review.clj
create mode 100644 src/metabase/models/moderation_review.clj
create mode 100644 src/metabase/moderation.clj
create mode 100644 test/metabase/api/moderation_review_test.clj
diff --git a/enterprise/frontend/src/metabase-enterprise/moderation/actions.js b/enterprise/frontend/src/metabase-enterprise/moderation/actions.js
new file mode 100644
index 00000000000..3d51a9b36bb
--- /dev/null
+++ b/enterprise/frontend/src/metabase-enterprise/moderation/actions.js
@@ -0,0 +1,31 @@
+import { createThunkAction } from "metabase/lib/redux";
+import { verifyItem, removeReview } from "./service";
+import { softReloadCard } from "metabase/query_builder/actions";
+
+export const VERIFY_CARD = "metabase-enterprise/moderation/VERIFY_CARD";
+export const verifyCard = createThunkAction(
+ VERIFY_CARD,
+ (cardId, text) => async (dispatch, getState) => {
+ await verifyItem({
+ itemId: cardId,
+ itemType: "card",
+ text,
+ });
+
+ return dispatch(softReloadCard());
+ },
+);
+
+export const REMOVE_CARD_REVIEW =
+ "metabase-enterprise/moderation/REMOVE_CARD_REVIEW";
+export const removeCardReview = createThunkAction(
+ REMOVE_CARD_REVIEW,
+ cardId => async (dispatch, getState) => {
+ await removeReview({
+ itemId: cardId,
+ itemType: "card",
+ });
+
+ return dispatch(softReloadCard());
+ },
+);
diff --git a/enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationActions/ModerationActions.jsx b/enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationActions/ModerationActions.jsx
new file mode 100644
index 00000000000..8b8f69a3a10
--- /dev/null
+++ b/enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationActions/ModerationActions.jsx
@@ -0,0 +1,35 @@
+import React from "react";
+import PropTypes from "prop-types";
+import { t } from "ttag";
+
+import { isItemVerified } from "metabase-enterprise/moderation/service";
+
+import { Container, Label, VerifyButton } from "./ModerationActions.styled";
+import Tooltip from "metabase/components/Tooltip";
+
+export default ModerationActions;
+
+ModerationActions.propTypes = {
+ className: PropTypes.string,
+ onVerify: PropTypes.func,
+ moderationReview: PropTypes.object,
+};
+
+function ModerationActions({ moderationReview, className, onVerify }) {
+ const isVerified = isItemVerified(moderationReview);
+ const hasActions = !!onVerify;
+
+ return hasActions ? (
+ <Container className={className}>
+ <Label>{t`Moderation`}</Label>
+ {!isVerified && (
+ <Tooltip tooltip={t`Verify this`}>
+ <VerifyButton
+ data-testid="moderation-verify-action"
+ onClick={onVerify}
+ />
+ </Tooltip>
+ )}
+ </Container>
+ ) : null;
+}
diff --git a/enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationActions/ModerationActions.styled.jsx b/enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationActions/ModerationActions.styled.jsx
new file mode 100644
index 00000000000..8c9af89d429
--- /dev/null
+++ b/enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationActions/ModerationActions.styled.jsx
@@ -0,0 +1,35 @@
+import styled from "styled-components";
+
+import { color } from "metabase/lib/colors";
+import { getVerifiedIcon } from "metabase-enterprise/moderation/service";
+
+const { icon: verifiedIcon, iconColor: verifiedIconColor } = getVerifiedIcon();
+
+import Button from "metabase/components/Button";
+
+export const Container = styled.div`
+ display: flex;
+ align-items: center;
+ justify-content: space-between;
+`;
+
+export const Label = styled.h5`
+ font-size: 14px;
+ color: ${color("text-medium")};
+ flex: 1;
+`;
+
+export const VerifyButton = styled(Button).attrs({
+ icon: verifiedIcon,
+ iconSize: 20,
+})`
+ border: none;
+
+ &:hover {
+ color: ${color(verifiedIconColor)};
+ }
+
+ &:disabled {
+ color: ${color("text-medium")};
+ }
+`;
diff --git a/enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationActions/ModerationActions.unit.spec.js b/enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationActions/ModerationActions.unit.spec.js
new file mode 100644
index 00000000000..2110349e783
--- /dev/null
+++ b/enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationActions/ModerationActions.unit.spec.js
@@ -0,0 +1,28 @@
+import React from "react";
+import ModerationActions from "./ModerationActions";
+import { render, screen } from "@testing-library/react";
+
+describe("ModerationActions", () => {
+ describe("when the user is not a moderator", () => {
+ it("should not render", () => {
+ const { queryByTestId } = render(
+ <ModerationActions isModerator={false} />,
+ );
+ expect(queryByTestId("moderation-verify-action")).toBeNull();
+ expect(screen.queryByText("Moderation")).toBeNull();
+ });
+ });
+
+ describe("when a moderator clicks on the verify button", () => {
+ it("should call the onVerify prop", () => {
+ const onVerify = jest.fn();
+ const { getByTestId } = render(
+ <ModerationActions isModerator onVerify={onVerify} />,
+ );
+
+ getByTestId("moderation-verify-action").click();
+
+ expect(onVerify).toHaveBeenCalled();
+ });
+ });
+});
diff --git a/enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationReviewBanner/ModerationReviewBanner.jsx b/enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationReviewBanner/ModerationReviewBanner.jsx
new file mode 100644
index 00000000000..3970f651728
--- /dev/null
+++ b/enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationReviewBanner/ModerationReviewBanner.jsx
@@ -0,0 +1,98 @@
+import React from "react";
+import PropTypes from "prop-types";
+import _ from "underscore";
+import { connect } from "react-redux";
+
+import { color, alpha } from "metabase/lib/colors";
+import { getUser } from "metabase/selectors/user";
+import { getRelativeTimeAbbreviated } from "metabase/lib/time";
+import {
+ getTextForReviewBanner,
+ getIconForReview,
+} from "metabase-enterprise/moderation/service";
+import User from "metabase/entities/users";
+
+import {
+ Container,
+ Text,
+ Time,
+ IconButton,
+ StatusIcon,
+} from "./ModerationReviewBanner.styled";
+import Tooltip from "metabase/components/Tooltip";
+
+const ICON_BUTTON_SIZE = 20;
+const TOOLTIP_X_OFFSET = ICON_BUTTON_SIZE / 4;
+
+const mapStateToProps = (state, props) => ({
+ currentUser: getUser(state),
+});
+
+export default _.compose(
+ User.load({
+ id: (state, props) => props.moderationReview.moderator_id,
+ loadingAndErrorWrapper: false,
+ }),
+ connect(mapStateToProps),
+)(ModerationReviewBanner);
+
+ModerationReviewBanner.propTypes = {
+ moderationReview: PropTypes.object.isRequired,
+ user: PropTypes.object,
+ currentUser: PropTypes.object.isRequired,
+ onRemove: PropTypes.func,
+};
+
+export function ModerationReviewBanner({
+ moderationReview,
+ user: moderator,
+ currentUser,
+ onRemove,
+}) {
+ const [isHovering, setIsHovering] = React.useState(false);
+ const [isActive, setIsActive] = React.useState(false);
+
+ const { bannerText, tooltipText } = getTextForReviewBanner(
+ moderationReview,
+ moderator,
+ currentUser,
+ );
+ const relativeCreationTime = getRelativeTimeAbbreviated(
+ moderationReview.created_at,
+ );
+ const { icon, iconColor } = getIconForReview(moderationReview);
+ const showClose = isHovering || isActive;
+
+ return (
+ <Container
+ backgroundColor={alpha(iconColor, 0.2)}
+ onMouseEnter={() => setIsHovering(true)}
+ onMouseLeave={() => setIsHovering(false)}
+ >
+ <Tooltip
+ targetOffsetX={TOOLTIP_X_OFFSET}
+ tooltip={onRemove && tooltipText}
+ >
+ {onRemove ? (
+ <IconButton
+ data-testid="moderation-remove-review-action"
+ onFocus={() => setIsActive(true)}
+ onBlur={() => setIsActive(false)}
+ icon={showClose ? "close" : icon}
+ color={color(showClose ? "text-medium" : iconColor)}
+ onClick={onRemove}
+ iconSize={ICON_BUTTON_SIZE}
+ />
+ ) : (
+ <StatusIcon
+ name={icon}
+ color={color(iconColor)}
+ size={ICON_BUTTON_SIZE}
+ />
+ )}
+ </Tooltip>
+ <Text>{bannerText}</Text>
+ <Time dateTime={moderationReview.created_at}>{relativeCreationTime}</Time>
+ </Container>
+ );
+}
diff --git a/enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationReviewBanner/ModerationReviewBanner.styled.jsx b/enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationReviewBanner/ModerationReviewBanner.styled.jsx
new file mode 100644
index 00000000000..60439334ad7
--- /dev/null
+++ b/enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationReviewBanner/ModerationReviewBanner.styled.jsx
@@ -0,0 +1,40 @@
+import styled from "styled-components";
+import { color } from "metabase/lib/colors";
+import Button from "metabase/components/Button";
+import Icon from "metabase/components/Icon";
+
+export const Container = styled.div`
+ padding: 1rem 1rem 1rem 0.5rem;
+ background-color: ${props => props.backgroundColor};
+ display: flex;
+ justify-content: space-between;
+ align-items: center;
+ column-gap: 0.5rem;
+ border-radius: 8px;
+`;
+
+export const Text = styled.span`
+ flex: 1;
+ font-size: 14px;
+ font-weight: 700;
+`;
+
+export const Time = styled.time`
+ color: ${color("text-medium")};
+ font-size: 12px;
+`;
+
+export const IconButton = styled(Button)`
+ padding: 0 0 0 0.5rem !important;
+ border: none;
+ background-color: transparent;
+
+ &:hover {
+ background-color: transparent;
+ color: ${color("danger")};
+ }
+`;
+
+export const StatusIcon = styled(Icon)`
+ padding: 0 0.5rem;
+`;
diff --git a/enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationReviewBanner/ModerationReviewBanner.unit.spec.js b/enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationReviewBanner/ModerationReviewBanner.unit.spec.js
new file mode 100644
index 00000000000..20507f7131a
--- /dev/null
+++ b/enterprise/frontend/src/metabase-enterprise/moderation/components/ModerationReviewBanner/ModerationReviewBanner.unit.spec.js
@@ -0,0 +1,104 @@
+import React from "react";
+import { ModerationReviewBanner } from "./ModerationReviewBanner";
+import { render, fireEvent } from "@testing-library/react";
+
+const VERIFIED_ICON_SELECTOR = ".Icon-verified";
+const CLOSE_ICON_SELECTOR = ".Icon-close";
+
+const moderationReview = {
+ status: "verified",
+ moderator_id: 1,
+ created_at: Date.now(),
+};
+const moderator = { id: 1, display_name: "Foo" };
+const currentUser = { id: 2, display_name: "Bar" };
+
+describe("ModerationReviewBanner", () => {
+ it("should show text concerning the given review", () => {
+ const { getByText } = render(
+ <ModerationReviewBanner
+ moderationReview={moderationReview}
+ user={moderator}
+ currentUser={currentUser}
+ />,
+ );
+ expect(getByText("Foo verified this")).toBeTruthy();
+ });
+
+ describe("when not provided an onRemove prop", () => {
+ let getByRole;
+ let container;
+ beforeEach(() => {
+ const wrapper = render(
+ <ModerationReviewBanner
+ moderationReview={moderationReview}
+ user={moderator}
+ currentUser={currentUser}
+ />,
+ );
+
+ getByRole = wrapper.getByRole;
+ container = wrapper.container;
+ });
+
+ it("should render a status icon, not a button", () => {
+ expect(() => getByRole("button")).toThrow();
+ });
+
+ it("should render with the icon relevant to the review's status", () => {
+ expect(container.querySelector(VERIFIED_ICON_SELECTOR)).toBeTruthy();
+ });
+ });
+
+ describe("when provided an onRemove callback prop", () => {
+ let onRemove;
+ let container;
+ let getByRole;
+ beforeEach(() => {
+ onRemove = jest.fn();
+ const wrapper = render(
+ <ModerationReviewBanner
+ moderationReview={moderationReview}
+ user={moderator}
+ currentUser={currentUser}
+ onRemove={onRemove}
+ />,
+ );
+
+ container = wrapper.container;
+ getByRole = wrapper.getByRole;
+ });
+
+ it("should render a button", () => {
+ expect(getByRole("button")).toBeTruthy();
+ });
+
+ it("should render the button with the icon relevant to the review's status", () => {
+ expect(container.querySelector(VERIFIED_ICON_SELECTOR)).toBeTruthy();
+ });
+
+ it("should render the button as a close icon when the user is hovering their mouse over the banner", () => {
+ const banner = container.firstChild;
+ fireEvent.mouseEnter(banner);
+ expect(container.querySelector(CLOSE_ICON_SELECTOR)).toBeTruthy();
+ fireEvent.mouseLeave(banner);
+ expect(container.querySelector(VERIFIED_ICON_SELECTOR)).toBeTruthy();
+ });
+
+ it("should render the button as a close icon when the user focuses the button", () => {
+ fireEvent.focus(getByRole("button"));
+ expect(container.querySelector(CLOSE_ICON_SELECTOR)).toBeTruthy();
+ fireEvent.blur(getByRole("button"));
+ expect(container.querySelector(VERIFIED_ICON_SELECTOR)).toBeTruthy();
+ });
+
+ it("should render the button as a close icon when focused, even when the mouse leaves the banner", () => {
+ const banner = container.firstChild;
+ fireEvent.mouseEnter(banner);
+ fireEvent.focus(getByRole("button"));
+ expect(container.querySelector(CLOSE_ICON_SELECTOR)).toBeTruthy();
+ fireEvent.mouseLeave(banner);
+ expect(container.querySelector(CLOSE_ICON_SELECTOR)).toBeTruthy();
+ });
+ });
+});
diff --git a/enterprise/frontend/src/metabase-enterprise/moderation/components/QuestionModerationSection/QuestionModerationSection.jsx b/enterprise/frontend/src/metabase-enterprise/moderation/components/QuestionModerationSection/QuestionModerationSection.jsx
new file mode 100644
index 00000000000..aafe91711f8
--- /dev/null
+++ b/enterprise/frontend/src/metabase-enterprise/moderation/components/QuestionModerationSection/QuestionModerationSection.jsx
@@ -0,0 +1,69 @@
+import React from "react";
+import PropTypes from "prop-types";
+import { connect } from "react-redux";
+
+import { getLatestModerationReview } from "metabase-enterprise/moderation/service";
+import { getIsModerator } from "metabase-enterprise/moderation/selectors";
+import {
+ verifyCard,
+ removeCardReview,
+} from "metabase-enterprise/moderation/actions";
+
+import { BorderedModerationActions } from "./QuestionModerationSection.styled";
+import ModerationReviewBanner from "../ModerationReviewBanner/ModerationReviewBanner";
+
+const mapStateToProps = (state, props) => ({
+ isModerator: getIsModerator(state, props),
+});
+const mapDispatchToProps = {
+ verifyCard,
+ removeCardReview,
+};
+
+export default connect(
+ mapStateToProps,
+ mapDispatchToProps,
+)(QuestionModerationSection);
+
+QuestionModerationSection.propTypes = {
+ question: PropTypes.object.isRequired,
+ verifyCard: PropTypes.func.isRequired,
+ removeCardReview: PropTypes.func.isRequired,
+ isModerator: PropTypes.bool.isRequired,
+};
+
+function QuestionModerationSection({
+ question,
+ verifyCard,
+ removeCardReview,
+ isModerator,
+}) {
+ const latestModerationReview = getLatestModerationReview(
+ question.getModerationReviews(),
+ );
+
+ const onVerify = () => {
+ const id = question.id();
+ verifyCard(id);
+ };
+
+ const onRemoveModerationReview = () => {
+ const id = question.id();
+ removeCardReview(id);
+ };
+
+ return (
+ <React.Fragment>
+ <BorderedModerationActions
+ moderationReview={latestModerationReview}
+ onVerify={isModerator && onVerify}
+ />
+ {latestModerationReview && (
+ <ModerationReviewBanner
+ moderationReview={latestModerationReview}
+ onRemove={isModerator && onRemoveModerationReview}
+ />
+ )}
+ </React.Fragment>
+ );
+}
diff --git a/enterprise/frontend/src/metabase-enterprise/moderation/components/QuestionModerationSection/QuestionModerationSection.styled.jsx b/enterprise/frontend/src/metabase-enterprise/moderation/components/QuestionModerationSection/QuestionModerationSection.styled.jsx
new file mode 100644
index 00000000000..373a7c28535
--- /dev/null
+++ b/enterprise/frontend/src/metabase-enterprise/moderation/components/QuestionModerationSection/QuestionModerationSection.styled.jsx
@@ -0,0 +1,8 @@
+import styled from "styled-components";
+import { color } from "metabase/lib/colors";
+import ModerationActions from "../ModerationActions/ModerationActions";
+
+export const BorderedModerationActions = styled(ModerationActions)`
+ border-top: 1px solid ${color("border")};
+ padding-top: 1rem;
+`;
diff --git a/enterprise/frontend/src/metabase-enterprise/moderation/constants.js b/enterprise/frontend/src/metabase-enterprise/moderation/constants.js
new file mode 100644
index 00000000000..9ba34d2bf42
--- /dev/null
+++ b/enterprise/frontend/src/metabase-enterprise/moderation/constants.js
@@ -0,0 +1,7 @@
+export const ACTIONS = {
+ verified: {
+ type: "verified",
+ icon: "verified",
+ color: "brand",
+ },
+};
diff --git a/enterprise/frontend/src/metabase-enterprise/moderation/index.js b/enterprise/frontend/src/metabase-enterprise/moderation/index.js
new file mode 100644
index 00000000000..0cef8f836be
--- /dev/null
+++ b/enterprise/frontend/src/metabase-enterprise/moderation/index.js
@@ -0,0 +1,9 @@
+import { PLUGIN_MODERATION } from "metabase/plugins";
+import QuestionModerationSection from "./components/QuestionModerationSection/QuestionModerationSection";
+
+import { getStatusIconForReviews } from "./service";
+
+Object.assign(PLUGIN_MODERATION, {
+ QuestionModerationSection,
+ getStatusIconForReviews,
+});
diff --git a/enterprise/frontend/src/metabase-enterprise/moderation/selectors.js b/enterprise/frontend/src/metabase-enterprise/moderation/selectors.js
new file mode 100644
index 00000000000..c0b8f577872
--- /dev/null
+++ b/enterprise/frontend/src/metabase-enterprise/moderation/selectors.js
@@ -0,0 +1,5 @@
+import { getUserIsAdmin } from "metabase/selectors/user";
+
+export const getIsModerator = (state, props) => {
+ return getUserIsAdmin(state, props);
+};
diff --git a/enterprise/frontend/src/metabase-enterprise/moderation/service.js b/enterprise/frontend/src/metabase-enterprise/moderation/service.js
new file mode 100644
index 00000000000..25530e0d633
--- /dev/null
+++ b/enterprise/frontend/src/metabase-enterprise/moderation/service.js
@@ -0,0 +1,87 @@
+import { t } from "ttag";
+import _ from "underscore";
+
+import { ModerationReviewApi } from "metabase/services";
+import { ACTIONS } from "./constants";
+
+export function verifyItem({ text, itemId, itemType }) {
+ return ModerationReviewApi.create({
+ status: "verified",
+ moderated_item_id: itemId,
+ moderated_item_type: itemType,
+ text,
+ });
+}
+
+export function removeReview({ itemId, itemType }) {
+ return ModerationReviewApi.create({
+ status: null,
+ moderated_item_id: itemId,
+ moderated_item_type: itemType,
+ });
+}
+
+export function getVerifiedIcon() {
+ const { icon, color } = ACTIONS["verified"];
+ return { icon, iconColor: color };
+}
+
+export function getIconForReview(review) {
+ if (review && review.status !== null) {
+ const { status } = review;
+ const { icon, color } = ACTIONS[status] || {};
+ return { icon, iconColor: color };
+ }
+
+ return {};
+}
+
+export function getTextForReviewBanner(
+ moderationReview,
+ moderator,
+ currentUser,
+) {
+ const moderatorName = getModeratorDisplayName(moderator, currentUser);
+ const { status } = moderationReview;
+
+ if (status === "verified") {
+ const bannerText = t`${moderatorName} verified this`;
+ const tooltipText = t`Remove verification`;
+ return { bannerText, tooltipText };
+ }
+
+ return {};
+}
+
+function getModeratorDisplayName(user, currentUser) {
+ const { id: userId, display_name } = user || {};
+ const { id: currentUserId } = currentUser || {};
+
+ if (currentUserId != null && userId === currentUserId) {
+ return t`You`;
+ } else if (userId != null) {
+ return display_name;
+ } else {
+ return t`A moderator`;
+ }
+}
+
+export function isItemVerified(review) {
+ return review != null && review.status === "verified";
+}
+
+export function getLatestModerationReview(reviews) {
+ const review = _.findWhere(reviews, {
+ most_recent: true,
+ });
+
+ // since we can't delete reviews, consider a most recent review with a status of null to mean there is no review
+ if (review && review.status !== null) {
+ return review;
+ }
+}
+
+export function getStatusIconForReviews(reviews) {
+ const review = getLatestModerationReview(reviews);
+ return getIconForReview(review);
+}
diff --git a/enterprise/frontend/src/metabase-enterprise/moderation/service.unit.spec.js b/enterprise/frontend/src/metabase-enterprise/moderation/service.unit.spec.js
new file mode 100644
index 00000000000..6fcf0dd9260
--- /dev/null
+++ b/enterprise/frontend/src/metabase-enterprise/moderation/service.unit.spec.js
@@ -0,0 +1,196 @@
+import {
+ verifyItem,
+ removeReview,
+ getVerifiedIcon,
+ getIconForReview,
+ getTextForReviewBanner,
+ isItemVerified,
+ getLatestModerationReview,
+ getStatusIconForReviews,
+} from "./service";
+
+jest.mock("metabase/services", () => ({
+ ModerationReviewApi: {
+ create: jest.fn(() => Promise.resolve({ id: 123 })),
+ },
+}));
+
+import { ModerationReviewApi } from "metabase/services";
+
+describe("moderation/service", () => {
+ beforeEach(() => {
+ jest.clearAllMocks();
+ });
+
+ describe("verifyItem", () => {
+ it("should create a new moderation review", async () => {
+ const review = await verifyItem({
+ itemId: 123,
+ itemType: "card",
+ text: "bar",
+ });
+
+ expect(ModerationReviewApi.create).toHaveBeenCalledWith({
+ status: "verified",
+ moderated_item_id: 123,
+ moderated_item_type: "card",
+ text: "bar",
+ });
+
+ expect(review).toEqual({ id: 123 });
+ });
+ });
+
+ describe("removeReview", () => {
+ it("should create a new moderation review with a null status", async () => {
+ const review = await removeReview({
+ itemId: 123,
+ itemType: "card",
+ });
+
+ expect(ModerationReviewApi.create).toHaveBeenCalledWith({
+ status: null,
+ moderated_item_id: 123,
+ moderated_item_type: "card",
+ });
+
+ expect(review).toEqual({ id: 123 });
+ });
+ });
+
+ describe("getVerifiedIcon", () => {
+ it("should return verified icon name/color", () => {
+ expect(getVerifiedIcon()).toEqual({
+ icon: "verified",
+ iconColor: "brand",
+ });
+ });
+ });
+
+ describe("getIconForReview", () => {
+ it("should return icon name/color for given review", () => {
+ expect(getIconForReview({ status: "verified" })).toEqual(
+ getVerifiedIcon(),
+ );
+ });
+
+ it("should be an empty object for a null review", () => {
+ expect(getIconForReview({ status: null })).toEqual({});
+ });
+ });
+
+ describe("getTextForReviewBanner", () => {
+ it("should return text for a verified review", () => {
+ expect(getTextForReviewBanner({ status: "verified" })).toEqual({
+ bannerText: "A moderator verified this",
+ tooltipText: "Remove verification",
+ });
+ });
+
+ it("should include the moderator name", () => {
+ expect(
+ getTextForReviewBanner(
+ { status: "verified" },
+ {
+ display_name: "Foo",
+ id: 1,
+ },
+ { id: 2 },
+ ),
+ ).toEqual({
+ bannerText: "Foo verified this",
+ tooltipText: "Remove verification",
+ });
+ });
+
+ it("should handle the moderator being the current user", () => {
+ expect(
+ getTextForReviewBanner(
+ { status: "verified" },
+ {
+ display_name: "Foo",
+ id: 1,
+ },
+ { id: 1 },
+ ),
+ ).toEqual({
+ bannerText: "You verified this",
+ tooltipText: "Remove verification",
+ });
+ });
+ });
+
+ describe("isItemVerified", () => {
+ it("should return true for a verified review", () => {
+ expect(isItemVerified({ status: "verified" })).toBe(true);
+ });
+
+ it("should return false for a null review", () => {
+ expect(isItemVerified({ status: null })).toBe(false);
+ });
+
+ it("should return false for no review", () => {
+ expect(isItemVerified()).toBe(false);
+ });
+ });
+
+ describe("getLatestModerationReview", () => {
+ it("should return the review flagged as most recent", () => {
+ const reviews = [
+ { id: 1, status: "verified" },
+ { id: 2, status: "verified", most_recent: true },
+ { id: 3, status: null },
+ ];
+
+ expect(getLatestModerationReview(reviews)).toEqual({
+ id: 2,
+ status: "verified",
+ most_recent: true,
+ });
+ });
+
+ it("should return undefined when there is no review flagged as most recent", () => {
+ const reviews = [
+ { id: 1, status: "verified" },
+ { id: 2, status: "verified" },
+ { id: 3, status: null },
+ ];
+
+ expect(getLatestModerationReview(reviews)).toEqual(undefined);
+ expect(getLatestModerationReview([])).toEqual(undefined);
+ });
+
+ it("should return undefined when there is a review with a status of null flagged as most recent", () => {
+ const reviews = [
+ { id: 1, status: "verified" },
+ { id: 2, status: "verified" },
+ { id: 3, status: null, most_recent: true },
+ ];
+
+ expect(getLatestModerationReview(reviews)).toEqual(undefined);
+ });
+ });
+});
+
+describe("getStatusIconForReviews", () => {
+ 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 },
+ ];
+
+ expect(getStatusIconForReviews(reviews)).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);
+ });
+});
diff --git a/enterprise/frontend/src/metabase-enterprise/plugins.js b/enterprise/frontend/src/metabase-enterprise/plugins.js
index b0e9c0a8246..3adb1cc98b7 100644
--- a/enterprise/frontend/src/metabase-enterprise/plugins.js
+++ b/enterprise/frontend/src/metabase-enterprise/plugins.js
@@ -18,3 +18,4 @@ import "./embedding";
import "./store";
import "./snippets";
import "./sharing";
+import "./moderation";
diff --git a/frontend/src/metabase-lib/lib/Question.js b/frontend/src/metabase-lib/lib/Question.js
index f90c67333b2..aa7146ee499 100644
--- a/frontend/src/metabase-lib/lib/Question.js
+++ b/frontend/src/metabase-lib/lib/Question.js
@@ -1,5 +1,5 @@
import _ from "underscore";
-import { chain, assoc, dissoc, assocIn } from "icepick";
+import { chain, assoc, dissoc, assocIn, getIn } from "icepick";
// NOTE: the order of these matters due to circular dependency issues
import StructuredQuery, {
@@ -1045,6 +1045,10 @@ export default class Question {
const query = this.isNative() ? this._parameterValues : undefined;
return question.getUrl({ originalQuestion: this, query });
}
+
+ getModerationReviews() {
+ return getIn(this, ["_card", "moderation_reviews"]) || [];
+ }
}
window.Question = Question;
diff --git a/frontend/src/metabase/icon_paths.js b/frontend/src/metabase/icon_paths.js
index f92c84a8962..edb87238105 100644
--- a/frontend/src/metabase/icon_paths.js
+++ b/frontend/src/metabase/icon_paths.js
@@ -100,10 +100,6 @@ export const ICON_PATHS = {
chevronleft: "M20 1 L24 5 L14 16 L24 27 L20 31 L6 16 z",
chevronright: "M12 1 L26 16 L12 31 L8 27 L18 16 L8 5 z ",
chevronup: "M1 20 L16 6 L31 20 L27 24 L16 14 L5 24 z",
- clarification: {
- svg:
- '<rect width="32" height="32" rx="16" fill="#E1D9E8"/><path d="M11.2 8.307a9.244 9.244 0 0 1 1.024-.742c.375-.23.777-.431 1.203-.602.427-.179.883-.316 1.37-.41a7.894 7.894 0 0 1 1.6-.153c.802 0 1.523.107 2.163.32.649.213 1.199.52 1.651.922.452.392.798.87 1.037 1.433.247.555.371 1.174.371 1.856 0 .649-.09 1.212-.269 1.69-.179.47-.405.879-.678 1.229-.273.35-.576.652-.909.908a40.2 40.2 0 0 1-.934.717c-.29.214-.546.427-.768.64a1.46 1.46 0 0 0-.41.717l-.358 1.792h-2.714l-.281-2.06c-.069-.419-.026-.782.128-1.089.153-.316.362-.597.627-.845.273-.256.576-.495.909-.716.332-.23.644-.474.934-.73a4.12 4.12 0 0 0 .73-.87c.204-.325.307-.709.307-1.152 0-.512-.17-.918-.512-1.216-.333-.308-.794-.461-1.383-.461-.452 0-.832.047-1.139.14-.298.094-.559.201-.78.32-.214.112-.402.214-.564.308a.963.963 0 0 1-.486.14.881.881 0 0 1-.82-.473L11.2 8.307zm2.355 14.938c0-.307.056-.593.167-.858.119-.264.277-.495.473-.691.205-.196.444-.35.717-.46.273-.12.572-.18.896-.18.316 0 .61.06.883.18.273.11.512.264.717.46.205.196.363.427.474.691.119.265.179.55.179.858 0 .307-.06.597-.18.87a2.038 2.038 0 0 1-.473.691c-.205.197-.444.35-.717.461a2.323 2.323 0 0 1-.883.167c-.324 0-.623-.056-.896-.167a2.245 2.245 0 0 1-.717-.46 2.31 2.31 0 0 1-.473-.692 2.289 2.289 0 0 1-.167-.87z" fill="#A989C5"/>',
- },
click:
"M5.38519 1C2.41104 1 0 3.41103 0 6.38519V26.779C0 29.7532 2.41103 32.1642 5.38519 32.1642H13.3818V27.7911H5.38519C4.82624 27.7911 4.37313 27.338 4.37313 26.779V6.38519C4.37313 5.82624 4.82624 5.37313 5.38519 5.37313H22.779C23.338 5.37313 23.7911 5.82624 23.7911 6.38519V7.98451H28.1642V6.38519C28.1642 3.41103 25.7532 1 22.779 1H5.38519ZM12 10.6L17.5213 29.1906L21.8777 24.8341L27.6436 30.6L32 26.2436L26.2341 20.4777L30.5906 16.1213L12 10.6Z",
clipboard:
@@ -396,7 +392,7 @@ export const ICON_PATHS = {
"M32,3.85760518 C32,5.35923081 31.5210404,6.55447236 30.5631068,7.4433657 C29.6051732,8.33225903 28.4358214,8.77669903 27.0550162,8.77669903 C26.2265331,8.77669903 25.4110072,8.67314019 24.6084142,8.46601942 C23.8058212,8.25889864 23.111114,8.05178097 22.5242718,7.84466019 C22.2481108,8.03452091 21.8425054,8.44875625 21.3074434,9.08737864 C20.7723814,9.72600104 20.1682882,10.5026923 19.4951456,11.4174757 C20.116508,14.0582656 20.6170423,15.9352695 20.9967638,17.0485437 C21.3764852,18.1618179 21.7389411,19.2880202 22.0841424,20.4271845 C22.3775635,21.3419679 22.8090586,22.0582498 23.3786408,22.5760518 C23.9482229,23.0938537 24.8457328,23.3527508 26.0711974,23.3527508 C26.5199591,23.3527508 27.0809028,23.2664518 27.7540453,23.0938511 C28.4271878,22.9212505 28.9795016,22.7486524 29.4110032,22.5760518 L28.8414239,24.9061489 C27.1326775,25.6310716 25.6397043,26.1574957 24.3624595,26.4854369 C23.0852148,26.8133781 21.9460676,26.9773463 20.9449838,26.9773463 C20.2200611,26.9773463 19.5037792,26.9083071 18.7961165,26.7702265 C18.0884539,26.632146 17.4412111,26.3818788 16.8543689,26.0194175 C16.2157465,25.6396961 15.6763776,25.1650514 15.236246,24.5954693 C14.7961143,24.0258871 14.4207135,23.2319361 14.1100324,22.2135922 C13.9029116,21.5749698 13.7130537,20.850058 13.5404531,20.038835 C13.3678524,19.2276119 13.1952544,18.51133 13.0226537,17.8899676 C12.5221118,18.6321504 12.1596559,19.1844642 11.9352751,19.5469256 C11.7108942,19.9093869 11.3829579,20.4185512 10.9514563,21.0744337 C9.5879112,23.1629015 8.4056145,24.6515597 7.40453074,25.5404531 C6.40344699,26.4293464 5.20389049,26.8737864 3.80582524,26.8737864 C2.75296129,26.8737864 1.85545139,26.5199604 1.11326861,25.8122977 C0.371085825,25.1046351 0,24.1812355 0,23.0420712 C0,21.5059254 0.478959612,20.2934241 1.4368932,19.4045307 C2.3948268,18.5156374 3.56417864,18.0711974 4.94498382,18.0711974 C5.77346693,18.0711974 6.56741799,18.1704413 7.32686084,18.368932 C8.08630369,18.5674228 8.80258563,18.7874853 9.47572816,19.0291262 C9.73462913,18.8220054 10.1359196,18.4164 10.6796117,17.8122977 C11.2233037,17.2081955 11.814452,16.4573939 12.4530744,15.5598706 C11.8834923,13.2470219 11.4174775,11.5037815 11.0550162,10.3300971 C10.6925548,9.15641269 10.321469,7.99137579 9.94174757,6.83495146 C9.63106641,5.90290796 9.18231146,5.18231107 8.59546926,4.67313916 C8.00862706,4.16396725 7.12837696,3.90938511 5.95469256,3.90938511 C5.43689061,3.90938511 4.85868712,3.99999909 4.22006472,4.18122977 C3.58144233,4.36246045 3.04638835,4.53074356 2.61488673,4.68608414 L3.18446602,2.35598706 C4.73787184,1.66558447 6.20927029,1.14779029 7.5987055,0.802588997 C8.98814071,0.457387702 10.1488627,0.284789644 11.0809061,0.284789644 C11.9266493,0.284789644 12.6515612,0.345198964 13.2556634,0.466019417 C13.8597657,0.586839871 14.4983785,0.845736958 15.171521,1.24271845 C15.7928834,1.62243987 16.3322523,2.10139948 16.789644,2.67961165 C17.2470357,3.25782382 17.6224365,4.04745994 17.9158576,5.04854369 C18.1229784,5.73894628 18.3128362,6.45522822 18.4854369,7.197411 C18.6580375,7.93959379 18.8047459,8.5782066 18.9255663,9.11326861 C19.2880277,8.56094654 19.6634285,7.99137294 20.0517799,7.40453074 C20.4401314,6.81768854 20.7723827,6.29989437 21.0485437,5.85113269 C22.3775687,3.76266485 23.5684953,2.2653767 24.6213592,1.3592233 C25.6742232,0.453069903 26.8651498,0 28.1941748,0 C29.2815588,0 30.1876986,0.358140971 30.9126214,1.07443366 C31.6375441,1.79072634 32,2.71844091 32,3.85760518 L32,3.85760518 Z",
verified: {
svg:
- '<path fill-rule="evenodd" clip-rule="evenodd" d="M32 16.64a6.397 6.397 0 0 0-3.15-5.514 6.4 6.4 0 0 0-7.42-8.115A6.396 6.396 0 0 0 16 0a6.396 6.396 0 0 0-5.43 3.01 6.4 6.4 0 0 0-7.42 8.115A6.397 6.397 0 0 0 0 16.64c0 2.25 1.162 4.23 2.919 5.371a6.4 6.4 0 0 0 7.652 6.979A6.396 6.396 0 0 0 16 32a6.396 6.396 0 0 0 5.429-3.01 6.4 6.4 0 0 0 7.652-6.979A6.395 6.395 0 0 0 32 16.64z" fill="#D3E7F8"/><path fill-rule="evenodd" clip-rule="evenodd" d="M8.32 16.852l2.392-2.485 3.649 3.769 7.312-7.576 2.4 2.584-9.71 10.02-6.043-6.312z" fill="#509EE3"/>',
+ '<path fill-rule="evenodd" clip-rule="evenodd" d="M32 16.64a6.397 6.397 0 0 0-3.15-5.514 6.4 6.4 0 0 0-7.42-8.115A6.396 6.396 0 0 0 16 0a6.396 6.396 0 0 0-5.43 3.01 6.4 6.4 0 0 0-7.42 8.115A6.397 6.397 0 0 0 0 16.64c0 2.25 1.162 4.23 2.919 5.371a6.4 6.4 0 0 0 7.652 6.979A6.396 6.396 0 0 0 16 32a6.396 6.396 0 0 0 5.429-3.01 6.4 6.4 0 0 0 7.652-6.979A6.395 6.395 0 0 0 32 16.64z M8.32 16.852l2.392-2.485 3.649 3.769 7.312-7.576 2.4 2.584-9.71 10.02-6.043-6.312z" />',
},
view_archive: {
path:
@@ -408,10 +404,6 @@ export const ICON_PATHS = {
"M12.3069589,4.52260192 C14.3462632,1.2440969 17.653446,1.24541073 19.691933,4.52260192 L31.2249413,23.0637415 C33.2642456,26.3422466 31.7889628,29 27.9115531,29 L4.08733885,29 C0.218100769,29 -1.26453645,26.3409327 0.77395061,23.0637415 L12.3069589,4.52260192 Z M18.0499318,23.0163223 C18.0499772,23.0222378 18.05,23.0281606 18.05,23.0340907 C18.05,23.3266209 17.9947172,23.6030345 17.8840476,23.8612637 C17.7737568,24.1186089 17.6195847,24.3426723 17.4224081,24.5316332 C17.2266259,24.7192578 16.998292,24.8660439 16.7389806,24.9713892 C16.4782454,25.0773129 16.1979962,25.1301134 15.9,25.1301134 C15.5950083,25.1301134 15.3111795,25.0774024 15.0502239,24.9713892 C14.7901813,24.8657469 14.5629613,24.7183609 14.3703047,24.5298034 C14.177545,24.3411449 14.0258626,24.1177208 13.9159524,23.8612637 C13.8052827,23.6030345 13.75,23.3266209 13.75,23.0340907 C13.75,22.7411889 13.8054281,22.4661013 13.9165299,22.2109786 C14.0264627,21.9585404 14.1779817,21.7374046 14.3703047,21.5491736 C14.5621821,21.3613786 14.7883231,21.2126553 15.047143,21.1034656 C15.3089445,20.9930181 15.593871,20.938068 15.9,20.938068 C16.1991423,20.938068 16.4804862,20.9931136 16.7420615,21.1034656 C17.0001525,21.2123478 17.2274115,21.360472 17.4224081,21.5473437 C17.6191428,21.7358811 17.7731504,21.957652 17.88347,22.2109786 C17.9124619,22.2775526 17.9376628,22.3454862 17.9590769,22.414741 C18.0181943,22.5998533 18.05,22.7963729 18.05,23 C18.05,23.0054459 18.0499772,23.0108867 18.0499318,23.0163223 L18.0499318,23.0163223 Z M17.7477272,14.1749999 L17.7477272,8.75 L14.1170454,8.75 L14.1170454,14.1749999 C14.1170454,14.8471841 14.1572355,15.5139742 14.2376219,16.1753351 C14.3174838,16.8323805 14.4227217,17.5019113 14.5533248,18.1839498 L14.5921937,18.3869317 L17.272579,18.3869317 L17.3114479,18.1839498 C17.442051,17.5019113 17.5472889,16.8323805 17.6271507,16.1753351 C17.7075371,15.5139742 17.7477272,14.8471841 17.7477272,14.1749999 Z",
attrs: { fillRule: "evenodd" },
},
- warning_colorized: {
- svg:
- '<path fill-rule="evenodd" clip-rule="evenodd" d="M11.764 4.36c1.957-3.146 6.535-3.146 8.492 0l11.002 17.687c2.072 3.33-.323 7.64-4.246 7.64H5.008c-3.922 0-6.317-4.31-4.246-7.64L11.765 4.359zm6.414 19.002v.019c0 .309-.059.601-.175.874-.117.272-.28.509-.488.709a2.269 2.269 0 0 1-.723.465 2.336 2.336 0 0 1-.887.168c-.322 0-.623-.056-.898-.168a2.179 2.179 0 0 1-1.2-1.174 2.199 2.199 0 0 1-.175-.874c0-.31.059-.601.176-.87.116-.268.276-.501.48-.7a2.279 2.279 0 0 1 1.617-.646c.316 0 .614.058.89.174.273.116.513.272.72.47a2.153 2.153 0 0 1 .663 1.535v.018zm-.32-15.083v5.735c0 .711-.042 1.416-.127 2.115a31.535 31.535 0 0 1-.334 2.124l-.041.214h-2.834l-.04-.214a31.537 31.537 0 0 1-.335-2.124c-.085-.699-.127-1.404-.127-2.115V8.28h3.838z" fill="#F2A86F"/>',
- },
waterfall: {
path: "M12 0h8v13h-8V0zM0 13h8v19H0V13zM32 0h-8v21h8V0z",
attrs: { fillRule: "evenodd" },
diff --git a/frontend/src/metabase/lib/time.js b/frontend/src/metabase/lib/time.js
index 584980ecc5e..0fc903bbaec 100644
--- a/frontend/src/metabase/lib/time.js
+++ b/frontend/src/metabase/lib/time.js
@@ -1,6 +1,38 @@
import moment from "moment-timezone";
import { t } from "ttag";
+addAbbreviatedLocale();
+
+// when you define a custom locale, moment automatically makes it the active global locale,
+// so we need to return to the user's initial locale.
+// also, you can't define a custom locale on a local instance
+function addAbbreviatedLocale() {
+ const initialLocale = moment.locale();
+
+ moment.locale("en-abbreviated", {
+ relativeTime: {
+ future: "in %s",
+ past: "%s",
+ s: "%d s",
+ ss: "%d s",
+ m: "%d m",
+ mm: "%d m",
+ h: "%d h",
+ hh: "%d h",
+ d: "%d d",
+ dd: "%d d",
+ w: "%d wk",
+ ww: "%d wks",
+ M: "a mth",
+ MM: "%d mths",
+ y: "%d y",
+ yy: "%d y",
+ },
+ });
+
+ moment.locale(initialLocale);
+}
+
const NUMERIC_UNIT_FORMATS = {
// workaround for https://github.com/metabase/metabase/issues/1992
year: value =>
@@ -123,3 +155,15 @@ export function formatFrame(frame) {
export function getRelativeTime(timestamp) {
return moment(timestamp).fromNow();
}
+
+export function getRelativeTimeAbbreviated(timestamp) {
+ const locale = moment().locale();
+
+ if (locale === "en") {
+ const ts = moment(timestamp);
+ ts.locale("en-abbreviated");
+ return ts.fromNow();
+ }
+
+ return getRelativeTime(timestamp);
+}
diff --git a/frontend/src/metabase/plugins/index.js b/frontend/src/metabase/plugins/index.js
index a4e309b4dd2..70530ab1f32 100644
--- a/frontend/src/metabase/plugins/index.js
+++ b/frontend/src/metabase/plugins/index.js
@@ -1,6 +1,8 @@
import { t } from "ttag";
import PluginPlaceholder from "metabase/plugins/components/PluginPlaceholder";
+
// Plugin integration points. All exports must be objects or arrays so they can be mutated by plugins.
+const object = () => ({});
// functions called when the application is started
export const PLUGIN_APP_INIT_FUCTIONS = [];
@@ -76,3 +78,8 @@ export const PLUGIN_COLLECTIONS = {
export const PLUGIN_COLLECTION_COMPONENTS = {
CollectionAuthorityLevelIcon: PluginPlaceholder,
};
+
+export const PLUGIN_MODERATION = {
+ QuestionModerationSection: PluginPlaceholder,
+ getStatusIconForReviews: object,
+};
diff --git a/frontend/src/metabase/query_builder/components/SavedQuestionHeaderButton.jsx b/frontend/src/metabase/query_builder/components/SavedQuestionHeaderButton/SavedQuestionHeaderButton.jsx
similarity index 70%
rename from frontend/src/metabase/query_builder/components/SavedQuestionHeaderButton.jsx
rename to frontend/src/metabase/query_builder/components/SavedQuestionHeaderButton/SavedQuestionHeaderButton.jsx
index 8fbd9bc82d6..ba7bba3f97d 100644
--- a/frontend/src/metabase/query_builder/components/SavedQuestionHeaderButton.jsx
+++ b/frontend/src/metabase/query_builder/components/SavedQuestionHeaderButton/SavedQuestionHeaderButton.jsx
@@ -1,8 +1,11 @@
import React from "react";
import PropTypes from "prop-types";
+import { PLUGIN_MODERATION } from "metabase/plugins";
import { HeaderButton } from "./SavedQuestionHeaderButton.styled";
+const { getStatusIconForReviews } = PLUGIN_MODERATION;
+
export default SavedQuestionHeaderButton;
SavedQuestionHeaderButton.propTypes = {
@@ -13,11 +16,18 @@ SavedQuestionHeaderButton.propTypes = {
};
function SavedQuestionHeaderButton({ className, question, onClick, isActive }) {
+ const {
+ icon: reviewIcon,
+ iconColor: reviewIconColor,
+ } = getStatusIconForReviews(question.getModerationReviews());
+
return (
<HeaderButton
className={className}
onClick={onClick}
iconRight="chevrondown"
+ icon={reviewIcon}
+ leftIconColor={reviewIconColor}
isActive={isActive}
iconSize={20}
data-testid="saved-question-header-button"
diff --git a/frontend/src/metabase/query_builder/components/SavedQuestionHeaderButton.styled.jsx b/frontend/src/metabase/query_builder/components/SavedQuestionHeaderButton/SavedQuestionHeaderButton.styled.jsx
similarity index 83%
rename from frontend/src/metabase/query_builder/components/SavedQuestionHeaderButton.styled.jsx
rename to frontend/src/metabase/query_builder/components/SavedQuestionHeaderButton/SavedQuestionHeaderButton.styled.jsx
index aa484586f90..9e67e5ac806 100644
--- a/frontend/src/metabase/query_builder/components/SavedQuestionHeaderButton.styled.jsx
+++ b/frontend/src/metabase/query_builder/components/SavedQuestionHeaderButton/SavedQuestionHeaderButton.styled.jsx
@@ -10,6 +10,10 @@ export const HeaderButton = styled(Button)`
color: ${props => (props.isActive ? color("brand") : "unset")};
background-color: ${props => (props.isActive ? color("bg-light") : "unset")};
+ .Icon:not(.Icon-chevrondown) {
+ color: ${props => color(props.leftIconColor)};
+ }
+
.Icon-chevrondown {
height: 13px;
}
diff --git a/frontend/src/metabase/query_builder/components/SavedQuestionHeaderButton/SavedQuestionHeaderButton.unit.spec.js b/frontend/src/metabase/query_builder/components/SavedQuestionHeaderButton/SavedQuestionHeaderButton.unit.spec.js
new file mode 100644
index 00000000000..592819dfd06
--- /dev/null
+++ b/frontend/src/metabase/query_builder/components/SavedQuestionHeaderButton/SavedQuestionHeaderButton.unit.spec.js
@@ -0,0 +1,75 @@
+import React from "react";
+
+import "@testing-library/jest-dom/extend-expect";
+import { render, screen } from "@testing-library/react";
+
+import SavedQuestionHeaderButton from "./SavedQuestionHeaderButton";
+
+describe("SavedQuestionHeaderButton", () => {
+ let onClick;
+ let question;
+ let componentContainer;
+
+ beforeEach(() => {
+ onClick = jest.fn();
+ question = {
+ displayName: () => "foo",
+ getModerationReviews: () => [],
+ };
+
+ const { container } = render(
+ <SavedQuestionHeaderButton
+ question={question}
+ onClick={onClick}
+ isActive={false}
+ />,
+ );
+
+ componentContainer = container;
+ });
+
+ it("renders the name of the question", () => {
+ expect(screen.getByText("foo")).toBeInTheDocument();
+ });
+
+ it("is clickable", () => {
+ screen.getByText("foo").click();
+ expect(onClick).toHaveBeenCalled();
+ });
+
+ describe("when the question does not have a latest moderation review", () => {
+ it("should contain no additional icons", () => {
+ expect(
+ componentContainer.querySelector(".Icon:not(.Icon-chevrondown)"),
+ ).toEqual(null);
+ });
+ });
+
+ describe("when the question has a latest moderation review", () => {
+ beforeEach(() => {
+ question = {
+ displayName: () => "foo",
+ getModerationReviews: () => [
+ { status: null },
+ { most_recent: true, status: "verified" },
+ ],
+ };
+
+ const { container } = render(
+ <SavedQuestionHeaderButton
+ question={question}
+ onClick={onClick}
+ isActive={false}
+ />,
+ );
+
+ componentContainer = container;
+ });
+
+ it("should have an additional icon to signify the question's moderation status", () => {
+ expect(
+ componentContainer.querySelector(".Icon:not(.Icon-chevrondown)"),
+ ).toBeDefined();
+ });
+ });
+});
diff --git a/frontend/src/metabase/query_builder/components/view/ViewHeader.jsx b/frontend/src/metabase/query_builder/components/view/ViewHeader.jsx
index bcec8107ab8..816dc2a2049 100644
--- a/frontend/src/metabase/query_builder/components/view/ViewHeader.jsx
+++ b/frontend/src/metabase/query_builder/components/view/ViewHeader.jsx
@@ -8,7 +8,7 @@ import Link from "metabase/components/Link";
import ButtonBar from "metabase/components/ButtonBar";
import CollectionBadge from "metabase/questions/components/CollectionBadge";
import LastEditInfoLabel from "metabase/components/LastEditInfoLabel";
-import SavedQuestionHeaderButton from "metabase/query_builder/components/SavedQuestionHeaderButton";
+import SavedQuestionHeaderButton from "metabase/query_builder/components/SavedQuestionHeaderButton/SavedQuestionHeaderButton";
import ViewSection, { ViewHeading, ViewSubHeading } from "./ViewSection";
import ViewButton from "metabase/query_builder/components/view/ViewButton";
diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionDetailsSidebarPanel.jsx b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionDetailsSidebarPanel.jsx
index e19f4d883b3..f391cf6a947 100644
--- a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionDetailsSidebarPanel.jsx
+++ b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionDetailsSidebarPanel.jsx
@@ -3,35 +3,52 @@ import PropTypes from "prop-types";
import SidebarContent from "metabase/query_builder/components/SidebarContent";
import QuestionActionButtons from "metabase/query_builder/components/QuestionActionButtons";
-import QuestionActivityTimeline from "metabase/query_builder/components/QuestionActivityTimeline";
import { ClampedDescription } from "metabase/query_builder/components/ClampedDescription";
-import { SidebarContentContainer } from "./QuestionDetailsSidebarPanel.styled";
+import {
+ SidebarContentContainer,
+ BorderedQuestionActivityTimeline,
+} from "./QuestionDetailsSidebarPanel.styled";
+import { PLUGIN_MODERATION } from "metabase/plugins";
+
+const { QuestionModerationSection } = PLUGIN_MODERATION;
+
+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 }) {
+function QuestionDetailsSidebarPanel({
+ question,
+ onOpenModal,
+ moderatorVerifyCard,
+ removeModerationReview,
+}) {
const canWrite = question.canWrite();
const description = question.description();
+ const onDescriptionEdit = canWrite
+ ? () => {
+ onOpenModal("edit");
+ }
+ : undefined;
+
return (
<SidebarContent>
<SidebarContentContainer>
<QuestionActionButtons canWrite={canWrite} onOpenModal={onOpenModal} />
<ClampedDescription
- description={description}
+ className="pb2"
visibleLines={8}
- onEdit={canWrite && (() => onOpenModal("edit"))}
- />
- <QuestionActivityTimeline
- className="border-top mt2 pt4"
- question={question}
+ description={description}
+ onEdit={onDescriptionEdit}
/>
+ <QuestionModerationSection question={question} />
+ <BorderedQuestionActivityTimeline question={question} />
</SidebarContentContainer>
</SidebarContent>
);
}
-
-export default QuestionDetailsSidebarPanel;
diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionDetailsSidebarPanel.styled.jsx b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionDetailsSidebarPanel.styled.jsx
index 38ad86d45b9..93bef16ff65 100644
--- a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionDetailsSidebarPanel.styled.jsx
+++ b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionDetailsSidebarPanel.styled.jsx
@@ -1,4 +1,6 @@
import styled from "styled-components";
+import { color } from "metabase/lib/colors";
+import QuestionActivityTimeline from "metabase/query_builder/components/QuestionActivityTimeline";
export const SidebarContentContainer = styled.div`
display: flex;
@@ -6,3 +8,14 @@ export const SidebarContentContainer = styled.div`
row-gap: 1rem;
padding: 0.5rem 1.5rem;
`;
+
+export const PanelSection = styled.div`
+ border-top: 1px solid ${color("border")};
+`;
+
+export const BorderedQuestionActivityTimeline = styled(
+ QuestionActivityTimeline,
+)`
+ border-top: 1px solid ${color("border")};
+ padding-top: 1rem;
+`;
diff --git a/frontend/src/metabase/selectors/user.unit.spec.js b/frontend/src/metabase/selectors/user.unit.spec.js
new file mode 100644
index 00000000000..9bde3e13bdb
--- /dev/null
+++ b/frontend/src/metabase/selectors/user.unit.spec.js
@@ -0,0 +1,23 @@
+import { getUserIsAdmin } from "./user";
+
+describe("metabase/selectors/user", () => {
+ it("should return true if user is an admin", () => {
+ const state = {
+ currentUser: {
+ is_superuser: true,
+ },
+ };
+
+ expect(getUserIsAdmin(state)).toBe(true);
+ });
+
+ it("should return false if user is not an admin", () => {
+ const state = {
+ currentUser: {
+ is_superuser: false,
+ },
+ };
+
+ expect(getUserIsAdmin(state)).toBe(false);
+ });
+});
diff --git a/frontend/src/metabase/services.js b/frontend/src/metabase/services.js
index a6f881d8d56..1580a3a592c 100644
--- a/frontend/src/metabase/services.js
+++ b/frontend/src/metabase/services.js
@@ -307,6 +307,11 @@ export const MetabaseApi = {
}),
};
+export const ModerationReviewApi = {
+ create: POST("/api/moderation-review"),
+ update: PUT("/api/moderation-review/:id"),
+};
+
export const PulseApi = {
list: GET("/api/pulse"),
create: POST("/api/pulse"),
diff --git a/frontend/test/metabase/query_builder/components/SavedQuestionHeaderButton.unit.spec.js b/frontend/test/metabase/query_builder/components/SavedQuestionHeaderButton.unit.spec.js
deleted file mode 100644
index 92c13bdae6a..00000000000
--- a/frontend/test/metabase/query_builder/components/SavedQuestionHeaderButton.unit.spec.js
+++ /dev/null
@@ -1,34 +0,0 @@
-import React from "react";
-
-import "@testing-library/jest-dom/extend-expect";
-import { render, screen } from "@testing-library/react";
-
-import SavedQuestionHeaderButton from "metabase/query_builder/components/SavedQuestionHeaderButton";
-
-describe("SavedQuestionHeaderButton", () => {
- let onClick;
- let question;
- beforeEach(() => {
- onClick = jest.fn();
- question = {
- displayName: () => "foo",
- };
-
- render(
- <SavedQuestionHeaderButton
- question={question}
- onClick={onClick}
- isActive={false}
- />,
- );
- });
-
- it("renders the name of the question", () => {
- expect(screen.getByText("foo")).toBeInTheDocument();
- });
-
- it("is clickable", () => {
- screen.getByText("foo").click();
- expect(onClick).toHaveBeenCalled();
- });
-});
diff --git a/frontend/test/metabase/scenarios/question/moderation.cy.spec.js b/frontend/test/metabase/scenarios/question/moderation.cy.spec.js
new file mode 100644
index 00000000000..211bde4944d
--- /dev/null
+++ b/frontend/test/metabase/scenarios/question/moderation.cy.spec.js
@@ -0,0 +1,70 @@
+import { describeWithToken, restore } from "__support__/e2e/cypress";
+
+describeWithToken("scenarios > saved question moderation", () => {
+ describe("as an admin", () => {
+ beforeEach(() => {
+ restore();
+ cy.signInAsAdmin();
+
+ cy.visit("/question/1");
+ });
+
+ it("should be able to verify a saved question", () => {
+ cy.findByTestId("saved-question-header-button").click();
+
+ cy.findByTestId("moderation-verify-action").click();
+
+ cy.findByText("You verified this").should("be.visible");
+
+ cy.findByTestId("saved-question-header-button").click();
+ cy.icon("verified").should("be.visible");
+ });
+
+ it("should be able to unverify a verified saved question", () => {
+ cy.findByTestId("saved-question-header-button").click();
+
+ cy.findByTestId("moderation-verify-action").click();
+ cy.findByTestId("moderation-remove-review-action").click();
+
+ cy.findByText("You verified this").should("not.exist");
+ cy.findByTestId("saved-question-header-button").click();
+
+ cy.icon("verified").should("not.exist");
+ });
+ });
+
+ describe("as a non-admin user", () => {
+ beforeEach(() => {
+ restore();
+ cy.signInAsNormalUser();
+
+ 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");
+ });
+
+ it("should be able to see that a question has been verified", () => {
+ cy.visit(`/question/1`);
+
+ cy.wait("@cardGet");
+
+ cy.icon("verified").should("exist");
+
+ cy.findByTestId("saved-question-header-button").click();
+ cy.findByText("A moderator verified this").should("exist");
+ });
+ });
+});
diff --git a/resources/migrations/000_migrations.yaml b/resources/migrations/000_migrations.yaml
index acfb2cabc4f..7d74ab20055 100644
--- a/resources/migrations/000_migrations.yaml
+++ b/resources/migrations/000_migrations.yaml
@@ -8282,6 +8282,70 @@ databaseChangeLog:
- sql:
- sql: ALTER SEQUENCE collection_revision_id_seq RENAME TO collection_permission_graph_revision_id_seq
+ - changeSet:
+ id: 303
+ author: tsmacdonald
+ comment: Added 0.40.0
+ changes:
+ - createTable:
+ tableName: moderation_review
+ remarks: "Reviews (from moderators) for a given question/dashboard (BUCM)"
+ columns:
+ - column:
+ name: id
+ type: int
+ autoIncrement: true
+ constraints:
+ primaryKey: true
+ nullable: false
+ - column:
+ name: updated_at
+ type: ${timestamp_type}
+ defaultValueComputed: current_timestamp
+ remarks: "most recent modification time"
+ constraints:
+ nullable: false
+ - column:
+ name: created_at
+ type: ${timestamp_type}
+ defaultValueComputed: current_timestamp
+ remarks: "creation time"
+ constraints:
+ nullable: false
+ - column:
+ name: status
+ type: varchar(255)
+ remarks: "verified, misleading, confusing, not_misleading, pending"
+ - column:
+ name: text
+ type: text
+ remarks: "Explanation of the review"
+ # I don't think it needs to be non-nullable
+ - column:
+ name: moderated_item_id
+ type: int
+ remarks: "either a document or question ID; the item that needs review"
+ constraints:
+ nullable: false
+ - column:
+ name: moderated_item_type
+ type: varchar(255)
+ remarks: "whether it's a question or dashboard"
+ constraints:
+ nullable: false
+ - column:
+ name: moderator_id
+ type: int
+ remarks: "ID of the user who did the review"
+ constraints:
+ nullable: false
+ - column:
+ name: most_recent
+ type: boolean
+ remarks: "tag for most recent review"
+ constraints:
+ nullable: false
+
- changeSet:
id: 304
author: camsaul
diff --git a/src/metabase/api/card.clj b/src/metabase/api/card.clj
index 12c26dc0766..16d19da4a7b 100644
--- a/src/metabase/api/card.clj
+++ b/src/metabase/api/card.clj
@@ -165,7 +165,7 @@
"Get `Card` with ID."
[id]
(u/prog1 (-> (Card id)
- (hydrate :creator :dashboard_count :can_write :collection)
+ (hydrate :creator :dashboard_count :can_write :collection :moderation_reviews)
api/read-check
(last-edit/with-last-edit-info :card))
(events/publish-event! :card-read (assoc <> :actor_id api/*current-user-id*))))
@@ -228,7 +228,7 @@
;; include same information returned by GET /api/card/:id since frontend replaces the Card it
;; currently has with returned one -- See #4283
(-> card
- (hydrate :creator :dashboard_count :can_write :collection)
+ (hydrate :creator :dashboard_count :can_write :collection :moderation_reviews)
(assoc :last-edit-info (last-edit/edit-information-for-user user))))))
(defn- create-card-async!
@@ -426,7 +426,7 @@
;; include same information returned by GET /api/card/:id since frontend replaces the Card it currently
;; has with returned one -- See #4142
(-> card
- (hydrate :creator :dashboard_count :can_write :collection)
+ (hydrate :creator :dashboard_count :can_write :collection :moderation_reviews)
(assoc :last-edit-info (last-edit/edit-information-for-user @api/*current-user*))))))
(api/defendpoint ^:returns-chan PUT "/:id"
diff --git a/src/metabase/api/collection.clj b/src/metabase/api/collection.clj
index 3976e6bb20b..a9029dbff81 100644
--- a/src/metabase/api/collection.clj
+++ b/src/metabase/api/collection.clj
@@ -167,7 +167,7 @@
(defmethod post-process-collection-children :pulse
[_ rows]
(for [row rows]
- (dissoc row :description :display :authority_level)))
+ (dissoc row :description :display :authority_level :moderated_status)))
(defmethod collection-children-query :snippet
[_ collection {:keys [archived? pinned-state]}]
@@ -180,14 +180,14 @@
(defmethod post-process-collection-children :snippet
[_ rows]
(for [row rows]
- (dissoc row :description :collection_position :display :authority_level)))
+ (dissoc row :description :collection_position :display :authority_level :moderated_status)))
(defmethod collection-children-query :card
[_ collection {:keys [archived? pinned-state]}]
(-> {:select [:c.id :c.name :c.description :c.collection_position :c.display [(hx/literal "card") :model]
[:u.id :last_edit_user] [:u.email :last_edit_email]
[:u.first_name :last_edit_first_name] [:u.last_name :last_edit_last_name]
- [:r.timestamp :last_edit_timestamp]]
+ [:r.timestamp :last_edit_timestamp] [:mr.status :moderated_status]]
:from [[Card :c]]
;; todo: should there be a flag, or a realized view?
:left-join [[{:select [:r1.*]
@@ -200,7 +200,11 @@
[:= :r2.id nil]
[:= :r1.model (hx/literal "Card")]]} :r]
[:= :r.model_id :c.id]
- [:core_user :u] [:= :u.id :r.user_id]]
+ [:core_user :u] [:= :u.id :r.user_id]
+ [:moderation_review :mr] [:and
+ [:= :mr.moderated_item_id :c.id]
+ [:= :mr.moderated_item_type "card"]
+ [:= :mr.most_recent true]]]
:where [:and
[:= :collection_id (:id collection)]
[:= :archived (boolean archived?)]]}
@@ -235,7 +239,7 @@
(defmethod post-process-collection-children :dashboard
[_ rows]
- (hydrate (map #(dissoc % :display :authority_level) rows) :favorite))
+ (hydrate (map #(dissoc % :display :authority_level :moderated_status) rows) :favorite))
(defmethod collection-children-query :collection
[_ collection {:keys [archived? collection-namespace pinned-state]}]
@@ -260,7 +264,7 @@
;; don't get models back from ulterior over-query
;; Previous examination with logging to DB says that there's no N+1 query for this.
;; However, this was only tested on H2 and Postgres
- (assoc (dissoc row :collection_position :display)
+ (assoc (dissoc row :collection_position :display :moderated_status)
:can_write
(mi/can-write? Collection (:id row)))))
@@ -319,7 +323,7 @@
are optional (not id, but last_edit_user for example) must have a type so that the union-all can unify the nil with
the correct column type."
[:id :name :description :display :model :collection_position :authority_level
- :last_edit_email :last_edit_first_name :last_edit_last_name
+ :last_edit_email :last_edit_first_name :last_edit_last_name :moderated_status
[:last_edit_user :integer] [:last_edit_timestamp :timestamp]])
(defn- add-missing-columns
diff --git a/src/metabase/api/dashboard.clj b/src/metabase/api/dashboard.clj
index b4701777369..f6fd4529c1d 100644
--- a/src/metabase/api/dashboard.clj
+++ b/src/metabase/api/dashboard.clj
@@ -205,7 +205,10 @@
[id]
(-> (Dashboard id)
api/check-404
- (hydrate [:ordered_cards :card :series] :collection_authority_level :can_write :param_fields :param_values)
+ ;; i'm a bit worried that this is an n+1 situation here. The cards can be batch hydrated i think because they
+ ;; have a hydration key and an id. moderation_reviews currently aren't batch hydrated but i'm worried they
+ ;; cannot be in this situation
+ (hydrate [:ordered_cards [:card :moderation_reviews] :series] :collection_authority_level :can_write :param_fields :param_values)
api/read-check
api/check-not-archived
hide-unreadable-cards
diff --git a/src/metabase/api/moderation_review.clj b/src/metabase/api/moderation_review.clj
new file mode 100644
index 00000000000..6a04bb83325
--- /dev/null
+++ b/src/metabase/api/moderation_review.clj
@@ -0,0 +1,25 @@
+(ns metabase.api.moderation-review
+ (:require [compojure.core :refer [POST]]
+ [metabase.api.common :as api]
+ [metabase.models.moderation-review :as moderation-review]
+ [metabase.moderation :as moderation]
+ [metabase.util.schema :as su]
+ [schema.core :as s]))
+
+(api/defendpoint POST "/"
+ "Create a new `ModerationReview`."
+ [:as {{:keys [text moderated_item_id moderated_item_type status]} :body}]
+ {text (s/maybe s/Str)
+ moderated_item_id su/IntGreaterThanZero
+ moderated_item_type moderation/moderated-item-types
+ status moderation-review/Statuses}
+ (api/check-superuser)
+ (let [review-data {:text text
+ :moderated_item_id moderated_item_id
+ :moderated_item_type moderated_item_type
+ :moderator_id api/*current-user-id*
+ :status status}]
+ (api/check-404 (moderation/moderated-item review-data))
+ (moderation-review/create-review! review-data)))
+
+(api/define-routes)
diff --git a/src/metabase/api/routes.clj b/src/metabase/api/routes.clj
index d122052240e..9d11a44d20b 100644
--- a/src/metabase/api/routes.clj
+++ b/src/metabase/api/routes.clj
@@ -17,6 +17,7 @@
[metabase.api.login-history :as login-history]
[metabase.api.metastore :as metastore]
[metabase.api.metric :as metric]
+ [metabase.api.moderation-review :as moderation-review]
[metabase.api.native-query-snippet :as native-query-snippet]
[metabase.api.notify :as notify]
[metabase.api.permissions :as permissions]
@@ -84,6 +85,7 @@
(context "/login-history" [] (+auth login-history/routes))
(context "/metastore" [] (+auth metastore/routes))
(context "/metric" [] (+auth metric/routes))
+ (context "/moderation-review" [] (+auth moderation-review/routes))
(context "/native-query-snippet" [] (+auth native-query-snippet/routes))
(context "/notify" [] (+apikey notify/routes))
(context "/permissions" [] (+auth permissions/routes))
diff --git a/src/metabase/cmd/copy.clj b/src/metabase/cmd/copy.clj
index 6ee8fab0653..f89ed0b5c31 100644
--- a/src/metabase/cmd/copy.clj
+++ b/src/metabase/cmd/copy.clj
@@ -11,8 +11,8 @@
[metabase.db.setup :as mdb.setup]
[metabase.models :refer [Activity Card CardFavorite Collection CollectionPermissionGraphRevision Dashboard
DashboardCard DashboardCardSeries DashboardFavorite Database Dependency Dimension Field
- FieldValues LoginHistory Metric MetricImportantField NativeQuerySnippet Permissions
- PermissionsGroup PermissionsGroupMembership PermissionsRevision Pulse PulseCard
+ FieldValues LoginHistory Metric MetricImportantField ModerationReview NativeQuerySnippet
+ Permissions PermissionsGroup PermissionsGroupMembership PermissionsRevision Pulse PulseCard
PulseChannel PulseChannelRecipient Revision Segment Session Setting Table User
ViewLog]]
[metabase.util :as u]
@@ -54,6 +54,7 @@
Segment
Metric
MetricImportantField
+ ModerationReview
Revision
ViewLog
Session
diff --git a/src/metabase/models.clj b/src/metabase/models.clj
index 2b5b159db2c..e3be5e0d169 100644
--- a/src/metabase/models.clj
+++ b/src/metabase/models.clj
@@ -16,6 +16,7 @@
[metabase.models.login-history :as login-history]
[metabase.models.metric :as metric]
[metabase.models.metric-important-field :as metric-important-field]
+ [metabase.models.moderation-review :as moderation-review]
[metabase.models.native-query-snippet :as native-query-snippet]
[metabase.models.permissions :as permissions]
[metabase.models.permissions-group :as permissions-group]
@@ -54,6 +55,7 @@
field-values/keep-me
login-history/keep-me
metric/keep-me
+ moderation-review/keep-me
metric-important-field/keep-me
native-query-snippet/keep-me
permissions/keep-me
@@ -92,6 +94,7 @@
[field-values FieldValues]
[login-history LoginHistory]
[metric Metric]
+ [moderation-review ModerationReview]
[metric-important-field MetricImportantField]
[native-query-snippet NativeQuerySnippet]
[permissions Permissions]
diff --git a/src/metabase/models/card.clj b/src/metabase/models/card.clj
index a2d69633577..6a1cecc341e 100644
--- a/src/metabase/models/card.clj
+++ b/src/metabase/models/card.clj
@@ -13,6 +13,7 @@
[metabase.models.permissions :as perms]
[metabase.models.query :as query]
[metabase.models.revision :as revision]
+ [metabase.moderation :as moderation]
[metabase.plugins.classloader :as classloader]
[metabase.public-settings :as public-settings]
[metabase.query-processor.util :as qputil]
@@ -33,6 +34,8 @@
[{:keys [id]}]
(db/count 'DashboardCard, :card_id id))
+;; There's more hydration in the shared metabase.moderation namespace, but it needs to be required:
+(comment moderation/keep-me)
;;; -------------------------------------------------- Dependencies --------------------------------------------------
@@ -199,6 +202,7 @@
;; Cards don't normally get deleted (they get archived instead) so this mostly affects tests
(defn- pre-delete [{:keys [id]}]
+ (db/delete! 'ModerationReview :moderated_item_type "card", :moderated_item_id id)
(db/delete! 'Revision :model "Card", :model_id id))
(defn- result-metadata-out
diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj
index 5d0435bd093..4e1120d6c08 100644
--- a/src/metabase/models/dashboard.clj
+++ b/src/metabase/models/dashboard.clj
@@ -17,6 +17,7 @@
[metabase.models.pulse-card :as pulse-card :refer [PulseCard]]
[metabase.models.revision :as revision]
[metabase.models.revision.diff :refer [build-sentence]]
+ [metabase.moderation :as moderation]
[metabase.public-settings :as public-settings]
[metabase.query-processor.async :as qp.async]
[metabase.util :as u]
@@ -58,6 +59,8 @@
(for [dashboard dashboards]
(assoc dashboard :collection_authority_level (get coll-id->level (u/the-id dashboard))))))
+(comment moderation/keep-me)
+
(models/defmodel Dashboard :report_dashboard)
;;; ----------------------------------------------- Entity & Lifecycle -----------------------------------------------
diff --git a/src/metabase/models/moderation_review.clj b/src/metabase/models/moderation_review.clj
new file mode 100644
index 00000000000..e57e78634c7
--- /dev/null
+++ b/src/metabase/models/moderation_review.clj
@@ -0,0 +1,76 @@
+(ns metabase.models.moderation-review
+ (:require [metabase.models.interface :as i]
+ [metabase.models.permissions :as perms]
+ [metabase.moderation :as moderation]
+ [metabase.util :as u]
+ [metabase.util.schema :as su]
+ [schema.core :as s]
+ [toucan.db :as db]
+ [toucan.models :as models]))
+
+(def statuses
+ "Schema enum of the acceptable values for the `status` column"
+ #{"verified" nil})
+
+(def Statuses
+ "Schema of valid statuses"
+ (apply s/enum statuses))
+
+(def ReviewChanges
+ "Schema for a ModerationReview that's being updated (so most keys are optional)"
+ {(s/optional-key :id) su/IntGreaterThanZero
+ (s/optional-key :moderated_item_id) su/IntGreaterThanZero
+ (s/optional-key :moderated_item_type) moderation/moderated-item-types
+ (s/optional-key :status) Statuses
+ (s/optional-key :text) (s/maybe s/Str)
+ s/Any s/Any})
+
+(models/defmodel ModerationReview :moderation_review)
+
+(u/strict-extend (class ModerationReview)
+ models/IModel
+ (merge models/IModelDefaults
+ {:properties (constantly {:timestamped? true})
+ :types (constantly {:moderated_item_type :keyword})})
+
+ ;; Todo: this is wrong, but what should it be?
+ i/IObjectPermissions
+ perms/IObjectPermissionsForParentCollection)
+
+(def max-moderation-reviews
+ "The amount of moderation reviews we will keep on hand."
+ 10)
+
+(s/defn delete-extra-reviews!
+ "Delete extra reviews to maintain an invariant of only `max-moderation-reviews`. Called before inserting so actuall
+ insures there are one fewer than that so you can add afterwards."
+ [item-id :- s/Int item-type :- s/Str]
+ (let [ids (into #{} (comp (map :id)
+ (drop (dec max-moderation-reviews)))
+ (db/query {:select [:id]
+ :from [:moderation_review]
+ :where [:and
+ [:= :moderated_item_id item-id]
+ [:= :moderated_item_type item-type]]
+ ;; cannot put the offset in this query as mysql doesnt place nice. It requires a limit
+ ;; as well which we do not want to give. The offset is only 10 though so its not a huge
+ ;; savings and we run this on every entry so the max number is 10, delete the extra,
+ ;; and insert a new one to arrive at 10 again, our invariant.
+ :order-by [[:id :desc]]}))]
+ (when (seq ids)
+ (db/delete! ModerationReview :id [:in ids]))))
+
+(s/defn create-review!
+ "Create a new ModerationReview"
+ [params :-
+ {:moderated_item_id su/IntGreaterThanZero
+ :moderated_item_type moderation/moderated-item-types
+ :moderator_id su/IntGreaterThanZero
+ (s/optional-key :status) Statuses
+ (s/optional-key :text) (s/maybe s/Str)}]
+ (db/transaction
+ (delete-extra-reviews! (:moderated_item_id params) (:moderated_item_type params))
+ (db/update-where! ModerationReview {:moderated_item_id (:moderated_item_id params)
+ :moderated_item_type (:moderated_item_type params)}
+ :most_recent false)
+ (db/insert! ModerationReview (assoc params :most_recent true))))
diff --git a/src/metabase/moderation.clj b/src/metabase/moderation.clj
new file mode 100644
index 00000000000..d972a29a98b
--- /dev/null
+++ b/src/metabase/moderation.clj
@@ -0,0 +1,45 @@
+(ns metabase.moderation
+ (:require [clojure.string :as str]
+ [metabase.util :as u]
+ [schema.core :as s]
+ [toucan.db :as db]))
+
+(def moderated-item-types
+ "Schema enum of the acceptable values for the `moderated_item_type` column"
+ (s/enum "card" "dashboard" :card :dashboard))
+
+(def moderated-item-type->model
+ "Maps DB name of the moderated item type to the model symbol (used for db/select and such)"
+ {"card" 'Card
+ :card 'Card
+ "dashboard" 'Dashboard
+ :dashboard 'Dashboard})
+
+(defn- object->type
+ "Convert a moderated item instance to the keyword stored in the database"
+ [moderated-item]
+ (str/lower-case (name moderated-item)))
+
+(defn moderation-reviews-for-items
+ "Hydrate moderation reviews onto a seq of items. All are cards or the nils that end up here on text dashboard
+ cards. In the future could have dashboards here as well."
+ {:batched-hydrate :moderation_reviews}
+ [items]
+ ;; no need to do work on empty items. Also, can have nil here due to text cards. I think this is a bug in toucan. To
+ ;; get here we are `(hydrate dashboard [:ordered_cards [:card :moderation_reviews] :series] ...)` But ordered_cards dont have to have cards. but the hydration will pass the nil card id into
+ (when-some [items (seq (keep identity items))]
+ (let [all-reviews (group-by (juxt :moderated_item_type :moderated_item_id)
+ (db/select 'ModerationReview
+ :moderated_item_type "card"
+ :moderated_item_id [:in (map :id items)]
+ {:order-by [[:id :desc]]}))]
+ (for [item items]
+ (let [k ((juxt (comp keyword object->type) u/the-id) item)]
+ (assoc item :moderation_reviews (get all-reviews k ())))))))
+
+(defn moderated-item
+ "The moderated item for a given request or review"
+ {:hydrate :moderated_item}
+ [{:keys [moderated_item_id moderated_item_type]}]
+ (when (and moderated_item_type moderated_item_id)
+ (db/select-one (moderated-item-type->model moderated_item_type) :id moderated_item_id)))
diff --git a/src/metabase/util/schema.clj b/src/metabase/util/schema.clj
index fafaf275cae..a9c8a524bc4 100644
--- a/src/metabase/util/schema.clj
+++ b/src/metabase/util/schema.clj
@@ -99,7 +99,7 @@
(deferred-tru "value may be nil, or if non-nil, {0}" message)))
;; we can do something similar for enum schemas which are also likely to be defined inline
(when (instance? schema.core.EnumSchema schema)
- (deferred-tru "value must be one of: {0}." (str/join ", " (for [v (sort (:vs schema))]
+ (deferred-tru "value must be one of: {0}." (str/join ", " (for [v (sort (map str (:vs schema)))]
(str "`" v "`")))))
;; For cond-pre schemas we'll generate something like
;; value must satisfy one of the following requirements:
diff --git a/test/metabase/api/card_test.clj b/test/metabase/api/card_test.clj
index 7503ecbf5d8..031da8d82e8 100644
--- a/test/metabase/api/card_test.clj
+++ b/test/metabase/api/card_test.clj
@@ -10,8 +10,8 @@
[metabase.api.pivots :as pivots]
[metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.http-client :as http]
- [metabase.models :refer [Card CardFavorite Collection Dashboard Database Pulse PulseCard PulseChannel
- PulseChannelRecipient Table ViewLog]]
+ [metabase.models :refer [Card CardFavorite Collection Dashboard Database ModerationReview
+ Pulse PulseCard PulseChannel PulseChannelRecipient Table ViewLog]]
[metabase.models.permissions :as perms]
[metabase.models.permissions-group :as perms-group]
[metabase.models.revision :as revision :refer [Revision]]
@@ -48,6 +48,7 @@
:enable_embedding false
:embedding_params nil
:made_public_by_id nil
+ :moderation_reviews ()
:public_uuid nil
:query_type nil
:cache_ttl nil
@@ -593,7 +594,20 @@
(is (= {:id true :email "user@test.com" :first_name "Test" :last_name "User" :timestamp true}
(-> (mt/user-http-request :rasta :get 200 (str "card/" (u/the-id card)))
mt/boolean-ids-and-timestamps
- :last-edit-info)))))))))
+ :last-edit-info)))))
+ (testing "Card should include moderation reviews"
+ (letfn [(clean [mr] (select-keys [mr] [:status :text])) ]
+ (mt/with-temp* [ModerationReview [review {:moderated_item_id (:id card)
+ :moderated_item_type "card"
+ :moderator_id (mt/user->id :rasta)
+ :most_recent true
+ :status "verified"
+ :text "lookin good"}]]
+ (is (= [(clean review)]
+ (->> (mt/user-http-request :rasta :get 200 (str "card/" (u/the-id card)))
+ mt/boolean-ids-and-timestamps
+ :moderation_reviews
+ (map clean)))))))))))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | UPDATING A CARD |
diff --git a/test/metabase/api/collection_test.clj b/test/metabase/api/collection_test.clj
index 478681dc9fc..8e32cd4f8ce 100644
--- a/test/metabase/api/collection_test.clj
+++ b/test/metabase/api/collection_test.clj
@@ -4,9 +4,9 @@
[clojure.test :refer :all]
[honeysql.core :as hsql]
[metabase.api.collection :as api-coll]
- [metabase.models :refer [Card Collection Dashboard DashboardCard NativeQuerySnippet PermissionsGroup
- PermissionsGroupMembership Pulse PulseCard PulseChannel PulseChannelRecipient
- Revision User]]
+ [metabase.models :refer [Card Collection Dashboard DashboardCard ModerationReview NativeQuerySnippet
+ PermissionsGroup PermissionsGroupMembership Pulse PulseCard PulseChannel
+ PulseChannelRecipient Revision User]]
[metabase.models.collection :as collection]
[metabase.models.collection-test :as collection-test]
[metabase.models.collection.graph :as graph]
@@ -329,6 +329,8 @@
(merge {:id true, :collection_position nil}
(when (= model "collection")
{:authority_level nil})
+ (when (= model "card")
+ {:moderated_status nil})
item-map))
(defn- collection-item [collection-name & {:as extra-keypairs}]
@@ -343,18 +345,26 @@
(deftest collection-items-test
(testing "GET /api/collection/:id/items"
(testing "check that cards are returned with the collection/items endpoint"
- (mt/with-temp* [Collection [collection]
- Card [card {:collection_id (u/the-id collection)}]]
+ (mt/with-temp* [Collection [collection]
+ User [{user-id :id} {:first_name "x" :last_name "x" :email "zzzz@example.com"}]
+ Card [{card-id :id :as card} {:collection_id (u/the-id collection)}]
+ ModerationReview [_ {:moderated_item_type "card"
+ :moderated_item_id card-id
+ :status "verified"
+ :moderator_id user-id
+ :most_recent true}]]
(is (= (mt/obj->json->obj
- [{:id (u/the-id card)
+ [{:id card-id
:name (:name card)
:collection_position nil
:display "table"
:description nil
+ :moderated_status "verified"
:favorite false
:model "card"}])
(mt/obj->json->obj
- (:data (mt/user-http-request :crowberto :get 200 (str "collection/" (u/the-id collection) "/items"))))))))
+ (:data (mt/user-http-request :crowberto :get 200
+ (str "collection/" (u/the-id collection) "/items"))))))))
(testing "check that limit and offset work and total comes back"
(mt/with-temp* [Collection [collection]
Card [card3 {:collection_id (u/the-id collection)}]
@@ -901,6 +911,7 @@
:description nil
:collection_position nil
:display "table"
+ :moderated_status nil
:favorite false
:model "card"}]
(for [item (:data (mt/user-http-request :crowberto :get 200 "collection/root/items?archived=true"))]
diff --git a/test/metabase/api/moderation_review_test.clj b/test/metabase/api/moderation_review_test.clj
new file mode 100644
index 00000000000..d8ce37fc0ab
--- /dev/null
+++ b/test/metabase/api/moderation_review_test.clj
@@ -0,0 +1,86 @@
+(ns metabase.api.moderation-review-test
+ (:require [clojure.test :refer :all]
+ [metabase.models.card :refer [Card]]
+ [metabase.models.moderation-review :as mod-review :refer [ModerationReview]]
+ [metabase.test :as mt]
+ [toucan.db :as db]))
+
+(defn- normalized-response
+ [moderation-review]
+ (dissoc moderation-review :id :updated_at :created_at))
+
+;;todo: check it can review dashboards, and that it cannot review other models
+(deftest create-test
+ (testing "POST /api/moderation-review"
+ (mt/with-temp* [Card [{card-id :id :as card} {:name "Test Card"}]]
+ (mt/with-model-cleanup [ModerationReview]
+ (letfn [(moderate! [status text]
+ (normalized-response
+ (mt/user-http-request :crowberto :post 200 "moderation-review"
+ {:text text
+ :status status
+ :moderated_item_id card-id
+ :moderated_item_type "card"})))
+ (review-count [] (db/count ModerationReview
+ :moderated_item_id card-id
+ :moderated_item_type "card"))]
+ (testing "Non admin cannot create a moderation review"
+ (is (= 0 (review-count)))
+ (is (= "You don't have permissions to do that."
+ (mt/user-http-request :rasta :post 403 "moderation-review"
+ {:text "review"
+ :status "verified"
+ :moderated_item_id card-id
+ :moderated_item_type "card"})))
+ (is (= 0 (review-count))))
+ (is (= {:text "Looks good to me"
+ :moderated_item_id card-id
+ :moderated_item_type "card"
+ :moderator_id (mt/user->id :crowberto)
+ :status "verified"
+ :most_recent true}
+ (moderate! "verified" "Looks good to me")))
+ (testing "When adding a new moderation review, marks it as most recent"
+ (is (= {:text "hmm"
+ :status nil
+ :most_recent true}
+ (select-keys (moderate! nil "hmm") [:text :status :most_recent])))
+ (testing "And previous moderation reviews are marked as not :most_recent"
+ (is (= #{{:text "hmm" :most_recent true :status nil}
+ {:text "Looks good to me" :most_recent false :status "verified"}}
+ (into #{}
+ (map #(select-keys % [:text :status :most_recent]))
+ (db/select ModerationReview
+ :moderated_item_id card-id
+ :moderated_item_type "card"))))))
+ (testing "Ensures we never have more than `modreview/max-moderation-reviews`"
+ (db/insert-many! ModerationReview (repeat (* 2 mod-review/max-moderation-reviews)
+ {:moderated_item_id card-id
+ :moderated_item_type "card"
+ :moderator_id (mt/user->id :crowberto)
+ :most_recent false
+ :status "verified"
+ :text "old review"}))
+ ;; manually inserted many
+
+ (is (> (review-count) mod-review/max-moderation-reviews))
+ (moderate! "verified" "lookin good")
+ ;; api ensures we never have more than our limit
+
+ (is (<= (review-count) mod-review/max-moderation-reviews)))
+ (testing "Only allows for valid status"
+ (doseq [status mod-review/statuses]
+ (is (= status (:status (moderate! status "good")))))
+ ;; i wish this was better. Should have a better error message and honestly shouldn't be a 500
+ (tap> (mt/user-http-request :crowberto :post 400 "moderation-review"
+ {:text "not a chance this works"
+ :status "invalid status"
+ :moderated_item_id card-id
+ :moderated_item_type "card"})))
+ (testing "Can't moderate a card that doesn't exist"
+ (is (= "Not found."
+ (mt/user-http-request :crowberto :post 404 "moderation-review"
+ {:text "card doesn't exist"
+ :status "verified"
+ :moderated_item_id Integer/MAX_VALUE
+ :moderated_item_type "card"})))))))))
--
GitLab