Skip to content
This repository has been archived by the owner on Jan 13, 2022. It is now read-only.

Fix bug 1011001: Add support for optin param back to subscribe. #98

Merged
merged 1 commit into from
May 21, 2014

Conversation

pmclanahan
Copy link

Setting optin=Y will now require an API key and SSL to avoid people
being signed up without a confirmation step.

@pmclanahan
Copy link
Author

@Osmose feel like r? some Basket :)

@@ -120,7 +120,8 @@ The following URLs are available (assuming "/news" is app url):
"newsletters" field, which should be a comma-delimited list of
newsletters. "email" and "newsletters" are required. "optin" should
be Y or N depending if the user should automatically be opted in,
default is Y. "trigger_welcome" should be Y to fire off a welcome email::
default is N and setting Y requires a valid API key. "trigger_welcome"
Copy link

Choose a reason for hiding this comment

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

Future improvement suggestion: Explain what opting in means. :P

@Osmose
Copy link

Osmose commented May 19, 2014

r+. The additional test would be nice, the other two are just random thoughts. Nice work!

@pmclanahan
Copy link
Author

@Osmose updated. mind taking another look?

@pmclanahan
Copy link
Author

Well... We're planning a push soon, so I'll get this in. If you have opinions on things further I'll send another PR. Thanks again!

* Fix flake8 issue. Add flake8 config.
* Improve optin documentation in README and doc string.
* Also add some case-insensitivity tests.
@pmclanahan pmclanahan merged commit 8eddf59 into mozilla:master May 21, 2014
@pmclanahan pmclanahan deleted the fix-subscribe-optin branch May 21, 2014 16:58
@Osmose
Copy link

Osmose commented May 21, 2014

Luckily for you it looks great so that I have no more input! :P

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants