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.

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

♻️ enhances emails plugin and 🐛 fixes missing fields ( ⚠️ devops ) #3822 #3835

Merged
merged 21 commits into from
Feb 6, 2023

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Feb 3, 2023

What do these changes do?

Follows up on #3822: emails sent by osparc are classified as spam because some fields are missing. This PR adds missing MIME fields and creates a new entrypoint to test emails directly in the deploy (admin privileges needed). This should help spotting other issues that might be missing in the email server configured in the stack.

⚠️ devops:

There should be one (and one only) admin account per deployment (for now, best with mail our common devops mail adress osparcio-devops =at= speag.swiss). No other user (even team members) should have admin privileges. The admin-account will go into the e2e-ops, into the credentials page. This needs to be done upon releasing this PR.

Hightlights

  • ♻️ Moves email and text templates helpers to email plugin
    • New POST /email:test for admin to manually test templates and email server (i.e. admin privileges required)
  • 🐛 Completes required MIME date field

Related issue/s

How to test

  • New entrypoint was designed to manually test with the actual email server

image

Checklist

  • make version-*
  • make openapi.json
  • Unit tests for the changes exist
  • Runs in the swarm
  • Documentation reflects the changes

@pcrespov pcrespov self-assigned this Feb 3, 2023
@codecov
Copy link

codecov bot commented Feb 3, 2023

Codecov Report

Merging #3835 (503ea4c) into master (d4874cb) will increase coverage by 5.1%.
The diff coverage is 79.4%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #3835     +/-   ##
========================================
+ Coverage    81.5%   86.7%   +5.1%     
========================================
  Files         883     747    -136     
  Lines       38876   33017   -5859     
  Branches      766     393    -373     
========================================
- Hits        31721   28646   -3075     
+ Misses       6963    4262   -2701     
+ Partials      192     109     -83     
Flag Coverage Δ
integrationtests 52.3% <50.6%> (?)
unittests 84.6% <79.4%> (+3.0%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../web/server/src/simcore_service_webserver/utils.py 56.3% <66.6%> (+0.4%) ⬆️
...server/src/simcore_service_webserver/email_core.py 72.2% <72.2%> (ø)
...er/src/simcore_service_webserver/email_handlers.py 91.3% <91.3%> (ø)
.../web/server/src/simcore_service_webserver/email.py 93.7% <100.0%> (+0.8%) ⬆️
...src/simcore_service_webserver/login/utils_email.py 93.3% <100.0%> (+26.2%) ⬆️
...er/src/simcore_service_webserver/security_roles.py 100.0% <100.0%> (ø)
...elib/fastapi/long_running_tasks/_error_handlers.py
...c/servicelib/aiohttp/long_running_tasks/_server.py
...rvice-library/src/servicelib/aiohttp/monitoring.py
...dask_task_models_library/container_tasks/docker.py
... and 275 more

@pcrespov pcrespov force-pushed the is3822/mime-emails branch 2 times, most recently from e6565f6 to e58eb70 Compare February 6, 2023 12:29
@pcrespov pcrespov added the a:webserver issue related to the webserver service label Feb 6, 2023
@pcrespov pcrespov added this to the Resistance Is Futile milestone Feb 6, 2023
@pcrespov pcrespov changed the title ♻️ enhances emails plugin and addressed #3822 ♻️ enhances emails plugin and 🐛 fixes missing fields #3822 Feb 6, 2023
@pcrespov pcrespov marked this pull request as ready for review February 6, 2023 13:01
@mrnicegyu11 mrnicegyu11 changed the title ♻️ enhances emails plugin and 🐛 fixes missing fields #3822 ♻️ enhances emails plugin and 🐛 fixes missing fields ( ⚠️ devops ) #3822 Feb 6, 2023
Copy link
Member

@mrnicegyu11 mrnicegyu11 left a comment

Choose a reason for hiding this comment

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

Thanks a lot, truly <3 !

Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

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

👍

@codeclimate
Copy link

codeclimate bot commented Feb 6, 2023

Code Climate has analyzed commit 503ea4c and detected 0 issues on this pull request.

View more on Code Climate.

@sonarcloud
Copy link

sonarcloud bot commented Feb 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:webserver issue related to the webserver service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants