Dashboard

I would like to rename this to NotificationChannel, to indicate that it's not simply a type of message, but represents the channel through which the message is sent.

I would like to rename this to NotificationChannel, to indicate that it's not simply a type of message, but represents the channel through which the message is sent.

Looks good to me. Will there be a review for the cross-service migration?

Looks good to me. Will there be a review for the cross-service migration?

Do we think it's a good idea to have random phone numbers, especially once we implement SMS notification?

Do we think it's a good idea to have random phone numbers, especially once we implement SMS notification?

Is isTrue necessary? Isn't emailVerified a boolean?

Is isTrue necessary? Isn't emailVerified a boolean?

Does this schema allow each message to have a key like "email" or "sms"?

Does this schema allow each message to have a key like "email" or "sms"?

There are a lot of things happening here, but it's still not clear. Can this be refactored, either by putting some in descriptive helper functions, or encapsulating some of the condition checking i...

There are a lot of things happening here, but it's still not clear. Can this be refactored, either by putting some in descriptive helper functions, or encapsulating some of the condition checking in more readable terms?

I mean why plain text vs JSON?

I mean why plain text vs JSON?

I mean ref distro uses a Postgres 9.6 db image.

I mean ref distro uses a Postgres 9.6 db image.

agreed. I have updated the tests.

agreed. I have updated the tests.

OLMIS-4867: address code review feedbacks

renamed test cases for better clarity.

Seems like this error message doesn't quite seem to describe correctly when this constraint is violated, so it seems like we should have a separate message.

Seems like this error message doesn't quite seem to describe correctly when this constraint is violated, so it seems like we should have a separate message.

We might just want to say it's "request is invalid" here. We might require service-level tokens now, but that could potentially change in the future.

We might just want to say it's "request is invalid" here. We might require service-level tokens now, but that could potentially change in the future.

I thought it returns a 401 if a token is not provided.

I thought it returns a 401 if a token is not provided.

We should make this more generic, "with provided messages."

We should make this more generic, "with provided messages."

So perhaps we should describe the test cases as "should show error message when server responded with OK but transfer was not complete" and "should show error message when server responded with err...

So perhaps we should describe the test cases as "should show error message when server responded with OK but transfer was not complete" and "should show error message when server responded with error message". I like these test cases to be clear because sometimes they are the best way to understand what the code is supposed to do.

OLMIS-4963: Add a Jenkinsfile

OLMIS-4963: Add a Jenkinsfile

OLMIS-4963: Add a Jenkinsfile

OLMIS-4963: Add a Jenkinsfile

OLMIS-4963: Add a Jenkinsfile

OLMIS-4963: Add a Jenkinsfile

OLMIS-4963: Add a Jenkinsfile