Skip to content

[Fix] Convert Markdown To HTML Email Body#266

Merged
canihavesomecoffee merged 7 commits intoCCExtractor:masterfrom
thealphadollar:fixes_#264
May 24, 2019
Merged

[Fix] Convert Markdown To HTML Email Body#266
canihavesomecoffee merged 7 commits intoCCExtractor:masterfrom
thealphadollar:fixes_#264

Conversation

@thealphadollar
Copy link
Copy Markdown
Contributor

@thealphadollar thealphadollar commented May 20, 2019

Please prefix your pull request with one of the following: [FEATURE] [FIX] [IMPROVEMENT].

In raising this pull request, I confirm the following (please check boxes):

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

My familiarity with the project is as follows (check one):

  • I have never used the project.
  • I have used the project briefly.
  • I have used the project extensively, but have not contributed previously.
  • I am an active contributor to the project.

In reference to issue #264 , the PR uses the HTML method of mailgun API to send the issue body (originally in markdown) by first converting it into HTML using markdown2 along with extras to add more features and convert the task list.

Below is an example of how the emails would look after the PR is merged.

OLD

Screenshot from 2019-05-21 00-48-17

NEW

Screenshot from 2019-05-21 00-47-46

The unit-test for inform_mailing_list function has been updated accordingly.

fixes #264

@pep8speaks
Copy link
Copy Markdown

pep8speaks commented May 20, 2019

Hello @thealphadollar, Thank you for updating the PR!

Great work! I discovered no PEP8 issues in this Pull Request. 🥇

Comment last updated at 2019-05-23 06:52:24 UTC

@thealphadollar
Copy link
Copy Markdown
Contributor Author

cc/ @canihavesomecoffee @satyammittal Please approve :)

@codecov
Copy link
Copy Markdown

codecov Bot commented May 20, 2019

Codecov Report

Merging #266 into master will increase coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #266      +/-   ##
==========================================
+ Coverage   65.06%   65.14%   +0.07%     
==========================================
  Files          34       34              
  Lines        3051     3058       +7     
  Branches      375      375              
==========================================
+ Hits         1985     1992       +7     
  Misses        960      960              
  Partials      106      106
Impacted Files Coverage Δ
mod_ci/controllers.py 41.28% <100%> (+0.6%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6c3c670...086251a. Read the comment docs.

@aadibajpai
Copy link
Copy Markdown
Member

before/after pics?

@thealphadollar
Copy link
Copy Markdown
Contributor Author

before/after pics?

I had added them but were not properly uploaded; rectified now. Please leave your views and suggestions 😄

Copy link
Copy Markdown
Member

@canihavesomecoffee canihavesomecoffee left a comment

Choose a reason for hiding this comment

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

Remind me this evening to merge & install the new dependency on the live version :)

Copy link
Copy Markdown
Member

@canihavesomecoffee canihavesomecoffee left a comment

Choose a reason for hiding this comment

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

Could you add a unittest that tests the new method you added? :)

@thealphadollar
Copy link
Copy Markdown
Contributor Author

Could you add a unittest that tests the new method you added? :)

Will do :)

@thealphadollar
Copy link
Copy Markdown
Contributor Author

@canihavesomecoffee Test has been added.

Remove by-mistake commited file.
Comment thread tests/test_ci/TestControllers.py Outdated
Comment thread test.txt Outdated
Comment thread tests/test_ci/TestControllers.py Outdated
Comment thread tests/test_ci/TestControllers.py Outdated
Comment thread tests/test_ci/TestControllers.py Outdated
Copy link
Copy Markdown
Member

@canihavesomecoffee canihavesomecoffee left a comment

Choose a reason for hiding this comment

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

Almost there 👍

Comment thread tests/test_ci/TestControllers.py
Comment thread tests/test_ci/TestControllers.py Outdated
Comment thread tests/test_ci/TestMarkdown.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Markdown rendering in email

4 participants