Skip to content

Commit

Permalink
fix(approval-status-tag): show custom tag text for unauthorized users (
Browse files Browse the repository at this point in the history
…#106)

* fix(auth): distinguish between hasAppAccess and hasApprovalAuthorities

* fix(approval-status-tag): show custom tag text for unauthorized users
  • Loading branch information
HendrikThePendric authored Oct 4, 2021
1 parent 40b4851 commit bb2a483
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 14 deletions.
7 changes: 5 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: 2021-10-04T09:20:46.071Z\n"
"PO-Revision-Date: 2021-10-04T09:20:46.071Z\n"
"POT-Creation-Date: 2021-10-04T12:25:48.548Z\n"
"PO-Revision-Date: 2021-10-04T12:25:48.548Z\n"

msgid "Not authorized"
msgstr "Not authorized"
Expand Down Expand Up @@ -140,6 +140,9 @@ msgstr "Approved by {{- name}} {{timeAgo}}"
msgid "Approved at higher level {{timeAgo}}"
msgstr "Approved at higher level {{timeAgo}}"

msgid "You do not have authority to approve data"
msgstr "You do not have authority to approve data"

msgid "Cannot be approved"
msgstr "Cannot be approved"

Expand Down
4 changes: 2 additions & 2 deletions src/auth/auth-wall.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import { ErrorMessage } from '../shared/index.js'
import { useIsAuthorized } from './use-is-authorized.js'

const AuthWall = ({ children }) => {
const isAuthorized = useIsAuthorized()
const { hasAppAccess } = useIsAuthorized()

if (!isAuthorized) {
if (!hasAppAccess) {
return (
<ErrorMessage title={i18n.t('Not authorized')}>
{i18n.t(
Expand Down
13 changes: 11 additions & 2 deletions src/auth/auth-wall.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,24 @@ afterEach(() => {

describe('<AuthWall>', () => {
it('shows a noticebox for unauthorized users', () => {
useIsAuthorized.mockImplementation(() => false)
useIsAuthorized.mockImplementation(() => ({
hasAppAccess: false,
hasApprovalAuthorities: false,
}))

const wrapper = shallow(<AuthWall>Child</AuthWall>)

expect(wrapper.find(ErrorMessage)).toHaveLength(1)
expect(wrapper.prop('children')).toBe(
"You don't have access to the Data Approval App. Contact a system administrator to request access."
)
})

it('renders the children for authorised users', () => {
useIsAuthorized.mockImplementation(() => true)
useIsAuthorized.mockImplementation(() => ({
hasAppAccess: true,
hasApprovalAuthorities: true,
}))

const wrapper = shallow(<AuthWall>Child</AuthWall>)

Expand Down
12 changes: 11 additions & 1 deletion src/auth/use-is-authorized.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,17 @@ import { useAppContext } from '../app-context/index.js'

export const useIsAuthorized = () => {
const { authorities } = useAppContext()
return authorities.some(
const hasAppAccess = authorities.some(
authority => authority === 'ALL' || authority === 'M_dhis-web-approval'
)

const hasApprovalAuthorities = authorities.some(
authority =>
authority === 'ALL' ||
authority === 'F_APPROVE_DATA' ||
authority === 'F_APPROVE_DATA_LOWER_LEVELS' ||
authority === 'F_ACCEPT_DATA_LOWER_LEVELS'
)

return { hasAppAccess, hasApprovalAuthorities }
}
72 changes: 66 additions & 6 deletions src/auth/use-is-authorized.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { AppContext } from '../app-context/index.js'
import { useIsAuthorized } from './use-is-authorized.js'

describe('useIsAuthorized', () => {
it('returns false for unauthorised users', () => {
it('returns the correct object for unauthorised users', () => {
const value = {
authorities: ['dummy'],
}
Expand All @@ -15,10 +15,13 @@ describe('useIsAuthorized', () => {

const { result } = renderHook(() => useIsAuthorized(), { wrapper })

expect(result.current).toEqual(false)
expect(result.current).toEqual({
hasAppAccess: false,
hasApprovalAuthorities: false,
})
})

it('returns true for authorised users', () => {
it('returns the correct object for authorised users', () => {
const value = {
authorities: ['M_dhis-web-approval'],
}
Expand All @@ -29,10 +32,64 @@ describe('useIsAuthorized', () => {

const { result } = renderHook(() => useIsAuthorized(), { wrapper })

expect(result.current).toEqual(true)
expect(result.current).toEqual({
hasAppAccess: true,
hasApprovalAuthorities: false,
})
})

it('returns true for superusers', () => {
it('returns the correct object for authorised users with F_APPROVE_DATA authority', () => {
const value = {
authorities: ['M_dhis-web-approval', 'F_APPROVE_DATA'],
}

const wrapper = ({ children }) => (
<AppContext.Provider value={value}>{children}</AppContext.Provider>
)

const { result } = renderHook(() => useIsAuthorized(), { wrapper })

expect(result.current).toEqual({
hasAppAccess: true,
hasApprovalAuthorities: true,
})
})

it('returns the correct object for authorised users with F_APPROVE_DATA_LOWER_LEVELS authority', () => {
const value = {
authorities: ['M_dhis-web-approval', 'F_APPROVE_DATA_LOWER_LEVELS'],
}

const wrapper = ({ children }) => (
<AppContext.Provider value={value}>{children}</AppContext.Provider>
)

const { result } = renderHook(() => useIsAuthorized(), { wrapper })

expect(result.current).toEqual({
hasAppAccess: true,
hasApprovalAuthorities: true,
})
})

it('returns the correct object for authorised users with F_ACCEPT_DATA_LOWER_LEVELS authority', () => {
const value = {
authorities: ['M_dhis-web-approval', 'F_ACCEPT_DATA_LOWER_LEVELS'],
}

const wrapper = ({ children }) => (
<AppContext.Provider value={value}>{children}</AppContext.Provider>
)

const { result } = renderHook(() => useIsAuthorized(), { wrapper })

expect(result.current).toEqual({
hasAppAccess: true,
hasApprovalAuthorities: true,
})
})

it('returns the correct object for superusers', () => {
const value = {
authorities: ['ALL'],
}
Expand All @@ -43,6 +100,9 @@ describe('useIsAuthorized', () => {

const { result } = renderHook(() => useIsAuthorized(), { wrapper })

expect(result.current).toEqual(true)
expect(result.current).toEqual({
hasAppAccess: true,
hasApprovalAuthorities: true,
})
})
})
8 changes: 7 additions & 1 deletion src/bottom-bar/bottom-bar.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react'
import { useIsAuthorized } from '../auth/use-is-authorized.js'
import { ApprovalStatusTag, APPROVAL_STATUSES } from '../shared/index.js'
import { useWorkflowContext } from '../workflow-context/index.js'
import { AcceptButton } from './accept-button/index.js'
Expand All @@ -17,6 +18,7 @@ const approvedStatuses = new Set([
const BottomBar = () => {
const { allowedActions, approvalStatus, approvedBy, approvedAt } =
useWorkflowContext()
const { hasApprovalAuthorities } = useIsAuthorized()
const { mayAccept, mayApprove, mayUnaccept, mayUnapprove } = allowedActions
const disableApproveBtn =
/* We want to signal that the user can't approve or unapprove anything
Expand All @@ -29,7 +31,11 @@ const BottomBar = () => {
<div className={styles.bottomBar} data-test="bottom-bar">
<BottomBarItem>
<ApprovalStatusTag
approvalStatus={approvalStatus}
approvalStatus={
hasApprovalAuthorities
? approvalStatus
: APPROVAL_STATUSES.UNAUTHORIZED
}
approvedBy={approvedBy}
approvedAt={approvedAt}
/>
Expand Down
4 changes: 4 additions & 0 deletions src/shared/approval-status/get-approval-status.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const APPROVAL_STATUSES = {
APPROVED_HERE: 'APPROVED_HERE',
APPROVED_ABOVE: 'APPROVED_ABOVE',
UNAPPROVABLE: 'UNAPPROVABLE',
UNAUTHORIZED: 'UNAUTHORIZED',
LOADING: 'LOADING',
ERROR: 'ERROR',
}
Expand All @@ -36,6 +37,7 @@ const getApprovalStatusIcon = approvalStatus => {
type: 'positive',
}
case APPROVAL_STATUSES.UNAPPROVABLE:
case APPROVAL_STATUSES.UNAUTHORIZED:
return {
icon: IconBlock16,
type: 'negative',
Expand Down Expand Up @@ -76,6 +78,8 @@ const getApprovalStatusText = ({
return i18n.t('Approved at higher level {{timeAgo}}', {
timeAgo: moment(approvalDateTime).fromNow(),
})
case APPROVAL_STATUSES.UNAUTHORIZED:
return i18n.t('You do not have authority to approve data')
case APPROVAL_STATUSES.UNAPPROVABLE:
return i18n.t('Cannot be approved')
case APPROVAL_STATUSES.ERROR:
Expand Down

0 comments on commit bb2a483

Please sign in to comment.