From 3d7a638b2081e0ff1706dab9ab80412a2f9fc01a Mon Sep 17 00:00:00 2001
From: Krzysztof Wolski <krzysztof.k.wolski@gmail.com>
Date: Wed, 29 Apr 2020 08:47:08 +0200
Subject: [PATCH 1/2] Handle errors on group deletion

---
 .../PermissionGroupDeleteDialog.stories.tsx   |  8 +++++-
 .../PermissionGroupDeleteDialog.tsx           |  8 ++++--
 .../PermissionGroupList.tsx                   | 18 +++++++------
 src/permissionGroups/fixtures.ts              |  7 +++++
 src/permissionGroups/queries.ts               |  1 +
 .../types/PermissionGroupCreate.ts            |  1 +
 .../types/PermissionGroupDetails.ts           |  1 +
 .../types/PermissionGroupDetailsFragment.ts   |  1 +
 .../types/PermissionGroupFragment.ts          |  1 +
 .../types/PermissionGroupList.ts              |  1 +
 .../types/PermissionGroupUpdate.ts            |  1 +
 .../PermissionGroupList.tsx                   | 11 ++++++++
 .../__snapshots__/Stories.test.ts.snap        | 27 +++++--------------
 13 files changed, 54 insertions(+), 32 deletions(-)

diff --git a/src/permissionGroups/components/PermissionGroupDeleteDialog/PermissionGroupDeleteDialog.stories.tsx b/src/permissionGroups/components/PermissionGroupDeleteDialog/PermissionGroupDeleteDialog.stories.tsx
index b76c86949..4b43cffa7 100644
--- a/src/permissionGroups/components/PermissionGroupDeleteDialog/PermissionGroupDeleteDialog.stories.tsx
+++ b/src/permissionGroups/components/PermissionGroupDeleteDialog/PermissionGroupDeleteDialog.stories.tsx
@@ -16,4 +16,10 @@ const props: PermissionDeleteDialogProps = {
 
 storiesOf("Views / Permission Groups / Permission Group Delete", module)
   .addDecorator(Decorator)
-  .add("remove single", () => <PermissionGroupDeleteDialog {...props} />);
+  .add("remove single", () => <PermissionGroupDeleteDialog {...props} />)
+  .add("Got error", () => (
+    <PermissionGroupDeleteDialog
+      {...props}
+      error={"Something went realy wrong."}
+    />
+  ));
diff --git a/src/permissionGroups/components/PermissionGroupDeleteDialog/PermissionGroupDeleteDialog.tsx b/src/permissionGroups/components/PermissionGroupDeleteDialog/PermissionGroupDeleteDialog.tsx
index 1c66b87ee..fc539cf34 100644
--- a/src/permissionGroups/components/PermissionGroupDeleteDialog/PermissionGroupDeleteDialog.tsx
+++ b/src/permissionGroups/components/PermissionGroupDeleteDialog/PermissionGroupDeleteDialog.tsx
@@ -4,17 +4,20 @@ import { FormattedMessage, useIntl } from "react-intl";
 
 import ActionDialog from "@saleor/components/ActionDialog";
 import { ConfirmButtonTransitionState } from "@saleor/components/ConfirmButton";
+import { Typography } from "@material-ui/core";
 
 export interface PermissionDeleteDialogProps {
   confirmButtonState: ConfirmButtonTransitionState;
-  open: boolean;
+  error?: string;
   name: string;
-  onConfirm: () => void;
   onClose: () => void;
+  onConfirm: () => void;
+  open: boolean;
 }
 
 const PermissionGroupDeleteDialog: React.FC<PermissionDeleteDialogProps> = ({
   confirmButtonState,
+  error,
   name,
   onClose,
   onConfirm,
@@ -43,6 +46,7 @@ const PermissionGroupDeleteDialog: React.FC<PermissionDeleteDialogProps> = ({
           }}
         />
       </DialogContentText>
+      {!!error && <Typography color="error">{error}</Typography>}
     </ActionDialog>
   );
 };
diff --git a/src/permissionGroups/components/PermissionGroupList/PermissionGroupList.tsx b/src/permissionGroups/components/PermissionGroupList/PermissionGroupList.tsx
index 3018c2a5b..694c696a5 100644
--- a/src/permissionGroups/components/PermissionGroupList/PermissionGroupList.tsx
+++ b/src/permissionGroups/components/PermissionGroupList/PermissionGroupList.tsx
@@ -148,14 +148,16 @@ const PermissionGroupList: React.FC<PermissionGroupListProps> = props => {
               <TableCell className={classes.colActions}>
                 {permissionGroup ? (
                   <>
-                    <IconButton
-                      color="primary"
-                      onClick={stopPropagation(() =>
-                        onDelete(permissionGroup.id)
-                      )}
-                    >
-                      <DeleteIcon />
-                    </IconButton>
+                    {permissionGroup.userCanManage && (
+                      <IconButton
+                        color="primary"
+                        onClick={stopPropagation(() =>
+                          onDelete(permissionGroup.id)
+                        )}
+                      >
+                        <DeleteIcon />
+                      </IconButton>
+                    )}
                     <IconButton color="primary">
                       <EditIcon />
                     </IconButton>
diff --git a/src/permissionGroups/fixtures.ts b/src/permissionGroups/fixtures.ts
index 57ba617a5..1fc235bfb 100644
--- a/src/permissionGroups/fixtures.ts
+++ b/src/permissionGroups/fixtures.ts
@@ -16,6 +16,7 @@ export const permissionGroups: PermissionGroupList_permissionGroups_edges_node[]
     node: {
       id: "R3JvdXA6Mg==",
       name: "Customer Support",
+      userCanManage: true,
       users: [
         {
           id: "VXNlcjoyMQ==",
@@ -32,6 +33,7 @@ export const permissionGroups: PermissionGroupList_permissionGroups_edges_node[]
     node: {
       id: "R3JvdXA6MQ==",
       name: "Full Access",
+      userCanManage: false,
       users: [
         {
           id: "VXNlcjoyMQ==",
@@ -49,6 +51,7 @@ export const permissionGroups: PermissionGroupList_permissionGroups_edges_node[]
       id: "R3JvdXA6NA==",
       name: "Management",
       users: [],
+      userCanManage: true,
       __typename: "Group" as "Group"
     },
     __typename: "GroupCountableEdge" as "GroupCountableEdge"
@@ -57,6 +60,7 @@ export const permissionGroups: PermissionGroupList_permissionGroups_edges_node[]
     node: {
       id: "R3JvdXA6Mw==",
       name: "Editors",
+      userCanManage: true,
       users: [
         {
           id: "VXNlcjoyMw==",
@@ -79,6 +83,7 @@ export const permissionGroups: PermissionGroupList_permissionGroups_edges_node[]
     node: {
       id: "R3JvdXA6NQ==",
       name: "Publishers",
+      userCanManage: true,
       users: [],
       __typename: "Group" as "Group"
     },
@@ -104,6 +109,7 @@ export const userPermissionGroups: StaffMemberDetails_user_permissionGroups[] =
 export const emptyPermissionGroup: PermissionGroupDetails_permissionGroup = {
   id: "R3JvdXA6Mw==",
   name: "Editors",
+  userCanManage: true,
   users: [],
   __typename: "Group",
   permissions: [
@@ -131,6 +137,7 @@ export const errorsOfPermissionGroupCreate: PermissionGroupErrorFragment[] = [
 export const permissionGroup: PermissionGroupDetails_permissionGroup = {
   id: "R3JvdXA6Mw==",
   name: "Editors",
+  userCanManage: true,
   users: [
     {
       id: "VXNlcjoyMg==",
diff --git a/src/permissionGroups/queries.ts b/src/permissionGroups/queries.ts
index 80f84e6d0..9050a502a 100644
--- a/src/permissionGroups/queries.ts
+++ b/src/permissionGroups/queries.ts
@@ -16,6 +16,7 @@ export const permissionGroupFragment = gql`
   fragment PermissionGroupFragment on Group {
     id
     name
+    userCanManage
     users {
       id
       firstName
diff --git a/src/permissionGroups/types/PermissionGroupCreate.ts b/src/permissionGroups/types/PermissionGroupCreate.ts
index b80003a2c..6eee4ebd4 100644
--- a/src/permissionGroups/types/PermissionGroupCreate.ts
+++ b/src/permissionGroups/types/PermissionGroupCreate.ts
@@ -39,6 +39,7 @@ export interface PermissionGroupCreate_permissionGroupCreate_group {
   __typename: "Group";
   id: string;
   name: string;
+  userCanManage: boolean;
   users: (PermissionGroupCreate_permissionGroupCreate_group_users | null)[] | null;
   permissions: (PermissionGroupCreate_permissionGroupCreate_group_permissions | null)[] | null;
 }
diff --git a/src/permissionGroups/types/PermissionGroupDetails.ts b/src/permissionGroups/types/PermissionGroupDetails.ts
index bfe0a6a1a..cde1dea2b 100644
--- a/src/permissionGroups/types/PermissionGroupDetails.ts
+++ b/src/permissionGroups/types/PermissionGroupDetails.ts
@@ -33,6 +33,7 @@ export interface PermissionGroupDetails_permissionGroup {
   __typename: "Group";
   id: string;
   name: string;
+  userCanManage: boolean;
   users: (PermissionGroupDetails_permissionGroup_users | null)[] | null;
   permissions: (PermissionGroupDetails_permissionGroup_permissions | null)[] | null;
 }
diff --git a/src/permissionGroups/types/PermissionGroupDetailsFragment.ts b/src/permissionGroups/types/PermissionGroupDetailsFragment.ts
index b6cc4b41c..189247a10 100644
--- a/src/permissionGroups/types/PermissionGroupDetailsFragment.ts
+++ b/src/permissionGroups/types/PermissionGroupDetailsFragment.ts
@@ -33,6 +33,7 @@ export interface PermissionGroupDetailsFragment {
   __typename: "Group";
   id: string;
   name: string;
+  userCanManage: boolean;
   users: (PermissionGroupDetailsFragment_users | null)[] | null;
   permissions: (PermissionGroupDetailsFragment_permissions | null)[] | null;
 }
diff --git a/src/permissionGroups/types/PermissionGroupFragment.ts b/src/permissionGroups/types/PermissionGroupFragment.ts
index e7adff753..78f46b529 100644
--- a/src/permissionGroups/types/PermissionGroupFragment.ts
+++ b/src/permissionGroups/types/PermissionGroupFragment.ts
@@ -17,5 +17,6 @@ export interface PermissionGroupFragment {
   __typename: "Group";
   id: string;
   name: string;
+  userCanManage: boolean;
   users: (PermissionGroupFragment_users | null)[] | null;
 }
diff --git a/src/permissionGroups/types/PermissionGroupList.ts b/src/permissionGroups/types/PermissionGroupList.ts
index d797e562f..204a417d0 100644
--- a/src/permissionGroups/types/PermissionGroupList.ts
+++ b/src/permissionGroups/types/PermissionGroupList.ts
@@ -19,6 +19,7 @@ export interface PermissionGroupList_permissionGroups_edges_node {
   __typename: "Group";
   id: string;
   name: string;
+  userCanManage: boolean;
   users: (PermissionGroupList_permissionGroups_edges_node_users | null)[] | null;
 }
 
diff --git a/src/permissionGroups/types/PermissionGroupUpdate.ts b/src/permissionGroups/types/PermissionGroupUpdate.ts
index d47ea0051..4ff8bd464 100644
--- a/src/permissionGroups/types/PermissionGroupUpdate.ts
+++ b/src/permissionGroups/types/PermissionGroupUpdate.ts
@@ -39,6 +39,7 @@ export interface PermissionGroupUpdate_permissionGroupUpdate_group {
   __typename: "Group";
   id: string;
   name: string;
+  userCanManage: boolean;
   users: (PermissionGroupUpdate_permissionGroupUpdate_group_users | null)[] | null;
   permissions: (PermissionGroupUpdate_permissionGroupUpdate_group_permissions | null)[] | null;
 }
diff --git a/src/permissionGroups/views/PermissionGroupList/PermissionGroupList.tsx b/src/permissionGroups/views/PermissionGroupList/PermissionGroupList.tsx
index 74ca807b7..661ee88ed 100644
--- a/src/permissionGroups/views/PermissionGroupList/PermissionGroupList.tsx
+++ b/src/permissionGroups/views/PermissionGroupList/PermissionGroupList.tsx
@@ -18,6 +18,7 @@ import { usePermissionGroupListQuery } from "@saleor/permissionGroups/queries";
 import { PermissionGroupDelete } from "@saleor/permissionGroups/types/PermissionGroupDelete";
 import { usePermissionGroupDelete } from "@saleor/permissionGroups/mutations";
 import { getStringOrPlaceholder } from "@saleor/misc";
+import getPermissionGroupErrorMessage from "@saleor/utils/errors/permissionGroups";
 import PermissionGroupListPage from "../../components/PermissionGroupListPage";
 import {
   permissionGroupListUrl,
@@ -74,6 +75,7 @@ export const PermissionGroupList: React.FC<PermissionGroupListProps> = ({
   >(navigate, permissionGroupListUrl, params);
 
   const permissionGroups = data?.permissionGroups?.edges.map(edge => edge.node);
+  const [deleteError, setDeleteError] = React.useState<string>();
 
   const handleDeleteSuccess = (data: PermissionGroupDelete) => {
     if (data.permissionGroupDelete.errors.length === 0) {
@@ -83,7 +85,15 @@ export const PermissionGroupList: React.FC<PermissionGroupListProps> = ({
         })
       });
       refetch();
+      setDeleteError("");
       closeModal();
+    } else {
+      setDeleteError(
+        getPermissionGroupErrorMessage(
+          data.permissionGroupDelete.errors[0],
+          intl
+        )
+      );
     }
   };
 
@@ -115,6 +125,7 @@ export const PermissionGroupList: React.FC<PermissionGroupListProps> = ({
             }
           })
         }
+        error={deleteError}
         name={getStringOrPlaceholder(
           permissionGroups?.find(group => group.id === params.id)?.name
         )}
diff --git a/src/storybook/__snapshots__/Stories.test.ts.snap b/src/storybook/__snapshots__/Stories.test.ts.snap
index 256772bbf..c0e8783ee 100644
--- a/src/storybook/__snapshots__/Stories.test.ts.snap
+++ b/src/storybook/__snapshots__/Stories.test.ts.snap
@@ -87464,6 +87464,12 @@ exports[`Storyshots Views / Permission Groups / Permission Group Create loading
 </div>
 `;
 
+exports[`Storyshots Views / Permission Groups / Permission Group Delete Got error 1`] = `
+<div
+  style="padding:24px"
+/>
+`;
+
 exports[`Storyshots Views / Permission Groups / Permission Group Delete remove single 1`] = `
 <div
   style="padding:24px"
@@ -90331,27 +90337,6 @@ exports[`Storyshots Views / Permission Groups / Permission Group List default 1`
               <td
                 class="MuiTableCell-root-id MuiTableCell-body-id PermissionGroupList-colActions-id"
               >
-                <button
-                  class="MuiButtonBase-root-id MuiIconButton-root-id MuiIconButton-colorPrimary-id"
-                  tabindex="0"
-                  type="button"
-                >
-                  <span
-                    class="MuiIconButton-label-id"
-                  >
-                    <svg
-                      aria-hidden="true"
-                      class="MuiSvgIcon-root-id"
-                      focusable="false"
-                      role="presentation"
-                      viewBox="0 0 24 24"
-                    >
-                      <path
-                        d="M6 19c0 1.1.9 2 2 2h8c1.1 0 2-.9 2-2V7H6v12zM19 4h-3.5l-1-1h-5l-1 1H5v2h14V4z"
-                      />
-                    </svg>
-                  </span>
-                </button>
                 <button
                   class="MuiButtonBase-root-id MuiIconButton-root-id MuiIconButton-colorPrimary-id"
                   tabindex="0"

From da950ce0795bacf69a88ab422b12bde93a652207 Mon Sep 17 00:00:00 2001
From: Krzysztof Wolski <krzysztof.k.wolski@gmail.com>
Date: Thu, 30 Apr 2020 11:06:09 +0200
Subject: [PATCH 2/2] Review fixes

---
 .../PermissionGroupDeleteDialog.stories.tsx   | 24 +++++++++++++++----
 .../PermissionGroupDeleteDialog.tsx           | 18 ++++++++++++--
 .../PermissionGroupList.tsx                   | 15 +++++-------
 .../__snapshots__/Stories.test.ts.snap        |  8 ++++++-
 4 files changed, 48 insertions(+), 17 deletions(-)

diff --git a/src/permissionGroups/components/PermissionGroupDeleteDialog/PermissionGroupDeleteDialog.stories.tsx b/src/permissionGroups/components/PermissionGroupDeleteDialog/PermissionGroupDeleteDialog.stories.tsx
index 4b43cffa7..ee5bb800d 100644
--- a/src/permissionGroups/components/PermissionGroupDeleteDialog/PermissionGroupDeleteDialog.stories.tsx
+++ b/src/permissionGroups/components/PermissionGroupDeleteDialog/PermissionGroupDeleteDialog.stories.tsx
@@ -5,6 +5,20 @@ import PermissionGroupDeleteDialog, {
   PermissionDeleteDialogProps
 } from "@saleor/permissionGroups/components/PermissionGroupDeleteDialog";
 import Decorator from "@saleor/storybook/Decorator";
+import { PermissionGroupErrorFragment } from "@saleor/permissionGroups/types/PermissionGroupErrorFragment";
+import { PermissionGroupErrorCode } from "@saleor/types/globalTypes";
+
+const permissionsError: PermissionGroupErrorFragment = {
+  __typename: "PermissionGroupError",
+  code: PermissionGroupErrorCode.OUT_OF_SCOPE_PERMISSION,
+  field: null
+};
+
+const requiredError: PermissionGroupErrorFragment = {
+  __typename: "PermissionGroupError",
+  code: PermissionGroupErrorCode.REQUIRED,
+  field: null
+};
 
 const props: PermissionDeleteDialogProps = {
   confirmButtonState: "default",
@@ -17,9 +31,9 @@ const props: PermissionDeleteDialogProps = {
 storiesOf("Views / Permission Groups / Permission Group Delete", module)
   .addDecorator(Decorator)
   .add("remove single", () => <PermissionGroupDeleteDialog {...props} />)
-  .add("Got error", () => (
-    <PermissionGroupDeleteDialog
-      {...props}
-      error={"Something went realy wrong."}
-    />
+  .add("Got permissions error", () => (
+    <PermissionGroupDeleteDialog {...props} error={permissionsError} />
+  ))
+  .add("Get random permission group error", () => (
+    <PermissionGroupDeleteDialog {...props} error={requiredError} />
   ));
diff --git a/src/permissionGroups/components/PermissionGroupDeleteDialog/PermissionGroupDeleteDialog.tsx b/src/permissionGroups/components/PermissionGroupDeleteDialog/PermissionGroupDeleteDialog.tsx
index fc539cf34..930951170 100644
--- a/src/permissionGroups/components/PermissionGroupDeleteDialog/PermissionGroupDeleteDialog.tsx
+++ b/src/permissionGroups/components/PermissionGroupDeleteDialog/PermissionGroupDeleteDialog.tsx
@@ -5,10 +5,13 @@ import { FormattedMessage, useIntl } from "react-intl";
 import ActionDialog from "@saleor/components/ActionDialog";
 import { ConfirmButtonTransitionState } from "@saleor/components/ConfirmButton";
 import { Typography } from "@material-ui/core";
+import { PermissionGroupErrorFragment } from "@saleor/permissionGroups/types/PermissionGroupErrorFragment";
+import getPermissionGroupErrorMessage from "@saleor/utils/errors/permissionGroups";
+import { PermissionGroupErrorCode } from "@saleor/types/globalTypes";
 
 export interface PermissionDeleteDialogProps {
   confirmButtonState: ConfirmButtonTransitionState;
-  error?: string;
+  error?: PermissionGroupErrorFragment;
   name: string;
   onClose: () => void;
   onConfirm: () => void;
@@ -25,6 +28,17 @@ const PermissionGroupDeleteDialog: React.FC<PermissionDeleteDialogProps> = ({
 }) => {
   const intl = useIntl();
 
+  let errorMessage;
+  if (error?.code === PermissionGroupErrorCode.OUT_OF_SCOPE_PERMISSION) {
+    errorMessage = intl.formatMessage({
+      defaultMessage:
+        "Cant's delete group which is out of your permission scope",
+      description: "deletion error message"
+    });
+  } else if (!!error) {
+    errorMessage = getPermissionGroupErrorMessage(error, intl);
+  }
+
   return (
     <ActionDialog
       open={open}
@@ -46,7 +60,7 @@ const PermissionGroupDeleteDialog: React.FC<PermissionDeleteDialogProps> = ({
           }}
         />
       </DialogContentText>
-      {!!error && <Typography color="error">{error}</Typography>}
+      {!!errorMessage && <Typography color="error">{errorMessage}</Typography>}
     </ActionDialog>
   );
 };
diff --git a/src/permissionGroups/views/PermissionGroupList/PermissionGroupList.tsx b/src/permissionGroups/views/PermissionGroupList/PermissionGroupList.tsx
index 661ee88ed..b6f602f5d 100644
--- a/src/permissionGroups/views/PermissionGroupList/PermissionGroupList.tsx
+++ b/src/permissionGroups/views/PermissionGroupList/PermissionGroupList.tsx
@@ -18,7 +18,7 @@ import { usePermissionGroupListQuery } from "@saleor/permissionGroups/queries";
 import { PermissionGroupDelete } from "@saleor/permissionGroups/types/PermissionGroupDelete";
 import { usePermissionGroupDelete } from "@saleor/permissionGroups/mutations";
 import { getStringOrPlaceholder } from "@saleor/misc";
-import getPermissionGroupErrorMessage from "@saleor/utils/errors/permissionGroups";
+import { PermissionGroupErrorFragment } from "@saleor/permissionGroups/types/PermissionGroupErrorFragment";
 import PermissionGroupListPage from "../../components/PermissionGroupListPage";
 import {
   permissionGroupListUrl,
@@ -75,7 +75,9 @@ export const PermissionGroupList: React.FC<PermissionGroupListProps> = ({
   >(navigate, permissionGroupListUrl, params);
 
   const permissionGroups = data?.permissionGroups?.edges.map(edge => edge.node);
-  const [deleteError, setDeleteError] = React.useState<string>();
+  const [deleteError, setDeleteError] = React.useState<
+    PermissionGroupErrorFragment
+  >();
 
   const handleDeleteSuccess = (data: PermissionGroupDelete) => {
     if (data.permissionGroupDelete.errors.length === 0) {
@@ -85,15 +87,10 @@ export const PermissionGroupList: React.FC<PermissionGroupListProps> = ({
         })
       });
       refetch();
-      setDeleteError("");
+      setDeleteError(undefined);
       closeModal();
     } else {
-      setDeleteError(
-        getPermissionGroupErrorMessage(
-          data.permissionGroupDelete.errors[0],
-          intl
-        )
-      );
+      setDeleteError(data.permissionGroupDelete.errors[0]);
     }
   };
 
diff --git a/src/storybook/__snapshots__/Stories.test.ts.snap b/src/storybook/__snapshots__/Stories.test.ts.snap
index c0e8783ee..0b2e2a331 100644
--- a/src/storybook/__snapshots__/Stories.test.ts.snap
+++ b/src/storybook/__snapshots__/Stories.test.ts.snap
@@ -87464,7 +87464,13 @@ exports[`Storyshots Views / Permission Groups / Permission Group Create loading
 </div>
 `;
 
-exports[`Storyshots Views / Permission Groups / Permission Group Delete Got error 1`] = `
+exports[`Storyshots Views / Permission Groups / Permission Group Delete Get random permission group error 1`] = `
+<div
+  style="padding:24px"
+/>
+`;
+
+exports[`Storyshots Views / Permission Groups / Permission Group Delete Got permissions error 1`] = `
 <div
   style="padding:24px"
 />