Add support for alternate URL

leafac / kill-the-newsletter

Convert email newsletters into Atom feeds

https://kill-the-newsletter.com

MIT License

2.31k stars 113 forks source link

Add support for alternate URL #21

Closed johanholmerin closed 3 years ago

johanholmerin commented 3 years ago

When using Kill the Newsletter with IFTTT and Instapaper, the alternate link is always used, which leads to the wrong content being saved. This adds support for saving and viewing the original email content.

leafac commented 3 years ago

This looks great! Thanks for your contribution. I’ll review it in detail and deploy it soon.

In the meanwhile, can you please add some tests?

Oh, and also something to delete stuff from static/alternate/ when removing old entries? Otherwise these files will just pile up and take up all the storage space…

johanholmerin commented 3 years ago

How's this?

leafac commented 3 years ago

Thank you very much for the pull request 😃

I’m checking in to tell you that I’m working on it and will merge it soon. I’m doing things such as refactoring the code to remove repeated logic between adding the first entry to the feed (the one that says “You Inbox was Created”) and the other entries as emails arrive. I’m also going to change the tests to use JSDOM as you did instead of regular expressions like I had. Your idea is much better.

leafac commented 3 years ago

Your contribution was merged and deployed to https://www.kill-the-newsletter.com. Thanks.

You may interested in reading https://github.com/leafac/www.kill-the-newsletter.com/commit/80ae8fab52b38b7d76a79edab3f58b455c79b436, which are the changes I made before merging. Here are the highlights:

I’m only creating alternate HTML files if the inbox exists. Emails to non-existent@kill-the-newsletter.com are considered spam and are silently ignored.
I’m only creating alternate HTML files if the email isn’t too big as to be discarded.
I extracted the addition of an entry to the feed into its own function, which is called both when the feed is created to add the “Your inbox was created” entry as well as when an email arrives.
I modified the tests to use JSDOM instead of regular expressions.

johanholmerin commented 3 years ago

Great. I checked it with a newsletter and it seems to be working. Thank you.