From 326c865d378941e79ba63fb96a6617daade5be07 Mon Sep 17 00:00:00 2001 From: Thomas Zemp Date: Wed, 18 Dec 2024 09:28:32 +0100 Subject: [PATCH] fix: add tests, clean up --- i18n/en.pot | 22 +- jest.config.js | 2 +- src/AppWrapper.js | 2 +- src/components/GroupForm/GroupForm.js | 3 +- .../RoleForm/AssignmentRestrictionsWarning.js | 11 +- .../RoleForm/LegacyAuthoritiesTable.js | 12 +- src/components/RoleForm/RoleForm.js | 7 +- src/components/RoleForm/RoleForm.test.js | 461 ++++++++++++++++++ .../useOrgUnitSearchResults.js | 2 +- .../SearchableOrgUnitTree.js | 2 +- src/components/SectionNavigation.js | 2 +- src/components/UserForm/RolesSection.js | 13 +- src/components/UserForm/RolesSection.test.js | 98 ++++ src/components/UserForm/UserForm.js | 5 +- src/components/UserForm/useFormData.js | 2 - src/components/UserForm/useFormData.test.js | 44 ++ .../GroupList/ContextMenu/ContextMenu.js | 3 +- .../GroupList/ContextMenu/Modals/JoinModal.js | 2 +- .../ContextMenu/Modals/LeaveModal.js | 2 +- src/pages/GroupList/GroupTable.js | 3 +- src/pages/GroupList/GroupTable.test.js | 4 +- src/pages/Home/Home.js | 2 +- src/pages/RoleDetails/RoleDetails.js | 3 +- src/pages/RoleDetails/use-role.js | 4 - src/pages/RoleList/RoleTable.js | 2 +- src/pages/UserList/ContextMenu/ContextMenu.js | 3 +- src/pages/UserList/UserTable.js | 2 +- .../current-user}/CurrentUserProvider.js | 4 +- src/providers/current-user/index.js | 2 + .../current-user}/useCurrentUser.js | 2 +- src/providers/index.js | 6 +- .../{ => referrer}/ReferrerContext.js | 0 .../{ => referrer}/ReferrerProvider.js | 0 src/providers/referrer/index.js | 2 + src/providers/{ => referrer}/useReferrer.js | 0 35 files changed, 663 insertions(+), 71 deletions(-) create mode 100644 src/components/RoleForm/RoleForm.test.js create mode 100644 src/components/UserForm/RolesSection.test.js create mode 100644 src/components/UserForm/useFormData.test.js rename src/{components => providers/current-user}/CurrentUserProvider.js (98%) create mode 100644 src/providers/current-user/index.js rename src/{hooks => providers/current-user}/useCurrentUser.js (58%) rename src/providers/{ => referrer}/ReferrerContext.js (100%) rename src/providers/{ => referrer}/ReferrerProvider.js (100%) create mode 100644 src/providers/referrer/index.js rename src/providers/{ => referrer}/useReferrer.js (100%) diff --git a/i18n/en.pot b/i18n/en.pot index 850d8eac0..42c951692 100644 --- a/i18n/en.pot +++ b/i18n/en.pot @@ -5,8 +5,8 @@ msgstr "" "Content-Type: text/plain; charset=utf-8\n" "Content-Transfer-Encoding: 8bit\n" "Plural-Forms: nplurals=2; plural=(n != 1)\n" -"POT-Creation-Date: 2024-12-12T11:59:53.609Z\n" -"PO-Revision-Date: 2024-12-12T11:59:53.610Z\n" +"POT-Creation-Date: 2024-12-18T07:54:11.061Z\n" +"PO-Revision-Date: 2024-12-18T07:54:11.062Z\n" msgid "Yes" msgstr "Yes" @@ -53,12 +53,6 @@ msgstr "Will be added" msgid "Will be removed" msgstr "Will be removed" -msgid "Network error" -msgstr "Network error" - -msgid "Could not load current user information." -msgstr "Could not load current user information." - msgid "Filter options" msgstr "Filter options" @@ -1012,12 +1006,6 @@ msgstr "Last login" msgid "Status" msgstr "Status" -msgid "Verified" -msgstr "Verified" - -msgid "Not verified" -msgstr "Not verified" - msgid "Disabled" msgstr "Disabled" @@ -1081,5 +1069,11 @@ msgstr "Interests" msgid "Birthday" msgstr "Birthday" +msgid "Network error" +msgstr "Network error" + +msgid "Could not load current user information." +msgstr "Could not load current user information." + msgid "Could not load system information." msgstr "Could not load system information." diff --git a/jest.config.js b/jest.config.js index 3daec13ba..d9f4e0400 100644 --- a/jest.config.js +++ b/jest.config.js @@ -1,4 +1,4 @@ module.exports = { setupFilesAfterEnv: ['/src/test-utils/setup-tests.js'], - transformIgnorePatterns: ['/node_modules/(?!(lodash-es)/)'], + transformIgnorePatterns: ['/node_modules/(?!(lodash-es|p-debounce)/)'], } diff --git a/src/AppWrapper.js b/src/AppWrapper.js index 04dd7d16f..39ea4bfb1 100644 --- a/src/AppWrapper.js +++ b/src/AppWrapper.js @@ -3,7 +3,7 @@ import './locales/index.js' import { CssVariables } from '@dhis2/ui' import React from 'react' import App from './App.js' -import { CurrentUserProvider } from './components/CurrentUserProvider.js' +import { CurrentUserProvider } from './providers/current-user/CurrentUserProvider.js' import { ReferrerProvider, SystemProvider } from './providers/index.js' const AppWrapper = () => ( diff --git a/src/components/GroupForm/GroupForm.js b/src/components/GroupForm/GroupForm.js index de7aa23e9..88ab64b63 100644 --- a/src/components/GroupForm/GroupForm.js +++ b/src/components/GroupForm/GroupForm.js @@ -4,8 +4,7 @@ import { NoticeBox, FinalForm } from '@dhis2/ui' import PropTypes from 'prop-types' import React from 'react' import { useHistory } from 'react-router-dom' -import { useCurrentUser } from '../../hooks/useCurrentUser.js' -import { useReferrerInfo } from '../../providers/index.js' +import { useCurrentUser, useReferrerInfo } from '../../providers/index.js' import navigateTo from '../../utils/navigateTo.js' import Attributes from '../Attributes/index.js' import Form, { FormSection } from '../Form.js' diff --git a/src/components/RoleForm/AssignmentRestrictionsWarning.js b/src/components/RoleForm/AssignmentRestrictionsWarning.js index 2c5c42c45..6a89f42da 100644 --- a/src/components/RoleForm/AssignmentRestrictionsWarning.js +++ b/src/components/RoleForm/AssignmentRestrictionsWarning.js @@ -9,8 +9,11 @@ export const AssignmentRestrictionWarning = ({ currentUser, roleId, roleAuthorities, + initiallyExpanded, }) => { - const [detailsExpanded, setDetailsExpanded] = useState(false) + const [detailsExpanded, setDetailsExpanded] = useState( + initiallyExpanded ?? false + ) // check if user is able to assign this role if they are a member const { @@ -57,6 +60,7 @@ export const AssignmentRestrictionWarning = ({ onClick={() => { setDetailsExpanded((prev) => !prev) }} + data-test="roles-details-expand" > {detailsExpanded ? ( @@ -67,7 +71,7 @@ export const AssignmentRestrictionWarning = ({ {detailsExpanded && ( -
    +
      {authoritiesUserDoesNotHave .sort((a, b) => ( @@ -77,7 +81,7 @@ export const AssignmentRestrictionWarning = ({ ) ) .map((auth) => ( -
    • +
    • {authorityIdToNameMap.get(auth) ?? auth}
      • ))} @@ -92,5 +96,6 @@ export const AssignmentRestrictionWarning = ({ AssignmentRestrictionWarning.propTypes = { roleId: PropTypes.string.isRequired, currentUser: PropTypes.object, + initiallyExpanded: PropTypes.bool, roleAuthorities: PropTypes.arrayOf(PropTypes.string), } diff --git a/src/components/RoleForm/LegacyAuthoritiesTable.js b/src/components/RoleForm/LegacyAuthoritiesTable.js index 024fe5270..cc9079f12 100644 --- a/src/components/RoleForm/LegacyAuthoritiesTable.js +++ b/src/components/RoleForm/LegacyAuthoritiesTable.js @@ -60,7 +60,10 @@ const LegacyAuthoritiesTable = React.memo( ) } return ( -
      +
      @@ -99,15 +102,16 @@ const LegacyAuthoritiesTableFF = ({ meta, ...props }) => { + const { value: inputValue, onChange: inputOnChange } = input const handleAuthorityRemove = useCallback( ({ id }) => { - const newSelectedAuthorities = new Set(input.value) + const newSelectedAuthorities = new Set(inputValue) if (newSelectedAuthorities.has(id)) { newSelectedAuthorities.delete(id) } - input.onChange(newSelectedAuthorities) + inputOnChange(newSelectedAuthorities) }, - [input.value, input.onChange] + [inputValue, inputOnChange] ) return ( diff --git a/src/components/RoleForm/RoleForm.js b/src/components/RoleForm/RoleForm.js index 6d38a1026..5521d8563 100644 --- a/src/components/RoleForm/RoleForm.js +++ b/src/components/RoleForm/RoleForm.js @@ -5,8 +5,7 @@ import { flatMap } from 'lodash-es' import PropTypes from 'prop-types' import React from 'react' import { useHistory } from 'react-router-dom' -import { useCurrentUser } from '../../hooks/useCurrentUser.js' -import { useReferrerInfo } from '../../providers/index.js' +import { useCurrentUser, useReferrerInfo } from '../../providers/index.js' import navigateTo from '../../utils/navigateTo.js' import Form, { FormSection, TextField, TransferField } from '../Form.js' import { AssignmentRestrictionWarning } from './AssignmentRestrictionsWarning.js' @@ -56,7 +55,9 @@ const getRoleAuthorityIDs = ({ tracker: role.authorities.filter((id) => trackerIDs.has(id)), importExport: role.authorities.filter((id) => importExportIDs.has(id)), system: role.authorities.filter((id) => systemIDs.has(id)), - legacy: role.authorities.filter((id) => !allKnownIDs.has(id)).sort(), + legacy: role.authorities + .filter((id) => !allKnownIDs.has(id)) + .sort((a, b) => a.localeCompare(b)), } } diff --git a/src/components/RoleForm/RoleForm.test.js b/src/components/RoleForm/RoleForm.test.js new file mode 100644 index 000000000..cab6a2458 --- /dev/null +++ b/src/components/RoleForm/RoleForm.test.js @@ -0,0 +1,461 @@ +import { CustomDataProvider, Provider } from '@dhis2/app-runtime' +import { ReactFinalForm } from '@dhis2/ui' +import { act, render, screen, within, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import PropTypes from 'prop-types' +import React from 'react' +import { useCurrentUser } from '../../providers/current-user/useCurrentUser.js' +import { useSystemInformation } from '../../providers/system/useSystemInformation.js' +import RoleForm from './RoleForm.js' + +const noop = () => {} + +jest.mock('@dhis2/ui', () => ({ + ...jest.requireActual('@dhis2/ui'), + Transfer: jest.fn(() =>

      Transfer

      ), +})) + +jest.mock('../../utils/navigateTo.js', () => { + return { + default: noop, + } +}) + +const CONFIG_DEFAULTS = { + baseUrl: 'https://debug.dhis2.org/dev', + apiVersion: '41', + systemInfo: { + serverTimeZoneId: 'Etc/UTC', + }, +} + +const MOCK_ROLE = { + name: 'Mock User Role', + description: 'This is a mock user role', + authorities: ['lutefisk'], + restrictions: [], + displayName: 'Mock User Role', + id: 'user-role-1-id', +} + +const MOCK_AUTHORITIES = [ + { id: 'a', name: 'apple' }, + { id: 'b', name: 'banana' }, + { id: 'd', name: 'Dance' }, + { id: 'bb', name: 'beat box' }, +] + +const MOCK_CURRENT_USER = { + name: 'Ola Nordmann', + id: 'ola-nordmann-id-1', + authorities: ['lutefisk'], + userRoleIds: ['user-role-JUL-id'], + userGroupIds: [], + hasAllAuthority: false, +} + +jest.mock('../../providers/current-user/useCurrentUser.js', () => ({ + useCurrentUser: jest.fn(() => MOCK_CURRENT_USER), +})) + +const MOCK_SYSTEM_INFORMATION = { + authorities: MOCK_AUTHORITIES, + authorityIdToNameMap: new Map( + MOCK_AUTHORITIES.map(({ id, name }) => [id, name]) + ), + usersCanAssignOwnUserRoles: false, +} + +jest.mock('../../providers/system/useSystemInformation.js', () => ({ + useSystemInformation: jest.fn(() => MOCK_SYSTEM_INFORMATION), +})) + +const mockPatchUserRole = jest.fn() + +const customProviderData = { + 'userRoles/user-role-1-id': (type, query) => { + if (type === 'json-patch') { + mockPatchUserRole(query?.data) + } + return {} + }, +} + +const { Form } = ReactFinalForm + +const RenderWrapper = ({ children }) => ( + + +
      {() => children}
      +       +       +) + +RenderWrapper.propTypes = { + children: PropTypes.node, +} + +describe('Legacy Authorities', () => { + afterEach(() => { + jest.clearAllMocks() + }) + + it('does not show any legacy authorities when in edit mode (no role prop) ', async () => { + act(() => { + render( + + + + ) + }) + + const legacyAuthoritiesTable = await screen.queryByTestId( + 'legacy-authorities-table' + ) + expect(legacyAuthoritiesTable).toBe(null) + }) + + it('displays a message saying there are no legacy authorities when role authorities all exist in system response', async () => { + const MOCK_AUTHORITIES = [ + { id: 'ka', name: 'kasper' }, + { id: 'je', name: 'jesper' }, + { id: 'jo', name: 'jonatan' }, + ] + useSystemInformation.mockReturnValueOnce({ + authorities: MOCK_AUTHORITIES, + }) + const role_with_no_legacy_authorities = { + ...MOCK_ROLE, + authorities: MOCK_AUTHORITIES.map(({ id }) => id), + } + act(() => { + render( + + + + ) + }) + + const noLegacyAuthoritiesMessage = await screen.findByText( + 'There are no legacy or nonstandard authorities assigned to this user role.' + ) + expect(noLegacyAuthoritiesMessage).toBeInTheDocument() + }) + + it('shows any authorities not in the system response in a case-insensitive alphabetical order ', async () => { + const role_with_multiple_legacy_authorities = { + ...MOCK_ROLE, + authorities: ['zebras', 'antelopes', 'Moose'], + } + act(() => { + render( + + + + ) + }) + + const legacyAuthoritiesTable = await screen.findByTestId( + 'legacy-authorities-table' + ) + const rows = within(legacyAuthoritiesTable).getAllByRole('row') + expect(rows[1]).toHaveTextContent('antelopes') + expect(rows[2]).toHaveTextContent('Moose') + expect(rows[3]).toHaveTextContent('zebras') + expect(rows.length).toBe(4) // legacy count + 1 header row + }) + + it('removes authorities when you click remove and then save the role', async () => { + const role_with_multiple_legacy_authorities = { + ...MOCK_ROLE, + authorities: ['zebras', 'antelopes', 'Moose'], + } + const SAVE_BUTTON_TEXT = 'save this role' + + const expected_post_after_removal = [ + { op: 'add', path: '/name', value: 'Mock User Role' }, + { + op: 'add', + path: '/description', + value: 'This is a mock user role', + }, + { op: 'add', path: '/authorities', value: ['Moose', 'zebras'] }, + ] + + act(() => { + render( + + + + ) + }) + const legacyAuthoritiesTable = await screen.findByTestId( + 'legacy-authorities-table' + ) + const rows = within(legacyAuthoritiesTable).getAllByRole('row') + within(rows[1]).queryByRole('button', { name: 'Remove' }).click() + + const saveButton = await screen.findByText(SAVE_BUTTON_TEXT) + await waitFor(() => { + userEvent.click(saveButton) + }) + expect(mockPatchUserRole).toHaveBeenCalled() + expect(mockPatchUserRole).toHaveBeenCalledWith( + expected_post_after_removal + ) + }) +}) + +describe('User roles assignment warnings', () => { + afterEach(() => { + jest.clearAllMocks() + }) + + it('does not show warning if user has ALL authority', async () => { + const user_with_all_authority = { + ...MOCK_CURRENT_USER, + authorities: ['ALL'], + hasAllAuthority: true, + userRoleIds: ['user-role-for-all'], + } + useCurrentUser.mockReturnValueOnce(user_with_all_authority) + + const role_with_authorities = { + ...MOCK_ROLE, + id: 'user-role-for-all', + authorities: ['d', 'bb'], // corresponds to 'Dance' and 'beat box' in id:name map + } + + // NB: default mock has keyCanGrantOwnUserAuthorityGroups:false + + act(() => { + render( + + + + ) + }) + + await waitFor(async () => { + const cannotAssignWarningAuthorities = await screen.queryByText( + 'You cannot assign this role because it has authorities that you do not have.' + ) + expect(cannotAssignWarningAuthorities).toBe(null) + const cannotAssignWarningMemberOfRole = await screen.queryByText( + 'You cannot assign this role because you are assigned to this role, and your system does not allow you to assign roles of which you are a member.' + ) + expect(cannotAssignWarningMemberOfRole).toBe(null) + }) + }) + + it('shows a warning if user cannot assign role because role contains authorities that user does not have', async () => { + const user_with_limited_authorities = { + ...MOCK_CURRENT_USER, + authorities: ['code', 'debug'], + } + useCurrentUser.mockReturnValueOnce(user_with_limited_authorities) + + const role_with_authority_user_does_not_have = { + ...MOCK_ROLE, + authorities: ['d', 'bb'], // corresponds to 'Dance' and 'beat box' in id:name map + } + + act(() => { + render( + + + + ) + }) + + const cannotAssignWarning = await screen.findByText( + 'You cannot assign this role because it has authorities that you do not have.' + ) + expect(cannotAssignWarning).toBeInTheDocument() + }) + + it('does not show warning about missing roles if user has all the authorities that exist on role', async () => { + const user_with_limited_authorities = { + ...MOCK_CURRENT_USER, + authorities: ['code', 'debug'], + } + useCurrentUser.mockReturnValueOnce(user_with_limited_authorities) + + const role_with_authorities_user_has = { + ...MOCK_ROLE, + authorities: ['code', 'debug'], + } + + act(() => { + render( + + + + ) + }) + await waitFor(() => { + const cannotAssignWarning = screen.queryByText( + 'You cannot assign this role because it has authorities that you do not have.' + ) + expect(cannotAssignWarning).toBeNull() + }) + }) + + it('user roles details are toggleable and displayed alphabetically', async () => { + const user_with_limited_authorities = { + ...MOCK_CURRENT_USER, + authorities: ['code', 'debug'], + } + useCurrentUser.mockReturnValueOnce(user_with_limited_authorities) + + const role_with_authority_user_does_not_have = { + ...MOCK_ROLE, + authorities: ['d', 'bb'], // corresponds to 'Dance' and 'beat box' in id:name map + } + + useSystemInformation.mockReturnValueOnce(MOCK_SYSTEM_INFORMATION) + + act(() => { + render( + + + + ) + }) + + await waitFor(async () => { + const expandIcon = await screen.findByTestId('roles-details-expand') + userEvent.click(expandIcon) + const missingAuthoritiesList = await screen.findByTestId( + 'authorities-user-does-not-have-list' + ) + const missingAuthorities = within( + missingAuthoritiesList + ).getAllByRole('listitem') + expect(missingAuthorities[0]).toHaveTextContent('beat box') + expect(missingAuthorities[1]).toHaveTextContent('Dance') + expect(missingAuthorities.length).toBe(2) + userEvent.click(expandIcon) + expect(missingAuthoritiesList).not.toBeVisible() + }) + }) + + it('shows a warning if user cannot assign role because role is assigned to user and keyCanGrantOwnUserAuthorityGroups:false ', async () => { + // NB: default mock has keyCanGrantOwnUserAuthorityGroups:false + const user_assigned_to_role = { + ...MOCK_CURRENT_USER, + userRoleIds: ['user-role-id-pi'], + } + useCurrentUser.mockReturnValueOnce(user_assigned_to_role) + + const role_assigned_to_user = { + ...MOCK_ROLE, + id: 'user-role-id-pi', + } + + act(() => { + render( + + + + ) + }) + + const cannotAssignWarning = await screen.findByText( + 'You cannot assign this role because you are assigned to this role, and your system does not allow you to assign roles of which you are a member.' + ) + expect(cannotAssignWarning).toBeInTheDocument() + }) + + it('does not show a warning if role is not assigned to user and keyCanGrantOwnUserAuthorityGroups:false ', async () => { + // NB: default mock has keyCanGrantOwnUserAuthorityGroups:false + const user_not_assigned_to_role = { + ...MOCK_CURRENT_USER, + userRoleIds: ['some-other-user-role'], + } + useCurrentUser.mockReturnValueOnce(user_not_assigned_to_role) + + const role_not_assigned_to_user = { + ...MOCK_ROLE, + id: 'user-role-id-e', + } + + act(() => { + render( + + + + ) + }) + + await waitFor(async () => { + const cannotAssignWarning = await screen.queryByText( + 'You cannot assign this role because you are assigned to this role, and your system does not allow you to assign roles of which you are a member.' + ) + expect(cannotAssignWarning).toBe(null) + }) + }) + + it.skip('does not show a warning if user cannot assign role because role is assigned to user and keyCanGrantOwnUserAuthorityGroups:true ', async () => { + const user_assigned_to_role = { + ...MOCK_CURRENT_USER, + userRoleIds: ['user-role-id-pi'], + } + useCurrentUser.mockReturnValueOnce(user_assigned_to_role) + + const role_assigned_to_user = { + ...MOCK_ROLE, + id: 'user-role-id-pi', + } + + useSystemInformation.mockReturnValueOnce({ + ...MOCK_SYSTEM_INFORMATION, + usersCanAssignOwnUserRoles: true, + }) + + act(() => { + render( + + + + ) + }) + + await waitFor(async () => { + const cannotAssignWarning = await screen.queryByText( + 'You cannot assign this role because you are assigned to this role, and your system does not allow you to assign roles of which you are a member.' + ) + expect(cannotAssignWarning).toBe(null) + }) + }) +}) diff --git a/src/components/SearchableOrgUnitTree/AsyncAutoComplete/useOrgUnitSearchResults.js b/src/components/SearchableOrgUnitTree/AsyncAutoComplete/useOrgUnitSearchResults.js index 85ea9e0dc..1c460af41 100644 --- a/src/components/SearchableOrgUnitTree/AsyncAutoComplete/useOrgUnitSearchResults.js +++ b/src/components/SearchableOrgUnitTree/AsyncAutoComplete/useOrgUnitSearchResults.js @@ -1,7 +1,7 @@ import { useDataQuery } from '@dhis2/app-runtime' import { debounce } from 'lodash-es' import { useState, useEffect, useRef } from 'react' -import { useCurrentUser } from '../../../hooks/useCurrentUser.js' +import { useCurrentUser } from '../../../providers/index.js' import { PAGE_SIZE, MIN_CHAR_LENGTH, DEBOUNCE_TIME } from './constants.js' import { getRestrictedOrgUnits } from './getRestrictedOrgUnits.js' diff --git a/src/components/SearchableOrgUnitTree/SearchableOrgUnitTree.js b/src/components/SearchableOrgUnitTree/SearchableOrgUnitTree.js index cb01bfd60..b3c0d9948 100644 --- a/src/components/SearchableOrgUnitTree/SearchableOrgUnitTree.js +++ b/src/components/SearchableOrgUnitTree/SearchableOrgUnitTree.js @@ -11,7 +11,7 @@ import cx from 'classnames' import { defer } from 'lodash-es' import PropTypes from 'prop-types' import React, { useState, useCallback, useMemo } from 'react' -import { useCurrentUser } from '../../hooks/useCurrentUser.js' +import { useCurrentUser } from '../../providers/index.js' import AsyncAutoComplete from './AsyncAutoComplete/index.js' import getInitiallyExpandedUnits from './getInitiallyExpandedUnits.js' import getInitiallySelectedUnits from './getInitiallySelectedUnits.js' diff --git a/src/components/SectionNavigation.js b/src/components/SectionNavigation.js index e4ba56838..c9ba05e59 100644 --- a/src/components/SectionNavigation.js +++ b/src/components/SectionNavigation.js @@ -2,7 +2,6 @@ import i18n from '@dhis2/d2-i18n' import { NoticeBox } from '@dhis2/ui' import React, { useEffect } from 'react' import { Route, useRouteMatch, useLocation } from 'react-router-dom' -import { useCurrentUser } from '../hooks/useCurrentUser.js' import CreateGroup from '../pages/CreateGroup.js' import CreateRole from '../pages/CreateRole.js' import CreateUser from '../pages/CreateUser.js' @@ -16,6 +15,7 @@ import RoleDetails from '../pages/RoleDetails/index.js' import RoleList from '../pages/RoleList/index.js' import UserList from '../pages/UserList/index.js' import UserProfile from '../pages/UserProfile/index.js' +import { useCurrentUser } from '../providers/index.js' import navigateTo from '../utils/navigateTo.js' import styles from './SectionNavigation.module.css' import { SideNav, NavItem } from './SideNav.js' diff --git a/src/components/UserForm/RolesSection.js b/src/components/UserForm/RolesSection.js index 3a944153c..5d7053bef 100644 --- a/src/components/UserForm/RolesSection.js +++ b/src/components/UserForm/RolesSection.js @@ -7,13 +7,7 @@ import { FormSection, TransferField } from '../Form.js' import { hasSelectionValidator } from './validators.js' const RolesSection = React.memo( - ({ - user, - userRoleOptions, - userGroupOptions, - userRolesAreHidden, - userRolesHidden, - }) => ( + ({ user, userRoleOptions, userGroupOptions, userRolesHidden }) => (
      - {userRolesAreHidden && ( + {userRolesHidden?.length > 0 && ( id) || []} + initialValue={user?.userGroups?.map(({ id }) => id) || []} /> ) @@ -67,7 +61,6 @@ RolesSection.propTypes = { userGroupOptions: PropTypes.array.isRequired, userRoleOptions: PropTypes.array.isRequired, user: PropTypes.object, - userRolesAreHidden: PropTypes.bool, userRolesHidden: PropTypes.array, } diff --git a/src/components/UserForm/RolesSection.test.js b/src/components/UserForm/RolesSection.test.js new file mode 100644 index 000000000..ec569c22f --- /dev/null +++ b/src/components/UserForm/RolesSection.test.js @@ -0,0 +1,98 @@ +import { ReactFinalForm } from '@dhis2/ui' +import { render, screen } from '@testing-library/react' +import PropTypes from 'prop-types' +import React from 'react' +import { MemoryRouter } from 'react-router-dom' +import RolesSection from './RolesSection.js' + +jest.mock('@dhis2/ui', () => ({ + ...jest.requireActual('@dhis2/ui'), + Transfer: jest.fn(() =>

      Transfer

      ), +})) + +const { Form } = ReactFinalForm + +const MOCK_CURRENT_USER = { + name: 'Ola Nordmann', + id: 'ola-nordmann-id-1', + authorities: ['lutefisk'], + userRoleIds: ['user-role-JUL-id'], + userGroupIds: [], + hasAllAuthority: false, +} + +const DEFAULT_HIDDEN_ROLES = [ + { id: 'a', displayName: 'Antagonize aardvarks' }, + { id: 'b', displayName: 'Befriend badgers' }, +] + +const RenderWrapper = ({ children }) => ( + +
      {}}>{() => children}
      +       +) + +RenderWrapper.propTypes = { + children: PropTypes.node, +} + +describe('RolesSection', () => { + it('shows a warning message if there are hidden roles', () => { + render( + + + + ) + const warningMessage = screen.getByText( + 'You do not have permission to assign certain user roles' + ) + expect(warningMessage).toBeInTheDocument() + }) + + it('does not show a warning message if there are no hidden roles', () => { + render( + + + + ) + const warningMessage = screen.queryByText( + 'You do not have permission to assign certain user roles' + ) + expect(warningMessage).toBe(null) + }) + + it('shows list of hidden roles with a link', () => { + render( + + + + ) + const linkOne = screen.getByRole('link', { + name: 'Antagonize aardvarks', + }) + expect(linkOne).toHaveProperty( + 'href', + 'http://localhost/user-roles/view/a' + ) + const linkTwo = screen.getByRole('link', { name: 'Befriend badgers' }) + expect(linkTwo).toHaveProperty( + 'href', + 'http://localhost/user-roles/view/b' + ) + }) +}) diff --git a/src/components/UserForm/UserForm.js b/src/components/UserForm/UserForm.js index 10bd84f9e..db6ce0370 100644 --- a/src/components/UserForm/UserForm.js +++ b/src/components/UserForm/UserForm.js @@ -5,8 +5,7 @@ import { keyBy } from 'lodash-es' import PropTypes from 'prop-types' import React, { useState } from 'react' import { useHistory } from 'react-router-dom' -import { useCurrentUser } from '../../hooks/useCurrentUser.js' -import { useReferrerInfo } from '../../providers/index.js' +import { useCurrentUser, useReferrerInfo } from '../../providers/index.js' import navigateTo from '../../utils/navigateTo.js' import Attributes from '../Attributes/index.js' import Form, { FormSection } from '../Form.js' @@ -40,7 +39,6 @@ const UserForm = ({ databaseLanguageOptions, userRoleOptions, userRolesHidden, - userRolesAreHidden, userGroupOptions, dimensionConstraints, dimensionConstraintOptions, @@ -211,7 +209,6 @@ const UserForm = ({ user={user} userGroupOptions={userGroupOptions} userRoleOptions={userRoleOptions} - userRolesAreHidden={userRolesAreHidden} userRolesHidden={userRolesHidden} /> { all: userRolesAll, assignable: userRoles, }), - userRolesAreHidden: - userRoles.length !== data.userRolesCount?.pager?.total, userGroupOptions: makeOptions(userGroups), dimensionConstraints, dimensionConstraintOptions: makeOptions(dimensionConstraints), diff --git a/src/components/UserForm/useFormData.test.js b/src/components/UserForm/useFormData.test.js new file mode 100644 index 000000000..024d622cc --- /dev/null +++ b/src/components/UserForm/useFormData.test.js @@ -0,0 +1,44 @@ +import { renderHook } from '@testing-library/react-hooks' +import { useFormData } from './useFormData.js' + +jest.mock('@dhis2/app-runtime', () => ({ + ...jest.requireActual('@dhis2/app-runtime'), + useDataQuery: () => ({ + data: { + interfaceLanguages: [], + databaseLanguages: [], + userRoles: { + userRoles: [ + { id: 'a', displayName: 'aerobics' }, + { id: 'b', displayName: 'baseball' }, + ], + }, + userRolesAll: { + userRoles: [ + { id: 'a', displayName: 'aerobics' }, + { id: 'b', displayName: 'baseball' }, + { id: 'c', displayName: 'cycling' }, + { id: 'd', displayName: 'diving' }, + ], + }, + userGroups: { userGroups: [] }, + dimensionConstraints: { dimensions: [] }, + attributes: { attributes: [] }, + filledOrganisationUnitLevels: {}, + }, + }), +})) + +describe('useFormData', () => { + it('correctly sorts userRoles into visible and hidden roles', () => { + const { result } = renderHook(() => useFormData()) + expect(result.current).toHaveProperty('userRoleOptions', [ + { label: 'aerobics', value: 'a' }, + { label: 'baseball', value: 'b' }, + ]) + expect(result.current).toHaveProperty('userRolesHidden', [ + { displayName: 'cycling', id: 'c' }, + { displayName: 'diving', id: 'd' }, + ]) + }) +}) diff --git a/src/pages/GroupList/ContextMenu/ContextMenu.js b/src/pages/GroupList/ContextMenu/ContextMenu.js index e08809744..d13d16556 100644 --- a/src/pages/GroupList/ContextMenu/ContextMenu.js +++ b/src/pages/GroupList/ContextMenu/ContextMenu.js @@ -12,8 +12,7 @@ import { } from '@dhis2/ui' import PropTypes from 'prop-types' import React, { useState } from 'react' -import { useCurrentUser } from '../../../hooks/useCurrentUser.js' -import { useReferrerInfo } from '../../../providers/index.js' +import { useCurrentUser, useReferrerInfo } from '../../../providers/index.js' import navigateTo from '../../../utils/navigateTo.js' import DeleteModal from './Modals/DeleteModal.js' import JoinModal from './Modals/JoinModal.js' diff --git a/src/pages/GroupList/ContextMenu/Modals/JoinModal.js b/src/pages/GroupList/ContextMenu/Modals/JoinModal.js index f22a2ca2a..a0e823591 100644 --- a/src/pages/GroupList/ContextMenu/Modals/JoinModal.js +++ b/src/pages/GroupList/ContextMenu/Modals/JoinModal.js @@ -10,8 +10,8 @@ import { } from '@dhis2/ui' import PropTypes from 'prop-types' import React, { useState } from 'react' -import { useCurrentUser } from '../../../../hooks/useCurrentUser.js' import { useFetchAlert } from '../../../../hooks/useFetchAlert.js' +import { useCurrentUser } from '../../../../providers/index.js' const JoinModal = ({ group, refetchGroups, onClose }) => { const currentUser = useCurrentUser() diff --git a/src/pages/GroupList/ContextMenu/Modals/LeaveModal.js b/src/pages/GroupList/ContextMenu/Modals/LeaveModal.js index dba04febe..33364292c 100644 --- a/src/pages/GroupList/ContextMenu/Modals/LeaveModal.js +++ b/src/pages/GroupList/ContextMenu/Modals/LeaveModal.js @@ -10,8 +10,8 @@ import { } from '@dhis2/ui' import PropTypes from 'prop-types' import React, { useState } from 'react' -import { useCurrentUser } from '../../../../hooks/useCurrentUser.js' import { useFetchAlert } from '../../../../hooks/useFetchAlert.js' +import { useCurrentUser } from '../../../../providers/index.js' const LeaveModal = ({ group, refetchGroups, onClose }) => { const engine = useDataEngine() diff --git a/src/pages/GroupList/GroupTable.js b/src/pages/GroupList/GroupTable.js index e791ee694..15a900021 100644 --- a/src/pages/GroupList/GroupTable.js +++ b/src/pages/GroupList/GroupTable.js @@ -15,8 +15,7 @@ import PropTypes from 'prop-types' import React from 'react' import DataTableInfoWrapper from '../../components/DataTableInfoWrapper.js' import EmptyTableInfo from '../../components/EmptyTableInfo.js' -import { useCurrentUser } from '../../hooks/useCurrentUser.js' -import { useReferrerInfo } from '../../providers/useReferrer.js' +import { useCurrentUser, useReferrerInfo } from '../../providers/index.js' import navigateTo from '../../utils/navigateTo.js' import ContextMenuButton from './ContextMenu/ContextMenuButton.js' diff --git a/src/pages/GroupList/GroupTable.test.js b/src/pages/GroupList/GroupTable.test.js index 0ff7153ab..a57ec474d 100644 --- a/src/pages/GroupList/GroupTable.test.js +++ b/src/pages/GroupList/GroupTable.test.js @@ -1,10 +1,10 @@ import { render, screen, within } from '@testing-library/react' import userEvent from '@testing-library/user-event' import React from 'react' -import { useCurrentUser } from '../../hooks/useCurrentUser.js' +import { useCurrentUser } from '../../providers/current-user/useCurrentUser.js' import GroupTable from './GroupTable.js' -jest.mock('../../hooks/useCurrentUser.js', () => ({ +jest.mock('../../providers/current-user/useCurrentUser.js', () => ({ useCurrentUser: jest.fn(), })) diff --git a/src/pages/Home/Home.js b/src/pages/Home/Home.js index de57a25a6..a066d9982 100644 --- a/src/pages/Home/Home.js +++ b/src/pages/Home/Home.js @@ -1,7 +1,7 @@ import i18n from '@dhis2/d2-i18n' import { IconAdd16, IconList16 } from '@dhis2/ui' import React from 'react' -import { useCurrentUser } from '../../hooks/useCurrentUser.js' +import { useCurrentUser } from '../../providers/index.js' import styles from './Home.module.css' import SectionCard from './SectionCard.js' diff --git a/src/pages/RoleDetails/RoleDetails.js b/src/pages/RoleDetails/RoleDetails.js index be9ba2dc1..67580675a 100644 --- a/src/pages/RoleDetails/RoleDetails.js +++ b/src/pages/RoleDetails/RoleDetails.js @@ -5,7 +5,7 @@ import React from 'react' import { useHistory } from 'react-router-dom' import Details, { Section, Field } from '../../components/Details/index.js' import { AssignmentRestrictionWarning } from '../../components/RoleForm/AssignmentRestrictionsWarning.js' -import { useCurrentUser } from '../../hooks/useCurrentUser.js' +import { useCurrentUser } from '../../providers/index.js' import useRole from './use-role.js' const RoleDetails = ({ roleId }) => { @@ -53,6 +53,7 @@ const RoleDetails = ({ roleId }) => { roleId={role.id} roleAuthorities={role.authorities} currentUser={currentUser} + initiallyExpanded={true} /> )} diff --git a/src/pages/RoleDetails/use-role.js b/src/pages/RoleDetails/use-role.js index 497c0899a..3bf31ac9d 100644 --- a/src/pages/RoleDetails/use-role.js +++ b/src/pages/RoleDetails/use-role.js @@ -9,10 +9,6 @@ const query = { fields: ['id', 'access', 'displayName', 'authorities'], }, }, - // move this elsewhere presumably? (e.g. to provider) - authorities: { - resource: 'authorities', - }, } const useRole = (roleId) => { diff --git a/src/pages/RoleList/RoleTable.js b/src/pages/RoleList/RoleTable.js index 00af930ee..ffb447f66 100644 --- a/src/pages/RoleList/RoleTable.js +++ b/src/pages/RoleList/RoleTable.js @@ -15,7 +15,7 @@ import PropTypes from 'prop-types' import React from 'react' import DataTableInfoWrapper from '../../components/DataTableInfoWrapper.js' import EmptyTableInfo from '../../components/EmptyTableInfo.js' -import { useReferrerInfo } from '../../providers/useReferrer.js' +import { useReferrerInfo } from '../../providers/index.js' import navigateTo from '../../utils/navigateTo.js' import ContextMenuButton from './ContextMenu/ContextMenuButton.js' diff --git a/src/pages/UserList/ContextMenu/ContextMenu.js b/src/pages/UserList/ContextMenu/ContextMenu.js index 36ac2247d..c440aae24 100644 --- a/src/pages/UserList/ContextMenu/ContextMenu.js +++ b/src/pages/UserList/ContextMenu/ContextMenu.js @@ -17,8 +17,7 @@ import { } from '@dhis2/ui' import PropTypes from 'prop-types' import React, { useState } from 'react' -import { useCurrentUser } from '../../../hooks/useCurrentUser.js' -import { useReferrerInfo } from '../../../providers/useReferrer.js' +import { useCurrentUser, useReferrerInfo } from '../../../providers/index.js' import navigateTo from '../../../utils/navigateTo.js' import DeleteModal from './Modals/DeleteModal.js' import Disable2FaModal from './Modals/Disable2FaModal.js' diff --git a/src/pages/UserList/UserTable.js b/src/pages/UserList/UserTable.js index 439a6ae66..a2a059aa8 100644 --- a/src/pages/UserList/UserTable.js +++ b/src/pages/UserList/UserTable.js @@ -17,7 +17,7 @@ import PropTypes from 'prop-types' import React from 'react' import DataTableInfoWrapper from '../../components/DataTableInfoWrapper.js' import EmptyTableInfo from '../../components/EmptyTableInfo.js' -import { useReferrerInfo } from '../../providers/useReferrer.js' +import { useReferrerInfo } from '../../providers/index.js' import navigateTo from '../../utils/navigateTo.js' import ContextMenuButton from './ContextMenu/ContextMenuButton.js' diff --git a/src/components/CurrentUserProvider.js b/src/providers/current-user/CurrentUserProvider.js similarity index 98% rename from src/components/CurrentUserProvider.js rename to src/providers/current-user/CurrentUserProvider.js index a7b3d123d..05c73ca5d 100644 --- a/src/components/CurrentUserProvider.js +++ b/src/providers/current-user/CurrentUserProvider.js @@ -8,7 +8,7 @@ const CurrentUserContext = createContext({}) const query = { me: { - resource: '/me', + resource: 'me', params: { fields: [ 'id', @@ -25,7 +25,7 @@ const query = { }, }, systemOrganisationUnitRoots: { - resource: '/organisationUnits', + resource: 'organisationUnits', params: { paging: false, level: 1, diff --git a/src/providers/current-user/index.js b/src/providers/current-user/index.js new file mode 100644 index 000000000..ba2005e5a --- /dev/null +++ b/src/providers/current-user/index.js @@ -0,0 +1,2 @@ +export { useCurrentUser } from './useCurrentUser.js' +export { CurrentUserProvider } from './CurrentUserProvider.js' diff --git a/src/hooks/useCurrentUser.js b/src/providers/current-user/useCurrentUser.js similarity index 58% rename from src/hooks/useCurrentUser.js rename to src/providers/current-user/useCurrentUser.js index 2f62ed622..9f4eab220 100644 --- a/src/hooks/useCurrentUser.js +++ b/src/providers/current-user/useCurrentUser.js @@ -1,4 +1,4 @@ import { useContext } from 'react' -import { CurrentUserContext } from '../components/CurrentUserProvider.js' +import { CurrentUserContext } from './CurrentUserProvider.js' export const useCurrentUser = () => useContext(CurrentUserContext) diff --git a/src/providers/index.js b/src/providers/index.js index 85ffc889f..6f7bfb60d 100644 --- a/src/providers/index.js +++ b/src/providers/index.js @@ -1,3 +1,3 @@ -export { useReferrerInfo } from './useReferrer.js' -export { ReferrerProvider } from './ReferrerProvider.js' -export { SystemProvider, useSystemInformation } from './system/index.js' +export * from './current-user/index.js' +export * from './referrer/index.js' +export * from './system/index.js' diff --git a/src/providers/ReferrerContext.js b/src/providers/referrer/ReferrerContext.js similarity index 100% rename from src/providers/ReferrerContext.js rename to src/providers/referrer/ReferrerContext.js diff --git a/src/providers/ReferrerProvider.js b/src/providers/referrer/ReferrerProvider.js similarity index 100% rename from src/providers/ReferrerProvider.js rename to src/providers/referrer/ReferrerProvider.js diff --git a/src/providers/referrer/index.js b/src/providers/referrer/index.js new file mode 100644 index 000000000..2fa0c475c --- /dev/null +++ b/src/providers/referrer/index.js @@ -0,0 +1,2 @@ +export { useReferrerInfo } from './useReferrer.js' +export { ReferrerProvider } from './ReferrerProvider.js' diff --git a/src/providers/useReferrer.js b/src/providers/referrer/useReferrer.js similarity index 100% rename from src/providers/useReferrer.js rename to src/providers/referrer/useReferrer.js