-
Notifications
You must be signed in to change notification settings - Fork 13
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
News moderation email magic links #1600
Conversation
Looks good, But let's change the expiration to 1 day. ok to merge once that is done and Dave signs off on it |
One note about the "Decline" button - we had some discussions and at least for this iteration we are not going to implement it, and it would significantly increase the scope of the project, and would leave other questions unanswered. Specifically:
Considering these issues, we are going to move forward with the current solution, and create a new issue if it's decided that more needs to be done. |
news/notifications.py
Outdated
|
||
from .acl import moderators | ||
|
||
User = get_user_model() | ||
NEWS_APPROVAL_SALT = "news-approval" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor thing: I'd move these to the constants file
news/tests/test_notifications.py
Outdated
|
||
def test_generate_magic_approval_link(make_entry, make_user): | ||
entry = make_entry() | ||
# entry = make_entry(NEWS_MODELS[0]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor: this comment could be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of very small things, otherwise looks good to go.
0f8e0a3
to
08dbd65
Compare
Here's what the email looks like:
Fixes #1586