fix(bottom-bar): disable approve button when it is allowed but pointless

src/bottom-bar/bottom-bar.js

@@ -1,5 +1,5 @@
 import React from 'react'
-import { ApprovalStatusTag } from '../shared/index.js'
+import { ApprovalStatusTag, APPROVAL_STATUSES } from '../shared/index.js'
 import { useWorkflowContext } from '../workflow-context/index.js'
 import { AcceptButton } from './accept-button/index.js'
 import { ApproveButton } from './approve-button/index.js'
@@ -8,11 +8,21 @@ import styles from './bottom-bar.module.css'
 import { UnacceptButton } from './unaccept-button/index.js'
 import { UnapproveButton } from './unapprove-button/index.js'
 
+const approvedStatuses = new Set([
+    APPROVAL_STATUSES.APPROVED_HERE,
+    APPROVAL_STATUSES.APPROVED_ABOVE,
+    APPROVAL_STATUSES.ACCEPTED_HERE,
+])
+
 const BottomBar = () => {
     const { allowedActions, approvalStatus, approvedBy, approvedAt } =
         useWorkflowContext()
     const { mayAccept, mayApprove, mayUnaccept, mayUnapprove } = allowedActions
-    const disableApproveBtn = !mayApprove && !mayUnapprove
+    const disableApproveBtn =
+        /* We want to signal that the user can't approve or unapprove anything 
+           by showing a disabled button rather than an empty space */
+        (!mayApprove && !mayUnapprove) ||
+        (mayApprove && approvedStatuses.has(approvalStatus))
 
     return (
         <>

src/shared/approval-status/approval-status-icon.js

@@ -2,7 +2,10 @@ import { colors } from '@dhis2/ui'
 import PropTypes from 'prop-types'
 import React from 'react'
 import classes from './approval-status-icon.module.css'
-import { getApprovalStatusDisplayData } from './get-approval-status.js'
+import {
+    getApprovalStatusDisplayData,
+    APPROVAL_STATUSES,
+} from './get-approval-status.js'
 
 const getIconColorForType = type => {
     switch (type) {
@@ -39,15 +42,7 @@ ApprovalStatusIcon.defaultProps = {
 }
 
 ApprovalStatusIcon.propTypes = {
-    approvalStatus: PropTypes.oneOf([
-        'APPROVED_HERE',
-        'APPROVED_ABOVE',
-        'ACCEPTED_HERE',
-        'UNAPPROVED_READY',
-        'UNAPPROVED_WAITING',
-        'UNAPPROVED_ABOVE',
-        'UNAPPROVABLE',
-    ]),
+    approvalStatus: PropTypes.oneOf(Object.values(APPROVAL_STATUSES)),
     showTitle: PropTypes.bool,
 }
 

src/shared/approval-status/approval-status-tag.js

@@ -6,6 +6,7 @@ import React from 'react'
 import {
     getApprovalStatusDisplayData,
     isApproved,
+    APPROVAL_STATUSES,
 } from './get-approval-status.js'
 import { useServerDateTimeAsLocal } from './use-server-date-time-as-local.js'
 
@@ -42,15 +43,7 @@ const ApprovalStatusTag = ({ approvalStatus, approvedAt, approvedBy }) => {
 }
 
 ApprovalStatusTag.propTypes = {
-    approvalStatus: PropTypes.oneOf([
-        'APPROVED_HERE',
-        'APPROVED_ABOVE',
-        'ACCEPTED_HERE',
-        'UNAPPROVED_READY',
-        'UNAPPROVED_WAITING',
-        'UNAPPROVED_ABOVE',
-        'UNAPPROVABLE',
-    ]),
+    approvalStatus: PropTypes.oneOf(Object.values(APPROVAL_STATUSES)),
     approvedAt: PropTypes.string,
     approvedBy: PropTypes.string,
 }

src/shared/approval-status/get-approval-status.js

@@ -3,32 +3,44 @@ import { IconBlock16, IconError16 } from '@dhis2/ui'
 import moment from 'moment'
 import { Approved, Ready, Waiting } from './icons.js'
 
+const APPROVAL_STATUSES = {
+    UNAPPROVED_READY: 'UNAPPROVED_READY',
+    ACCEPTED_HERE: 'ACCEPTED_HERE',
+    UNAPPROVED_WAITING: 'UNAPPROVED_WAITING',
+    UNAPPROVED_ABOVE: 'UNAPPROVED_ABOVE',
+    APPROVED_HERE: 'APPROVED_HERE',
+    APPROVED_ABOVE: 'APPROVED_ABOVE',
+    UNAPPROVABLE: 'UNAPPROVABLE',
+    LOADING: 'LOADING',
+    ERROR: 'ERROR',
+}
+
 const getApprovalStatusIcon = approvalStatus => {
     switch (approvalStatus) {
-        case 'UNAPPROVED_READY':
-        case 'ACCEPTED_HERE':
+        case APPROVAL_STATUSES.UNAPPROVED_READY:
+        case APPROVAL_STATUSES.ACCEPTED_HERE:
             return {
                 icon: Ready,
                 type: 'neutral',
             }
-        case 'UNAPPROVED_WAITING':
-        case 'UNAPPROVED_ABOVE':
+        case APPROVAL_STATUSES.UNAPPROVED_WAITING:
+        case APPROVAL_STATUSES.UNAPPROVED_ABOVE:
             return {
                 icon: Waiting,
                 type: 'default',
             }
-        case 'APPROVED_HERE':
-        case 'APPROVED_ABOVE':
+        case APPROVAL_STATUSES.APPROVED_HERE:
+        case APPROVAL_STATUSES.APPROVED_ABOVE:
             return {
                 icon: Approved,
                 type: 'positive',
             }
-        case 'UNAPPROVABLE':
+        case APPROVAL_STATUSES.UNAPPROVABLE:
             return {
                 icon: IconBlock16,
                 type: 'negative',
             }
-        case 'ERROR':
+        case APPROVAL_STATUSES.ERROR:
             return {
                 icon: IconError16,
                 type: 'negative',
@@ -58,28 +70,29 @@ const getApprovalStatusText = ({
     approvedBy,
 }) => {
     switch (approvalStatus) {
-        case 'UNAPPROVED_READY':
+        case APPROVAL_STATUSES.UNAPPROVED_READY:
             return i18n.t('Ready for approval')
-        case 'ACCEPTED_HERE':
+        case APPROVAL_STATUSES.ACCEPTED_HERE:
             return i18n.t('Ready for approval — Accepted')
-        case 'UNAPPROVED_WAITING':
+        case APPROVAL_STATUSES.UNAPPROVED_WAITING:
             return i18n.t('Waiting for lower level approval')
-        case 'UNAPPROVED_ABOVE':
+        case APPROVAL_STATUSES.UNAPPROVED_ABOVE:
             return i18n.t('Waiting for higher level approval')
-        case 'APPROVED_HERE':
-        case 'APPROVED_ABOVE':
+        case APPROVAL_STATUSES.APPROVED_HERE:
+        case APPROVAL_STATUSES.APPROVED_ABOVE:
             return getApprovedStatusText({ approvalDateTime, approvedBy })
-        case 'UNAPPROVABLE':
+        case APPROVAL_STATUSES.UNAPPROVABLE:
             return i18n.t('Cannot be approved')
-        case 'ERROR':
+        case APPROVAL_STATUSES.ERROR:
             return i18n.t('Could not retrieve approval status')
         default:
             throw new Error(`Unknown approval status: '${approvalStatus}'`)
     }
 }
 
 const isApproved = approvalStatus =>
-    approvalStatus === 'APPROVED_HERE' || approvalStatus === 'APPROVED_ABOVE'
+    approvalStatus === APPROVAL_STATUSES.APPROVED_HERE ||
+    approvalStatus === APPROVAL_STATUSES.APPROVED_ABOVE
 
 const getApprovalStatusDisplayData = ({
     approvalStatus,
@@ -96,4 +109,4 @@ const getApprovalStatusDisplayData = ({
     return { displayName, icon, type }
 }
 
-export { getApprovalStatusDisplayData, isApproved }
+export { APPROVAL_STATUSES, getApprovalStatusDisplayData, isApproved }

src/shared/approval-status/get-approval-status.test.js

@@ -1,6 +1,9 @@
 import { IconBlock16, IconError16 } from '@dhis2/ui'
 import moment from 'moment'
-import { getApprovalStatusDisplayData } from './get-approval-status.js'
+import {
+    getApprovalStatusDisplayData,
+    APPROVAL_STATUSES,
+} from './get-approval-status.js'
 import { Approved, Ready, Waiting } from './icons.js'
 
 jest.mock('moment', () => {
@@ -10,7 +13,9 @@ jest.mock('moment', () => {
 describe('getApprovalStatusDisplayData', () => {
     it('returns the correct display data for approval status "UNAPPROVED_READY"', () => {
         expect(
-            getApprovalStatusDisplayData({ approvalStatus: 'UNAPPROVED_READY' })
+            getApprovalStatusDisplayData({
+                approvalStatus: APPROVAL_STATUSES.UNAPPROVED_READY,
+            })
         ).toEqual({
             displayName: 'Ready for approval',
             icon: Ready,
@@ -19,7 +24,9 @@ describe('getApprovalStatusDisplayData', () => {
     })
     it('returns the correct display data for approval status "ACCEPTED_HERE"', () => {
         expect(
-            getApprovalStatusDisplayData({ approvalStatus: 'ACCEPTED_HERE' })
+            getApprovalStatusDisplayData({
+                approvalStatus: APPROVAL_STATUSES.ACCEPTED_HERE,
+            })
         ).toEqual({
             displayName: 'Ready for approval — Accepted',
             icon: Ready,
@@ -29,7 +36,7 @@ describe('getApprovalStatusDisplayData', () => {
     it('returns the correct display data for approval status "UNAPPROVED_WAITING"', () => {
         expect(
             getApprovalStatusDisplayData({
-                approvalStatus: 'UNAPPROVED_WAITING',
+                approvalStatus: APPROVAL_STATUSES.UNAPPROVED_WAITING,
             })
         ).toEqual({
             displayName: 'Waiting for lower level approval',
@@ -39,15 +46,20 @@ describe('getApprovalStatusDisplayData', () => {
     })
     it('returns the correct display data for approval status "UNAPPROVED_ABOVE"', () => {
         expect(
-            getApprovalStatusDisplayData({ approvalStatus: 'UNAPPROVED_ABOVE' })
+            getApprovalStatusDisplayData({
+                approvalStatus: APPROVAL_STATUSES.UNAPPROVED_ABOVE,
+            })
         ).toEqual({
             displayName: 'Waiting for higher level approval',
             icon: Waiting,
             type: 'default',
         })
     })
     describe('approved approval statuses "APPROVED_HERE" and "APPROVED_ABOVE"', () => {
-        for (const approvalStatus of ['APPROVED_HERE', 'APPROVED_ABOVE']) {
+        for (const approvalStatus of [
+            APPROVAL_STATUSES.APPROVED_HERE,
+            APPROVAL_STATUSES.APPROVED_ABOVE,
+        ]) {
             it(`returns the correct diplay data for ${approvalStatus} when only approvalStatus is supplied`, () => {
                 expect(
                     getApprovalStatusDisplayData({ approvalStatus })
@@ -98,7 +110,9 @@ describe('getApprovalStatusDisplayData', () => {
     })
     it('returns the correct display data for approval status "UNAPPROVABLE"', () => {
         expect(
-            getApprovalStatusDisplayData({ approvalStatus: 'UNAPPROVABLE' })
+            getApprovalStatusDisplayData({
+                approvalStatus: APPROVAL_STATUSES.UNAPPROVABLE,
+            })
         ).toEqual({
             displayName: 'Cannot be approved',
             icon: IconBlock16,
@@ -107,7 +121,9 @@ describe('getApprovalStatusDisplayData', () => {
     })
     it('returns the correct display data for approval status "ERROR"', () => {
         expect(
-            getApprovalStatusDisplayData({ approvalStatus: 'ERROR' })
+            getApprovalStatusDisplayData({
+                approvalStatus: APPROVAL_STATUSES.ERROR,
+            })
         ).toEqual({
             displayName: 'Could not retrieve approval status',
             icon: IconError16,

src/shared/approval-status/index.js

@@ -1,3 +1,6 @@
 export { ApprovalStatusTag } from './approval-status-tag.js'
 export { ApprovalStatusIcon } from './approval-status-icon.js'
-export { getApprovalStatusDisplayData } from './get-approval-status.js'
+export {
+    getApprovalStatusDisplayData,
+    APPROVAL_STATUSES,
+} from './get-approval-status.js'

src/top-bar/org-unit-select/approval-status-icons-legend.js

@@ -1,17 +1,29 @@
 import i18n from '@dhis2/d2-i18n'
 import React from 'react'
-import { ApprovalStatusIcon } from '../../shared/approval-status/index.js'
+import {
+    ApprovalStatusIcon,
+    APPROVAL_STATUSES,
+} from '../../shared/approval-status/index.js'
 import classes from './approval-status-icons-legend.module.css'
 
 // Not all approval statuses are defined here as some share the same icons
 const approvalStatuses = [
     {
-        status: 'UNAPPROVED_WAITING',
+        status: APPROVAL_STATUSES.UNAPPROVED_WAITING,
         displayName: i18n.t('Waiting for approval'),
     },
-    { status: 'UNAPPROVED_READY', displayName: i18n.t('Ready for approval') },
-    { status: 'APPROVED_HERE', displayName: i18n.t('Approved') },
-    { status: 'UNAPPROVABLE', displayName: i18n.t('Cannot be approved') },
+    {
+        status: APPROVAL_STATUSES.UNAPPROVED_READY,
+        displayName: i18n.t('Ready for approval'),
+    },
+    {
+        status: APPROVAL_STATUSES.APPROVED_HERE,
+        displayName: i18n.t('Approved'),
+    },
+    {
+        status: APPROVAL_STATUSES.UNAPPROVABLE,
+        displayName: i18n.t('Cannot be approved'),
+    },
 ]
 
 const ApprovalStatusIconsLegend = () => (

src/top-bar/org-unit-select/approval-status-label.js

@@ -3,14 +3,14 @@ import { IconWarning16, colors, Tooltip } from '@dhis2/ui'
 import PropTypes from 'prop-types'
 import React, { useEffect } from 'react'
 import { useSelectionContext } from '../../selection-context/index.js'
-import { ApprovalStatusIcon } from '../../shared/approval-status/index.js'
+import { ApprovalStatusIcon, APPROVAL_STATUSES } from '../../shared/index.js'
 import classes from './approval-status-label.module.css'
 import { useApprovalStatus } from './approval-statuses.js'
 
 const renderIcon = approvalStatus => {
-    if (approvalStatus === 'LOADING') {
+    if (approvalStatus === APPROVAL_STATUSES.LOADING) {
         return <span className={classes.loadingIcon}></span>
-    } else if (approvalStatus === 'FETCH_ERROR') {
+    } else if (approvalStatus === APPROVAL_STATUSES.ERROR) {
         return (
             <Tooltip content={i18n.t('Failed to load approval state')}>
                 {({ onMouseOver, onMouseOut, ref }) => (

src/top-bar/org-unit-select/approval-statuses.js

@@ -2,6 +2,7 @@ import { useDataEngine } from '@dhis2/app-runtime'
 import PropTypes from 'prop-types'
 import React, { createContext, useContext, useRef, useState } from 'react'
 import { useDebouncedCallback } from 'use-debounce'
+import { APPROVAL_STATUSES } from '../../shared/index.js'
 
 const ApprovalStatusesContext = createContext()
 
@@ -61,7 +62,7 @@ const useFetchApprovalStatus = ({ updateApprovalStatuses }) => {
                 workflowId,
                 approvalStatusUpdates: orgUnitIds.reduce(
                     (statuses, orgUnitId) => {
-                        statuses[orgUnitId] = 'LOADING'
+                        statuses[orgUnitId] = APPROVAL_STATUSES.LOADING
                         return statuses
                     },
                     {}
@@ -81,11 +82,11 @@ const useFetchApprovalStatus = ({ updateApprovalStatuses }) => {
                     },
                 })
                 approvalStatuses.forEach(({ ou, state }) => {
-                    updateObject[ou] = state || 'UNAPPROVABLE'
+                    updateObject[ou] = state || APPROVAL_STATUSES.UNAPPROVABLE
                 })
             } catch (error) {
                 orgUnitIds.forEach(orgUnitId => {
-                    updateObject[orgUnitId] = 'FETCH_ERROR'
+                    updateObject[orgUnitId] = APPROVAL_STATUSES.ERROR
                 })
             }
             updateApprovalStatuses({