From bb2a4839daf0d7608a6bc1e0b1c4c756ffa86ac6 Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Mon, 4 Oct 2021 14:47:22 +0200 Subject: [PATCH] fix(approval-status-tag): show custom tag text for unauthorized users (#106) * fix(auth): distinguish between hasAppAccess and hasApprovalAuthorities * fix(approval-status-tag): show custom tag text for unauthorized users --- i18n/en.pot | 7 +- src/auth/auth-wall.js | 4 +- src/auth/auth-wall.test.js | 13 +++- src/auth/use-is-authorized.js | 12 +++- src/auth/use-is-authorized.test.js | 72 +++++++++++++++++-- src/bottom-bar/bottom-bar.js | 8 ++- .../approval-status/get-approval-status.js | 4 ++ 7 files changed, 106 insertions(+), 14 deletions(-) diff --git a/i18n/en.pot b/i18n/en.pot index f945a4b5..efae4a98 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: 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" @@ -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" diff --git a/src/auth/auth-wall.js b/src/auth/auth-wall.js index 5c33d6da..1ab26663 100644 --- a/src/auth/auth-wall.js +++ b/src/auth/auth-wall.js @@ -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 ( {i18n.t( diff --git a/src/auth/auth-wall.test.js b/src/auth/auth-wall.test.js index dbf2c458..e8006f71 100644 --- a/src/auth/auth-wall.test.js +++ b/src/auth/auth-wall.test.js @@ -15,15 +15,24 @@ afterEach(() => { describe('', () => { it('shows a noticebox for unauthorized users', () => { - useIsAuthorized.mockImplementation(() => false) + useIsAuthorized.mockImplementation(() => ({ + hasAppAccess: false, + hasApprovalAuthorities: false, + })) const wrapper = shallow(Child) 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(Child) diff --git a/src/auth/use-is-authorized.js b/src/auth/use-is-authorized.js index c55b44ed..518de0cc 100644 --- a/src/auth/use-is-authorized.js +++ b/src/auth/use-is-authorized.js @@ -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 } } diff --git a/src/auth/use-is-authorized.test.js b/src/auth/use-is-authorized.test.js index dfee27b3..32560aca 100644 --- a/src/auth/use-is-authorized.test.js +++ b/src/auth/use-is-authorized.test.js @@ -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'], } @@ -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'], } @@ -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 }) => ( + {children} + ) + + 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 }) => ( + {children} + ) + + 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 }) => ( + {children} + ) + + const { result } = renderHook(() => useIsAuthorized(), { wrapper }) + + expect(result.current).toEqual({ + hasAppAccess: true, + hasApprovalAuthorities: true, + }) + }) + + it('returns the correct object for superusers', () => { const value = { authorities: ['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, + }) }) }) diff --git a/src/bottom-bar/bottom-bar.js b/src/bottom-bar/bottom-bar.js index f0716f5c..57ff29e4 100644 --- a/src/bottom-bar/bottom-bar.js +++ b/src/bottom-bar/bottom-bar.js @@ -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' @@ -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 @@ -29,7 +31,11 @@ const BottomBar = () => {
diff --git a/src/shared/approval-status/get-approval-status.js b/src/shared/approval-status/get-approval-status.js index 5bef8ce3..76febf04 100644 --- a/src/shared/approval-status/get-approval-status.js +++ b/src/shared/approval-status/get-approval-status.js @@ -11,6 +11,7 @@ const APPROVAL_STATUSES = { APPROVED_HERE: 'APPROVED_HERE', APPROVED_ABOVE: 'APPROVED_ABOVE', UNAPPROVABLE: 'UNAPPROVABLE', + UNAUTHORIZED: 'UNAUTHORIZED', LOADING: 'LOADING', ERROR: 'ERROR', } @@ -36,6 +37,7 @@ const getApprovalStatusIcon = approvalStatus => { type: 'positive', } case APPROVAL_STATUSES.UNAPPROVABLE: + case APPROVAL_STATUSES.UNAUTHORIZED: return { icon: IconBlock16, type: 'negative', @@ -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: