Add plaintext toggle

[janboll]

Description

Adding a toggle, that makes it possible to send E-Mails as unformatted Plain text.

Motivation and Context

I want to send E-Mails as unformatted Plain Text. Right now the mail service sends out e-mails as HTML only.

How Has This Been Tested?

• Tested locally using implementation.
• Added some tests to ensure fields are set correctly.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation (no code change)
• Refactor (refactoring production code)
• Other

Checklist:

• My code follows the code style of this project.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

[janboll]

Hi, @nikoksr, @svaloumas, is there anything I can do to get this PR reviewd?

[nikoksr]

Hi @janboll, sorry for the delay! We're both rather caught up atm. I'll throw a look at it later today tho, would that help you out? We greatly appreciate your contribution!

[janboll]

Of course, thank you for the feedback!

[nikoksr]

Hi @janboll, looking at your PR, everything looks very good! We appreciate it.

The only thing that came to my mind is the maintainability of your solution. A solution I would personally like better would be if instead of using fixed text formats, which require functions, like func (m Mail) UsePlainTextBody() { ... }, we rather use the text formats as parameters to a function like func (m Mail) UseTextFormat(format string) { ... } and continue working with it internally.

The advantage would be that we can repeat the same pattern for other services and not use n functions in the style UseXXXTextBody() { ... }. What if we need to support HTML, JSON, XML, Text etc? I am sure you get my point.

Please let me know what you think and if you have a better suggestion, so that we can get this merged asap!

cc @svaloumas adding you in case you have any additions to this.

[janboll]

That makes sense. I'll refactor the PR!

[janboll]

I refactored the change a little to use a single function to set the correct body format settings (With the naming of the method I sicked to E-Mail terminology).

I am unsure if the solution I proposed meets your expectations. Cause reading your comment again, did you think about changing the Notifier interface and adding methods to format the message?

[nikoksr]

@janboll appreciate the quick fix! No, you did absolutely the right thing, no worries there! I don't want it to be part of the Notifier interface since this behavior should stay service specific imo.

I'll fix the linter issues for you real quick and then we're ready to merge! Thank you so much for your time and efforts.

[codecov-commenter]

Codecov Report

Base: 73.73% // Head: 73.43% // Decreases project coverage by -0.29% ⚠️

Coverage data is based on head (664763a) compared to base (1e6d668).
Patch coverage: 70.58% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #476      +/-   ##
==========================================
- Coverage   73.73%   73.43%   -0.30%     
==========================================
  Files          36       37       +1     
  Lines        1104     1148      +44     
==========================================
+ Hits          814      843      +29     
- Misses        213      228      +15     
  Partials       77       77              

Impacted Files	Coverage Δ
service/mail/mail.go	65.90% <70.58%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[nikoksr]

Ignoring codecov complaining here. Yes, send method should be tested but since the mail package was untested prior to your PR and you've only added funcitonality and did not implement the whole service, I don't expect you to write tests for the whole service and see this as an overall improvement nonetheless.