feat(support-notifications): use notification content type #2699
Conversation
|
I just looked through the docs and the diagrams already mention content type and defaulting to text/plain, I am not sure there is anything to do there. https://docs.edgexfoundry.org/1.2/microservices/support/notifications/Ch-AlertsNotifications/ |
|
Build failure appears to be when trying to build Kong. Don't think it is related to my change but please let me know if I missed something. |
|
|
||
| stringResult := string(result) | ||
|
|
||
| assert.Equal(t, fmt.Sprintf("Subject: %s\r\n\r\n%s\r\n", subject, message), stringResult) |
There was a problem hiding this comment.
Like to see expected set outside the assert for readability. Here and the other tests
There was a problem hiding this comment.
fixed both of these and rebased
|
@AlexCuse , code looks good. Please tweak your commit message to follow our new Conventional Commit guidelines here: Specifically. The issue number should be in the body as |
splitting Use ContentType defined on notification on HTTP header for HTTP channel and MIME header for SMTP channel. Also add chunking with newlines for long SMTP message lines. Closes #2699 Signed-off-by: Alex Ullrich <alexullrich@technotects.com>
|
Kudos, SonarCloud Quality Gate passed!
|
|
@lenny-intel should be good now thanks. I wanted to include the line splitting in title but worried it'd be too long. I rebased the branch again too just in case. |
Use ContentType defined on notification on HTTP header for HTTP channel
and MIME header for SMTP channel. Also add chunking with newlines for
long SMTP messages.
Signed-off-by: Alex Ullrich alexullrich@technotects.com
PR Checklist
Please check if your PR fulfills the following requirements:
make testhas completed successfullyPR Type
What kind of change does this PR introduce?
Not sure if a bug per se but current implementation disregards the content type set on the notification, and also runs into problems sending long SMTP messages
What is the current behavior?
Issue Number: #2682
What is the new behavior?
Includes content type specified on the notification in the HTTP header for REST transmission, and in the MIME header for SMTP transmission. Also introduces chunking of SMTP payloads for messages longer than 1000 characters.
Does this PR introduce a breaking change?
Are there any new imports or modules? If so, what are they used for and why?
No
Are there any specific instructions or things that should be known prior to reviewing?
I mentioned ability to specify character set and general sending service refactoring in the linked issue but not addressing at this time. Not going to pursue 'smarter' chunking using bytes.Runes until I find a failure case with the current implementation - not sure if this will come with expanding charset support or not.
Other information