-
Notifications
You must be signed in to change notification settings - Fork 7
feat(authz): add roles table for Audit user page #106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
8f36eaf
refactor: moving files from libraries to authz module and minor impro…
jacobo-dominguez-wgu 5bb05b6
feat: roles table for audit user page
jacobo-dominguez-wgu 215d0bc
feat: integrating api for permissions assignments on audit user page …
jacobo-dominguez-wgu 2eac66f
test: adding unit test for audit user page components
jacobo-dominguez-wgu b61207d
fix: addressing pr comments
jacobo-dominguez-wgu 16e1e91
fix: refactor useUserAssignedRoles function
jesusbalderramawgu 4b88663
fix: unnecessary refetchWindowFocus removed
jesusbalderramawgu 6766a57
fix: minor conflict fixes
jacobo-dominguez-wgu 4bc061a
fix: addressing pr comments
jacobo-dominguez-wgu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,188 @@ | ||
| import { render, screen, waitFor } from '@testing-library/react'; | ||
| import userEvent from '@testing-library/user-event'; | ||
| import { MemoryRouter, Route, Routes } from 'react-router-dom'; | ||
| import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; | ||
| import { IntlProvider } from '@edx/frontend-platform/i18n'; | ||
| import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; | ||
| import AuditUserPage from './index'; | ||
|
|
||
| jest.mock('@edx/frontend-platform/auth', () => ({ | ||
| getAuthenticatedHttpClient: jest.fn(), | ||
| configure: jest.fn(), | ||
| })); | ||
|
|
||
| const mockUser = { | ||
| username: 'johndoe', | ||
| email: '[email protected]', | ||
| profile_image: { has_image: false }, | ||
| }; | ||
| const mockAssignments = { | ||
| count: 1, | ||
| results: [ | ||
| { | ||
| id: '1', | ||
| role: 'library_admin', | ||
| org: 'Test Org', | ||
| scope: 'lib:test', | ||
| permissionCount: 5, | ||
| }, | ||
| ], | ||
| next: null, | ||
| previous: null, | ||
| }; | ||
|
|
||
| const renderWithRouter = (route = '/audit/johndoe') => { | ||
| const queryClient = new QueryClient({ | ||
| defaultOptions: { | ||
| queries: { | ||
| retry: false, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| return render( | ||
| <QueryClientProvider client={queryClient}> | ||
| <IntlProvider locale="en"> | ||
| <MemoryRouter initialEntries={[route]}> | ||
| <Routes> | ||
| <Route path="/audit/:username" element={<AuditUserPage />} /> | ||
| <Route path="/authz" element={<div>Home Page</div>} /> | ||
| </Routes> | ||
| </MemoryRouter> | ||
| </IntlProvider> | ||
| </QueryClientProvider>, | ||
| ); | ||
| }; | ||
|
|
||
| describe('AuditUserPage', () => { | ||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('renders user info and table when data is loaded', async () => { | ||
| (getAuthenticatedHttpClient as jest.Mock).mockReturnValue({ | ||
| get: jest | ||
| .fn() | ||
| .mockResolvedValueOnce({ data: mockUser }) | ||
| .mockResolvedValueOnce({ data: mockAssignments }), | ||
| }); | ||
|
|
||
| renderWithRouter(); | ||
|
|
||
| await waitFor(() => { | ||
| expect(screen.getByRole('heading', { name: 'johndoe' })).toBeInTheDocument(); | ||
| expect(screen.getByText('[email protected]')).toBeInTheDocument(); | ||
| expect(screen.getByRole('button', { name: /assign role/i })).toBeInTheDocument(); | ||
| expect(screen.getByText('Library Admin')).toBeInTheDocument(); | ||
| expect(screen.getByText('Test Org')).toBeInTheDocument(); | ||
| expect(screen.getByText('lib:test')).toBeInTheDocument(); | ||
| expect(screen.getByText('5 permissions available')).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
|
||
| it('navigates to home if user is not found', async () => { | ||
| (getAuthenticatedHttpClient as jest.Mock).mockReturnValue({ | ||
| get: jest | ||
| .fn() | ||
| .mockResolvedValueOnce({ data: null }) | ||
| .mockResolvedValueOnce({ data: mockAssignments }), | ||
| }); | ||
|
|
||
| renderWithRouter(); | ||
|
|
||
| await waitFor(() => { | ||
| expect(screen.getByText('Home Page')).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
|
||
| it('allows user to interact with Assign Role button', async () => { | ||
| (getAuthenticatedHttpClient as jest.Mock).mockReturnValue({ | ||
| get: jest | ||
| .fn() | ||
| .mockResolvedValueOnce({ data: mockUser }) | ||
| .mockResolvedValueOnce({ data: mockAssignments }), | ||
| }); | ||
|
|
||
| renderWithRouter(); | ||
|
|
||
| await waitFor(() => { | ||
| expect(screen.getByRole('button', { name: /assign role/i })).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| const user = userEvent.setup(); | ||
| const button = screen.getByRole('button', { name: /assign role/i }); | ||
| await user.click(button); | ||
| expect(button).not.toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('renders empty state when user has no assignments', async () => { | ||
| (getAuthenticatedHttpClient as jest.Mock).mockReturnValue({ | ||
| get: jest | ||
| .fn() | ||
| .mockResolvedValueOnce({ data: mockUser }) | ||
| .mockResolvedValueOnce({ | ||
| data: { | ||
| count: 0, results: [], next: null, previous: null, | ||
| }, | ||
| }), | ||
| }); | ||
|
|
||
| renderWithRouter(); | ||
|
|
||
| await waitFor(() => { | ||
| expect(screen.getByRole('heading', { name: 'johndoe' })).toBeInTheDocument(); | ||
| expect(screen.queryByText('5 permissions available')).not.toBeInTheDocument(); | ||
| expect(screen.getByRole('table')).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
|
||
| it('renders correct table headers', async () => { | ||
| (getAuthenticatedHttpClient as jest.Mock).mockReturnValue({ | ||
| get: jest | ||
| .fn() | ||
| .mockResolvedValueOnce({ data: mockUser }) | ||
| .mockResolvedValueOnce({ data: mockAssignments }), | ||
| }); | ||
|
|
||
| renderWithRouter(); | ||
|
|
||
| await waitFor(() => { | ||
| expect(screen.getByText('Role')).toBeInTheDocument(); | ||
| expect(screen.getByText('Organization')).toBeInTheDocument(); | ||
| expect(screen.getByText('Scope')).toBeInTheDocument(); | ||
| expect(screen.getByText('Permissions')).toBeInTheDocument(); | ||
| expect(screen.getByText('Actions')).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
|
||
| it('renders the pagination controls when assignments are present', async () => { | ||
| (getAuthenticatedHttpClient as jest.Mock).mockReturnValue({ | ||
| get: jest | ||
| .fn() | ||
| .mockResolvedValueOnce({ data: mockUser }) | ||
| .mockResolvedValueOnce({ data: mockAssignments }), | ||
| }); | ||
|
|
||
| renderWithRouter(); | ||
|
|
||
| await waitFor(() => { | ||
| expect(screen.getByText('Showing 1 of 1.')).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
|
||
| it('renders the breadcrumb navigation with home link', async () => { | ||
| (getAuthenticatedHttpClient as jest.Mock).mockReturnValue({ | ||
| get: jest | ||
| .fn() | ||
| .mockResolvedValueOnce({ data: mockUser }) | ||
| .mockResolvedValueOnce({ data: mockAssignments }), | ||
| }); | ||
|
|
||
| renderWithRouter(); | ||
|
|
||
| await waitFor(() => { | ||
| expect(screen.getByRole('link', { name: /roles and permissions management/i })).toBeInTheDocument(); | ||
| expect(screen.getByText(mockUser.username, { selector: 'li[aria-current="page"]' })).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| import { useEffect, useMemo } from 'react'; | ||
| import { useIntl } from '@edx/frontend-platform/i18n'; | ||
| import debounce from 'lodash.debounce'; | ||
| import { | ||
| Container, DataTable, | ||
| } from '@openedx/paragon'; | ||
| import TableFooter from '@src/authz-module/components/TableFooter/TableFooter'; | ||
| import { AUTHZ_HOME_PATH, TABLE_DEFAULT_PAGE_SIZE } from '@src/authz-module/constants'; | ||
| import AuthZLayout from '@src/authz-module/components/AuthZLayout'; | ||
| import { useNavigate, useParams } from 'react-router-dom'; | ||
| import { useUserAccount } from '@src/data/hooks'; | ||
| import baseMessages from '@src/authz-module/messages'; | ||
| import AddRoleButton from '@src/authz-module/components/AddRoleButton'; | ||
| import { | ||
| OrgCell, RoleCell, ScopeCell, PermissionsCell, ViewAllPermissionsCell, ActionsCell, | ||
| } from '@src/authz-module/components/TableCells'; | ||
| import { useQuerySettings } from '@src/authz-module/hooks/useQuerySettings'; | ||
| import { useUserAssignedRoles } from '@src/authz-module/data/hooks'; | ||
| import messages from './messages'; | ||
|
|
||
| const AuditUserPage = () => { | ||
| const { formatMessage } = useIntl(); | ||
| const { username } = useParams(); | ||
| const navigate = useNavigate(); | ||
| const { isLoading: isLoadingUser, data: user } = useUserAccount(username ?? ''); | ||
| const { querySettings, handleTableFetch } = useQuerySettings(); | ||
| const { isLoading: isLoadingUserAssignments, data: { results: userAssignments, count } = { results: [], count: 0 } } = useUserAssignedRoles(username ?? '', querySettings); | ||
|
|
||
| useEffect(() => { | ||
| if (!user && !isLoadingUser) { | ||
| navigate(AUTHZ_HOME_PATH); | ||
| } | ||
| }, [user, isLoadingUser, navigate]); | ||
|
|
||
| const navLinks = [ | ||
| { | ||
| label: formatMessage(baseMessages['authz.management.home.nav.link']), | ||
| to: AUTHZ_HOME_PATH, | ||
| }, | ||
| ]; | ||
| const additionalColumns = [ | ||
| { | ||
| id: 'view_permissions', | ||
| Header: '', | ||
| Cell: ViewAllPermissionsCell, | ||
| }, | ||
| { | ||
| id: 'action', | ||
| Header: formatMessage(messages['authz.user.table.action.column.header']), | ||
| Cell: ActionsCell, | ||
| }, | ||
| ]; | ||
| const columns = [ | ||
| { | ||
| Header: formatMessage(messages['authz.user.table.role.column.header']), | ||
| accessor: 'role', | ||
| Cell: RoleCell, | ||
| }, | ||
| { | ||
| Header: formatMessage(messages['authz.user.table.organization.column.header']), | ||
| accessor: 'org', | ||
| Cell: OrgCell, | ||
| }, | ||
| { | ||
| Header: formatMessage(messages['authz.user.table.scope.column.header']), | ||
| accessor: 'scope', | ||
| Cell: ScopeCell, | ||
| disableFilters: true, | ||
| }, | ||
| { | ||
| Header: formatMessage(messages['authz.user.table.permissions.column.header']), | ||
| Cell: PermissionsCell, | ||
| disableFilters: true, | ||
| disableSortBy: true, | ||
| }, | ||
| ]; | ||
| const pageCount = Math.ceil(count / TABLE_DEFAULT_PAGE_SIZE); | ||
|
|
||
| const fetchData = useMemo(() => debounce(handleTableFetch, 500), [handleTableFetch]); | ||
|
|
||
| return ( | ||
| <div className="authz-module"> | ||
| <AuthZLayout | ||
| context={{ | ||
| id: '', | ||
| org: '', | ||
| title: '', | ||
| }} | ||
| navLinks={navLinks} | ||
| activeLabel={username || ''} | ||
| pageTitle={user?.username || ''} | ||
| pageSubtitle={user?.email || ''} | ||
| actions={ | ||
| [ | ||
| <AddRoleButton presetUsername={user?.username} key="add-role-button" />, | ||
| ] | ||
| } | ||
| > | ||
| <Container className="bg-light-200 p-5"> | ||
| <DataTable | ||
| isPaginated | ||
| manualPagination | ||
| data={userAssignments} | ||
| fetchData={fetchData} | ||
| itemCount={count} | ||
| pageCount={pageCount} | ||
| initialState={{ pageSize: TABLE_DEFAULT_PAGE_SIZE }} | ||
| additionalColumns={additionalColumns} | ||
| columns={columns} | ||
| isLoading={isLoadingUserAssignments} | ||
| > | ||
| <DataTable.Table /> | ||
| <TableFooter /> | ||
| </DataTable> | ||
|
|
||
| </Container> | ||
| </AuthZLayout> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| export default AuditUserPage; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| import { defineMessages } from '@edx/frontend-platform/i18n'; | ||
|
|
||
| const messages = defineMessages( | ||
| { | ||
| 'authz.user.table.role.column.header': { | ||
| id: 'authz.user.table.role.column.header', | ||
| defaultMessage: 'Role', | ||
| description: 'Header for the role column in the user table', | ||
| }, | ||
| 'authz.user.table.organization.column.header': { | ||
| id: 'authz.user.table.organization.column.header', | ||
| defaultMessage: 'Organization', | ||
| description: 'Header for the organization column in the user table', | ||
| }, | ||
| 'authz.user.table.scope.column.header': { | ||
| id: 'authz.user.table.scope.column.header', | ||
| defaultMessage: 'Scope', | ||
| description: 'Header for the scope column in the user table', | ||
| }, | ||
| 'authz.user.table.permissions.column.header': { | ||
| id: 'authz.user.table.permissions.column.header', | ||
| defaultMessage: 'Permissions', | ||
| description: 'Header for the permissions column in the user table', | ||
| }, | ||
| 'authz.user.table.action.column.header': { | ||
| id: 'authz.user.table.action.column.header', | ||
| defaultMessage: 'Actions', | ||
| description: 'Header for the actions column in the user table', | ||
| }, | ||
| }, | ||
| ); | ||
|
|
||
| export default messages; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid this
username ?? ''conditional addenabled: !!usernameas a flag in the hook instead, so the API call is only trigger when a username exist. I'm not counting this as a blocker because it can be solved in a subsequent PR as a minor fix. @jacobo-dominguez-wguUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same with this behavior found by Claude:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All has been gathered here #132
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have addressed all of them on the last commit, thanks @dcoa
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was still a couple of comments left so they are in the issue #132