[IOCIT-141] Handle user preferences for push notification format #232
Conversation
Example of PR titles that include pivotal stories:
|
utils/domain-errors.ts
Outdated
| title: NonEmptyString, | ||
| detail: NonEmptyString, | ||
| objectName: NonEmptyString | ||
| ): NotFoundError => ({ detail, kind: "NotFound", objectName, title }); |
There was a problem hiding this comment.
Why not defining an enum for error kinds instead of always writing a string?
There was a problem hiding this comment.
It's not "always", because this is a method that builds a NotFoundError, so it's the only point in which that string will be written.
But I agree that having an enum is a best practice.
Done in 7908370
WebhookNotification/handler.ts
Outdated
| @@ -125,11 +130,14 @@ export const sendToWebhook = ( | |||
| notifyApiCall({ | |||
| notification: { | |||
| // If the service requires secure channels | |||
| // or user did not allow to receave verbose notifications | |||
WebhookNotification/handler.ts
Outdated
| fiscalCode: message.fiscalCode | ||
| }), | ||
| // We are using T.map(E.getOrElseW(..)) because | ||
| // TE.getOrElseW is unable to infer `userProfile` type |
There was a problem hiding this comment.
Is this an fp-ts bug?
There was a problem hiding this comment.
My bad. With TE.getOrElse it works fine.
Done in [#IOCIT-141] fix TE.getOrElse return type on userProfile
| @@ -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 { GetUserProfileReader } from "../../readers/user-profile.readers"; | |||
There was a problem hiding this comment.
The folder is readers, why calling the file .readers.ts?
There was a problem hiding this comment.
gquadrati
force-pushed
the
IOCIT-141--verbose-notification
branch
from
3a35752 to
60ac8fb
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
| fiscalCode: message.fiscalCode | ||
| }), | ||
| TE.getOrElse(err => { | ||
| throw new Error(err.title); |
There was a problem hiding this comment.
it's better to throw the err title or the err message?
There was a problem hiding this comment.
Not sure, since this error is needed for replaying the operation, I thought it was not useful to write detailed info.
List of Changes
Motivation and Context
Send push notification as defined by user.
How Has This Been Tested?
Unit test
Screenshots (if appropriate):
Types of changes
Checklist: