fix: revert pr#102 because the bug was actually expected behavior (#104)

* chore: revert "show message to users with no authority to approve data" This reverts commit 1462f6d. * chore: revert "remove unused import" This reverts commit ad0ffac.
dhis2 · Sep 30, 2021 · 6709bc2 · 6709bc2
1 parent aa8ac30
commit 6709bc2
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 97 deletions.
diff --git a/src/auth/auth-wall.js b/src/auth/auth-wall.js
@@ -5,23 +5,19 @@ import { ErrorMessage } from '../shared/index.js'
 import { useIsAuthorized } from './use-is-authorized.js'
 
 const AuthWall = ({ children }) => {
-    const { hasAppAccess, hasApprovalAuthorities } = useIsAuthorized()
+    const isAuthorized = useIsAuthorized()
 
-    if (hasAppAccess && hasApprovalAuthorities) {
-        return children
+    if (!isAuthorized) {
+        return (
+            <ErrorMessage title={i18n.t('Not authorized')}>
+                {i18n.t(
+                    "You don't have access to the Data Approval App. Contact a system administrator to request access."
+                )}
+            </ErrorMessage>
+        )
     }
 
-    const message = !hasAppAccess
-        ? i18n.t(
-              "You don't have access to the Data Approval App. Contact a system administrator to request access."
-          )
-        : i18n.t(
-              'You are not allowed to approve data. Contact a system administrator to request the appropriate authorities.'
-          )
-
-    return (
-        <ErrorMessage title={i18n.t('Not authorized')}>{message}</ErrorMessage>
-    )
+    return children
 }
 
 AuthWall.propTypes = {

diff --git a/src/auth/auth-wall.test.js b/src/auth/auth-wall.test.js
@@ -1,3 +1,4 @@
+import { NoticeBox } from '@dhis2/ui'
 import { shallow } from 'enzyme'
 import React from 'react'
 import { ErrorMessage } from '../shared/index.js'
@@ -14,38 +15,15 @@ afterEach(() => {
 
 describe('<AuthWall>', () => {
     it('shows a noticebox for unauthorized users', () => {
-        useIsAuthorized.mockImplementation(() => ({
-            hasAppAccess: false,
-            hasApprovalAuthorities: false,
-        }))
+        useIsAuthorized.mockImplementation(() => 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('shows a noticebox for users without appropriate authorities', () => {
-        useIsAuthorized.mockImplementation(() => ({
-            hasAppAccess: true,
-            hasApprovalAuthorities: false,
-        }))
-
-        const wrapper = shallow(<AuthWall>Child</AuthWall>)
-
-        expect(wrapper.find(ErrorMessage)).toHaveLength(1)
-        expect(wrapper.prop('children')).toBe(
-            'You are not allowed to approve data. Contact a system administrator to request the appropriate authorities.'
-        )
     })
 
     it('renders the children for authorised users', () => {
-        useIsAuthorized.mockImplementation(() => ({
-            hasAppAccess: true,
-            hasApprovalAuthorities: true,
-        }))
+        useIsAuthorized.mockImplementation(() => true)
 
         const wrapper = shallow(<AuthWall>Child</AuthWall>)
 

diff --git a/src/auth/use-is-authorized.js b/src/auth/use-is-authorized.js
@@ -2,15 +2,7 @@ import { useAppContext } from '../app-context/index.js'
 
 export const useIsAuthorized = () => {
     const { authorities } = useAppContext()
-    const hasAppAccess = authorities.some(
+    return 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'
-    )
-
-    return { hasAppAccess, hasApprovalAuthorities }
 }
diff --git 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 the correct object for unauthorised users', () => {
+    it('returns false for unauthorised users', () => {
         const value = {
             authorities: ['dummy'],
         }
@@ -15,13 +15,10 @@ describe('useIsAuthorized', () => {
 
         const { result } = renderHook(() => useIsAuthorized(), { wrapper })
 
-        expect(result.current).toEqual({
-            hasAppAccess: false,
-            hasApprovalAuthorities: false,
-        })
+        expect(result.current).toEqual(false)
     })
 
-    it('returns the correct object for authorised users', () => {
+    it('returns true for authorised users', () => {
         const value = {
             authorities: ['M_dhis-web-approval'],
         }
@@ -32,47 +29,10 @@ describe('useIsAuthorized', () => {
 
         const { result } = renderHook(() => useIsAuthorized(), { wrapper })
 
-        expect(result.current).toEqual({
-            hasAppAccess: true,
-            hasApprovalAuthorities: false,
-        })
+        expect(result.current).toEqual(true)
     })
 
-    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 superusers', () => {
+    it('returns true for superusers', () => {
         const value = {
             authorities: ['ALL'],
         }
@@ -83,9 +43,6 @@ describe('useIsAuthorized', () => {
 
         const { result } = renderHook(() => useIsAuthorized(), { wrapper })
 
-        expect(result.current).toEqual({
-            hasAppAccess: true,
-            hasApprovalAuthorities: true,
-        })
+        expect(result.current).toEqual(true)
     })
 })