Fix dashboard and question actions visibility depending on user's permissions (#15654)

* Fix dashboard edit action visibility

Hiding "Change title and description" from users with read-only access to dashboard

* Fix dashboard archive action visibility

Hiding "Archive" action from users with read-only permission to dashboard

* Enable #13229 repro test

* Test user with read permissions can't revert question history

* Fix user with read-only permissions sees revert buttons

frontend/src/metabase/components/HistoryModal.jsx

@@ -20,7 +20,7 @@ function formatDate(date) {
 export default class HistoryModal extends Component {
   static propTypes = {
     revisions: PropTypes.array,
-    onRevert: PropTypes.func.isRequired,
+    onRevert: PropTypes.func,
     onClose: PropTypes.func.isRequired,
   };
 
@@ -60,7 +60,7 @@ export default class HistoryModal extends Component {
                   <span>{this.revisionDescription(revision)}</span>
                 </td>
                 <td className={cellClassName}>
-                  {index !== 0 && (
+                  {index !== 0 && onRevert && (
                     <ActionButton
                       actionFn={() => onRevert(revision)}
                       className="Button Button--small Button--danger text-uppercase"

frontend/src/metabase/containers/HistoryModal.jsx

 /* eslint-disable react/prop-types */
 import React from "react";
+import PropTypes from "prop-types";
 
 import HistoryModal from "metabase/components/HistoryModal";
 import Revision from "metabase/entities/revisions";
@@ -12,17 +13,24 @@ import Revision from "metabase/entities/revisions";
   wrapped: true,
 })
 export default class HistoryModalContainer extends React.Component {
+  static propTypes = {
+    canRevert: PropTypes.bool.isRequired,
+  };
+
+  onRevert = async revision => {
+    const { onReverted } = this.props;
+    await revision.revert();
+    if (onReverted) {
+      onReverted();
+    }
+  };
+
   render() {
-    const { revisions, onClose, onReverted } = this.props;
+    const { revisions, canRevert, onClose } = this.props;
     return (
       <HistoryModal
         revisions={revisions}
-        onRevert={async revision => {
-          await revision.revert();
-          if (onReverted) {
-            onReverted();
-          }
-        }}
+        onRevert={canRevert ? this.onRevert : null}
         onClose={onClose}
       />
     );

frontend/src/metabase/dashboard/components/DashboardHeader.jsx

@@ -289,15 +289,17 @@ export default class DashboardHeader extends Component {
     if (!isFullscreen && !isEditing) {
       const extraButtonClassNames =
         "bg-brand-hover text-white-hover py2 px3 text-bold block cursor-pointer";
-      extraButtons.push(
-        <Link
-          className={extraButtonClassNames}
-          to={location.pathname + "/details"}
-          data-metabase-event={"Dashboard;EditDetails"}
-        >
-          {t`Change title and description`}
-        </Link>,
-      );
+      if (canEdit) {
+        extraButtons.push(
+          <Link
+            className={extraButtonClassNames}
+            to={location.pathname + "/details"}
+            data-metabase-event={"Dashboard;EditDetails"}
+          >
+            {t`Change title and description`}
+          </Link>,
+        );
+      }
       extraButtons.push(
         <Link
           className={extraButtonClassNames}
@@ -327,15 +329,17 @@ export default class DashboardHeader extends Component {
           </Link>,
         );
       }
-      extraButtons.push(
-        <Link
-          className={extraButtonClassNames}
-          to={location.pathname + "/archive"}
-          data-metabase-event={"Dashboard;Archive"}
-        >
-          {t`Archive`}
-        </Link>,
-      );
+      if (canEdit) {
+        extraButtons.push(
+          <Link
+            className={extraButtonClassNames}
+            to={location.pathname + "/archive"}
+            data-metabase-event={"Dashboard;Archive"}
+          >
+            {t`Archive`}
+          </Link>,
+        );
+      }
     }
 
     buttons.push(...getDashboardActions(this, this.props));

frontend/src/metabase/dashboard/components/DashboardHistoryModal.jsx

@@ -3,24 +3,23 @@ import React from "react";
 
 import HistoryModal from "metabase/containers/HistoryModal";
 import { withRouter } from "react-router";
-import { connect } from "react-redux";
-import { fetchDashboard } from "metabase/dashboard/dashboard";
+import Dashboards from "metabase/entities/dashboards";
 
 @withRouter
-@connect(
-  null,
-  { fetchDashboard },
-)
+@Dashboards.load({
+  id: (state, props) => parseInt(props.params.dashboardId),
+  wrapped: false,
+})
 export default class DashboardHistoryModal extends React.Component {
   render() {
-    const { fetchDashboard, onClose, location, params } = this.props;
-    const dashboardId = parseInt(params.dashboardId);
+    const { dashboard, fetch, onClose, location } = this.props;
     return (
       <HistoryModal
         modelType="dashboard"
-        modelId={dashboardId}
+        modelId={dashboard.id}
+        canRevert={dashboard.can_write}
         onReverted={() => {
-          fetchDashboard(dashboardId, location.query);
+          fetch(dashboard.id, location.query);
           onClose();
         }}
         onClose={onClose}

frontend/src/metabase/query_builder/containers/QuestionHistoryModal.jsx

 import React from "react";
+import PropTypes from "prop-types";
 
+import Questions from "metabase/entities/questions";
 import HistoryModal from "metabase/containers/HistoryModal";
 
-type Props = {
-  questionId: number,
-  onClose: () => void,
-  onReverted: () => void,
-};
+@Questions.load({
+  id: (state, props) => props.questionId,
+})
+class QuestionHistoryModal extends React.Component {
+  static propTypes = {
+    question: PropTypes.object.isRequired,
+    questionId: PropTypes.number.isRequired,
+    onClose: PropTypes.func.isRequired,
+    onReverted: PropTypes.func.isRequired,
+  };
 
-const QuestionHistoryModal = ({ questionId, onClose, onReverted }: Props) => (
-  <HistoryModal
-    modelType={"card"}
-    modelId={questionId}
-    onClose={onClose}
-    onReverted={onReverted}
-  />
-);
+  render() {
+    const { question, onClose, onReverted } = this.props;
+    return (
+      <HistoryModal
+        modelType={"card"}
+        modelId={question.id}
+        canRevert={question.can_write}
+        onClose={onClose}
+        onReverted={onReverted}
+      />
+    );
+  }
+}
 
 export default QuestionHistoryModal;

frontend/test/metabase/scenarios/collections/permissions.cy.spec.js

@@ -399,6 +399,22 @@ describe("collection permissions", () => {
             });
 
             describe("managing dashboard from the dashboard's edit menu", () => {
+              it("should not be offered to change title and description for dashboard in collections they have `read` access to (metabase#15280)", () => {
+                cy.visit("/dashboard/1");
+                cy.icon("ellipsis").click();
+                popover()
+                  .findByText("Change title and description")
+                  .should("not.exist");
+              });
+
+              it("should not be offered to archive dashboard in collections they have `read` access to (metabase#15280)", () => {
+                cy.visit("/dashboard/1");
+                cy.icon("ellipsis").click();
+                popover()
+                  .findByText("Archive")
+                  .should("not.exist");
+              });
+
               it("should not be offered to duplicate dashboard in collections they have `read` access to", () => {
                 cy.visit("/dashboard/1");
                 cy.icon("ellipsis").click();
@@ -517,7 +533,7 @@ describe("collection permissions", () => {
 
           onlyOn(permission === "view", () => {
             describe(`${user} user`, () => {
-              it.skip("should not see revert buttons (metabase#13229)", () => {
+              it("should not see dashboard revert buttons (metabase#13229)", () => {
                 cy.signIn(user);
                 cy.visit("/dashboard/1");
                 cy.icon("ellipsis").click();
@@ -526,6 +542,16 @@ describe("collection permissions", () => {
                   "not.exist",
                 );
               });
+
+              it("should not see question revert buttons (metabase#13229)", () => {
+                cy.signIn(user);
+                cy.visit("/question/1");
+                cy.icon("pencil").click();
+                cy.findByText("View revision history").click();
+                cy.findAllByRole("button", { name: "Revert" }).should(
+                  "not.exist",
+                );
+              });
             });
           });
         });