Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IOCIT-141] Handle user preferences for push notification format #232

Merged
merged 7 commits into from
Nov 16, 2022
Merged

[IOCIT-141] Handle user preferences for push notification format #232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ec1696b
[#IOCIT-141] add user profile reader + domain errors
gquadrati Nov 9, 2022
dcd1063
[#IOCIT-141] add new management for verbose notification
gquadrati Nov 9, 2022
8e10d57
[#IOCIT-141] add unit tests
gquadrati Nov 9, 2022
0cd79bf
[#IOCIT-141] rename user-profile.reader.ts to user-profile.ts
gquadrati Nov 10, 2022
7908370
[#IOCIT-141] add ErrorKind enum and use it
gquadrati Nov 10, 2022
60ac8fb
[#IOCIT-141] fix after refactor based on comments
gquadrati Nov 10, 2022
5ded0e7
[#IOCIT-141] fix TE.getOrElse return type on userProfile
gquadrati Nov 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 129 additions & 3 deletions WebhookNotification/__tests__/handler.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
import { CreatedMessageEventSenderMetadata } from "@pagopa/io-functions-commons/dist/src/models/created_message_sender_metadata";
import { Notification } from "@pagopa/io-functions-commons/dist/src/models/notification";
import { isTransientError } from "@pagopa/io-functions-commons/dist/src/utils/errors";
import { PushNotificationsContentTypeEnum } from "@pagopa/io-functions-commons/dist/generated/definitions/PushNotificationsContentType";

import { getWebhookNotificationHandler, sendToWebhook } from "../handler";

Expand All @@ -38,6 +39,11 @@ import * as TE from "fp-ts/lib/TaskEither";
import * as O from "fp-ts/lib/Option";
import { StandardServiceCategoryEnum } from "../../generated/api-admin/StandardServiceCategory";

import { toInternalError, toNotFoundError } from "../../utils/domain-errors";
import { UserProfileReader } from "../../readers/user-profile";

import { aRetrievedProfile } from "../../__mocks__/mocks";

const mockAppinsights = {
trackDependency: jest.fn(),
trackEvent: jest.fn()
Expand Down Expand Up @@ -126,16 +132,25 @@ const aCommonMessageData = {
senderMetadata: aSenderMetadata
};

const aRetrievedProfileWithFullPushNotificationsContentType = {
...aRetrievedProfile,
pushNotificationsContentType: PushNotificationsContentTypeEnum.FULL
};

const mockRetrieveProcessingMessageData = jest
.fn()
.mockImplementation(() => TE.of(O.some(aCommonMessageData)));

const userProfileReaderMock = jest.fn(
_ => TE.of(aRetrievedProfile) as ReturnType<UserProfileReader>
);

beforeEach(() => {
jest.clearAllMocks();
});

describe("sendToWebhook", () => {
it("should succeded with right parameters", async () => {
it("should succeded with right parameters, with message content", async () => {
const expectedResponse = { message: "OK" };
const fetchApi = mockFetch(200, expectedResponse);
const notifyApiCall = getNotifyClient(fetchApi as any);
Expand All @@ -145,8 +160,15 @@ describe("sendToWebhook", () => {
aMessage as any,
aMessageContent,
aSenderMetadata,
aRetrievedProfileWithFullPushNotificationsContentType,
false
)();

expect(fetchApi.mock.calls[0][1]).toHaveProperty("body");
const body = JSON.parse(fetchApi.mock.calls[0][1].body);
expect(body).toHaveProperty("message");
expect(body).toHaveProperty("sender_metadata");
expect(body.message).toHaveProperty("content");
expect(E.isRight(ret)).toBeTruthy();
});

Expand All @@ -163,15 +185,17 @@ describe("sendToWebhook", () => {
...aSenderMetadata,
requireSecureChannels: true
},
aRetrievedProfileWithFullPushNotificationsContentType,
false
)();
expect(fetchApi.mock.calls[0][1]).toHaveProperty("body");
const body = JSON.parse(fetchApi.mock.calls[0][1].body);
expect(body).toHaveProperty("message");
expect(body).toHaveProperty("sender_metadata");
expect(body).not.toHaveProperty("content");
expect(body.message).not.toHaveProperty("content");
expect(E.isRight(ret)).toBeTruthy();
});

it("should remove message content if webhook message content is disabled", async () => {
const expectedResponse = { message: "OK" };
const fetchApi = mockFetch(200, expectedResponse);
Expand All @@ -182,15 +206,38 @@ describe("sendToWebhook", () => {
aMessage as any,
aMessageContent,
aSenderMetadata,
aRetrievedProfileWithFullPushNotificationsContentType,
true
)();
expect(fetchApi.mock.calls[0][1]).toHaveProperty("body");
const body = JSON.parse(fetchApi.mock.calls[0][1].body);
expect(body).toHaveProperty("message");
expect(body).toHaveProperty("sender_metadata");
expect(body).not.toHaveProperty("content");
expect(body.message).not.toHaveProperty("content");
expect(E.isRight(ret)).toBeTruthy();
});

it("should remove message content if user did not allow verbose notifications", async () => {
const expectedResponse = { message: "OK" };
const fetchApi = mockFetch(200, expectedResponse);
const notifyApiCall = getNotifyClient(fetchApi as any);
const ret = await sendToWebhook(
notifyApiCall,
"http://localhost/test" as HttpsUrl,
aMessage as any,
aMessageContent,
aSenderMetadata,
aRetrievedProfile,
false
)();
expect(fetchApi.mock.calls[0][1]).toHaveProperty("body");
const body = JSON.parse(fetchApi.mock.calls[0][1].body);
expect(body).toHaveProperty("message");
expect(body).toHaveProperty("sender_metadata");
expect(body.message).not.toHaveProperty("content");
expect(E.isRight(ret)).toBeTruthy();
});

it("should return a transient error in case of timeout", async () => {
const abortableFetch = AbortableFetch(agent.getHttpsFetch(process.env));
const fetchWithTimeout = setFetchTimeout(1 as Millisecond, abortableFetch);
Expand All @@ -201,6 +248,7 @@ describe("sendToWebhook", () => {
aMessage as any,
aMessageContent,
aSenderMetadata,
aRetrievedProfile,
false
)();
expect(E.isLeft(ret)).toBeTruthy();
Expand All @@ -218,6 +266,7 @@ describe("sendToWebhook", () => {
{} as any,
{} as any,
{} as any,
aRetrievedProfile,
false
)();
expect(fetchApi).toHaveBeenCalledTimes(1);
Expand All @@ -236,6 +285,7 @@ describe("sendToWebhook", () => {
{} as any,
{} as any,
{} as any,
aRetrievedProfile,
false
)();
expect(fetchApi).toHaveBeenCalledTimes(1);
Expand All @@ -260,6 +310,7 @@ describe("handler", () => {
notificationModelMock as any,
{} as any,
mockRetrieveProcessingMessageData,
userProfileReaderMock,
false
)(mockContext, JSON.stringify(aNotificationEvent))
).rejects.toThrow();
Expand All @@ -275,6 +326,76 @@ describe("handler", () => {
notificationModelMock as any,
{} as any,
mockRetrieveProcessingMessageData,
userProfileReaderMock,
false
)(mockContext, JSON.stringify(aNotificationEvent))
).rejects.toThrow();
});

it("should return a transient error when an error occurred retrieving user profile", async () => {
const notificationModelMock = {
find: jest.fn(() => TE.of(O.some(aNotification))),
update: jest.fn(() => TE.of(O.some(aNotification)))
};

const notifyCallApiMock = jest
.fn()
.mockReturnValue(Promise.resolve(E.right({ status: 200 })));

mockRetrieveProcessingMessageData.mockImplementationOnce(() =>
TE.of(O.some({ ...aCommonMessageData, content: aMessageContent }))
);

userProfileReaderMock.mockImplementationOnce(() =>
TE.left(
toInternalError(
"an Error" as NonEmptyString,
"an Error detail" as NonEmptyString
)
)
);

await expect(
getWebhookNotificationHandler(
notificationModelMock as any,
notifyCallApiMock as any,
mockRetrieveProcessingMessageData,
userProfileReaderMock,
false
)(mockContext, JSON.stringify(aNotificationEvent))
).rejects.toThrow();
});

it("should return a transient error when user profile has not been found", async () => {
const notificationModelMock = {
find: jest.fn(() => TE.of(O.some(aNotification))),
update: jest.fn(() => TE.of(O.some(aNotification)))
};

const notifyCallApiMock = jest
.fn()
.mockReturnValue(Promise.resolve(E.right({ status: 200 })));

mockRetrieveProcessingMessageData.mockImplementationOnce(() =>
TE.of(O.some({ ...aCommonMessageData, content: aMessageContent }))
);

userProfileReaderMock.mockImplementationOnce(() =>
TE.left(
toNotFoundError(
"an Error" as NonEmptyString,
"an Error detail" as NonEmptyString,
"profile" as NonEmptyString
)
)
);

await expect(
getWebhookNotificationHandler(
notificationModelMock as any,
notifyCallApiMock as any,
mockRetrieveProcessingMessageData,
userProfileReaderMock,
false
)(mockContext, JSON.stringify(aNotificationEvent))
).rejects.toThrow();
Expand All @@ -290,6 +411,7 @@ describe("handler", () => {
notificationModelMock as any,
{} as any,
mockRetrieveProcessingMessageData,
userProfileReaderMock,
false
)(mockContext, JSON.stringify(aNotificationEvent))
).resolves.toEqual({ kind: "FAILURE", reason: "DECODE_ERROR" });
Expand All @@ -313,6 +435,7 @@ describe("handler", () => {
notificationModelMock as any,
notifyCallApiMock as any,
mockRetrieveProcessingMessageData,
userProfileReaderMock,
false
)(mockContext, JSON.stringify(aNotificationEvent));

Expand Down Expand Up @@ -347,6 +470,7 @@ describe("handler", () => {
notificationModelMock as any,
notifyCallApiMock,
mockRetrieveProcessingMessageData,
userProfileReaderMock,
false
)(mockContext, JSON.stringify(aNotificationEvent));

Expand Down Expand Up @@ -375,6 +499,7 @@ describe("handler", () => {
notificationModelMock as any,
notifyCallApiMock,
mockRetrieveProcessingMessageData,
userProfileReaderMock,
false
)(mockContext, JSON.stringify(aNotificationEvent))
).resolves.toEqual({ kind: "FAILURE", reason: "SEND_TO_WEBHOOK_FAILED" });
Expand Down Expand Up @@ -403,6 +528,7 @@ describe("handler", () => {
notificationModelMock as any,
{} as any,
mockRetrieveProcessingMessageData,
userProfileReaderMock,
false
)(mockContext, JSON.stringify(aNotificationEvent));

Expand Down
18 changes: 18 additions & 0 deletions WebhookNotification/handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ import {
} from "@pagopa/ts-commons/lib/requests";
import { flow, pipe } from "fp-ts/lib/function";
import { TaskEither } from "fp-ts/lib/TaskEither";
import { Profile } from "@pagopa/io-functions-commons/dist/src/models/profile";
import { PushNotificationsContentTypeEnum } from "@pagopa/io-functions-commons/dist/generated/definitions/PushNotificationsContentType";
import { Notification } from "../generated/notifications/Notification";
import { withJsonInput } from "../utils/with-json-input";
import { withDecodedInput } from "../utils/with-decoded-input";
Expand All @@ -50,6 +52,7 @@ import {
NotificationCreatedEvent
} from "../utils/events/message";
import { DataFetcher, withExpandedInput } from "../utils/with-expanded-input";
import { UserProfileReader } from "../readers/user-profile";
import { WebhookNotifyT } from "./client";

export const WebhookNotificationInput = NotificationCreatedEvent;
Expand Down Expand Up @@ -116,6 +119,7 @@ export const sendToWebhook = (
message: NewMessageWithoutContent,
content: MessageContent,
senderMetadata: CreatedMessageEventSenderMetadata,
userProfile: Profile,
disableWebhookMessageContent: boolean
// eslint-disable-next-line max-params
): TaskEither<RuntimeError, TypeofApiResponse<WebhookNotifyT>> =>
Expand All @@ -125,11 +129,14 @@ export const sendToWebhook = (
notifyApiCall({
notification: {
// If the service requires secure channels
// or user did not allow to receive verbose notifications
// or the message content is disabled for all services
// we send an empty (generic) push notification
// generic content is provided by `io-backend` https://github.com/pagopa/io-backend/blob/v7.16.0/src/controllers/notificationController.ts#L62
message:
senderMetadata.requireSecureChannels ||
userProfile.pushNotificationsContentType !==
PushNotificationsContentTypeEnum.FULL ||
disableWebhookMessageContent
? newMessageToPublic(message)
: newMessageToPublic(message, content),
Expand Down Expand Up @@ -183,6 +190,7 @@ export const getWebhookNotificationHandler = (
lNotificationModel: NotificationModel,
notifyApiCall: TypeofApiCall<WebhookNotifyT>,
retrieveProcessingMessageData: DataFetcher<CommonMessageData>,
userProfileReader: UserProfileReader,
disableWebhookMessageContent: boolean
) =>
withJsonInput(
Expand Down Expand Up @@ -251,12 +259,22 @@ export const getWebhookNotificationHandler = (
const webhookNotification =
errorOrWebhookNotification.right.channels.WEBHOOK;

const userProfile = await pipe(
userProfileReader({
fiscalCode: message.fiscalCode
}),
TE.getOrElse(err => {
throw new Error(err.title);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's better to throw the err title or the err message?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, since this error is needed for replaying the operation, I thought it was not useful to write detailed info.

})
)();

const sendResult = await sendToWebhook(
notifyApiCall,
webhookNotification.url,
message,
content,
senderMetadata,
userProfile,
disableWebhookMessageContent
)();
if (E.isLeft(sendResult)) {
Expand Down
10 changes: 10 additions & 0 deletions WebhookNotification/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import {
NOTIFICATION_COLLECTION_NAME,
NotificationModel
} from "@pagopa/io-functions-commons/dist/src/models/notification";
import {
ProfileModel,
PROFILE_COLLECTION_NAME
} from "@pagopa/io-functions-commons/dist/src/models/profile";

import { agent } from "@pagopa/ts-commons";

Expand All @@ -12,6 +16,7 @@ import {
} from "@pagopa/ts-commons/lib/fetch";
import { Millisecond } from "@pagopa/ts-commons/lib/units";
import { createBlobService } from "azure-storage";
import { getUserProfileReader } from "../readers/user-profile";
import { getConfigOrThrow } from "../utils/config";
import { cosmosdbInstance } from "../utils/cosmosdb";
import { CommonMessageData } from "../utils/events/message";
Expand Down Expand Up @@ -47,9 +52,14 @@ const retrieveProcessingMessageData = makeRetrieveExpandedDataFromBlob(
config.PROCESSING_MESSAGE_CONTAINER_NAME
);

const profileModel = new ProfileModel(
cosmosdbInstance.container(PROFILE_COLLECTION_NAME)
);

export default getWebhookNotificationHandler(
notificationModel,
notifyApiCall,
retrieveProcessingMessageData,
getUserProfileReader(profileModel),
config.FF_DISABLE_WEBHOOK_MESSAGE_CONTENT
);
Loading