From 4348a47895a2d8a9c634250b05cea3a58ec5b51a Mon Sep 17 00:00:00 2001 From: abbas-khan10 Date: Mon, 26 Feb 2024 12:48:51 +0000 Subject: [PATCH] Add feature flag check to ARF journey --- app/src/helpers/hooks/useConfig.test.tsx | 8 +-- app/src/helpers/requests/getFeatureFlags.ts | 8 +-- app/src/helpers/test/testBuilders.ts | 25 +++++++++ app/src/helpers/utils/errorCodes.ts | 3 + .../AuthCallbackPage.test.tsx | 12 ++-- .../UploadDocumentsPage.test.tsx | 56 ++++++++++++++----- .../UploadDocumentsPage.tsx | 21 ++++++- .../configProvider/ConfigProvider.test.tsx | 11 ++-- .../configProvider/ConfigProvider.tsx | 3 +- app/src/types/generic/featureFlags.ts | 12 +++- lambdas/enums/lambda_error.py | 5 ++ .../create_document_reference_handler.py | 21 ++++++- .../test_create_document_reference_handler.py | 54 ++++++++++++++++-- 13 files changed, 193 insertions(+), 46 deletions(-) diff --git a/app/src/helpers/hooks/useConfig.test.tsx b/app/src/helpers/hooks/useConfig.test.tsx index 6b4e544f0..980bae44e 100644 --- a/app/src/helpers/hooks/useConfig.test.tsx +++ b/app/src/helpers/hooks/useConfig.test.tsx @@ -1,7 +1,7 @@ import { render, screen } from '@testing-library/react'; import useConfig from './useConfig'; import ConfigProvider, { GlobalConfig } from '../../providers/configProvider/ConfigProvider'; -import { defaultFeatureFlags } from '../requests/getFeatureFlags'; +import { defaultFeatureFlags } from '../../types/generic/featureFlags'; describe('useConfig', () => { beforeEach(() => { @@ -14,7 +14,7 @@ describe('useConfig', () => { it('returns true when feature flag in context', () => { const config: GlobalConfig = { - featureFlags: { ...defaultFeatureFlags, testFeature1: true }, + featureFlags: { ...defaultFeatureFlags, uploadLloydGeorgeWorkflowEnabled: true }, mockLocal: {}, }; renderHook(config); @@ -23,7 +23,7 @@ describe('useConfig', () => { it('returns false when there is no feature flag in context', () => { const config: GlobalConfig = { - featureFlags: { ...defaultFeatureFlags, testFeature1: false }, + featureFlags: { ...defaultFeatureFlags, uploadLloydGeorgeWorkflowEnabled: false }, mockLocal: {}, }; renderHook(config); @@ -33,7 +33,7 @@ describe('useConfig', () => { const TestApp = () => { const config = useConfig(); - return
{`FLAG: ${!!config.featureFlags.testFeature1}`.normalize()}
; + return
{`FLAG: ${config.featureFlags.uploadLloydGeorgeWorkflowEnabled}`.normalize()}
; }; const renderHook = (config?: GlobalConfig) => { diff --git a/app/src/helpers/requests/getFeatureFlags.ts b/app/src/helpers/requests/getFeatureFlags.ts index 3a45ffd78..497a7d560 100644 --- a/app/src/helpers/requests/getFeatureFlags.ts +++ b/app/src/helpers/requests/getFeatureFlags.ts @@ -2,7 +2,7 @@ import { AuthHeaders } from '../../types/blocks/authHeaders'; import { endpoints } from '../../types/generic/endpoints'; import axios from 'axios'; -import { FeatureFlags } from '../../types/generic/featureFlags'; +import { defaultFeatureFlags, FeatureFlags } from '../../types/generic/featureFlags'; type Args = { baseUrl: string; @@ -13,12 +13,6 @@ type GetFeatureFlagsResponse = { data: FeatureFlags; }; -export const defaultFeatureFlags = { - testFeature1: false, - testFeature2: false, - testFeature3: false, -}; - const getFeatureFlags = async ({ baseUrl, baseHeaders }: Args) => { const gatewayUrl = baseUrl + endpoints.FEATURE_FLAGS; try { diff --git a/app/src/helpers/test/testBuilders.ts b/app/src/helpers/test/testBuilders.ts index 53a837d59..2ede54043 100644 --- a/app/src/helpers/test/testBuilders.ts +++ b/app/src/helpers/test/testBuilders.ts @@ -11,6 +11,8 @@ import { LloydGeorgeStitchResult } from '../requests/getLloydGeorgeRecord'; import { REPOSITORY_ROLE } from '../../types/generic/authRole'; import { v4 as uuidv4 } from 'uuid'; import moment from 'moment'; +import { FeatureFlags } from '../../types/generic/featureFlags'; +import { GlobalConfig, LocalFlags } from '../../providers/configProvider/ConfigProvider'; const buildUserAuth = (userAuthOverride?: Partial) => { const auth: UserAuth = { @@ -110,6 +112,28 @@ const buildLgSearchResult = () => { return result; }; +const buildConfig = ( + localFlagsOverride?: Partial, + featureFlagsOverride?: Partial, +) => { + const globalConfig: GlobalConfig = { + mockLocal: { + isBsol: true, + recordUploaded: true, + userRole: REPOSITORY_ROLE.GP_ADMIN, + ...localFlagsOverride, + }, + featureFlags: { + uploadLloydGeorgeWorkflowEnabled: false, + uploadLambdaEnabled: false, + uploadArfWorkflowEnabled: false, + ...featureFlagsOverride, + }, + }; + + return globalConfig; +}; + export { buildPatientDetails, buildTextFile, @@ -118,4 +142,5 @@ export { buildLgSearchResult, buildUserAuth, buildLgFile, + buildConfig, }; diff --git a/app/src/helpers/utils/errorCodes.ts b/app/src/helpers/utils/errorCodes.ts index ab88559a4..eb241ea2b 100644 --- a/app/src/helpers/utils/errorCodes.ts +++ b/app/src/helpers/utils/errorCodes.ts @@ -27,6 +27,9 @@ const errorCodes: { [key: string]: string } = { GWY_5001: 'Failed to utilise AWS client/resource', SFB_5001: 'Error occur when sending email by SES', SFB_5002: 'Failed to fetch parameters for sending email from SSM param store', + FFL_5001: 'Failed to parse feature flag/s from AppConfig response', + FFL_5002: 'Failed to retrieve feature flag/s from AppConfig profile', + FFL_5003: 'Feature is not enabled', }; export default errorCodes; diff --git a/app/src/pages/authCallbackPage/AuthCallbackPage.test.tsx b/app/src/pages/authCallbackPage/AuthCallbackPage.test.tsx index 315adb5ee..fbc391409 100644 --- a/app/src/pages/authCallbackPage/AuthCallbackPage.test.tsx +++ b/app/src/pages/authCallbackPage/AuthCallbackPage.test.tsx @@ -9,7 +9,7 @@ import { routes } from '../../types/generic/routes'; import ConfigProvider, { useConfigContext } from '../../providers/configProvider/ConfigProvider'; import { act } from 'react-dom/test-utils'; import { endpoints } from '../../types/generic/endpoints'; -import { defaultFeatureFlags } from '../../helpers/requests/getFeatureFlags'; +import { defaultFeatureFlags } from '../../types/generic/featureFlags'; jest.mock('../../helpers/hooks/useConfig'); const mockedUseNavigate = jest.fn(); @@ -129,7 +129,7 @@ describe('AuthCallbackPage', () => { return Promise.resolve({ data: buildUserAuth() }); } else { return Promise.resolve({ - data: { ...defaultFeatureFlags, testFeature1: true }, + data: { ...defaultFeatureFlags, uploadLloydGeorgeWorkflowEnabled: true }, }); } }); @@ -148,8 +148,12 @@ const TestApp = () => { return (
-
{`FLAG: ${JSON.stringify(config.featureFlags.testFeature1)}`.normalize()}
; -
{`LOGGEDIN: ${!!session.auth?.role}`.normalize()}
; +
+ {`FLAG: ${JSON.stringify( + config.featureFlags.uploadLloydGeorgeWorkflowEnabled, + )}`.normalize()} +
+ ;
{`LOGGEDIN: ${!!session.auth?.role}`.normalize()}
;
); }; diff --git a/app/src/pages/uploadDocumentsPage/UploadDocumentsPage.test.tsx b/app/src/pages/uploadDocumentsPage/UploadDocumentsPage.test.tsx index 5656aa368..8ef93cbc7 100644 --- a/app/src/pages/uploadDocumentsPage/UploadDocumentsPage.test.tsx +++ b/app/src/pages/uploadDocumentsPage/UploadDocumentsPage.test.tsx @@ -1,17 +1,19 @@ -import { render, screen } from '@testing-library/react'; +import { render, screen, waitFor } from '@testing-library/react'; import UploadDocumentsPage from './UploadDocumentsPage'; -import usePatient from '../../helpers/hooks/usePatient'; -import { buildPatientDetails } from '../../helpers/test/testBuilders'; +import { buildConfig } from '../../helpers/test/testBuilders'; import { UPLOAD_STAGE } from '../../types/pages/UploadDocumentsPage/types'; import { useState } from 'react'; -jest.mock('../../helpers/hooks/useBaseAPIHeaders'); -jest.mock('../../helpers/hooks/useBaseAPIUrl'); -jest.mock('../../helpers/hooks/usePatient'); -jest.mock('react-router'); -const mockedUsePatient = usePatient as jest.Mock; +import useConfig from '../../helpers/hooks/useConfig'; +import { routes } from '../../types/generic/routes'; + const mockUseState = useState as jest.Mock; +const mockConfigContext = useConfig as jest.Mock; +const mockedUseNavigate = jest.fn(); -const mockPatient = buildPatientDetails(); +jest.mock('react-router', () => ({ + useNavigate: () => mockedUseNavigate, + useLocation: () => jest.fn(), +})); jest.mock('react', () => ({ ...jest.requireActual('react'), useState: jest.fn(), @@ -25,39 +27,65 @@ jest.mock('../../components/blocks/_arf/uploadingStage/UploadingStage', () => () jest.mock('../../components/blocks/_arf/completeStage/CompleteStage', () => () => (

Mock complete stage

)); +jest.mock('../../helpers/hooks/useConfig'); describe('UploadDocumentsPage', () => { beforeEach(() => { process.env.REACT_APP_ENVIRONMENT = 'jest'; - mockedUsePatient.mockReturnValue(mockPatient); + mockConfigContext.mockReturnValue(buildConfig({}, { uploadArfWorkflowEnabled: true })); mockUseState.mockImplementation(() => [UPLOAD_STAGE.Selecting, jest.fn()]); }); afterEach(() => { jest.clearAllMocks(); }); describe('Rendering', () => { - it('renders initial file input stage', () => { + it('renders initial file input stage', async () => { render(); + expect( screen.getByRole('heading', { name: 'Mock file input stage' }), ).toBeInTheDocument(); + await waitFor(() => { + expect(mockedUseNavigate).not.toHaveBeenCalledWith(routes.UNAUTHORISED); + }); }); - it('renders uploading stage when state is set', () => { + it('renders uploading stage when state is set', async () => { mockUseState.mockImplementation(() => [UPLOAD_STAGE.Uploading, jest.fn()]); + render(); + expect( screen.getByRole('heading', { name: 'Mock files are uploading stage' }), ).toBeInTheDocument(); + await waitFor(() => { + expect(mockedUseNavigate).not.toHaveBeenCalledWith(routes.UNAUTHORISED); + }); }); - it('renders upload complete stage when state is set', () => { + it('renders upload complete stage when state is set', async () => { mockUseState.mockImplementation(() => [UPLOAD_STAGE.Complete, jest.fn()]); + render(); + expect( screen.getByRole('heading', { name: 'Mock complete stage' }), ).toBeInTheDocument(); + await waitFor(() => { + expect(mockedUseNavigate).not.toHaveBeenCalledWith(routes.UNAUTHORISED); + }); + }); + }); + + describe('Navigation', () => { + it('redirects to unauthorised page if feature toggled off', async () => { + mockConfigContext.mockReturnValue(buildConfig({}, { uploadArfWorkflowEnabled: false })); + + render(); + + await waitFor(() => { + expect(mockedUseNavigate).toHaveBeenCalledWith(routes.UNAUTHORISED); + }); }); }); - describe('Navigation', () => {}); }); diff --git a/app/src/pages/uploadDocumentsPage/UploadDocumentsPage.tsx b/app/src/pages/uploadDocumentsPage/UploadDocumentsPage.tsx index 70eb0d25e..fad613bcf 100644 --- a/app/src/pages/uploadDocumentsPage/UploadDocumentsPage.tsx +++ b/app/src/pages/uploadDocumentsPage/UploadDocumentsPage.tsx @@ -1,12 +1,31 @@ -import React, { useState } from 'react'; +import React, { useEffect, useRef, useState } from 'react'; import { UPLOAD_STAGE, UploadDocument } from '../../types/pages/UploadDocumentsPage/types'; import SelectStage from '../../components/blocks/_arf/selectStage/SelectStage'; import UploadingStage from '../../components/blocks/_arf/uploadingStage/UploadingStage'; import CompleteStage from '../../components/blocks/_arf/completeStage/CompleteStage'; +import { routes } from '../../types/generic/routes'; +import { useNavigate } from 'react-router'; +import useConfig from '../../helpers/hooks/useConfig'; function UploadDocumentsPage() { const [stage, setStage] = useState(UPLOAD_STAGE.Selecting); const [documents, setDocuments] = useState>([]); + const config = useConfig(); + const navigate = useNavigate(); + + const mounted = useRef(false); + useEffect(() => { + const onPageLoad = () => { + if (!config.featureFlags.uploadArfWorkflowEnabled) { + navigate(routes.UNAUTHORISED); + } + }; + + if (!mounted.current) { + mounted.current = true; + void onPageLoad(); + } + }, [navigate, config]); if (stage === UPLOAD_STAGE.Selecting) { return ( diff --git a/app/src/providers/configProvider/ConfigProvider.test.tsx b/app/src/providers/configProvider/ConfigProvider.test.tsx index 375114bc0..8eef11112 100644 --- a/app/src/providers/configProvider/ConfigProvider.test.tsx +++ b/app/src/providers/configProvider/ConfigProvider.test.tsx @@ -1,7 +1,8 @@ import { act, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import ConfigProvider, { GlobalConfig, useConfigContext } from './ConfigProvider'; -import { defaultFeatureFlags } from '../../helpers/requests/getFeatureFlags'; +import { defaultFeatureFlags } from '../../types/generic/featureFlags'; + describe('SessionProvider', () => { beforeEach(() => { process.env.REACT_APP_ENVIRONMENT = 'jest'; @@ -37,14 +38,14 @@ const TestApp = () => { ...config, featureFlags: { ...defaultFeatureFlags, - testFeature1: true, + uploadLloydGeorgeWorkflowEnabled: true, }, }; const flagOff: GlobalConfig = { ...config, featureFlags: { ...defaultFeatureFlags, - testFeature1: false, + uploadLloydGeorgeWorkflowEnabled: false, }, }; return ( @@ -56,7 +57,9 @@ const TestApp = () => {

Flags

- testFeature - {`${!!config.featureFlags.testFeature1}`} + + testFeature - {`${config.featureFlags.uploadLloydGeorgeWorkflowEnabled}`} +
); diff --git a/app/src/providers/configProvider/ConfigProvider.tsx b/app/src/providers/configProvider/ConfigProvider.tsx index aeb1d43e4..a6557d7ea 100644 --- a/app/src/providers/configProvider/ConfigProvider.tsx +++ b/app/src/providers/configProvider/ConfigProvider.tsx @@ -1,8 +1,7 @@ import { createContext, useContext, useEffect, useMemo, useState } from 'react'; import type { Dispatch, ReactNode, SetStateAction } from 'react'; import { REPOSITORY_ROLE } from '../../types/generic/authRole'; -import { FeatureFlags } from '../../types/generic/featureFlags'; -import { defaultFeatureFlags } from '../../helpers/requests/getFeatureFlags'; +import { FeatureFlags, defaultFeatureFlags } from '../../types/generic/featureFlags'; import { isLocal } from '../../helpers/utils/isLocal'; type SetConfigOverride = (config: GlobalConfig) => void; diff --git a/app/src/types/generic/featureFlags.ts b/app/src/types/generic/featureFlags.ts index 275ba9eb7..f8fcd93dd 100644 --- a/app/src/types/generic/featureFlags.ts +++ b/app/src/types/generic/featureFlags.ts @@ -1,5 +1,11 @@ export type FeatureFlags = { - testFeature1: boolean; - testFeature2: boolean; - testFeature3: boolean; + uploadLloydGeorgeWorkflowEnabled: boolean; + uploadLambdaEnabled: boolean; + uploadArfWorkflowEnabled: boolean; +}; + +export const defaultFeatureFlags = { + uploadLloydGeorgeWorkflowEnabled: false, + uploadLambdaEnabled: false, + uploadArfWorkflowEnabled: false, }; diff --git a/lambdas/enums/lambda_error.py b/lambdas/enums/lambda_error.py index 9ef3be73c..8134a630b 100644 --- a/lambdas/enums/lambda_error.py +++ b/lambdas/enums/lambda_error.py @@ -254,6 +254,11 @@ def to_str(self) -> str: "message": "Failed to retrieve feature flag/s from AppConfig profile", } + FeatureFlagDisabled = { + "err_code": "FFL_5003", + "message": "Feature is not enabled", + } + """ Errors with no exception """ diff --git a/lambdas/handlers/create_document_reference_handler.py b/lambdas/handlers/create_document_reference_handler.py index 94522bb44..fb01764e0 100644 --- a/lambdas/handlers/create_document_reference_handler.py +++ b/lambdas/handlers/create_document_reference_handler.py @@ -3,15 +3,17 @@ import sys from json import JSONDecodeError +from enums.feature_flags import FeatureFlags from enums.lambda_error import LambdaError from enums.logging_app_interaction import LoggingAppInteraction from services.create_document_reference_service import CreateDocumentReferenceService +from services.feature_flags_service import FeatureFlagService from utils.audit_logging_setup import LoggingService from utils.decorators.ensure_env_var import ensure_environment_variables from utils.decorators.handle_lambda_exceptions import handle_lambda_exceptions from utils.decorators.override_error_check import override_error_check from utils.decorators.set_audit_arg import set_request_context_for_logging -from utils.lambda_exceptions import CreateDocumentRefException +from utils.lambda_exceptions import CreateDocumentRefException, FeatureFlagsException from utils.lambda_response import ApiGatewayResponse from utils.request_context import request_context @@ -23,10 +25,13 @@ @set_request_context_for_logging @ensure_environment_variables( names=[ - "LLOYD_GEORGE_BUCKET_NAME", - "LLOYD_GEORGE_DYNAMODB_NAME", + "APPCONFIG_APPLICATION", + "APPCONFIG_CONFIGURATION", + "APPCONFIG_ENVIRONMENT", "DOCUMENT_STORE_BUCKET_NAME", "DOCUMENT_STORE_DYNAMODB_NAME", + "LLOYD_GEORGE_BUCKET_NAME", + "LLOYD_GEORGE_DYNAMODB_NAME", ] ) @override_error_check @@ -34,6 +39,16 @@ def lambda_handler(event, context): request_context.app_interaction = LoggingAppInteraction.UPLOAD_RECORD.value + feature_flag_service = FeatureFlagService() + upload_flag_name = FeatureFlags.UPLOAD_LAMBDA_ENABLED.value + upload_lambda_enabled_flag_object = feature_flag_service.get_feature_flags_by_flag( + upload_flag_name + ) + + if not upload_lambda_enabled_flag_object[upload_flag_name]: + logger.info("Feature flag not enabled, event will not be processed") + raise FeatureFlagsException(500, LambdaError.FeatureFlagDisabled) + logger.info("Starting document reference creation process") nhs_number, doc_list = processing_event_details(event) diff --git a/lambdas/tests/unit/handlers/test_create_document_reference_handler.py b/lambdas/tests/unit/handlers/test_create_document_reference_handler.py index 311d1f111..954df07a1 100644 --- a/lambdas/tests/unit/handlers/test_create_document_reference_handler.py +++ b/lambdas/tests/unit/handlers/test_create_document_reference_handler.py @@ -6,6 +6,7 @@ lambda_handler, processing_event_details, ) +from services.feature_flags_service import FeatureFlagService from tests.unit.conftest import ( MOCK_ARF_BUCKET, MOCK_LG_BUCKET, @@ -58,8 +59,26 @@ def mock_processing_event_details(mocker): ) +@pytest.fixture +def mock_upload_lambda_enabled(mocker): + mock_function = mocker.patch.object(FeatureFlagService, "get_feature_flags_by_flag") + mock_upload_lambda_feature_flag = mock_function.return_value = { + "uploadLambdaEnabled": True + } + yield mock_upload_lambda_feature_flag + + +@pytest.fixture +def mock_upload_lambda_disabled(mocker): + mock_function = mocker.patch.object(FeatureFlagService, "get_feature_flags_by_flag") + mock_upload_lambda_feature_flag = mock_function.return_value = { + "uploadLambdaEnabled": False + } + yield mock_upload_lambda_feature_flag + + def test_create_document_reference_valid_both_lg_and_arf_type_returns_200( - set_env, both_type_event, context, mocker + set_env, both_type_event, context, mocker, mock_upload_lambda_enabled ): mock_service = mocker.patch( "handlers.create_document_reference_handler.CreateDocumentReferenceService", @@ -186,7 +205,12 @@ def test_processing_event_details_get_nhs_number_and_doc_list(arf_type_event): def test_lambda_handler_processing_event_details_raise_error( - mocker, arf_type_event, context, set_env, mock_processing_event_details + mocker, + arf_type_event, + context, + set_env, + mock_processing_event_details, + mock_upload_lambda_enabled, ): mock_processing_event_details.side_effect = CreateDocumentRefException( 400, MockError.Error @@ -202,7 +226,12 @@ def test_lambda_handler_processing_event_details_raise_error( def test_lambda_handler_service_raise_error( - mocker, arf_type_event, context, set_env, mock_processing_event_details + mocker, + arf_type_event, + context, + set_env, + mock_processing_event_details, + mock_upload_lambda_enabled, ): mock_processing_event_details.return_value = (TEST_NHS_NUMBER, ARF_FILE_LIST) @@ -222,7 +251,12 @@ def test_lambda_handler_service_raise_error( def test_lambda_handler_valid( - mocker, arf_type_event, context, set_env, mock_processing_event_details + mocker, + arf_type_event, + context, + set_env, + mock_processing_event_details, + mock_upload_lambda_enabled, ): mock_processing_event_details.return_value = (TEST_NHS_NUMBER, ARF_FILE_LIST) @@ -239,3 +273,15 @@ def test_lambda_handler_valid( actual = lambda_handler(arf_type_event, context) assert expected == actual mock_processing_event_details.assert_called_with(arf_type_event) + + +def test_no_event_processing_when_upload_lambda_flag_not_enabled( + set_env, + both_type_event, + context, + mock_processing_event_details, + mock_upload_lambda_disabled, +): + lambda_handler(both_type_event, context) + + mock_processing_event_details.assert_not_called()