[#IOPID-1604] applied review suggestions

ValidateProfileEmail/__tests__/handler.test.ts

@@ -9,7 +9,7 @@ import * as O from "fp-ts/Option";
 import { ProfileModel } from "@pagopa/io-functions-commons/dist/src/models/profile";
 import { aFiscalCode, aRetrievedProfile, anEmail } from "../__mocks__/profile";
 import { ValidUrl } from "@pagopa/ts-commons/lib/url";
-import { FlowChoiceEnum, TokenQueryParam } from "../../utils/middleware";
+import { FlowTypeEnum, TokenQueryParam } from "../../utils/middleware";
 import {
   confirmChoicePageUrl,
   validationFailureUrl,
@@ -42,11 +42,11 @@ const validationCallbackUrl = {
   href: "localhost/validation"
 } as ValidUrl;
 
-const confirmChoiceUrl = {
+const confirmValidationUrl = {
   href: "localhost/confirm-choice"
 } as ValidUrl;
 
-const timestampGeneratorMock = () => 1234567890;
+const emailValidationUrls = { confirmValidationUrl, validationCallbackUrl };
 
 const mockRetrieveEntity = jest
   .fn()
@@ -115,26 +115,20 @@ describe("ValidateProfileEmailHandler", () => {
         tableServiceMock as any,
         "",
         mockProfileModel,
-        validationCallbackUrl as any,
-        timestampGeneratorMock,
+        emailValidationUrls,
         profileEmailReader,
-        constTrue,
-        confirmChoiceUrl
+        constTrue
       );
 
       const response = await verifyProfileEmailHandler(
         contextMock as any,
         VALIDATION_TOKEN,
-        isConfirmFlow ? FlowChoiceEnum.CONFIRM : FlowChoiceEnum.VALIDATE
+        isConfirmFlow ? FlowTypeEnum.CONFIRM : FlowTypeEnum.VALIDATE
       );
 
       expect(response.kind).toBe("IResponseSeeOtherRedirect");
       expect(response.detail).toBe(
-        validationFailureUrl(
-          validationCallbackUrl,
-          expectedError,
-          timestampGeneratorMock
-        ).href
+        validationFailureUrl(validationCallbackUrl, expectedError).href
       );
       expect(mockFindLastVersionByModelId).not.toBeCalled();
       expect(mockUpdate).not.toBeCalled();
@@ -154,28 +148,22 @@ describe("ValidateProfileEmailHandler", () => {
         tableServiceMock as any,
         "",
         mockProfileModel,
-        validationCallbackUrl as any,
-        timestampGeneratorMock,
+        emailValidationUrls,
         {
           list: generateProfileEmails(1, isThrowing)
         },
-        constTrue,
-        confirmChoiceUrl
+        constTrue
       );
 
       const response = await verifyProfileEmailHandler(
         contextMock as any,
         VALIDATION_TOKEN,
-        isConfirmFlow ? FlowChoiceEnum.CONFIRM : FlowChoiceEnum.VALIDATE
+        isConfirmFlow ? FlowTypeEnum.CONFIRM : FlowTypeEnum.VALIDATE
       );
 
       expect(response.kind).toBe("IResponseSeeOtherRedirect");
       expect(response.detail).toBe(
-        validationFailureUrl(
-          validationCallbackUrl,
-          expectedError,
-          timestampGeneratorMock
-        ).href
+        validationFailureUrl(validationCallbackUrl, expectedError).href
       );
       expect(mockFindLastVersionByModelId).toBeCalledWith([aFiscalCode]);
       expect(mockUpdate).not.toBeCalled();
@@ -187,24 +175,22 @@ describe("ValidateProfileEmailHandler", () => {
       tableServiceMock as any,
       "",
       mockProfileModel,
-      validationCallbackUrl as any,
-      timestampGeneratorMock,
+      emailValidationUrls,
       {
         list: generateProfileEmails(0)
       },
-      constTrue,
-      confirmChoiceUrl
+      constTrue
     );
 
     const response = await verifyProfileEmailHandler(
       contextMock as any,
       VALIDATION_TOKEN,
-      FlowChoiceEnum.VALIDATE
+      FlowTypeEnum.VALIDATE
     );
 
     expect(response.kind).toBe("IResponseSeeOtherRedirect");
     expect(response.detail).toBe(
-      validationSuccessUrl(validationCallbackUrl, timestampGeneratorMock).href
+      validationSuccessUrl(validationCallbackUrl).href
     );
     expect(mockFindLastVersionByModelId).toBeCalledWith([aFiscalCode]);
     expect(mockUpdate).toBeCalledWith(
@@ -217,24 +203,22 @@ describe("ValidateProfileEmailHandler", () => {
       tableServiceMock as any,
       "",
       mockProfileModel,
-      validationCallbackUrl as any,
-      timestampGeneratorMock,
+      emailValidationUrls,
       {
         list: generateProfileEmails(0)
       },
-      constTrue,
-      confirmChoiceUrl
+      constTrue
     );
 
     const response = await verifyProfileEmailHandler(
       contextMock as any,
       VALIDATION_TOKEN,
-      FlowChoiceEnum.CONFIRM
+      FlowTypeEnum.CONFIRM
     );
 
     expect(response.kind).toBe("IResponseSeeOtherRedirect");
     expect(response.detail).toBe(
-      confirmChoicePageUrl(confirmChoiceUrl, VALIDATION_TOKEN, anEmail).href
+      confirmChoicePageUrl(confirmValidationUrl, VALIDATION_TOKEN, anEmail).href
     );
     expect(mockFindLastVersionByModelId).toBeCalledWith([aFiscalCode]);
     expect(mockUpdate).not.toHaveBeenCalled();

ValidateProfileEmail/handler.ts

@@ -34,9 +34,9 @@ import {
 } from "@pagopa/io-functions-commons/dist/src/utils/unique_email_enforcement";
 import { trackEvent } from "../utils/appinsights";
 import {
-  ChoiceConfirmQueryParamMiddleware,
-  FlowChoice,
-  FlowChoiceEnum,
+  ConfirmEmailFlowQueryParamMiddleware,
+  FlowType,
+  FlowTypeEnum,
   TokenQueryParam,
   TokenQueryParamMiddleware
 } from "../utils/middleware";
@@ -50,26 +50,28 @@ import { ValidationErrors } from "../utils/validation_errors";
 type IValidateProfileEmailHandler = (
   context: Context,
   token: TokenQueryParam,
-  flowChoice: FlowChoice
+  flowChoice: FlowType
 ) => Promise<IResponseSeeOtherRedirect | IResponseErrorValidation>;
 
 export const ValidateProfileEmailHandler = (
   tableService: TableService,
   validationTokensTableName: string,
   profileModel: ProfileModel,
-  validationCallbackUrl: ValidUrl,
-  timeStampGenerator: () => number,
+  emailValidationUrls: {
+    confirmValidationUrl: ValidUrl;
+    validationCallbackUrl: ValidUrl;
+  },
   profileEmails: IProfileEmailReader,
-  FF_UNIQUE_EMAIL_ENFORCEMENT_ENABLED: (fiscalCode: FiscalCode) => boolean,
-  confirmChoiceUrl: ValidUrl
+  FF_UNIQUE_EMAIL_ENFORCEMENT_ENABLED: (fiscalCode: FiscalCode) => boolean
 ): IValidateProfileEmailHandler => async (
   context,
   token,
   flowChoice
 ): Promise<IResponseSeeOtherRedirect | IResponseErrorValidation> => {
   const logPrefix = `ValidateProfileEmail|TOKEN=${token}`;
+  const { validationCallbackUrl, confirmValidationUrl } = emailValidationUrls;
   const vFailureUrl = (error: keyof typeof ValidationErrors): ValidUrl =>
-    validationFailureUrl(validationCallbackUrl, error, timeStampGenerator);
+    validationFailureUrl(validationCallbackUrl, error);
 
   // STEP 1: Find and verify validation token
 
@@ -195,10 +197,10 @@ export const ValidateProfileEmailHandler = (
   return await pipe(
     flowChoice,
     TE.fromPredicate(
-      flow => flow === FlowChoiceEnum.VALIDATE,
+      flow => flow === FlowTypeEnum.VALIDATE,
       () =>
         ResponseSeeOtherRedirect(
-          confirmChoicePageUrl(confirmChoiceUrl, token, email)
+          confirmChoicePageUrl(confirmValidationUrl, token, email)
         )
     ),
     TE.chain(() =>
@@ -226,7 +228,7 @@ export const ValidateProfileEmailHandler = (
 
           context.log.verbose(`${logPrefix}|The profile has been updated`);
           return ResponseSeeOtherRedirect(
-            validationSuccessUrl(validationCallbackUrl, timeStampGenerator)
+            validationSuccessUrl(validationCallbackUrl)
           );
         })
       )
@@ -243,27 +245,26 @@ export const ValidateProfileEmail = (
   tableService: TableService,
   validationTokensTableName: string,
   profileModel: ProfileModel,
-  validationCallbackUrl: ValidUrl,
-  timeStampGenerator: () => number,
+  emailValidationUrls: {
+    confirmValidationUrl: ValidUrl;
+    validationCallbackUrl: ValidUrl;
+  },
   profileEmails: IProfileEmailReader,
-  FF_UNIQUE_EMAIL_ENFORCEMENT_ENABLED: (fiscalCode: FiscalCode) => boolean,
-  confirmChoiceUrl: ValidUrl
+  FF_UNIQUE_EMAIL_ENFORCEMENT_ENABLED: (fiscalCode: FiscalCode) => boolean
 ): express.RequestHandler => {
   const handler = ValidateProfileEmailHandler(
     tableService,
     validationTokensTableName,
     profileModel,
-    validationCallbackUrl,
-    timeStampGenerator,
+    emailValidationUrls,
     profileEmails,
-    FF_UNIQUE_EMAIL_ENFORCEMENT_ENABLED,
-    confirmChoiceUrl
+    FF_UNIQUE_EMAIL_ENFORCEMENT_ENABLED
   );
 
   const middlewaresWrap = withRequestMiddlewares(
     ContextMiddleware(),
     TokenQueryParamMiddleware,
-    ChoiceConfirmQueryParamMiddleware
+    ConfirmEmailFlowQueryParamMiddleware
   );
   return wrapRequestHandler(middlewaresWrap(handler));
 };

ValidateProfileEmail/index.ts

@@ -65,11 +65,12 @@ app.get(
     tableService,
     VALIDATION_TOKEN_TABLE_NAME,
     profileModel,
-    validationCallbackValidUrl,
-    Date.now,
+    {
+      confirmValidationUrl: config.CONFIRM_CHOICE_PAGE_URL,
+      validationCallbackUrl: validationCallbackValidUrl
+    },
     profileEmailsReader,
-    FF_UNIQUE_EMAIL_ENFORCEMENT_ENABLED,
-    config.CONFIRM_CHOICE_PAGE_URL
+    FF_UNIQUE_EMAIL_ENFORCEMENT_ENABLED
   )
 );
 

utils/middleware.ts

@@ -19,19 +19,16 @@ export const TokenQueryParamMiddleware = RequiredQueryParamMiddleware(
 
 // CONFIRM -> verify token and on success redirect to confirm page
 // VALIDATE -> verify token and on success redirect to result page
-export enum FlowChoiceEnum {
+export enum FlowTypeEnum {
   "CONFIRM" = "CONFIRM",
   "VALIDATE" = "VALIDATE"
 }
-export const FlowChoice = enumType<FlowChoiceEnum>(
-  FlowChoiceEnum,
-  "FlowChoice"
-);
-export type FlowChoice = t.TypeOf<typeof FlowChoice>;
+export const FlowType = enumType<FlowTypeEnum>(FlowTypeEnum, "FlowChoice");
+export type FlowType = t.TypeOf<typeof FlowType>;
 
 // even if the query param is optional the withDefault type is covering the absence
 // of the value on the URL
-export const ChoiceConfirmQueryParamMiddleware = RequiredQueryParamMiddleware(
+export const ConfirmEmailFlowQueryParamMiddleware = RequiredQueryParamMiddleware(
   "flow",
-  withDefault(FlowChoice, FlowChoiceEnum.CONFIRM)
+  withDefault(FlowType, FlowTypeEnum.CONFIRM)
 );

utils/redirect_url.ts

@@ -20,25 +20,19 @@ export const confirmChoicePageUrl = (
  * Returns a ValidUrl that represents a successful validation
  */
 export const validationSuccessUrl = (
-  validationCallbackUrl: ValidUrl,
-  timeStampGenerator: () => number
+  validationCallbackUrl: ValidUrl
 ): ValidUrl =>
   ({
-    href: `${
-      validationCallbackUrl.href
-    }?result=success&time=${timeStampGenerator()}`
+    href: `${validationCallbackUrl.href}?result=success}`
   } as ValidUrl);
 
 /**
  * Returns a ValidUrl that represents a failed validation
  */
 export const validationFailureUrl = (
   validationCallbackUrl: ValidUrl,
-  error: keyof typeof ValidationErrors,
-  timeStampGenerator: () => number
+  error: keyof typeof ValidationErrors
 ): ValidUrl =>
   ({
-    href: `${
-      validationCallbackUrl.href
-    }?result=failure&error=${error}&time=${timeStampGenerator()}`
+    href: `${validationCallbackUrl.href}?result=failure&error=${error}}`
   } as ValidUrl);