From 3654c8ca09cdd47e3baa5239979d11d928f62346 Mon Sep 17 00:00:00 2001 From: Thomas Zemp Date: Fri, 6 Sep 2024 17:16:50 +0200 Subject: [PATCH] fix: exclude data non-readable exchanges from dropdown [DHIS2-18010] --- .../view/data-workspace/data-workspace.js | 8 +- .../exchange-select/exchange-select.js | 11 +-- src/context/app-context/app-provider.js | 11 +++ src/pages/data.test.js | 96 ++++++++++++++++--- src/utils/builders.js | 18 ++-- 5 files changed, 116 insertions(+), 28 deletions(-) diff --git a/src/components/view/data-workspace/data-workspace.js b/src/components/view/data-workspace/data-workspace.js index f6a547d..0338cbd 100644 --- a/src/components/view/data-workspace/data-workspace.js +++ b/src/components/view/data-workspace/data-workspace.js @@ -13,7 +13,7 @@ import { RequestsNavigation } from './requests-navigation/index.js' import { TitleBar } from './title-bar/title-bar.js' const DataWorkspace = () => { - const { aggregateDataExchanges } = useAppContext() + const { readableExchangeOptions } = useAppContext() const { exchange } = useExchangeContext() const [exchangeId] = useExchangeId() // memoize for stable reference? @@ -33,10 +33,12 @@ const DataWorkspace = () => { } }, [exchangeId, requests, selectedRequest, setSelectedRequest]) - if (aggregateDataExchanges.length === 0) { + if (readableExchangeOptions.length === 0) { return ( - {i18n.t('There are no exchanges available to you')} + + {i18n.t('There are no exchanges available to you')} + ) } diff --git a/src/components/view/top-bar/exchange-select/exchange-select.js b/src/components/view/top-bar/exchange-select/exchange-select.js index 8b21afa..ce3a176 100644 --- a/src/components/view/top-bar/exchange-select/exchange-select.js +++ b/src/components/view/top-bar/exchange-select/exchange-select.js @@ -6,21 +6,16 @@ import { useExchangeId } from '../../../../use-context-selection/use-context-sel import { MenuSelect } from '../menu-select/index.js' const ExchangeSelect = () => { - const { aggregateDataExchanges } = useAppContext() + const { readableExchangeOptions } = useAppContext() const [exchangeId, setExchangeId] = useExchangeId() const [exchangeSelectorOpen, setExchangeSelectorOpen] = useState(false) - const dataExchangeOptions = aggregateDataExchanges.map((exchange) => ({ - value: exchange.id, - label: exchange.displayName, - })) - return (
dExchange.value === exchangeId )?.label } @@ -30,7 +25,7 @@ const ExchangeSelect = () => { >
{ diff --git a/src/context/app-context/app-provider.js b/src/context/app-context/app-provider.js index f02ab22..c404e96 100644 --- a/src/context/app-context/app-provider.js +++ b/src/context/app-context/app-provider.js @@ -25,6 +25,14 @@ const query = { }, } +export const getReadableExchangeOptions = (aggregateDataExchanges) => + aggregateDataExchanges + .filter((exchange) => exchange?.access?.data?.read !== false) + .map((exchange) => ({ + value: exchange.id, + label: exchange.displayName, + })) + const AppProvider = ({ children }) => { const { data, @@ -61,6 +69,9 @@ const AppProvider = ({ children }) => { const providerValue = { aggregateDataExchanges, + readableExchangeOptions: getReadableExchangeOptions( + aggregateDataExchanges + ), refetchExchanges, } diff --git a/src/pages/data.test.js b/src/pages/data.test.js index 0b7908c..b6496ed 100644 --- a/src/pages/data.test.js +++ b/src/pages/data.test.js @@ -8,6 +8,7 @@ import { ReactRouter6Adapter } from 'use-query-params/adapters/react-router-6' import { formatData } from '../components/view/data-workspace/requests-display/index.js' import { getRelativeTimeDifference } from '../components/view/data-workspace/title-bar/index.js' import { getReportText } from '../components/view/submit-modal/submit-modal.js' +import { getReadableExchangeOptions } from '../context/app-context/app-provider.js' import { AppContext, UserContext } from '../context/index.js' import { addOnlyPermissionsUserContext, @@ -48,7 +49,11 @@ jest.mock('@dhis2/app-runtime', () => ({ const setUp = ( ui, { - aggregateDataExchanges = [testDataExchange(), testDataExchange()], + aggregateDataExchanges = [ + testDataExchange({ readDataAccess: true }), + testDataExchange({ readDataAccess: true }), + testDataExchange({ readDataAccess: false }), + ], exchangeId = null, exchangeData = [testDataExchangeSourceData()], importSummaryResponse = { importSummaries: [testImportSummary()] }, @@ -82,6 +87,9 @@ const setUp = ( {}, }} > @@ -105,7 +113,48 @@ beforeEach(() => { describe('', () => { it('should display a drop down to select an exchange', async () => { - const exchanges = [testDataExchange(), testDataExchange()] + const exchanges = [ + testDataExchange({ readDataAccess: true }), + testDataExchange({ readDataAccess: true }), + ] + const { screen } = setUp(, { + aggregateDataExchanges: exchanges, + }) + + screen.getByTestId('dhis2-ui-selectorbaritem').click() + const menuItems = await screen.findAllByTestId('dhis2-uicore-menuitem') + expect(menuItems).toHaveLength(2) + menuItems.map((menuItem, i) => { + expect(menuItem).toHaveTextContent(exchanges[i].displayName) + }) + }) + + it('should only display data readable exchanges for selection', async () => { + const exchanges = [ + testDataExchange({ readDataAccess: true }), + testDataExchange({ readDataAccess: false }), + testDataExchange({ readDataAccess: true }), + ] + const readableExchanges = exchanges.filter( + (exchange) => exchange.access.data.read + ) + const { screen } = setUp(, { + aggregateDataExchanges: exchanges, + }) + + screen.getByTestId('dhis2-ui-selectorbaritem').click() + const menuItems = await screen.findAllByTestId('dhis2-uicore-menuitem') + expect(menuItems).toHaveLength(2) + menuItems.map((menuItem, i) => { + expect(menuItem).toHaveTextContent(readableExchanges[i].displayName) + }) + }) + + it('should include exchanges with undefined data access for selection', async () => { + const exchanges = [ + testDataExchange({ includeDataAccess: false }), + testDataExchange({ includeDataAccess: false }), + ] const { screen } = setUp(, { aggregateDataExchanges: exchanges, }) @@ -124,6 +173,17 @@ describe('', () => { expect(screen.getByTestId('entry-screen-message')).toBeInTheDocument() }) + it('should display a message about no available exchanges if there are no data readable exchanges', async () => { + const exchanges = [testDataExchange({ readDataAccess: false })] + const { screen } = setUp(, { + aggregateDataExchanges: exchanges, + }) + + expect( + screen.getByTestId('no-exchanges-screen-message') + ).toBeInTheDocument() + }) + it('should show an edit configurations button when the user all permissions', async () => { const { screen } = setUp(, { userContext: allPermissionsUserContext(), @@ -151,8 +211,11 @@ describe('', () => { }) it('should select and clear the selected exchange', async () => { - const anExchange = testDataExchange() - const exchanges = [anExchange, testDataExchange()] + const anExchange = testDataExchange({ readDataAccess: true }) + const exchanges = [ + anExchange, + testDataExchange({ readDataAccess: true }), + ] const { screen } = setUp(, { aggregateDataExchanges: exchanges, }) @@ -178,7 +241,7 @@ describe('', () => { }) it('should show a progress bar when loading content', async () => { - const anExchange = testDataExchange() + const anExchange = testDataExchange({ readDataAccess: true }) const exchanges = [anExchange] const { screen } = setUp(, { @@ -191,7 +254,10 @@ describe('', () => { }) it('should display a warning if there are no requests', async () => { - const anExchange = testDataExchange({ requests: null }) + const anExchange = testDataExchange({ + requests: null, + readDataAccess: true, + }) const exchanges = [anExchange, testDataExchange()] const { screen } = setUp(, { aggregateDataExchanges: exchanges, @@ -204,7 +270,7 @@ describe('', () => { }) it('should display the correct exchange specified in url if the param is present', async () => { - const anExchange = testDataExchange() + const anExchange = testDataExchange({ readDataAccess: true }) const exchanges = [anExchange, testDataExchange()] const { screen } = setUp(, { aggregateDataExchanges: exchanges, @@ -220,7 +286,7 @@ describe('', () => { }) it('should display a preview table once an exchange is selected', async () => { - const anExchange = testDataExchange() + const anExchange = testDataExchange({ readDataAccess: true }) const exchanges = [anExchange] const exchangeData = testDataExchangeSourceData() @@ -272,6 +338,7 @@ describe('', () => { it('can show the correct data once another tab is clicked on', async () => { const anExchange = testDataExchange({ requests: [testRequest(), testRequest()], + readDataAccess: true, }) const exchanges = [anExchange] @@ -309,10 +376,11 @@ describe('', () => { }) it('should show a submit modal for internal exchange when the user clicks on the submit data button', async () => { - const anExchangeRequest = testRequest() + const anExchangeRequest = testRequest({ readDataAccess: true }) const anExchange = testDataExchange({ targetType: 'INTERNAL', requests: [anExchangeRequest], + readDataAccess: true, }) const exchanges = [anExchange] @@ -343,11 +411,12 @@ describe('', () => { it('should show a submit modal for external exchange when the user clicks on the submit data button', async () => { const externalURL = 'a/url' - const anExchangeRequest = testRequest() + const anExchangeRequest = testRequest({ readDataAccess: true }) const anExchange = testDataExchange({ targetType: 'EXTERNAL', externalURL: externalURL, requests: [anExchangeRequest], + readDataAccess: true, }) const exchanges = [anExchange] const exchangeData = testDataExchangeSourceData() @@ -379,6 +448,7 @@ describe('', () => { const importSummaries = [testImportSummary(), testImportSummary()] const anExchange = testDataExchange({ requests: [testRequest(), testRequest()], + readDataAccess: true, }) const exchanges = [anExchange] @@ -465,6 +535,7 @@ describe('', () => { ] const anExchange = testDataExchange({ requests: [testRequest(), testRequest()], + readDataAccess: true, }) const exchanges = [anExchange] @@ -502,7 +573,10 @@ describe('', () => { testImportSummary({ status: 'ERROR' }), ] const anExchange = testDataExchange({ - requests: [testRequest(), testRequest()], + requests: [ + testRequest({ readDataAccess: true }), + testRequest({ readDataAccess: true }), + ], }) const exchanges = [anExchange] diff --git a/src/utils/builders.js b/src/utils/builders.js index 99fb1e0..b3234ac 100644 --- a/src/utils/builders.js +++ b/src/utils/builders.js @@ -22,10 +22,11 @@ export const testDataExchange = ({ inputIdSchemes = { idScheme: 'UID' }, writeMetadataAccess = faker.datatype.boolean(), readMetadataAccess = faker.datatype.boolean(), - writeDataAccess = faker.datatype.boolean(), readDataAccess = faker.datatype.boolean(), + writeDataAccess = faker.datatype.boolean(), created = faker.date.recent(), externalURL = undefined, + includeDataAccess = true, } = {}) => ({ id, displayName: name, @@ -36,11 +37,16 @@ export const testDataExchange = ({ api: { url: externalURL }, request: inputIdSchemes, }, - access: { - write: writeMetadataAccess, - read: readMetadataAccess, - data: { write: writeDataAccess, read: readDataAccess }, - }, + access: includeDataAccess + ? { + write: writeMetadataAccess, + read: readMetadataAccess, + data: { write: writeDataAccess, read: readDataAccess }, + } + : { + write: writeMetadataAccess, + read: readMetadataAccess, + }, created, })