add moderation events to question timeline (#17401)

* add moderation events to question timeline

* tweak the height of the DrawerSection header

* replace history modal & control drawer state via redux

* add/fix cy tests

* fix banner unit tests

* prevent leakage of 'null' moderation icon

* rename styled component

* add tests / rmv redundant fn

* remove should('exist') from moderation cy tests

* refactor to make biz logic of a null mod status clearer

* add selector unit test

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

 import styled from "styled-components";
 
 import { color } from "metabase/lib/colors";
-import { getVerifiedIcon } from "metabase-enterprise/moderation/service";
-
-const { name: verifiedIconName, color: verifiedIconColor } = getVerifiedIcon();
+import {
+  MODERATION_STATUS,
+  getStatusIcon,
+} from "metabase-enterprise/moderation/service";
+
+const { name: verifiedIconName, color: verifiedIconColor } = getStatusIcon(
+  MODERATION_STATUS.verified,
+);
 
 import Button from "metabase/components/Button";
 

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

@@ -10,8 +10,8 @@ const moderationReview = {
   moderator_id: 1,
   created_at: Date.now(),
 };
-const moderator = { id: 1, display_name: "Foo" };
-const currentUser = { id: 2, display_name: "Bar" };
+const moderator = { id: 1, common_name: "Foo" };
+const currentUser = { id: 2, common_name: "Bar" };
 
 describe("ModerationReviewBanner", () => {
   it("should show text concerning the given review", () => {

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

-export const ACTIONS = {
+export const MODERATION_STATUS = {
+  verified: "verified",
+};
+
+export const MODERATION_STATUS_ICONS = {
   verified: {
-    type: "verified",
-    icon: {
-      name: "verified",
-      color: "brand",
-    },
+    name: "verified",
+    color: "brand",
+  },
+  null: {
+    name: "close",
+    color: "text-light",
   },
 };

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

@@ -2,11 +2,16 @@ import { PLUGIN_MODERATION } from "metabase/plugins";
 import QuestionModerationSection from "./components/QuestionModerationSection/QuestionModerationSection";
 import ModerationStatusIcon from "./components/ModerationStatusIcon/ModerationStatusIcon";
 
-import { getStatusIconForQuestion, getStatusIcon } from "./service";
+import {
+  getStatusIconForQuestion,
+  getStatusIcon,
+  getModerationTimelineEvents,
+} from "./service";
 
 Object.assign(PLUGIN_MODERATION, {
   QuestionModerationSection,
   ModerationStatusIcon,
   getStatusIconForQuestion,
   getStatusIcon,
+  getModerationTimelineEvents,
 });

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

@@ -2,7 +2,9 @@ import { t } from "ttag";
 import _ from "underscore";
 
 import { ModerationReviewApi } from "metabase/services";
-import { ACTIONS } from "./constants";
+import { MODERATION_STATUS_ICONS } from "./constants";
+
+export { MODERATION_STATUS } from "./constants";
 
 export function verifyItem({ text, itemId, itemType }) {
   return ModerationReviewApi.create({
@@ -21,18 +23,38 @@ export function removeReview({ itemId, itemType }) {
   });
 }
 
-const defaultIcon = {};
+const noIcon = {};
 export function getStatusIcon(status) {
-  const action = ACTIONS[status] || {};
-  return action.icon || defaultIcon;
+  if (isRemovedReviewStatus(status)) {
+    return noIcon;
+  }
+
+  return MODERATION_STATUS_ICONS[status] || noIcon;
 }
 
-export function getVerifiedIcon() {
-  return getStatusIcon("verified");
+export function getIconForReview(review, options) {
+  return getStatusIcon(review?.status, options);
 }
 
-export function getIconForReview(review) {
-  return getStatusIcon(review?.status);
+// we only want the icon that represents the removal of a review in special cases,
+// so you must ask for the icon explicitly
+export function getRemovedReviewStatusIcon() {
+  return MODERATION_STATUS_ICONS[null];
+}
+
+export function getLatestModerationReview(reviews) {
+  const maybeReview = _.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
+  return isRemovedReviewStatus(maybeReview?.status) ? undefined : maybeReview;
+}
+
+export function getStatusIconForQuestion(question) {
+  const reviews = question.getModerationReviews();
+  const review = getLatestModerationReview(reviews);
+  return getIconForReview(review);
 }
 
 export function getTextForReviewBanner(
@@ -53,35 +75,54 @@ export function getTextForReviewBanner(
 }
 
 function getModeratorDisplayName(user, currentUser) {
-  const { id: userId, display_name } = user || {};
+  const { id: userId, common_name } = user || {};
   const { id: currentUserId } = currentUser || {};
 
   if (currentUserId != null && userId === currentUserId) {
     return t`You`;
   } else if (userId != null) {
-    return display_name;
+    return common_name;
   } else {
     return t`A moderator`;
   }
 }
 
+// a `status` of `null` represents the removal of a review, since we can't delete reviews
+export function isRemovedReviewStatus(status) {
+  return String(status) === "null";
+}
+
 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;
+function getModerationReviewEventText(review, moderatorDisplayName) {
+  switch (review.status) {
+    case "verified":
+      return t`${moderatorDisplayName} verified this`;
+    case null:
+      return t`${moderatorDisplayName} removed verification`;
+    default:
+      return t`${moderatorDisplayName} changed status to ${review.status}`;
   }
 }
 
-export function getStatusIconForQuestion(question) {
-  const reviews = question.getModerationReviews();
-  const review = getLatestModerationReview(reviews);
-  return getIconForReview(review);
+export function getModerationTimelineEvents(reviews, usersById, currentUser) {
+  return reviews.map((review, index) => {
+    const moderator = usersById[review.moderator_id];
+    const moderatorDisplayName = getModeratorDisplayName(
+      moderator,
+      currentUser,
+    );
+    const text = getModerationReviewEventText(review, moderatorDisplayName);
+    const icon = isRemovedReviewStatus(review.status)
+      ? getRemovedReviewStatusIcon()
+      : getIconForReview(review);
+
+    return {
+      timestamp: new Date(review.created_at).valueOf(),
+      icon,
+      title: text,
+    };
+  });
 }

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

 import {
   verifyItem,
   removeReview,
-  getVerifiedIcon,
   getIconForReview,
   getTextForReviewBanner,
   isItemVerified,
   getLatestModerationReview,
   getStatusIconForQuestion,
+  getModerationTimelineEvents,
+  getStatusIcon,
+  getRemovedReviewStatusIcon,
 } from "./service";
 
 jest.mock("metabase/services", () => ({
@@ -58,25 +60,43 @@ describe("moderation/service", () => {
     });
   });
 
-  describe("getVerifiedIcon", () => {
-    it("should return verified icon name/color", () => {
-      expect(getVerifiedIcon()).toEqual({
+  describe("getStatusIcon", () => {
+    it("should return an empty icon if there is no matching status", () => {
+      expect(getStatusIcon("foo")).toEqual({});
+    });
+
+    it("should return an icon if there is a matching status", () => {
+      expect(getStatusIcon("verified")).toEqual({
         name: "verified",
         color: "brand",
       });
     });
+
+    it("should not return an icon for a status of null, which represents the removal of a review and is a special case", () => {
+      const removedReviewStatus = null;
+      const accidentallyStringCoercedRemvovedReviewStatus = "null";
+      expect(getStatusIcon(removedReviewStatus)).toEqual({});
+      expect(
+        getStatusIcon(accidentallyStringCoercedRemvovedReviewStatus),
+      ).toEqual({});
+    });
+  });
+
+  describe("getRemovedReviewStatusIcon", () => {
+    it("should return an icon for a removed review", () => {
+      expect(getRemovedReviewStatusIcon()).toEqual({
+        name: "close",
+        color: "text-light",
+      });
+    });
   });
 
   describe("getIconForReview", () => {
     it("should return icon name/color for given review", () => {
       expect(getIconForReview({ status: "verified" })).toEqual(
-        getVerifiedIcon(),
+        getStatusIcon("verified"),
       );
     });
-
-    it("should be an empty object for a null review", () => {
-      expect(getIconForReview({ status: null })).toEqual({});
-    });
   });
 
   describe("getTextForReviewBanner", () => {
@@ -92,7 +112,7 @@ describe("moderation/service", () => {
         getTextForReviewBanner(
           { status: "verified" },
           {
-            display_name: "Foo",
+            common_name: "Foo",
             id: 1,
           },
           { id: 2 },
@@ -108,7 +128,7 @@ describe("moderation/service", () => {
         getTextForReviewBanner(
           { status: "verified" },
           {
-            display_name: "Foo",
+            common_name: "Foo",
             id: 1,
           },
           { id: 1 },
@@ -170,48 +190,86 @@ describe("moderation/service", () => {
       expect(getLatestModerationReview(reviews)).toEqual(undefined);
     });
   });
-});
 
-describe("getStatusIconForQuestion", () => {
-  it('should return the status icon for the most recent "real" review', () => {
-    const questionWithReviews = {
-      getModerationReviews: () => [
-        { id: 1, status: "verified" },
-        { id: 2, status: "verified", most_recent: true },
-        { id: 3, status: null },
-      ],
-    };
+  describe("getStatusIconForQuestion", () => {
+    it('should return the status icon for the most recent "real" review', () => {
+      const questionWithReviews = {
+        getModerationReviews: () => [
+          { id: 1, status: "verified" },
+          { id: 2, status: "verified", most_recent: true },
+          { id: 3, status: null },
+        ],
+      };
+
+      expect(getStatusIconForQuestion(questionWithReviews)).toEqual(
+        getStatusIcon("verified"),
+      );
+    });
+
+    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: () => [],
+      };
 
-    expect(getStatusIconForQuestion(questionWithReviews)).toEqual(
-      getVerifiedIcon(),
-    );
+      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,
+      );
+    });
   });
 
-  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,
-    );
+  describe("getModerationTimelineEvents", () => {
+    it("should return the moderation timeline events", () => {
+      const reviews = [
+        {
+          id: 1,
+          status: "verified",
+          created_at: "2018-01-01T00:00:00.000Z",
+          moderator_id: 1,
+        },
+        {
+          id: 2,
+          status: null,
+          created_at: "2018-01-02T00:00:00.000Z",
+          moderator_id: 123,
+        },
+      ];
+      const usersById = {
+        1: {
+          id: 1,
+          common_name: "Foo",
+        },
+      };
+
+      expect(getModerationTimelineEvents(reviews, usersById)).toEqual([
+        {
+          timestamp: expect.any(Number),
+          icon: getStatusIcon("verified"),
+          title: "Foo verified this",
+        },
+        {
+          timestamp: expect.any(Number),
+          icon: getRemovedReviewStatusIcon(),
+          title: "A moderator removed verification",
+        },
+      ]);
+    });
   });
 });

frontend/src/metabase/components/DrawerSection/DrawerSection.info.js

+import React from "react";
+import DrawerSection from "./DrawerSection";
+
+import moment from "moment";
+import styled from "styled-components";
+import Timeline from "metabase/components/Timeline";
+import { color } from "metabase/lib/colors";
+
+export const component = DrawerSection;
+export const category = "layout";
+export const description = `
+  This component is similar to the CollapseSection component,
+  but instead of expanding downward, it expands upward.
+  The header situates itself at the bottom of the remaining space
+  in a parent component and when opened fills the remaining space
+  with what child components you have given it.
+
+  For this to work properly, the containing element (here, Container)
+  must handle overflow when the DrawerSection is open and have a display
+  of "flex" (plus a flex-direction of "column") so that the DrawerSection
+  can properly use the remaining space in the Container component.
+`;
+
+const Container = styled.div`
+  line-height: 1.5;
+  width: 350px;
+  border: 1px dashed ${color("bg-dark")};
+  border-radius: 0.5rem;
+  padding: 1rem;
+  height: 32rem;
+
+  overflow-y: auto;
+  display: flex;
+  flex-direction: column;
+`;
+
+const TextArea = styled.textarea`
+  width: 100%;
+  flex-shrink: 0;
+`;
+
+const items = [
+  {
+    icon: "verified",
+    title: "John Someone verified this",
+    description: "idk lol",
+    timestamp: moment()
+      .subtract(1, "day")
+      .valueOf(),
+    numComments: 5,
+  },
+  {
+    icon: "pencil",
+    title: "Foo edited this",
+    description: "Did a thing.",
+    timestamp: moment()
+      .subtract(1, "week")
+      .valueOf(),
+  },
+  {
+    icon: "close",
+    title: "foo foo foo",
+    timestamp: moment()
+      .subtract(2, "month")
+      .valueOf(),
+  },
+  {
+    icon: "number",
+    title: "bar bar bar",
+    timestamp: moment()
+      .subtract(1, "year")
+      .valueOf(),
+    numComments: 123,
+  },
+];
+
+export const examples = {
+  "Constrained container": (
+    <Container>
+      <TextArea placeholder="an element with variable height" />
+      <DrawerSection header="foo">
+        <Timeline items={items} />
+      </DrawerSection>
+    </Container>
+  ),
+};

frontend/src/metabase/components/DrawerSection/DrawerSection.jsx

+import React, { useState } from "react";
+import PropTypes from "prop-types";
+import _ from "underscore";
+
+import Icon from "metabase/components/Icon";
+import {
+  Container,
+  Transformer,
+  Children,
+  Header,
+} from "./DrawerSection.styled";
+
+export const STATES = {
+  closed: "closed",
+  open: "open",
+};
+
+DrawerSection.propTypes = {
+  header: PropTypes.node.isRequired,
+  children: PropTypes.node,
+  state: PropTypes.oneOf([STATES.closed, STATES.open]),
+  onStateChange: PropTypes.func,
+};
+
+function DrawerSection({ header, children, state, onStateChange }) {
+  return _.isFunction(onStateChange) ? (
+    <ControlledDrawerSection
+      header={header}
+      state={state}
+      onStateChange={onStateChange}
+    >
+      {children}
+    </ControlledDrawerSection>
+  ) : (
+    <UncontrolledDrawerSection header={header} initialState={state}>
+      {children}
+    </UncontrolledDrawerSection>
+  );
+}
+
+UncontrolledDrawerSection.propTypes = {
+  header: PropTypes.node.isRequired,
+  children: PropTypes.node,
+  initialState: PropTypes.oneOf([STATES.closed, STATES.open]),
+};
+
+function UncontrolledDrawerSection({ header, children, initialState }) {
+  const [state, setState] = useState(initialState);
+
+  return (
+    <ControlledDrawerSection
+      header={header}
+      state={state}
+      onStateChange={setState}
+    >
+      {children}
+    </ControlledDrawerSection>
+  );
+}
+
+ControlledDrawerSection.propTypes = {
+  header: PropTypes.node.isRequired,
+  children: PropTypes.node,
+  state: PropTypes.oneOf([STATES.closed, STATES.open]),
+  onStateChange: PropTypes.func.isRequired,
+};
+
+function ControlledDrawerSection({ header, children, state, onStateChange }) {
+  const isOpen = state === STATES.open;
+
+  const toggleState = () => {
+    if (state === STATES.open) {
+      onStateChange(STATES.closed);
+    } else {
+      onStateChange(STATES.open);
+    }
+  };
+
+  return (
+    <Container isOpen={isOpen}>
+      <Transformer isOpen={isOpen}>
+        <Header
+          isOpen={isOpen}
+          onClick={toggleState}
+          onKeyDown={e => e.key === "Enter" && toggleState()}
+        >
+          {header}
+          <Icon
+            className="mr1"
+            name={isOpen ? "chevrondown" : "chevronup"}
+            size={12}
+          />
+        </Header>
+        <Children isOpen={isOpen}>{children}</Children>
+      </Transformer>
+    </Container>
+  );
+}
+
+export default DrawerSection;

frontend/src/metabase/components/DrawerSection/DrawerSection.styled.js

+import styled from "styled-components";
+
+import { color } from "metabase/lib/colors";
+
+const HEADER_HEIGHT = "49px";
+
+export const Container = styled.div`
+  min-height: ${HEADER_HEIGHT};
+  height: ${props => (props.isOpen ? "auto" : "100%")};
+  overflow: ${props => (props.isOpen ? "unset" : "hidden")};
+  position: relative;
+  width: 100%;
+`;
+
+export const Transformer = styled.div`
+  height: 100%;
+  position: relative;
+  width: 100%;
+
+  will-change: transform;
+  transform: ${props =>
+    props.isOpen
+      ? "translateY(0)"
+      : `translateY(calc(100% - ${HEADER_HEIGHT}))`};
+  transition: transform 0.2s ease-in-out;
+`;
+
+export const Children = styled.div`
+  display: ${props => (props.isOpen ? "block" : "none")};
+  padding: 0 1.5rem;
+`;
+
+export const Header = styled.div.attrs({
+  role: "button",
+  tabIndex: "0",
+})`
+  height: ${HEADER_HEIGHT};
+  cursor: pointer;
+  display: flex;
+  align-items: center;
+  justify-content: space-between;
+  border-top: 1px solid ${color("border")};
+  font-weight: 700;
+  padding: 0 1.5rem;
+
+  &:hover {
+    color: ${color("brand")};
+  }
+`;

frontend/src/metabase/components/Timeline.jsx

@@ -19,7 +19,8 @@ Timeline.propTypes = {
   items: PropTypes.arrayOf(
     PropTypes.shape({
       timestamp: PropTypes.number.isRequired,
-      icon: PropTypes.string.isRequired,
+      icon: PropTypes.oneOfType([PropTypes.string, PropTypes.object])
+        .isRequired,
       title: PropTypes.string.isRequired,
       description: PropTypes.string,
       renderFooter: PropTypes.bool,
@@ -61,11 +62,16 @@ function Timeline({ className, items = [], renderFooter }) {
         } = item;
         const key = item.key == null ? index : item.key;
         const isNotLastEvent = index !== sortedFormattedItems.length - 1;
+        const iconProps = _.isObject(icon)
+          ? icon
+          : {
+              name: icon,
+            };
 
         return (
           <TimelineItem key={key} leftShift={halfIconSize}>
             {isNotLastEvent && <Border borderShift={halfIconSize} />}
-            <ItemIcon name={icon} size={iconSize} />
+            <ItemIcon {...iconProps} size={iconSize} />
             <ItemBody>
               <ItemHeader>{title}</ItemHeader>
               <Timestamp datetime={timestamp}>{formattedTimestamp}</Timestamp>

frontend/src/metabase/components/Timeline.styled.jsx

@@ -20,7 +20,7 @@ export const TimelineItem = styled.div`
 
 export const ItemIcon = styled(Icon)`
   position: relative;
-  color: ${color("text-light")};
+  color: ${props => (props.color ? color(props.color) : color("text-light"))};
 `;
 
 export const ItemBody = styled.div`

frontend/src/metabase/plugins/index.js

@@ -3,6 +3,7 @@ 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 = () => ({});
+const array = () => [];
 
 // functions called when the application is started
 export const PLUGIN_APP_INIT_FUCTIONS = [];
@@ -84,4 +85,5 @@ export const PLUGIN_MODERATION = {
   ModerationStatusIcon: PluginPlaceholder,
   getStatusIconForQuestion: object,
   getStatusIcon: object,
+  getModerationTimelineEvents: array,
 };

frontend/src/metabase/query_builder/actions.js

@@ -129,6 +129,12 @@ export const onOpenQuestionDetails = createAction(
 export const onCloseQuestionDetails = createAction(
   "metabase/qb/CLOSE_QUESTION_DETAILS",
 );
+export const onOpenQuestionHistory = createAction(
+  "metabase/qb/OPEN_QUESTION_HISTORY",
+);
+export const onCloseQuestionHistory = createAction(
+  "metabase/qb/CLOSE_QUESTION_HISTORY",
+);
 
 export const onCloseChartType = createAction("metabase/qb/CLOSE_CHART_TYPE");
 export const onCloseSidebars = createAction("metabase/qb/CLOSE_SIDEBARS");

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

-import React from "react";
+import React, { useMemo } from "react";
 import PropTypes from "prop-types";
 import { t } from "ttag";
 import { connect } from "react-redux";
 import _ from "underscore";
 
+import { PLUGIN_MODERATION } from "metabase/plugins";
 import { getRevisionEventsForTimeline } from "metabase/lib/revisions";
-import { revertToRevision } from "metabase/query_builder/actions";
+import {
+  revertToRevision,
+  onOpenQuestionHistory,
+  onCloseQuestionHistory,
+} from "metabase/query_builder/actions";
+import { getUser } from "metabase/selectors/user";
+import { getQuestionDetailsTimelineDrawerState } from "metabase/query_builder/selectors";
 
 import Revision from "metabase/entities/revisions";
+import User from "metabase/entities/users";
 import Timeline from "metabase/components/Timeline";
-import {
-  SidebarSectionHeader,
-  RevertButton,
-} from "./QuestionActivityTimeline.styled";
+import DrawerSection, {
+  STATES as DRAWER_STATES,
+} from "metabase/components/DrawerSection/DrawerSection";
+import { RevertButton } from "./QuestionActivityTimeline.styled";
+
+const { getModerationTimelineEvents } = PLUGIN_MODERATION;
+
+const mapStateToProps = (state, props) => ({
+  currentUser: getUser(state),
+  drawerState: getQuestionDetailsTimelineDrawerState(state, props),
+});
+
+const mapDispatchToProps = {
+  revertToRevision,
+  onOpenQuestionHistory,
+  onCloseQuestionHistory,
+};
 
 export default _.compose(
+  User.loadList({
+    loadingAndErrorWrapper: false,
+  }),
   Revision.loadList({
     query: (state, props) => ({
       model_type: "card",
@@ -23,10 +47,8 @@ export default _.compose(
     wrapped: true,
   }),
   connect(
-    null,
-    {
-      revertToRevision,
-    },
+    mapStateToProps,
+    mapDispatchToProps,
   ),
 )(QuestionActivityTimeline);
 
@@ -51,25 +73,55 @@ function RevisionEventFooter({ revision, onRevisionClick }) {
 
 QuestionActivityTimeline.propTypes = {
   question: PropTypes.object.isRequired,
-  className: PropTypes.string,
   revisions: PropTypes.array,
+  users: PropTypes.array,
+  currentUser: PropTypes.object.isRequired,
   revertToRevision: PropTypes.func.isRequired,
+  drawerState: PropTypes.oneOf([DRAWER_STATES.open, DRAWER_STATES.closed]),
+  onOpenQuestionHistory: PropTypes.func.isRequired,
+  onCloseQuestionHistory: PropTypes.func.isRequired,
 };
 
 export function QuestionActivityTimeline({
   question,
-  className,
   revisions,
+  users,
+  currentUser,
   revertToRevision,
+  drawerState,
+  onOpenQuestionHistory,
+  onCloseQuestionHistory,
 }) {
+  const usersById = useMemo(() => _.indexBy(users, "id"), [users]);
   const canWrite = question.canWrite();
-  const revisionEvents = getRevisionEventsForTimeline(revisions, canWrite);
+  const moderationReviews = question.getModerationReviews();
+
+  const events = useMemo(() => {
+    const moderationEvents = getModerationTimelineEvents(
+      moderationReviews,
+      usersById,
+      currentUser,
+    );
+    const revisionEvents = getRevisionEventsForTimeline(revisions, canWrite);
+    return [...revisionEvents, ...moderationEvents];
+  }, [canWrite, moderationReviews, revisions, usersById, currentUser]);
+
+  const onDrawerStateChange = state => {
+    if (state === DRAWER_STATES.open) {
+      onOpenQuestionHistory();
+    } else {
+      onCloseQuestionHistory();
+    }
+  };
 
   return (
-    <div className={className}>
-      <SidebarSectionHeader>{t`History`}</SidebarSectionHeader>
+    <DrawerSection
+      header={t`History`}
+      state={drawerState}
+      onStateChange={onDrawerStateChange}
+    >
       <Timeline
-        items={revisionEvents}
+        items={events}
         renderFooter={item => {
           const { isRevertable, revision } = item;
           if (isRevertable) {
@@ -82,6 +134,6 @@ export function QuestionActivityTimeline({
           }
         }}
       />
-    </div>
+    </DrawerSection>
   );
 }

frontend/src/metabase/query_builder/components/QuestionActivityTimeline.styled.jsx

 import styled from "styled-components";
 import { color } from "metabase/lib/colors";
 import ActionButton from "metabase/components/ActionButton";
-import Button from "metabase/components/Button";
-
-export const SidebarSectionHeader = styled.div`
-  color: ${color("text-medium")};
-  font-weight: bold;
-  padding-bottom: 1rem;
-`;
-
-export const RequestButton = styled(Button)`
-  padding: 0;
-  border: none;
-  color: ${props => color(props.color)};
-
-  &:hover {
-    text-decoration: underline;
-    background-color: transparent;
-    color: ${props => color(props.color)};
-  }
-`;
 
 export const RevertButton = styled(ActionButton).attrs({
   successClassName: "",

frontend/src/metabase/query_builder/components/view/ViewHeader.jsx

@@ -55,6 +55,7 @@ const viewTitleHeaderPropTypes = {
   onCloseFilter: PropTypes.func,
   onOpenQuestionDetails: PropTypes.func,
   onCloseQuestionDetails: PropTypes.func,
+  onOpenQuestionHistory: PropTypes.func,
 
   isPreviewable: PropTypes.bool,
   isPreviewing: PropTypes.bool,
@@ -119,6 +120,7 @@ export class ViewTitleHeader extends React.Component {
       isShowingQuestionDetailsSidebar,
       onOpenQuestionDetails,
       onCloseQuestionDetails,
+      onOpenQuestionHistory,
     } = this.props;
     const { isFiltersExpanded } = this.state;
     const isShowingNotebook = queryBuilderMode === "notebook";
@@ -161,7 +163,7 @@ export class ViewTitleHeader extends React.Component {
                 <LastEditInfoLabel
                   className="ml1 text-light"
                   item={question.card()}
-                  onClick={() => onOpenModal("history")}
+                  onClick={onOpenQuestionHistory}
                 />
               )}
             </div>

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

 import React from "react";
 import PropTypes from "prop-types";
 
-import SidebarContent from "metabase/query_builder/components/SidebarContent";
 import QuestionActionButtons from "metabase/query_builder/components/QuestionActionButtons";
 import { ClampedDescription } from "metabase/query_builder/components/ClampedDescription";
 import {
-  SidebarContentContainer,
-  BorderedQuestionActivityTimeline,
+  Container,
+  SidebarPaddedContent,
 } from "./QuestionDetailsSidebarPanel.styled";
+import QuestionActivityTimeline from "metabase/query_builder/components/QuestionActivityTimeline";
+
 import { PLUGIN_MODERATION } from "metabase/plugins";
 
 export default QuestionDetailsSidebarPanel;
@@ -33,8 +34,8 @@ function QuestionDetailsSidebarPanel({
     : undefined;
 
   return (
-    <SidebarContent>
-      <SidebarContentContainer>
+    <Container>
+      <SidebarPaddedContent>
         <QuestionActionButtons canWrite={canWrite} onOpenModal={onOpenModal} />
         <ClampedDescription
           className="pb2"
@@ -43,8 +44,8 @@ function QuestionDetailsSidebarPanel({
           onEdit={onDescriptionEdit}
         />
         <PLUGIN_MODERATION.QuestionModerationSection question={question} />
-        <BorderedQuestionActivityTimeline question={question} />
-      </SidebarContentContainer>
-    </SidebarContent>
+      </SidebarPaddedContent>
+      <QuestionActivityTimeline question={question} />
+    </Container>
   );
 }

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

 import styled from "styled-components";
-import { color } from "metabase/lib/colors";
-import QuestionActivityTimeline from "metabase/query_builder/components/QuestionActivityTimeline";
 
-export const SidebarContentContainer = styled.div`
+export const Container = styled.div`
+  height: 100%;
+  overflow-y: auto;
   display: flex;
   flex-direction: column;
-  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;
+export const SidebarPaddedContent = styled.div`
+  display: flex;
+  flex-direction: column;
+  row-gap: 1rem;
+  padding: 0.5rem 1.5rem 1rem 1.5rem;
 `;

frontend/src/metabase/query_builder/reducers.js

@@ -47,6 +47,8 @@ import {
   onCloseSidebars,
   onOpenQuestionDetails,
   onCloseQuestionDetails,
+  onOpenQuestionHistory,
+  onCloseQuestionHistory,
 } from "./actions";
 
 const DEFAULT_UI_CONTROLS = {
@@ -216,10 +218,24 @@ export const uiControls = handleActions(
       ...state,
       ...UI_CONTROLS_SIDEBAR_DEFAULTS,
       isShowingQuestionDetailsSidebar: true,
+      questionDetailsTimelineDrawerState: undefined,
     }),
     [onCloseQuestionDetails]: state => ({
       ...state,
       ...UI_CONTROLS_SIDEBAR_DEFAULTS,
+      questionDetailsTimelineDrawerState: undefined,
+    }),
+    [onOpenQuestionHistory]: state => ({
+      ...state,
+      ...UI_CONTROLS_SIDEBAR_DEFAULTS,
+      isShowingQuestionDetailsSidebar: true,
+      questionDetailsTimelineDrawerState: "open",
+    }),
+    [onCloseQuestionHistory]: state => ({
+      ...state,
+      ...UI_CONTROLS_SIDEBAR_DEFAULTS,
+      isShowingQuestionDetailsSidebar: true,
+      questionDetailsTimelineDrawerState: "closed",
     }),
     [onCloseSidebars]: state => ({
       ...state,

frontend/src/metabase/query_builder/selectors.js

@@ -462,3 +462,8 @@ export const getIsLiveResizable = createSelector(
     }
   },
 );
+
+export const getQuestionDetailsTimelineDrawerState = createSelector(
+  [getUiControls],
+  uiControls => uiControls && uiControls.questionDetailsTimelineDrawerState,
+);