From bb341f8cb90a7ffab540ee24fbc8fd60cbc65f99 Mon Sep 17 00:00:00 2001 From: Daniele Manni Date: Thu, 19 Aug 2021 12:54:12 +0200 Subject: [PATCH] [#IP368] Enable strictNullCheck on tsconfig.json --- MigrateServicePreferenceFromLegacy/handler.ts | 8 +++-- .../__tests__/handler.test.ts | 33 +++++++++++++++++-- StartEmailValidationProcess/handler.ts | 4 +-- UpdateProfile/__tests__/handler.test.ts | 5 +-- UpdateProfile/handler.ts | 2 +- UpsertServicePreferences/handler.ts | 4 +-- .../__tests__/handler.test.ts | 15 +++++---- UpsertedProfileOrchestrator/handler.ts | 15 ++++++--- UpsertedProfileOrchestrator/index.ts | 3 +- .../__tests__/handler.test.ts | 23 ++++++------- UpsertedProfileOrchestratorV2/handler.ts | 17 ++++++---- UpsertedProfileOrchestratorV2/index.ts | 3 +- tsconfig.json | 3 +- utils/appinsights.ts | 2 +- utils/profiles.ts | 15 ++++++--- utils/tracking.ts | 8 ++--- 16 files changed, 107 insertions(+), 53 deletions(-) diff --git a/MigrateServicePreferenceFromLegacy/handler.ts b/MigrateServicePreferenceFromLegacy/handler.ts index 4b143923..9cdd0bb7 100644 --- a/MigrateServicePreferenceFromLegacy/handler.ts +++ b/MigrateServicePreferenceFromLegacy/handler.ts @@ -57,9 +57,11 @@ export const createServicePreference = ( }); export const blockedsToServicesPreferences = ( - blocked: { - [x: string]: readonly BlockedInboxOrChannelEnum[]; - }, + blocked: + | { + [x: string]: readonly BlockedInboxOrChannelEnum[]; + } + | undefined, fiscalCode: FiscalCode, version: NonNegativeInteger ) => diff --git a/StartEmailValidationProcess/__tests__/handler.test.ts b/StartEmailValidationProcess/__tests__/handler.test.ts index e4cfc679..3da4a75f 100644 --- a/StartEmailValidationProcess/__tests__/handler.test.ts +++ b/StartEmailValidationProcess/__tests__/handler.test.ts @@ -28,7 +28,13 @@ describe("StartEmailValidationProcessHandler", () => { it("should start the orchestrator with the right input and return an accepted response", async () => { const profileModelMock = { findLastVersionByModelId: jest.fn(() => - taskEither.of(some({ ...aRetrievedProfile, isEmailValidated: false })) + taskEither.of( + some({ + ...aRetrievedProfile, + isEmailValidated: false, + email: "email@example.com" + }) + ) ) }; mockStartNew.mockImplementationOnce(() => Promise.resolve("start")); @@ -46,7 +52,13 @@ describe("StartEmailValidationProcessHandler", () => { it("should not start a new orchestrator if there is an already running orchestrator and return an accepted response", async () => { const profileModelMock = { findLastVersionByModelId: jest.fn(() => - taskEither.of(some({ ...aRetrievedProfile, isEmailValidated: false })) + taskEither.of( + some({ + ...aRetrievedProfile, + isEmailValidated: false, + email: "email@example.com" + }) + ) ) }; @@ -82,4 +94,21 @@ describe("StartEmailValidationProcessHandler", () => { ); expect(result.kind).toBe("IResponseErrorValidation"); }); + + it("should not start the orchestrator if the email is missing", async () => { + const profileModelMock = { + findLastVersionByModelId: jest.fn(() => + taskEither.of(some({ ...aRetrievedProfile, isEmailValidated: false })) + ) + }; + + const handler = StartEmailValidationProcessHandler(profileModelMock as any); + + await handler(contextMock as any, aRetrievedProfile.fiscalCode); + const result = await handler( + contextMock as any, + aRetrievedProfile.fiscalCode + ); + expect(result.kind).toBe("IResponseErrorValidation"); + }); }); diff --git a/StartEmailValidationProcess/handler.ts b/StartEmailValidationProcess/handler.ts index 1e6c58ec..3fb8d997 100644 --- a/StartEmailValidationProcess/handler.ts +++ b/StartEmailValidationProcess/handler.ts @@ -89,10 +89,10 @@ export function StartEmailValidationProcessHandler( const existingProfile = maybeExistingProfile.value; - if (existingProfile.isEmailValidated === true) { + if (existingProfile.isEmailValidated === true || !existingProfile.email) { return ResponseErrorValidation( "Validation error", - "The email is already validated" + "The email is missing or already validated" ); } diff --git a/UpdateProfile/__tests__/handler.test.ts b/UpdateProfile/__tests__/handler.test.ts index 924f9184..14490416 100644 --- a/UpdateProfile/__tests__/handler.test.ts +++ b/UpdateProfile/__tests__/handler.test.ts @@ -466,7 +466,7 @@ describe("UpdateProfileHandler", () => { expect(result.value).toEqual( expect.objectContaining({ service_preferences_settings: { - mode: autoApiProfileServicePreferencesSettings.mode, + mode: (autoApiProfileServicePreferencesSettings as any).mode, version: expectedServicePreferencesSettingsVersion } }) @@ -604,7 +604,8 @@ describe("UpdateProfileHandler", () => { isWebhookEnabled, expectedIsInboxEnabled, expectedIsWebHookEnabled, - acceptedTosVersion + acceptedTosVersion, + _ ) => { const profileModelMock = { findLastVersionByModelId: jest.fn(() => diff --git a/UpdateProfile/handler.ts b/UpdateProfile/handler.ts index b6bf64e3..4d1960a3 100644 --- a/UpdateProfile/handler.ts +++ b/UpdateProfile/handler.ts @@ -165,7 +165,7 @@ export function UpdateProfileHandler( const profile = apiProfileToProfile( profilePayload, fiscalCode, - emailChanged ? false : existingProfile.isEmailValidated, + emailChanged ? false : !!existingProfile.isEmailValidated, servicePreferencesSettingsVersion ); diff --git a/UpsertServicePreferences/handler.ts b/UpsertServicePreferences/handler.ts index 20d6e82e..e6812fa1 100644 --- a/UpsertServicePreferences/handler.ts +++ b/UpsertServicePreferences/handler.ts @@ -202,7 +202,7 @@ const getFeedOperation = ( * Return a type safe GetServicePreferences handler. */ export const GetUpsertServicePreferencesHandler = ( - telemetryClient: ReturnType, + telemetryClient: ReturnType | undefined, profileModels: ProfileModel, serviceModels: ServiceModel, servicePreferencesModel: ServicesPreferencesModel, @@ -315,7 +315,7 @@ export const GetUpsertServicePreferencesHandler = ( * Wraps a UpsertServicePreferences handler inside an Express request handler. */ export function UpsertServicePreferences( - telemetryClient: ReturnType, + telemetryClient: ReturnType | undefined, profileModels: ProfileModel, serviceModels: ServiceModel, servicePreferencesModel: ServicesPreferencesModel, diff --git a/UpsertedProfileOrchestrator/__tests__/handler.test.ts b/UpsertedProfileOrchestrator/__tests__/handler.test.ts index a641fd0d..2f0de280 100644 --- a/UpsertedProfileOrchestrator/__tests__/handler.test.ts +++ b/UpsertedProfileOrchestrator/__tests__/handler.test.ts @@ -24,6 +24,7 @@ import { import { BlockedInboxOrChannelEnum } from "@pagopa/io-functions-commons/dist/generated/definitions/BlockedInboxOrChannel"; import { readableReport } from "@pagopa/ts-commons/lib/reporters"; import { consumeGenerator } from "../../utils/durable"; +import { identity } from "fp-ts/lib/function"; const someRetryOptions = new df.RetryOptions(5000, 10); // tslint:disable-next-line: no-object-mutation @@ -203,7 +204,7 @@ describe("UpsertedProfileOrchestrator", () => { }; const orchestratorHandler = getUpsertedProfileOrchestratorHandler({ - notifyOn: fromArray([expectedQueueName]).getOrElseL(undefined), + notifyOn: fromArray([expectedQueueName]).fold(undefined, identity), sendCashbackMessage: true })(contextMockWithDf as any); @@ -310,7 +311,7 @@ describe("UpsertedProfileOrchestrator", () => { }; const orchestratorHandler = getUpsertedProfileOrchestratorHandler({ - notifyOn: fromArray([expectedQueueName]).getOrElseL(undefined), + notifyOn: fromArray([expectedQueueName]).fold(undefined, identity), sendCashbackMessage: true })(contextMockWithDf as any); @@ -421,7 +422,7 @@ describe("UpsertedProfileOrchestrator", () => { }; const orchestratorHandler = getUpsertedProfileOrchestratorHandler({ - notifyOn: fromArray([expectedQueueName]).getOrElseL(undefined), + notifyOn: fromArray([expectedQueueName]).fold(undefined, identity), sendCashbackMessage: true })(contextMockWithDf as any); @@ -532,7 +533,7 @@ describe("UpsertedProfileOrchestrator", () => { }; const orchestratorHandler = getUpsertedProfileOrchestratorHandler({ - notifyOn: fromArray([expectedQueueName]).getOrElseL(undefined), + notifyOn: fromArray([expectedQueueName]).fold(undefined, identity), sendCashbackMessage: true })(contextMockWithDf as any); @@ -650,7 +651,7 @@ describe("UpsertedProfileOrchestrator", () => { }; const orchestratorHandler = getUpsertedProfileOrchestratorHandler({ - notifyOn: fromArray([expectedQueueName]).getOrElseL(undefined), + notifyOn: fromArray([expectedQueueName]).fold(undefined, identity), sendCashbackMessage: true })(contextMockWithDf as any); @@ -768,7 +769,7 @@ describe("UpsertedProfileOrchestrator", () => { }; const orchestratorHandler = getUpsertedProfileOrchestratorHandler({ - notifyOn: fromArray([expectedQueueName]).getOrElseL(undefined), + notifyOn: fromArray([expectedQueueName]).fold(undefined, identity), sendCashbackMessage: true })(contextMockWithDf as any); @@ -891,7 +892,7 @@ describe("UpsertedProfileOrchestrator", () => { }; const orchestratorHandler = getUpsertedProfileOrchestratorHandler({ - notifyOn: fromArray([expectedQueueName]).getOrElseL(undefined), + notifyOn: fromArray([expectedQueueName]).fold(undefined, identity), sendCashbackMessage: true })(contextMockWithDf as any); diff --git a/UpsertedProfileOrchestrator/handler.ts b/UpsertedProfileOrchestrator/handler.ts index e98f7b27..a4287e63 100644 --- a/UpsertedProfileOrchestrator/handler.ts +++ b/UpsertedProfileOrchestrator/handler.ts @@ -89,12 +89,17 @@ export const getUpsertedProfileOrchestratorHandler = (params: { updatedAt } = upsertedProfileOrchestratorInput; - const profileOperation = oldProfile !== undefined ? "UPDATED" : "CREATED"; + const existsOldProfile = ( + p: RetrievedProfile | undefined + ): p is RetrievedProfile => p !== undefined; + const profileOperation = existsOldProfile(oldProfile) + ? "UPDATED" + : "CREATED"; // Check if the profile email is changed const isProfileEmailChanged = - profileOperation === "UPDATED" && newProfile.email !== oldProfile.email; - if (isProfileEmailChanged) { + existsOldProfile(oldProfile) && newProfile.email !== oldProfile.email; + if (isProfileEmailChanged && newProfile.email) { try { const { fiscalCode, email } = newProfile; @@ -151,7 +156,7 @@ export const getUpsertedProfileOrchestratorHandler = (params: { // Send welcome messages to the user const isInboxEnabled = newProfile.isInboxEnabled === true; const hasOldProfileWithInboxDisabled = - profileOperation === "UPDATED" && oldProfile.isInboxEnabled === false; + existsOldProfile(oldProfile) && oldProfile.isInboxEnabled === false; const hasJustEnabledInbox = isInboxEnabled && @@ -191,7 +196,7 @@ export const getUpsertedProfileOrchestratorHandler = (params: { } // Update subscriptions feed - if (profileOperation === "CREATED") { + if (!existsOldProfile(oldProfile)) { // When a profile get created we add an entry to the profile subscriptions context.log.verbose( `${logPrefix}|Calling UpdateSubscriptionsFeedActivity|OPERATION=SUBSCRIBED` diff --git a/UpsertedProfileOrchestrator/index.ts b/UpsertedProfileOrchestrator/index.ts index c8780f9b..fde86f99 100644 --- a/UpsertedProfileOrchestrator/index.ts +++ b/UpsertedProfileOrchestrator/index.ts @@ -1,4 +1,5 @@ import * as df from "durable-functions"; +import { identity } from "fp-ts/lib/function"; import * as NonEmptyArray from "fp-ts/lib/NonEmptyArray"; import { getConfigOrThrow } from "../utils/config"; @@ -11,7 +12,7 @@ const orchestrator = df.orchestrator( notifyOn: config.FF_NEW_USERS_EUCOVIDCERT_ENABLED ? NonEmptyArray.fromArray([ config.EUCOVIDCERT_PROFILE_CREATED_QUEUE_NAME - ]).getOrElse(undefined) + ]).fold(undefined, identity) : undefined, sendCashbackMessage: config.IS_CASHBACK_ENABLED }) diff --git a/UpsertedProfileOrchestratorV2/__tests__/handler.test.ts b/UpsertedProfileOrchestratorV2/__tests__/handler.test.ts index 59773d29..fc119788 100644 --- a/UpsertedProfileOrchestratorV2/__tests__/handler.test.ts +++ b/UpsertedProfileOrchestratorV2/__tests__/handler.test.ts @@ -24,6 +24,7 @@ import { import { BlockedInboxOrChannelEnum } from "@pagopa/io-functions-commons/dist/generated/definitions/BlockedInboxOrChannel"; import { readableReport } from "@pagopa/ts-commons/lib/reporters"; import { consumeGenerator } from "../../utils/durable"; +import { identity } from "fp-ts/lib/function"; const someRetryOptions = new df.RetryOptions(5000, 10); // tslint:disable-next-line: no-object-mutation @@ -203,7 +204,7 @@ describe("UpsertedProfileOrchestratorV2", () => { }; const orchestratorHandler = getUpsertedProfileOrchestratorHandler({ - notifyOn: fromArray([expectedQueueName]).getOrElseL(undefined), + notifyOn: fromArray([expectedQueueName]).fold(undefined, identity), sendCashbackMessage: true })(contextMockWithDf as any); @@ -310,7 +311,7 @@ describe("UpsertedProfileOrchestratorV2", () => { }; const orchestratorHandler = getUpsertedProfileOrchestratorHandler({ - notifyOn: fromArray([expectedQueueName]).getOrElseL(undefined), + notifyOn: fromArray([expectedQueueName]).fold(undefined, identity), sendCashbackMessage: true })(contextMockWithDf as any); @@ -421,7 +422,7 @@ describe("UpsertedProfileOrchestratorV2", () => { }; const orchestratorHandler = getUpsertedProfileOrchestratorHandler({ - notifyOn: fromArray([expectedQueueName]).getOrElseL(undefined), + notifyOn: fromArray([expectedQueueName]).fold(undefined, identity), sendCashbackMessage: true })(contextMockWithDf as any); @@ -532,7 +533,7 @@ describe("UpsertedProfileOrchestratorV2", () => { }; const orchestratorHandler = getUpsertedProfileOrchestratorHandler({ - notifyOn: fromArray([expectedQueueName]).getOrElseL(undefined), + notifyOn: fromArray([expectedQueueName]).fold(undefined, identity), sendCashbackMessage: true })(contextMockWithDf as any); @@ -656,7 +657,7 @@ describe("UpsertedProfileOrchestratorV2", () => { }; const orchestratorHandler = getUpsertedProfileOrchestratorHandler({ - notifyOn: fromArray([expectedQueueName]).getOrElseL(undefined), + notifyOn: fromArray([expectedQueueName]).fold(undefined, identity), sendCashbackMessage: true })(contextMockWithDf as any); @@ -711,8 +712,8 @@ describe("UpsertedProfileOrchestratorV2", () => { { fiscalCode: aFiscalCode, settingsVersion: - upsertedProfileOrchestratorInput.oldProfile.servicePreferencesSettings - .version + upsertedProfileOrchestratorInput.oldProfile + ?.servicePreferencesSettings.version } ); @@ -802,7 +803,7 @@ describe("UpsertedProfileOrchestratorV2", () => { }; const orchestratorHandler = getUpsertedProfileOrchestratorHandler({ - notifyOn: fromArray([expectedQueueName]).getOrElseL(undefined), + notifyOn: fromArray([expectedQueueName]).fold(undefined, identity), sendCashbackMessage: true })(contextMockWithDf as any); @@ -855,8 +856,8 @@ describe("UpsertedProfileOrchestratorV2", () => { { fiscalCode: aFiscalCode, settingsVersion: - upsertedProfileOrchestratorInput.oldProfile.servicePreferencesSettings - .version + upsertedProfileOrchestratorInput.oldProfile + ?.servicePreferencesSettings.version } ); @@ -945,7 +946,7 @@ describe("UpsertedProfileOrchestratorV2", () => { }; const orchestratorHandler = getUpsertedProfileOrchestratorHandler({ - notifyOn: fromArray([expectedQueueName]).getOrElseL(undefined), + notifyOn: fromArray([expectedQueueName]).fold(undefined, identity), sendCashbackMessage: true })(contextMockWithDf as any); diff --git a/UpsertedProfileOrchestratorV2/handler.ts b/UpsertedProfileOrchestratorV2/handler.ts index a9025e50..fadeff87 100644 --- a/UpsertedProfileOrchestratorV2/handler.ts +++ b/UpsertedProfileOrchestratorV2/handler.ts @@ -88,12 +88,17 @@ export const getUpsertedProfileOrchestratorHandler = (params: { updatedAt } = upsertedProfileOrchestratorInput; - const profileOperation = oldProfile !== undefined ? "UPDATED" : "CREATED"; + const existsOldProfile = ( + p: RetrievedProfile | undefined + ): p is RetrievedProfile => p !== undefined; + const profileOperation = existsOldProfile(oldProfile) + ? "UPDATED" + : "CREATED"; // Check if the profile email is changed const isProfileEmailChanged = - profileOperation === "UPDATED" && newProfile.email !== oldProfile.email; - if (isProfileEmailChanged) { + existsOldProfile(oldProfile) && newProfile.email !== oldProfile.email; + if (isProfileEmailChanged && newProfile.email) { try { const { fiscalCode, email } = newProfile; @@ -150,11 +155,11 @@ export const getUpsertedProfileOrchestratorHandler = (params: { // Send welcome messages to the user const isInboxEnabled = newProfile.isInboxEnabled === true; const hasOldProfileWithInboxDisabled = - profileOperation === "UPDATED" && oldProfile.isInboxEnabled === false; + existsOldProfile(oldProfile) && oldProfile.isInboxEnabled === false; const hasJustEnabledInbox = isInboxEnabled && - (profileOperation === "CREATED" || hasOldProfileWithInboxDisabled); + (!existsOldProfile(oldProfile) || hasOldProfileWithInboxDisabled); context.log.verbose( `${logPrefix}|OPERATION=${profileOperation}|INBOX_ENABLED=${isInboxEnabled}|INBOX_JUST_ENABLED=${hasJustEnabledInbox}` @@ -190,7 +195,7 @@ export const getUpsertedProfileOrchestratorHandler = (params: { } // Update subscriptions feed - if (profileOperation === "CREATED") { + if (!existsOldProfile(oldProfile)) { // When a profile get created we add an entry to the profile subscriptions context.log.verbose( `${logPrefix}|Calling UpdateSubscriptionsFeedActivity|OPERATION=SUBSCRIBED` diff --git a/UpsertedProfileOrchestratorV2/index.ts b/UpsertedProfileOrchestratorV2/index.ts index c8780f9b..fde86f99 100644 --- a/UpsertedProfileOrchestratorV2/index.ts +++ b/UpsertedProfileOrchestratorV2/index.ts @@ -1,4 +1,5 @@ import * as df from "durable-functions"; +import { identity } from "fp-ts/lib/function"; import * as NonEmptyArray from "fp-ts/lib/NonEmptyArray"; import { getConfigOrThrow } from "../utils/config"; @@ -11,7 +12,7 @@ const orchestrator = df.orchestrator( notifyOn: config.FF_NEW_USERS_EUCOVIDCERT_ENABLED ? NonEmptyArray.fromArray([ config.EUCOVIDCERT_PROFILE_CREATED_QUEUE_NAME - ]).getOrElse(undefined) + ]).fold(undefined, identity) : undefined, sendCashbackMessage: config.IS_CASHBACK_ENABLED }) diff --git a/tsconfig.json b/tsconfig.json index 049acb9e..f93cf144 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -6,7 +6,8 @@ "rootDir": ".", "sourceMap": true, "strict": false, - "resolveJsonModule": true + "resolveJsonModule": true, + "strictNullChecks": true }, "exclude": [ "node_modules", diff --git a/utils/appinsights.ts b/utils/appinsights.ts index b020d2e1..2ab75c22 100644 --- a/utils/appinsights.ts +++ b/utils/appinsights.ts @@ -13,7 +13,7 @@ export const initTelemetryClient = (env = process.env) => ai.defaultClient ? ai.defaultClient : NonEmptyString.decode(env.APPINSIGHTS_INSTRUMENTATIONKEY) - .map(k => + .map(k => initAppInsights(k, { disableAppInsights: env.APPINSIGHTS_DISABLE === "true", samplingPercentage: IntegerFromString.decode( diff --git a/utils/profiles.ts b/utils/profiles.ts index fb976e1e..59eb12d1 100644 --- a/utils/profiles.ts +++ b/utils/profiles.ts @@ -15,6 +15,7 @@ import { isObject } from "util"; import { FiscalCode } from "@pagopa/io-functions-commons/dist/generated/definitions/FiscalCode"; import { ServicesPreferencesModeEnum } from "@pagopa/io-functions-commons/dist/generated/definitions/ServicesPreferencesMode"; import { NonNegativeInteger } from "@pagopa/ts-commons/lib/numbers"; +import { fromNullable } from "fp-ts/lib/Option"; /** * Converts a ApiProfile in a Profile model @@ -77,11 +78,17 @@ export function retrievedProfileToExtendedProfile( const getInboxBlockedServices = ( blocked: Profile["blockedInboxOrChannels"] | undefined | null ): ReadonlyArray => - Object.keys(blocked) - .map(k => - blocked[k].includes(BlockedInboxOrChannelEnum.INBOX) ? k : undefined + fromNullable(blocked) + .map(_ => + Object.keys(_) + .reduce( + (p, k) => + _[k].includes(BlockedInboxOrChannelEnum.INBOX) ? [...p, k] : p, + [] + ) + .filter(k => k !== undefined) ) - .filter(k => k !== undefined); + .getOrElse([]); /** * Returns the services that exist in newServices but not in oldServices diff --git a/utils/tracking.ts b/utils/tracking.ts index e4c406c0..7ebecf45 100644 --- a/utils/tracking.ts +++ b/utils/tracking.ts @@ -21,7 +21,7 @@ export const createTracker = ( nextMode: ServicesPreferencesModeEnum, profileVersion: NonNegativeInteger ) => - telemetryClient.trackEvent({ + telemetryClient?.trackEvent({ name: eventName("change-service-preferences-mode"), properties: { nextMode, @@ -40,7 +40,7 @@ export const createTracker = ( newProfile: RetrievedProfile, action: "REQUESTING" | "DOING" | "DONE" ) => - telemetryClient.trackEvent({ + telemetryClient?.trackEvent({ name: eventName("migrate-legacy-preferences"), properties: { action, @@ -61,7 +61,7 @@ export const createTracker = ( { fiscalCode, version, updatedAt, ...input }: UpdateSubscriptionFeedInput, kind: "EXCEPTION" | "FAILURE" ) => { - telemetryClient.trackEvent({ + telemetryClient?.trackEvent({ name: "subscriptionFeed.upsertServicesPreferences.failure", properties: { ...input, @@ -75,7 +75,7 @@ export const createTracker = ( }; const traceEmailValidationSend = (messageInfo: object) => { - telemetryClient.trackEvent({ + telemetryClient?.trackEvent({ name: `SendValidationEmailActivity.success`, properties: messageInfo } as EventTelemetry);