Skip to content

Commit

Permalink
Allow user without app permission access apps view (#4738)
Browse files Browse the repository at this point in the history
* Allow user without app permission access apps view

* Use css subgrid instead of multiple map loops

* Add protection to queries and mutation that use App fragments

* Allow user to see manage app view

* Refactor app permission check to hook

* Protect routes

* shouldShowInstalledApps

* Refactor AppListPage, write test for loading, empty state with checking manage_app perm

* Change manage button label depend on manage_apps

* Improve typing

* Extract messages

* ButtonWithTooltip

* Refactor HeaderOptions, block action when no manage_apps

* Improve buttons

* Add changset

* Adjust singlePermission tests

* Remove required hasManagedAppsPermission when default

* Improve action icons

* Improve naming and refactor AppListCardInstallButton

* Improve changset

* Improve tests
  • Loading branch information
poulch committed Apr 10, 2024
1 parent 3b8ac6c commit f94eb20
Show file tree
Hide file tree
Showing 56 changed files with 688 additions and 323 deletions.
6 changes: 6 additions & 0 deletions .changeset/thin-avocados-grow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"saleor-dashboard": patch
---

Allow all user to access to APPS tab without checking any permissions. User will be able to see installed app list and enter to each apps.
Each app will be responsible for checking user permissions.
7 changes: 7 additions & 0 deletions locale/defaultMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -3859,6 +3859,9 @@
"context": "header",
"string": "{webhookName} Details"
},
"ORQvOg": {
"string": "You don't have permission to perform this action"
},
"OTDo9I": {
"context": "tab name",
"string": "All staff members"
Expand Down Expand Up @@ -4404,6 +4407,10 @@
"S8kqP9": {
"string": "Conditions"
},
"S90DJO": {
"context": "Button with app settings label",
"string": "App settings"
},
"SBb6Ej": {
"context": "select a warehouse to fulfill product from",
"string": "Select warehouse..."
Expand Down
4 changes: 2 additions & 2 deletions playwright/tests/singlePermissions/channelsWebhooks.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ test("TC: SALEOR_11 User should be able to navigate to channel list as a staff m

await page.goto(URL_LIST.homePage);
await mainMenuPage.openConfiguration();
await mainMenuPage.expectMenuItemsCount(2);
await mainMenuPage.expectMenuItemsCount(3);
await configurationPage.openChannels();

await expect(channelPage.createChannelButton).toBeVisible();
Expand All @@ -30,7 +30,7 @@ test("TC: SALEOR_12 User should be able to navigate to webhooks and events as a
const configurationPage = new ConfigurationPage(page);

await page.goto(URL_LIST.configuration);
await mainMenuPage.expectMenuItemsCount(2);
await mainMenuPage.expectMenuItemsCount(3);
await configurationPage.openWebhooksAndEvents();
await expect(webhooksEventsPage.createAppButton).toBeVisible();
});
4 changes: 2 additions & 2 deletions playwright/tests/singlePermissions/contentPage.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ test("TC: SALEOR_14 User should be able to navigate to content list as a staff m
await page.goto(URL_LIST.homePage);
await mainMenuPage.openContent();
await expect(contentPage.createContentButton).toBeVisible();
await mainMenuPage.expectMenuItemsCount(3);
await mainMenuPage.expectMenuItemsCount(4);
await basePage.expectGridToBeAttached();
});
test("TC: SALEOR_15 User should be able to navigate to page types list as a staff member using CONTENT aka PAGE permission @e2e", async ({
Expand All @@ -36,5 +36,5 @@ test("TC: SALEOR_15 User should be able to navigate to page types list as a staf

await configurationPage.openPageTypes();
await expect(pageTypesPage.createPageTypeButton).toBeVisible();
await mainMenuPage.expectMenuItemsCount(3);
await mainMenuPage.expectMenuItemsCount(4);
});
2 changes: 1 addition & 1 deletion playwright/tests/singlePermissions/customer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@ test("TC: SALEOR_13 User should be able to navigate to customer list as a staff
await mainMenuPage.openCustomers();
await expect(customersPage.createCustomerButton).toBeVisible();
await basePage.expectGridToBeAttached();
await mainMenuPage.expectMenuItemsCount(2);
await mainMenuPage.expectMenuItemsCount(3);
});
4 changes: 2 additions & 2 deletions playwright/tests/singlePermissions/discount.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ test("TC: SALEOR_6 User should be able to navigate to discount list as a staff m
await mainMenuPage.openDiscounts();
await expect(discountsPage.createDiscountButton).toBeVisible();
await basePage.expectGridToBeAttached();
await mainMenuPage.expectMenuItemsCount(3);
await mainMenuPage.expectMenuItemsCount(4);
});

test("TC: SALEOR_7 User should be able to navigate to voucher list as a staff member using DISCOUNTS permission @e2e", async ({
Expand All @@ -32,5 +32,5 @@ test("TC: SALEOR_7 User should be able to navigate to voucher list as a staff me
await mainMenuPage.openVouchers();
await expect(vouchersPage.createVoucherButton).toBeVisible();
await basePage.expectGridToBeAttached();
await mainMenuPage.expectMenuItemsCount(3);
await mainMenuPage.expectMenuItemsCount(4);
});
4 changes: 2 additions & 2 deletions playwright/tests/singlePermissions/orders.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ test("TC: SALEOR_8 User should be able to navigate to order list as a staff memb
await mainMenuPage.openOrders();
await expect(ordersPage.createOrderButton).toBeVisible();
await basePage.expectGridToBeAttached();
await mainMenuPage.expectMenuItemsCount(3);
await mainMenuPage.expectMenuItemsCount(4);
});
test("TC: SALEOR_9 User should be able to navigate to draft list as a staff member using ORDER permission @e2e", async ({
page,
Expand All @@ -31,5 +31,5 @@ test("TC: SALEOR_9 User should be able to navigate to draft list as a staff memb
await mainMenuPage.openDrafts();
await expect(draftOrdersPage.createDraftOrderButton).toBeVisible();
await basePage.expectGridToBeAttached();
await mainMenuPage.expectMenuItemsCount(3);
await mainMenuPage.expectMenuItemsCount(4);
});
2 changes: 1 addition & 1 deletion playwright/tests/singlePermissions/plugins.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@ test("TC: SALEOR_16 User should be able to navigate to plugin list as a staff me
await page.goto(URL_LIST.configuration);
await configurationPage.openPlugins();
await expect(pluginsPage.pluginRow.first()).toBeVisible();
await mainMenuPage.expectMenuItemsCount(2);
await mainMenuPage.expectMenuItemsCount(3);
});
6 changes: 3 additions & 3 deletions playwright/tests/singlePermissions/product.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ test("TC: SALEOR_23 User should be able to navigate to product list as a staff m
await page.goto(URL_LIST.homePage);
await mainMenuPage.openProducts();
await expect(productPage.addProductButton).toBeVisible();
await mainMenuPage.expectMenuItemsCount(5);
await mainMenuPage.expectMenuItemsCount(6);
await basePage.expectGridToBeAttached();
});
test("TC: SALEOR_24 User should be able to navigate to collections list as a staff member using PRODUCT permission @e2e", async ({
Expand All @@ -31,7 +31,7 @@ test("TC: SALEOR_24 User should be able to navigate to collections list as a sta
await page.goto(URL_LIST.homePage);
await mainMenuPage.openCollections();
await expect(collectionsPage.createCollectionButton).toBeVisible();
await mainMenuPage.expectMenuItemsCount(5);
await mainMenuPage.expectMenuItemsCount(6);
await basePage.expectGridToBeAttached();
});
test("TC: SALEOR_25 User should be able to navigate to categories list as a staff member using PRODUCT permission @e2e", async ({
Expand All @@ -44,6 +44,6 @@ test("TC: SALEOR_25 User should be able to navigate to categories list as a staf
await page.goto(URL_LIST.homePage);
await mainMenuPage.openCategories();
await expect(categoriesPage.createCategoryButton).toBeVisible();
await mainMenuPage.expectMenuItemsCount(5);
await mainMenuPage.expectMenuItemsCount(6);
await basePage.expectGridToBeAttached();
});
2 changes: 1 addition & 1 deletion playwright/tests/singlePermissions/productType.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@ test("TC: SALEOR_17 User should be able to navigate to product type list as a st
await page.goto(URL_LIST.configuration);
await configurationPage.openProductTypes();
await expect(productTypePage.addProductTypeButton).toBeVisible();
await mainMenuPage.expectMenuItemsCount(2);
await mainMenuPage.expectMenuItemsCount(3);
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@ test("TC: SALEOR_18 User should be able to navigate to configuration as a staff
await mainMenuPage.openConfiguration();
await configurationPage.openSiteSettings();
await expect(siteSettingsPage.companyInfoSection).toBeVisible();
await mainMenuPage.expectMenuItemsCount(2);
await mainMenuPage.expectMenuItemsCount(3);
});
2 changes: 1 addition & 1 deletion playwright/tests/singlePermissions/shipping.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@ test("TC: SALEOR_21 User should be able to navigate to shipping zones page as a
await page.waitForTimeout(8000);
await configurationPage.openShippingMethods();
await expect(shippingMethodsPage.createShippingZoneButton).toBeVisible();
await mainMenuPage.expectMenuItemsCount(2);
await mainMenuPage.expectMenuItemsCount(3);
});
4 changes: 2 additions & 2 deletions playwright/tests/singlePermissions/staffMembers.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ test("TC: SALEOR_19 User should be able to navigate to staff members list page a
await page.goto(URL_LIST.configuration);
await configurationPage.openStaffMembers();
await expect(staffMembersPage.inviteStaffMembersButton).toBeVisible();
await mainMenuPage.expectMenuItemsCount(2);
await mainMenuPage.expectMenuItemsCount(3);
await basePage.expectGridToBeAttached();
});

Expand All @@ -35,6 +35,6 @@ test("TC: SALEOR_20 User should be able to navigate to permission group list pag
await page.goto(URL_LIST.configuration);
await configurationPage.openPermissionGroups();
await expect(permissionGroupsPage.createPermissionGroupButton).toBeVisible();
await mainMenuPage.expectMenuItemsCount(2);
await mainMenuPage.expectMenuItemsCount(3);
await basePage.expectGridToBeAttached();
});
2 changes: 1 addition & 1 deletion playwright/tests/singlePermissions/translations.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ test("TC: SALEOR_22 User should be able to navigate to translations page as a st
await page.goto(URL_LIST.homePage);
await mainMenuPage.openTranslations();
await expect(basePage.pageHeader).toHaveText("Languages");
await mainMenuPage.expectMenuItemsCount(2);
await mainMenuPage.expectMenuItemsCount(3);
});
8 changes: 6 additions & 2 deletions src/apps/apps-routing.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import {
AppDetailsUrlQueryParams,
AppInstallUrlQueryParams,
} from "@dashboard/apps/urls";
import SectionRoute from "@dashboard/auth/components/SectionRoute";
import { PermissionEnum } from "@dashboard/graphql";
import { sectionNames } from "@dashboard/intl";
import { parse as parseQs } from "qs";
import React from "react";
Expand Down Expand Up @@ -55,8 +57,9 @@ export const AppsSectionRoot = () => {
<WindowTitle title={intl.formatMessage(sectionNames.apps)} />
<Switch>
<Route exact path={AppPaths.appListPath} component={AppListRoute} />
<Route
<SectionRoute
exact
permissions={[PermissionEnum.MANAGE_APPS]}
path={AppPaths.appInstallPath}
component={AppInstallRoute}
/>
Expand All @@ -65,8 +68,9 @@ export const AppsSectionRoot = () => {
path={AppPaths.resolveAppDetailsPath(":id")}
component={AppManageRoute}
/>
<Route
<SectionRoute
exact
permissions={[PermissionEnum.MANAGE_APPS]}
path={AppPaths.resolveRequestPermissionsPath(":id")}
component={AppPermissionRequestView}
/>
Expand Down
19 changes: 12 additions & 7 deletions src/apps/components/AllAppList/AllAppList.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { AppstoreApi } from "@dashboard/apps/appstore.types";
import { AppInstallationFragment } from "@dashboard/graphql";
import { Skeleton } from "@material-ui/lab";
import { Box } from "@saleor/macaw-ui-next";
import chunk from "lodash/chunk";
import { Box, useTheme } from "@saleor/macaw-ui-next";
import React from "react";

import AppListRow from "../AppListRow";
Expand All @@ -20,18 +19,24 @@ const AllAppList: React.FC<AllAppListProps> = ({
navigateToAppInstallPage,
navigateToGithubForkPage,
}) => {
const appsPairs = React.useMemo(() => chunk(appList, 2), [appList]);
const { themeValues } = useTheme();

if (!appList) {
return <Skeleton />;
}

return (
<Box display="flex" flexDirection="column" gap={5} marginTop={5}>
{appsPairs.map(appPair => (
<Box
display="grid"
__gridTemplateColumns="repeat(2, 1fr)"
__gap={`${themeValues.spacing[8]} ${themeValues.spacing[5]}`}
padding={5}
marginTop={5}
>
{appList.map(app => (
<AppListRow
key={appPair[0].name.en}
appPair={appPair}
key={app.name.en}
app={app}
appInstallationList={appInstallationList}
navigateToAppInstallPage={navigateToAppInstallPage}
navigateToGithubForkPage={navigateToGithubForkPage}
Expand Down
46 changes: 30 additions & 16 deletions src/apps/components/AppDetailsPage/HeaderOptions.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { ButtonWithTooltip } from "@dashboard/components/ButtonWithTooltip";
import { useHasManagedAppsPermission } from "@dashboard/hooks/useHasManagedAppsPermission";
import { buttonMessages } from "@dashboard/intl";
import { ButtonBase } from "@material-ui/core";
import { Box } from "@saleor/macaw-ui-next";
import React from "react";
import SVG from "react-inlinesvg";
import { FormattedMessage } from "react-intl";
import { FormattedMessage, useIntl } from "react-intl";

import activateIcon from "../../../../assets/images/activate-icon.svg";
import deleteIcon from "../../../../assets/images/delete.svg";
import { useStyles } from "./styles";

interface HeaderOptionsProps {
isActive: boolean;
Expand All @@ -22,7 +22,12 @@ const HeaderOptions: React.FC<HeaderOptionsProps> = ({
onAppDeactivateOpen,
onAppDeleteOpen,
}) => {
const classes = useStyles();
const intl = useIntl();
const { hasManagedAppsPermission } = useHasManagedAppsPermission();

const tooltipContent = !hasManagedAppsPermission
? intl.formatMessage(buttonMessages.noPermission)
: undefined;

return (
<Box
Expand All @@ -31,28 +36,37 @@ const HeaderOptions: React.FC<HeaderOptionsProps> = ({
borderColor="default1"
borderBottomWidth={1}
>
<div className={classes.appHeaderLinks}>
<ButtonBase
className={classes.headerLinkContainer}
disableRipple
<Box display="flex" gap={1} paddingY={2}>
<ButtonWithTooltip
variant="tertiary"
tooltip={tooltipContent}
disabled={!hasManagedAppsPermission}
onClick={isActive ? onAppDeactivateOpen : onAppActivateOpen}
display="flex"
alignItems="center"
gap={1}
>
<SVG src={activateIcon} />
<SVG src={activateIcon} width={18} height={18} />
{isActive ? (
<FormattedMessage {...buttonMessages.deactivate} />
) : (
<FormattedMessage {...buttonMessages.activate} />
)}
</ButtonBase>
<ButtonBase
className={classes.headerLinkContainer}
disableRipple
</ButtonWithTooltip>

<ButtonWithTooltip
variant="tertiary"
tooltip={tooltipContent}
disabled={!hasManagedAppsPermission}
onClick={onAppDeleteOpen}
display="flex"
alignItems="center"
gap={1}
>
<SVG src={deleteIcon} />
<SVG src={deleteIcon} width={16} height={16} />
<FormattedMessage {...buttonMessages.delete} />
</ButtonBase>
</div>
</ButtonWithTooltip>
</Box>
</Box>
);
};
Expand Down
18 changes: 14 additions & 4 deletions src/apps/components/AppDetailsPage/PermissionsCard.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { AppPermissionsDialog } from "@dashboard/apps/components/AppPermissionsDialog";
import { ButtonWithTooltip } from "@dashboard/components/ButtonWithTooltip";
import Skeleton from "@dashboard/components/Skeleton";
import { PermissionEnum } from "@dashboard/graphql";
import { Box, BoxProps, Button, Text } from "@saleor/macaw-ui-next";
import { useHasManagedAppsPermission } from "@dashboard/hooks/useHasManagedAppsPermission";
import { buttonMessages } from "@dashboard/intl";
import { Box, BoxProps, Text } from "@saleor/macaw-ui-next";
import React, { useState } from "react";
import { FormattedMessage, useIntl } from "react-intl";

Expand All @@ -25,14 +28,21 @@ export const PermissionsCard: React.FC<PermissionsCardProps> = ({
const [editPermissionDialogOpen, setEditPermissionDialogOpen] =
useState(false);
const intl = useIntl();
const { hasManagedAppsPermission } = useHasManagedAppsPermission();

const editPermissionsButton = (
<Button
variant={"secondary"}
<ButtonWithTooltip
tooltip={
!hasManagedAppsPermission
? intl.formatMessage(buttonMessages.noPermission)
: undefined
}
variant="secondary"
disabled={!hasManagedAppsPermission}
onClick={() => setEditPermissionDialogOpen(true)}
>
{intl.formatMessage(messages.editPermissionsButton)}
</Button>
</ButtonWithTooltip>
);

const renderContent = () => {
Expand Down
Loading

0 comments on commit f94eb20

Please sign in to comment.