Skip to content

Use site_configuration helper to get url base for activation email - #132

Merged
bryanlandia merged 1 commit into
appsembler/ficus/developfrom
appsembler/ficus/feature/fix-activation-email-for-sites
Jul 5, 2017
Merged

Use site_configuration helper to get url base for activation email#132
bryanlandia merged 1 commit into
appsembler/ficus/developfrom
appsembler/ficus/feature/fix-activation-email-for-sites

Conversation

@bryanlandia

Copy link
Copy Markdown

Student account activation email was not respecting SITE_NAME (which is actually a domain name) from openedx.core.djangoapps.site_configuration.helpers. This PR fixes that.

@bryanlandia
bryanlandia requested review from amirtds and tkeemon June 26, 2017 21:42
@bryanlandia

Copy link
Copy Markdown
Author

This is an issue on Eucalyptus, too, working through some conflicts on my local before fixing there.

@bryanlandia

Copy link
Copy Markdown
Author

@tkeemon

tkeemon commented Jun 27, 2017

Copy link
Copy Markdown

@bryanlandia I'm guessing this is a problem because the user is creating an account through a microsite. e.g.:

https://microsite1.customer_url.com/register

and the activation email is returning something like:

https://NONE/activate/1234...

Is that what you're seeing? It's been a while since I've played with microsites, but have you tried overriding SITE_NAME in the microsite JSON config (or are we using database driven microsites at this point?)?

With this PR, it looks like you're hardcoding the activation email to be:

https://customer_url.com/activate/1234...

Is there any case where you'd like that URL to be:

https://microsite1.customer_url.com/activate/1234..

?

@bryanlandia

Copy link
Copy Markdown
Author

@tkeemon - Yep, some users are signing up at microsite.domain.com/register but were getting activation emails with links to domain.com/activate/xxxxxx. This site uses the newer Site, Site Theme and Site Configuration setup for microsites. An override of SITE_NAMEcan be set in a SiteConfiguration object and paired with a Site (subdomain), so, yes, that's the approach I'm taking here. All the other email related templates I see in edx-platform have already been fixed to take this into account, just not the activation email for some reason.

@tkeemon

tkeemon commented Jun 28, 2017

Copy link
Copy Markdown

Great. All of that sounds good.

If this is still a problem upstream, can you please open a PR there as well.

@tkeemon tkeemon left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

@bryanlandia
bryanlandia merged commit c12764b into appsembler/ficus/develop Jul 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants