feat: create edit user group page #1732

CodyWMitchell · 2024-12-13T15:33:08Z

Description

Created the Edit User Group page

I can edit an existing user group by performing any or all of the following actions:
I can rename the user group.
I can update the description of the user group.
I am notified if the updated user group name has already been used for my organization.
I can confirm or cancel the request.
Upon cancellation, I am redirected to the previous page/task.
Upon saving, I am presented with a success message and redirected to the User Group list.
I am presented with an error message if the user group is not saved successfully.

Will be handled in future ticket

I can add or remove principal users and service accounts from the user group from their respective tabs.

Currently values are diffed and displayed, then logged to the console. Another ticket will handle sending out the appropriate requests.

RHCLOUD-32236

Improve the integration between DataView and data-driven-forms, and potentially create a custom DataView component which could simplify this logic and enable disabling the submit button when no changes have been made here:
RHCLOUD-37436 & RHCLOUD-37435

Screenshots

After:

Checklist ☑️

PR only fixes one issue or story
Change reviewed for extraneous code
UI best practices adhered to
Commits squashed and meaningfully named
All PR checks pass locally (build, lint, test, E2E)

(Optional) QE: Needs QE attention (OUIA changed, perceived impact to tests, no test coverage)
(Optional) QE: Has been mentioned
(Optional) UX: Needs UX attention (end user UX modified, missing designs)
(Optional) UX: Has been mentioned

fhlavac · 2025-01-07T15:39:41Z

@CodyWMitchell the navigation and breadcrumbs needs to be fixed to highlight and show correctly

after changes introduced in #1735, there should be also /user-groups section in the URL

fhlavac · 2025-01-07T15:42:00Z

we should introduce some loading state to prevent this from happening

fhlavac · 2025-01-07T15:43:03Z

src/Routing.tsx

@@ -75,6 +76,10 @@ const getRoutes = ({ enableServiceAccounts, isITLess, isWorkspacesFlag, isCommon
      },
    ],
  },
+  {


should be a child route

Rendering this as a child route adds the Users and Users Groups header above - is there a way to avoid this?

fhlavac · 2025-01-07T15:46:52Z

src/smart-components/access-management/EditUserGroup.tsx

+          { type: validatorTypes.REQUIRED },
+          (value: string) => {
+            if (value === group?.name) {
+              return undefined;
+            }
+
+            const isDuplicate = allGroups.some(
+              (existingGroup) => existingGroup.name.toLowerCase() === value?.toLowerCase() && existingGroup.uuid !== groupId
+            );
+
+            return isDuplicate ? intl.formatMessage(Messages.groupNameTakenTitle) : undefined;
+          },


I don't think this will work correctly. The groups' response is paginated so you probably won't get all the data to validate properly. Mabe we could reuse the denounced validator currently used in former add group wizard.

fhlavac · 2025-01-07T15:51:54Z

src/smart-components/access-management/EditUserGroup.tsx

+
+  return (
+    <React.Fragment>
+      <ContentHeader title={intl.formatMessage(Messages.usersAndUserGroupsEditUserGroup)} subtitle={''} />


Suggested change

<ContentHeader title={intl.formatMessage(Messages.usersAndUserGroupsEditUserGroup)} subtitle={''} />

<ContentHeader title={intl.formatMessage(Messages.usersAndUserGroupsEditUserGroup)} />

It looks like typescript requires the subtitle prop in the ContentHeader, it's not marked as optional. I see an error when I remove it.

Ahh, I see. I'll open an issue in the component groups

fhlavac · 2025-01-07T15:53:14Z

src/smart-components/access-management/EditUserGroupServiceAccounts.tsx

+import { ServiceAccountsState } from '../../redux/reducers/service-account-reducer';
+import { LAST_PAGE, ServiceAccount } from '../../helpers/service-account/service-account-helper';
+import { BulkSelect, BulkSelectValue } from '@patternfly/react-component-groups';
+import DateFormat from '@redhat-cloud-services/frontend-components/DateFormat';


we should rather use PatternFly Timestamp

The mocks show the time formatted as 1 month ago. I didn't see a straightforward way to do this with the PatternFly Timestamp in the docs without defining it custom somehow.

fhlavac · 2025-01-07T15:57:48Z

src/smart-components/access-management/EditUserGroupUsersAndServiceAccounts.tsx

+
+  return (
+    <React.Fragment>
+      <FormGroup label="Select users and/or service accounts">


Should be localized

fhlavac · 2025-01-07T15:58:02Z

src/smart-components/access-management/EditUserGroupUsersAndServiceAccounts.tsx

+    <React.Fragment>
+      <FormGroup label="Select users and/or service accounts">
+        <Tabs activeKey={activeTabKey} onSelect={handleTabSelect}>
+          <Tab eventKey={0} title="Users">


Should be localized

src/smart-components/access-management/EditUserGroupUsersAndServiceAccounts.tsx

fhlavac · 2025-01-07T16:00:16Z

src/smart-components/access-management/EditUserGroupUsers.tsx

+          />
+        }
+      />
+      <DataViewTable variant="compact" columns={['Org Admin', 'Username', 'Email', 'First name', 'Last name', 'Status']} rows={rows} />


Should be localized

fhlavac · 2025-01-07T16:01:15Z

Compared to mockups we should have local breadcrumbs and the "Name" field should be required

fhlavac · 2025-01-07T16:02:34Z

It would be also nice to disable "Submit" button when there are no changes - it should be pretty easy using the form state

fhlavac · 2025-01-07T16:06:24Z

src/smart-components/access-management/EditUserGroupUsers.tsx

Maybe we could just reuse the existingUsersTable, what do you think? Also, it might be good to add loading state when changing pages (the UsersTable already has them)

Since selection is being used for enabling and disabling users here - with the mocks showing the Org Admin column shuffled around and no action buttons - I made this into its own component instead of reusing the users table. I can try to make the original users table work for both cases if you think that's better though. Either way, I'll make sure to add loading states to this version. 👍

@CodyWMitchell columns shuffled around is an interesting thing --it might be good to ask Margot, who prepared the mockups if that's intentional. But, I agree that this table will probably be much simpler than the regular Users view - it's probably a safer solution in this case 👍

fhlavac · 2025-01-07T16:09:30Z

src/smart-components/access-management/EditUserGroupServiceAccounts.tsx

+  const calculateTotalCount = () => {
+    if (!serviceAccounts) return 0;
+    const currentCount = (page - 1) * perPage + serviceAccounts.length;
+    return status === LAST_PAGE ? currentCount : currentCount + 1;
+  };
+  const totalCount = calculateTotalCount();


could be memoized

fhlavac · 2025-01-07T16:13:20Z

@CodyWMitchell looks good! Just few comments

fhlavac · 2025-01-24T08:19:49Z

@CodyWMitchell when opening the detail, the URL should contain access-management, not user-access

Also, the link to groups in the local breadcrumbs has the same issue

fhlavac · 2025-01-24T08:20:46Z

The submit button looks to be enabled even if the form is pristine

If it is a more complex thing, we can also create a separate ticket to fix that so as not to block this one.

fhlavac · 2025-01-24T08:22:15Z

After submitting the form, the table does not seem to be updated.

fhlavac · 2025-01-24T08:22:48Z

@CodyWMitchell just three small comments, it looks much better now! Thank you 🙂

karelhala · 2025-01-29T14:45:57Z

@fhlavac for the breadcrumbs the workspaces effort has them broken and we'll have to fix a few of the screens. For the pristine state, that's connected to DDF and DW interaction and we'll have to write custom component and fix it in tech debt. As for the non updated values @CodyWMitchell will create a ticket to address this in a different ticket since this looks like API race condition.

CodyWMitchell requested a review from a team as a code owner December 13, 2024 15:33

CodyWMitchell mentioned this pull request Dec 13, 2024

Edit user group page #1714

Closed

18 tasks

CodyWMitchell force-pushed the edit-user-groups-page branch from eaccedb to 75b4e66 Compare December 13, 2024 15:40

CodyWMitchell force-pushed the edit-user-groups-page branch 4 times, most recently from 8595f6c to 4893df8 Compare January 7, 2025 07:13

fhlavac reviewed Jan 7, 2025

View reviewed changes

src/smart-components/access-management/EditUserGroupUsersAndServiceAccounts.tsx Outdated Show resolved Hide resolved

fhlavac reviewed Jan 7, 2025

View reviewed changes

feat: create edit user group page

fdad5f6

CodyWMitchell force-pushed the edit-user-groups-page branch from 4d273c4 to fdad5f6 Compare January 20, 2025 09:04

karelhala merged commit 6ee38d3 into RedHatInsights:master Jan 29, 2025
9 of 11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: create edit user group page #1732

feat: create edit user group page #1732

CodyWMitchell commented Dec 13, 2024 •

edited

Loading

fhlavac commented Jan 7, 2025 •

edited

Loading

fhlavac commented Jan 7, 2025

fhlavac Jan 7, 2025

CodyWMitchell Jan 14, 2025

fhlavac Jan 7, 2025

fhlavac Jan 7, 2025

CodyWMitchell Jan 8, 2025

fhlavac Jan 8, 2025

fhlavac Jan 7, 2025

CodyWMitchell Jan 14, 2025

fhlavac Jan 7, 2025

fhlavac Jan 7, 2025

fhlavac Jan 7, 2025

fhlavac commented Jan 7, 2025

fhlavac commented Jan 7, 2025

fhlavac Jan 7, 2025

CodyWMitchell Jan 8, 2025

fhlavac Jan 8, 2025

fhlavac Jan 7, 2025

fhlavac commented Jan 7, 2025

fhlavac commented Jan 24, 2025 •

edited

Loading

fhlavac commented Jan 24, 2025 •

edited

Loading

fhlavac commented Jan 24, 2025

fhlavac commented Jan 24, 2025

karelhala commented Jan 29, 2025

	<ContentHeader title={intl.formatMessage(Messages.usersAndUserGroupsEditUserGroup)} subtitle={''} />
	<ContentHeader title={intl.formatMessage(Messages.usersAndUserGroupsEditUserGroup)} />

feat: create edit user group page #1732

feat: create edit user group page #1732

Conversation

CodyWMitchell commented Dec 13, 2024 • edited Loading

Description

Will be handled in future ticket

Screenshots

After:

Checklist ☑️

fhlavac commented Jan 7, 2025 • edited Loading

fhlavac commented Jan 7, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fhlavac commented Jan 7, 2025

fhlavac commented Jan 7, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fhlavac commented Jan 7, 2025

fhlavac commented Jan 24, 2025 • edited Loading

fhlavac commented Jan 24, 2025 • edited Loading

fhlavac commented Jan 24, 2025

fhlavac commented Jan 24, 2025

karelhala commented Jan 29, 2025

CodyWMitchell commented Dec 13, 2024 •

edited

Loading

fhlavac commented Jan 7, 2025 •

edited

Loading

fhlavac commented Jan 24, 2025 •

edited

Loading

fhlavac commented Jan 24, 2025 •

edited

Loading