From 49f91577a5f4892f459eff85be53cfb9d3c78e8c Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Thu, 22 Aug 2024 22:28:32 +0000 Subject: [PATCH 1/5] fix: improve show/hide checkbox text --- .../CustomRolesPage/CreateEditRolePageView.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx index c48d1628a79c1..6bddd44afb47b 100644 --- a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx +++ b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx @@ -308,7 +308,13 @@ const ShowAllResourcesCheckbox: FC = ({ icon={} /> } - label={Show all permissions} + label={ + + {showAllResources + ? "Hide advanced permissions" + : "Show advanced permissions"} + + } /> ); }; From 800a49ee6f23ed3469162b909db82cd2a302e2ad Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Tue, 27 Aug 2024 16:23:43 +0000 Subject: [PATCH 2/5] feat: add parent checkbox for grouped resource permissions --- .../CreateEditRolePageView.tsx | 163 +++++++++++++----- 1 file changed, 123 insertions(+), 40 deletions(-) diff --git a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx index 6bddd44afb47b..eca189c83f1b2 100644 --- a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx +++ b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx @@ -146,12 +146,6 @@ export const CreateEditRolePageView: FC = ({ ); }; -interface ActionCheckboxesProps { - permissions: readonly Permission[] | undefined; - form: ReturnType> & { values: Role }; - allResources: boolean; -} - const ResourceActionComparator = ( p: Permission, resource: string, @@ -160,6 +154,7 @@ const ResourceActionComparator = ( p.resource_type === resource && (p.action.toString() === "*" || p.action === action); +// the subset of resources that are useful for most users const DEFAULT_RESOURCES = [ "audit_log", "group", @@ -177,6 +172,12 @@ const filteredRBACResourceActions = Object.fromEntries( ), ); +interface ActionCheckboxesProps { + permissions: readonly Permission[]; + form: ReturnType> & { values: Role }; + allResources: boolean; +} + const ActionCheckboxes: FC = ({ permissions, form, @@ -185,6 +186,10 @@ const ActionCheckboxes: FC = ({ const [checkedActions, setCheckActions] = useState(permissions); const [showAllResources, setShowAllResources] = useState(allResources); + const resourceActions = showAllResources + ? RBACResourceActions + : filteredRBACResourceActions; + const handleActionCheckChange = async ( e: ChangeEvent, form: ReturnType> & { values: Role }, @@ -194,7 +199,7 @@ const ActionCheckboxes: FC = ({ const newPermissions = checked ? [ - ...(checkedActions ?? []), + ...checkedActions, { negate: false, resource_type: resource_type as RBACResource, @@ -209,9 +214,36 @@ const ActionCheckboxes: FC = ({ await form.setFieldValue("organization_permissions", newPermissions); }; - const resourceActions = showAllResources - ? RBACResourceActions - : filteredRBACResourceActions; + const handleResourceCheckChange = async ( + e: ChangeEvent, + form: ReturnType> & { values: Role }, + indeterminate: boolean, + ) => { + const { name, checked } = e.currentTarget; + const resource = name as RBACResource; + + const resourceActionsForResource = resourceActions[resource] || {}; + + const newCheckedActions = + !checked || indeterminate + ? checkedActions?.filter((p) => p.resource_type !== resource) + : checkedActions; + + const newPermissions = + checked || indeterminate + ? [ + ...newCheckedActions, + ...Object.keys(resourceActionsForResource).map((resourceKey) => ({ + negate: false, + resource_type: resource as RBACResource, + action: resourceKey as RBACAction, + })), + ] + : [...newCheckedActions]; + + setCheckActions(newPermissions); + await form.setFieldValue("organization_permissions", newPermissions); + }; return ( @@ -233,36 +265,17 @@ const ActionCheckboxes: FC = ({ {Object.entries(resourceActions).map(([resourceKey, value]) => { return ( - - -
  • - {resourceKey} -
      - {Object.entries(value).map(([actionKey, value]) => ( -
    • - - - ResourceActionComparator( - p, - resourceKey, - actionKey, - ), - )} - onChange={(e) => handleActionCheckChange(e, form)} - /> - {actionKey} - {" "} - –{" "} - {value} -
    • - ))} -
    -
  • -
    -
    + a.resource_type === resourceKey, + )} + resourceKey={resourceKey} + value={value} + form={form} + handleActionCheckChange={handleActionCheckChange} + handleResourceCheckChange={handleResourceCheckChange} + /> ); })}
    @@ -285,6 +298,76 @@ const ActionCheckboxes: FC = ({ ); }; +interface PermissionCheckboxGroupProps { + checkedActions: readonly Permission[]; + resourceKey: string; + value: Partial>; + form: ReturnType> & { values: Role }; + handleActionCheckChange: ( + e: ChangeEvent, + form: ReturnType> & { values: Role }, + ) => Promise; + handleResourceCheckChange: ( + e: ChangeEvent, + form: ReturnType> & { values: Role }, + indeterminate: boolean, + ) => Promise; +} + +const PermissionCheckboxGroup: FC = ({ + checkedActions, + resourceKey, + value, + form, + handleActionCheckChange, + handleResourceCheckChange, +}) => { + return ( + + +
  • + 0 && + checkedActions.length < Object.keys(value).length + } + onChange={(e) => + handleResourceCheckChange( + e, + form, + checkedActions.length > 0 && + checkedActions.length < Object.keys(value).length, + ) + } + /> + {resourceKey} +
      + {Object.entries(value).map(([actionKey, value]) => ( +
    • + + + ResourceActionComparator(p, resourceKey, actionKey), + )} + onChange={(e) => handleActionCheckChange(e, form)} + /> + {actionKey} + {" "} + – {value} +
    • + ))} +
    +
  • +
    +
    + ); +}; + interface ShowAllResourcesCheckboxProps { showAllResources: boolean; setShowAllResources: React.Dispatch>; From 31b89e4d9634f25d44732b438b7a10f275c4d720 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Tue, 27 Aug 2024 18:22:45 +0000 Subject: [PATCH 3/5] fix: align action list item to a grid --- .../CustomRolesPage/CreateEditRolePageView.tsx | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx index eca189c83f1b2..5ed361d8aea5a 100644 --- a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx +++ b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx @@ -346,7 +346,7 @@ const PermissionCheckboxGroup: FC = ({ {resourceKey}
      {Object.entries(value).map(([actionKey, value]) => ( -
    • +
    • = ({ onChange={(e) => handleActionCheckChange(e, form)} /> {actionKey} - {" "} - – {value} + + {value}
    • ))}
    @@ -412,7 +412,12 @@ const styles = { }), actionDescription: (theme) => ({ color: theme.palette.text.secondary, + paddingTop: 6, }), + actionItem: { + display: "grid", + gridTemplateColumns: "270px 3fr", + }, } satisfies Record>; export default CreateEditRolePageView; From 0ecf02421f494ce367515ff640ed7e23c2743677 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Wed, 28 Aug 2024 21:55:38 +0000 Subject: [PATCH 4/5] chore: add additional tests --- .../CreateEditRolePageView.stories.tsx | 26 +++++++++++++++++++ .../CreateEditRolePageView.tsx | 3 ++- site/src/testHelpers/entities.ts | 15 +++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.stories.tsx b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.stories.tsx index 9dbf27077cffc..37901153a9fb5 100644 --- a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.stories.tsx +++ b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.stories.tsx @@ -1,10 +1,12 @@ import type { Meta, StoryObj } from "@storybook/react"; import { MockRoleWithOrgPermissions, + MockRole2WithOrgPermissions, assignableRole, mockApiError, } from "testHelpers/entities"; import { CreateEditRolePageView } from "./CreateEditRolePageView"; +import { userEvent, within, expect } from "@storybook/test"; const meta: Meta = { title: "pages/OrganizationCreateEditRolePage", @@ -25,6 +27,16 @@ export const Default: Story = { }, }; +export const CheckboxIndeterminate: Story = { + args: { + role: assignableRole(MockRole2WithOrgPermissions, true), + onSubmit: () => null, + isLoading: false, + organizationName: "my-org", + canAssignOrgRole: true, + }, +}; + export const WithError: Story = { args: { role: assignableRole(MockRoleWithOrgPermissions, true), @@ -61,3 +73,17 @@ export const ShowAllResources: Story = { allResources: true, }, }; + +export const ToggleParentCheckbox: Story = { + play: async ({ canvasElement }) => { + const user = userEvent.setup(); + const canvas = within(canvasElement); + const checkbox = await canvas + .getByTestId("audit_log") + .getElementsByTagName("input")[0]; + await user.click(checkbox); + await expect(checkbox).toBeChecked(); + await user.click(checkbox); + await expect(checkbox).not.toBeChecked(); + }, +}; diff --git a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx index 5ed361d8aea5a..fe183f33fc6a3 100644 --- a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx +++ b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx @@ -334,6 +334,7 @@ const PermissionCheckboxGroup: FC = ({ checkedActions.length > 0 && checkedActions.length < Object.keys(value).length } + data-testid={`${resourceKey}`} onChange={(e) => handleResourceCheckChange( e, @@ -416,7 +417,7 @@ const styles = { }), actionItem: { display: "grid", - gridTemplateColumns: "270px 3fr", + gridTemplateColumns: "270px 1fr", }, } satisfies Record>; diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index c620164bed754..1f6f034f79cff 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -391,6 +391,21 @@ export const MockRoleWithOrgPermissions: TypesGen.Role = { user_permissions: [], }; +export const MockRole2WithOrgPermissions: TypesGen.Role = { + name: "my-role-1", + display_name: "My Role 1", + organization_id: MockOrganization.id, + site_permissions: [], + organization_permissions: [ + { + negate: false, + resource_type: "audit_log", + action: "create", + }, + ], + user_permissions: [], +}; + // assignableRole takes a role and a boolean. The boolean implies if the // actor can assign (add/remove) the role from other users. export function assignableRole( From 14ae89d0bf76d741080a5747fb88c6256a5cad35 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Wed, 28 Aug 2024 22:02:04 +0000 Subject: [PATCH 5/5] fix: format --- .../CustomRolesPage/CreateEditRolePageView.stories.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.stories.tsx b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.stories.tsx index 37901153a9fb5..11d41480f4b1b 100644 --- a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.stories.tsx +++ b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.stories.tsx @@ -1,12 +1,12 @@ import type { Meta, StoryObj } from "@storybook/react"; +import { expect, userEvent, within } from "@storybook/test"; import { - MockRoleWithOrgPermissions, MockRole2WithOrgPermissions, + MockRoleWithOrgPermissions, assignableRole, mockApiError, } from "testHelpers/entities"; import { CreateEditRolePageView } from "./CreateEditRolePageView"; -import { userEvent, within, expect } from "@storybook/test"; const meta: Meta = { title: "pages/OrganizationCreateEditRolePage",