search

airbnb / knowledge-repo

A next-generation curated knowledge sharing platform for data scientists and other technical professions.
Apache License 2.0
5.48k stars 689 forks source link

Email default_recipients value for subscription emails #306

Open rahulravindran0108 opened 7 years ago

rahulravindran0108 commented 7 years ago

Auto-reviewers: @NiharikaRay @matthewwardrop @earthmancash @danfrankj

Why does this line exists ? https://github.com/airbnb/knowledge-repo/blob/master/knowledge_repo/app/utils/emails.py#L90 shouldn't it be an environment variable that an organization can set ?

matthewwardrop commented 7 years ago

Hi @rahulravindran0108 ,

Right you are, sir! There's still a lot of code cleanup to do before 1.0.0, so if you find anything else, let us know :). I'll clean this up soon!

M

rahulravindran0108 commented 7 years ago

Cool. Let me know if you want me to send over a PR ?

I would suggest removing the default recipient altogether and just use recipient bcc in here or if there is an environment variable for default recipient then use that instead.

Even with the code cleanups its a pretty solid product :). I enjoyed setting it up for my org.

matthewwardrop commented 7 years ago

Thanks for the offer. I'm about to land some fairly hefty changes around authentication in #305 , which will lead to a lot of legacy code like this needing cleaning up at some point. I plan to go over the code reasonably soon to fix this.

The authentication patches remove the notion of a 'default' user, and so a lot of this older logic will just disappear :).

jihonrado commented 5 years ago

Hi @matthewwardrop. This is just a reminder that this has not been made configurable yet through an environment variable. Thanks!