Skip to content
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

feat: add email verification in user list [DHIS2-18613] #1520

Merged
merged 2 commits into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 12 additions & 14 deletions cypress/e2e/user/search-users.feature
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
Feature: The user list can be searched

# SKIPPED because of search problems (7 Jan 2025)
# Scenario: The user list gets filtered by name
# Given some users exist
# And the user-manager navigated to the user list view
# When the user-manager searches the list by entering a name
# Then only the users whose username or display name contains the search term should be displayed
Scenario: The user list gets filtered by name
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated, but turning these back on because the issue with them is now resolved on the backend

Given some users exist
And the user-manager navigated to the user list view
When the user-manager searches the list by entering a name
Then only the users whose username or display name contains the search term should be displayed

# Scenario: The user list gets filtered by organisation unit
# Given some users exist
Expand All @@ -25,14 +24,13 @@ Feature: The user list can be searched
# When the user-manager chooses to view only self-registered users
# Then only the users who have registered themselves should be displayed

# SKIPPED because of search problems (7 Jan 2025)
# Scenario: The user-manager returns to the search results after opening the edit form
# Given some users exist
# And the user-manager navigated to the user list view
# And the user-manager filtered the list
# When the user-manager edits one of the displayed users
# And returns to the list view without saving
# Then the previously applied search should still be applied
Scenario: The user-manager returns to the search results after opening the edit form
Given some users exist
And the user-manager navigated to the user list view
And the user-manager filtered the list
When the user-manager edits one of the displayed users
And returns to the list view without saving
Then the previously applied search should still be applied

# Scenario: The user-manager returns to the search results after editing a user
# Given some users exist
Expand Down

Large diffs are not rendered by default.

126 changes: 81 additions & 45 deletions cypress/fixtures/network/42/static_resources.json

Large diffs are not rendered by default.

9 changes: 5 additions & 4 deletions cypress/fixtures/network/42/summary.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{
"count": 348,
"totalResponseSize": 258639,
"duplicates": 274,
"nonDeterministicResponses": 22,
"count": 398,
"totalResponseSize": 309677,
"duplicates": 307,
"nonDeterministicResponses": 24,
"apiVersion": "42",
"fixtureFiles": [
"static_resources.json",
Expand All @@ -11,6 +11,7 @@
"user_groups_can_be_listed.json",
"the_user_group_list_can_be_searched.json",
"users_can_be_listed.json",
"the_user_list_can_be_searched.json",
"user_roles_can_be_listed.json",
"the_user_role_list_can_be_searched.json"
]
Expand Down
10 changes: 5 additions & 5 deletions cypress/fixtures/network/42/the_app_has_a_main_navigation.json

Large diffs are not rendered by default.

Large diffs are not rendered by default.

508 changes: 508 additions & 0 deletions cypress/fixtures/network/42/the_user_list_can_be_searched.json

Large diffs are not rendered by default.

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions cypress/fixtures/network/42/user_groups_can_be_listed.json

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions cypress/fixtures/network/42/user_roles_can_be_listed.json

Large diffs are not rendered by default.

22 changes: 11 additions & 11 deletions cypress/fixtures/network/42/users_can_be_listed.json

Large diffs are not rendered by default.

22 changes: 20 additions & 2 deletions i18n/en.pot
Original file line number Diff line number Diff line change
Expand Up @@ -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-18T07:54:11.061Z\n"
"PO-Revision-Date: 2024-12-18T07:54:11.062Z\n"
"POT-Creation-Date: 2025-01-15T10:47:12.485Z\n"
"PO-Revision-Date: 2025-01-15T10:47:12.486Z\n"

msgid "Yes"
msgstr "Yes"
Expand Down Expand Up @@ -973,6 +973,18 @@ msgstr "{{months}} months"
msgid "Time inactive"
msgstr "Time inactive"

msgid "Email verification"
msgstr "Email verification"

msgid "All"
msgstr "All"

msgid "Email verified"
msgstr "Email verified"

msgid "Email not verified"
msgstr "Email not verified"

msgid "Invitation"
msgstr "Invitation"

Expand Down Expand Up @@ -1006,6 +1018,12 @@ msgstr "Last login"
msgid "Status"
msgstr "Status"

msgid "Verified"
msgstr "Verified"

msgid "Not verified"
msgstr "Not verified"

msgid "Disabled"
msgstr "Disabled"

Expand Down
12 changes: 12 additions & 0 deletions src/hooks/useFeatureToggle.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { useConfig } from '@dhis2/app-runtime'

export const useFeatureToggle = () => {
const config = useConfig()
const minorVersion = config?.serverVersion?.minor
const emailConfigured = config?.systemInfo?.emailConfigured
return {
displayEmailVerifiedStatus: Boolean(
emailConfigured && Number(minorVersion) >= 42
),
}
}
46 changes: 46 additions & 0 deletions src/hooks/useFeatureToggle.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { useConfig } from '@dhis2/app-runtime'
import { renderHook } from '@testing-library/react-hooks'
import { useFeatureToggle } from './useFeatureToggle.js'

jest.mock('@dhis2/app-runtime', () => ({
...jest.requireActual('@dhis2/app-runtime'),
useConfig: jest.fn(),
}))

describe('useFeatureToggle', () => {
beforeEach(() => {
jest.clearAllMocks()
})
it('has displayEmailVerifiedStatus:false if email is not configured', () => {
useConfig.mockReturnValue({
serverVersion: { minor: '42' },
systemInfo: { emailConfigured: false },
})
const { result } = renderHook(() => useFeatureToggle())
expect(result.current.displayEmailVerifiedStatus).toBe(false)
})

it('has displayEmailVerifiedStatus:false if api version is 41 or earlier ', () => {
useConfig.mockReturnValue({
serverVersion: { minor: '41' },
systemInfo: { emailConfigured: true },
})
const { result } = renderHook(() => useFeatureToggle())
expect(result.current.displayEmailVerifiedStatus).toBe(false)
})

it('has displayEmailVerifiedStatus:false if api version is 42 or greater and email is configured ', () => {
useConfig.mockReturnValue({
serverVersion: { minor: '42' },
systemInfo: { emailConfigured: true },
})
const { result } = renderHook(() => useFeatureToggle())
expect(result.current.displayEmailVerifiedStatus).toBe(true)
})

it('has displayEmailVerifiedStatus:false if config is missing information', () => {
useConfig.mockReturnValue({})
const { result } = renderHook(() => useFeatureToggle())
expect(result.current.displayEmailVerifiedStatus).toBe(false)
})
})
30 changes: 29 additions & 1 deletion src/pages/UserList/Filters.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,10 @@ const Filters = ({
onSelfRegisteredChange,
organisationUnits,
onOrganisationUnitsChange,
emailVerificationStatus,
onEmailVerificationStatusChange,
onClear,
displayEmailVerifiedStatus,
}) => (
<div className={styles.container}>
<InputField
Expand All @@ -79,6 +82,27 @@ const Filters = ({
inactiveMonths={inactiveMonths}
onInactiveMonthsChange={onInactiveMonthsChange}
/>
{displayEmailVerifiedStatus && (
<SingleSelectField
prefix={i18n.t('Email verification')}
selected={emailVerificationStatus}
onChange={({ selected }) =>
onEmailVerificationStatusChange(selected)
}
className={styles.input}
dense
>
<SingleSelectOption label={i18n.t('All')} value="all" />
<SingleSelectOption
label={i18n.t('Email verified')}
value="true"
/>
<SingleSelectOption
label={i18n.t('Email not verified')}
value="false"
/>
</SingleSelectField>
)}
<SingleSelectField
prefix={i18n.t('Invitation')}
selected={invitationStatus}
Expand Down Expand Up @@ -106,7 +130,8 @@ const Filters = ({
!inactiveMonths &&
!invitationStatus &&
!selfRegistered &&
organisationUnits.length === 0,
organisationUnits.length === 0 &&
!emailVerificationStatus,
})}
small
onClick={onClear}
Expand All @@ -117,13 +142,16 @@ const Filters = ({
)

Filters.propTypes = {
displayEmailVerifiedStatus: PropTypes.bool.isRequired,
organisationUnits: PropTypes.array.isRequired,
onClear: PropTypes.func.isRequired,
onEmailVerificationStatusChange: PropTypes.func.isRequired,
onInactiveMonthsChange: PropTypes.func.isRequired,
onInvitationStatusChange: PropTypes.func.isRequired,
onOrganisationUnitsChange: PropTypes.func.isRequired,
onQueryChange: PropTypes.func.isRequired,
onSelfRegisteredChange: PropTypes.func.isRequired,
emailVerificationStatus: PropTypes.string,
inactiveMonths: PropTypes.string,
invitationStatus: PropTypes.string,
query: PropTypes.string,
Expand Down
55 changes: 49 additions & 6 deletions src/pages/UserList/UserList.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,42 @@ import {
import React, { useState, useEffect } from 'react'
import { useDebounce } from 'use-debounce'
import PageHeader from '../../components/PageHeader.js'
import { useFeatureToggle } from '../../hooks/useFeatureToggle.js'
import navigateTo from '../../utils/navigateTo.js'
import Filters from './Filters.js'
import { useFilters } from './useFilters.js'
import styles from './UserList.module.css'
import UserTable from './UserTable.js'

const getUsersQueryFilters = ({
organisationUnits,
emailVerificationStatus,
displayEmailVerifiedStatus,
}) => {
const organisationUnitsFilter =
organisationUnits.length > 0
? `organisationUnits.id:in:[${organisationUnits.map(
({ id }) => id
)}]`
: undefined
// email verification filter is ignored if feature is not accessible
Chisomchima marked this conversation as resolved.
Show resolved Hide resolved
const emailVerificationFilter =
!displayEmailVerifiedStatus ||
!['true', 'false'].includes(emailVerificationStatus)
? undefined
: `emailVerified:eq:${emailVerificationStatus}`

if (
organisationUnitsFilter === undefined &&
emailVerificationFilter === undefined
) {
return undefined
}
return [organisationUnitsFilter, emailVerificationFilter]
.filter((f) => f)
.join(',')
}

const usersQuery = {
users: {
resource: 'users',
Expand All @@ -28,12 +58,15 @@ const usersQuery = {
selfRegistered,
nameSortDirection,
organisationUnits,
emailVerificationStatus,
displayEmailVerifiedStatus,
}) => ({
fields: [
'id',
'displayName',
'access',
'email',
'emailVerified',
Chisomchima marked this conversation as resolved.
Show resolved Hide resolved
'twoFactorEnabled',
'username',
'disabled',
Expand All @@ -52,12 +85,11 @@ const usersQuery = {
inactiveMonths,
invitationStatus,
selfRegistered,
filter:
organisationUnits.length > 0
? `organisationUnits.id:in:[${organisationUnits.map(
({ id }) => id
)}]`
: undefined,
filter: getUsersQueryFilters({
organisationUnits,
emailVerificationStatus,
displayEmailVerifiedStatus,
}),
}),
},
}
Expand Down Expand Up @@ -85,9 +117,12 @@ const UserList = () => {
toggleNameSortDirection,
organisationUnits,
setOrganisationUnits,
emailVerificationStatus,
setEmailVerificationStatus,
clearFilters,
} = useFilters()
const [debouncedQuery] = useDebounce(query, 375)
const { displayEmailVerifiedStatus } = useFeatureToggle()
const refetchUsers = () => {
setPrevUsers(users)
refetch({
Expand All @@ -99,6 +134,8 @@ const UserList = () => {
selfRegistered,
nameSortDirection,
organisationUnits,
emailVerificationStatus,
displayEmailVerifiedStatus,
})
}

Expand All @@ -113,6 +150,8 @@ const UserList = () => {
selfRegistered,
nameSortDirection,
JSON.stringify(organisationUnits),
emailVerificationStatus,
displayEmailVerifiedStatus,
])

return (
Expand All @@ -129,7 +168,10 @@ const UserList = () => {
onSelfRegisteredChange={setSelfRegistered}
organisationUnits={organisationUnits}
onOrganisationUnitsChange={setOrganisationUnits}
emailVerificationStatus={emailVerificationStatus}
onEmailVerificationStatusChange={setEmailVerificationStatus}
onClear={clearFilters}
displayEmailVerifiedStatus={displayEmailVerifiedStatus}
/>
<div className={styles.container}>
<DataTableToolbar>
Expand All @@ -148,6 +190,7 @@ const UserList = () => {
refetch={refetchUsers}
nameSortDirection={nameSortDirection}
onNameSortDirectionToggle={toggleNameSortDirection}
displayEmailVerifiedStatus={displayEmailVerifiedStatus}
/>
{(loading
? prevUsers?.users.length > 0
Expand Down
Loading
Loading