Skip to content
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

Add subscription link support to notification email verification template #573

Merged
merged 3 commits into from
Jan 21, 2020
Merged

Add subscription link support to notification email verification template #573

merged 3 commits into from
Jan 21, 2020

Conversation

paskal
Copy link
Collaborator

@paskal paskal commented Jan 19, 2020

Address #560, the backend part.

When NOTIFY_EMAIL_SUBSCRIBE_URL environment variable is provided, instead of embedding token to email it will be included as a link.

With SITE=http://127.0.0.1 and NOTIFY_EMAIL_SUBSCRIBE_URL=/subscribe?token=, link will look like http://127.0.0.1/subscribe?token=<subscription_token>. The token is not escaped and I'll escape it if it will be necessary when the frontend part will be written.

After the frontend counterpart will be created, NOTIFY_EMAIL_SUBSCRIBE_URL default should be set to newly created URL location.

That's how verification email looks like when subscribe_url is supplied:

image

@paskal paskal added the backend label Jan 19, 2020
@paskal paskal requested review from umputun and akellbl4 January 19, 2020 19:59
@akellbl4
Copy link
Collaborator

I thought it well be smth like this
Screenshot 2020-01-20 at 16 12 49

Two methods at the same time.

@umputun
Copy link
Owner

umputun commented Jan 20, 2020

I think we should keep both methods

upd: The reason why both - some people refuse to click the email's link as a part of their safety/security routine. We don't want to force them to click

@paskal
Copy link
Collaborator Author

paskal commented Jan 20, 2020

Updated, please see new look below.

image

@umputun
Copy link
Owner

umputun commented Jan 20, 2020

LGTM, but we can't merge it till UI side is ready

@umputun
Copy link
Owner

umputun commented Jan 20, 2020

my bad, looks like {{if .SubscribeURL}} makes it safe to merge

@umputun umputun merged commit f97d823 into umputun:master Jan 21, 2020
@paskal paskal deleted the subscription_by_link branch January 21, 2020 00:33
@umputun umputun added this to the v1.6 milestone Apr 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants