Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

UserMailer#reset_password_instructions has incorrect reset URL #511

Closed
mguterl opened this Issue · 8 comments

3 participants

@mguterl

UserMailer.default_url_options[:host] is being set to Spree::Config[:site_url]. This behavior is incorrect and results in the mailer generating URLs with "http://" twice. Example: http://http://store.recruitmilitary.com

I looked at Spree::Config and I did not see an actual host property, maybe it should be added? Spree::Config[:site_host] could be inferred from URI.parse(Spree::Config[:site_url]).host.

One other alternative is to use something like: https://github.com/ahoward/default_url_options to handle this more intelligently.

@mguterl

After looking at this some more, I have discovered that Spree::Config[:site_url] is actually expected to be Spree::Config[:site_host]. Should this be changed to reflect what it actually is?

@joneslee85
Collaborator

What version of spree are you on?

In fact, the Spree::Config[:site_host] does not exist in the upstream branch. And Spree::Config[:site_url] returns URL w/o the http://.

@mguterl

I'm probably using an older version, but I think it is important to be consistent. If it says it is a url, it should return a URL http:// and all that. If it says it is a host, then it should return just the host. I think it's reasonable to think that host would be set and site_url could be based off of the site_host.

So if what @joneslee85 says is correct, I think Spree::Config[:site_url] should be renamed to Spree::Config[:site_host].

@joneslee85
Collaborator

What version are you on? You're welcome to submit a pull request ;)

@radar
Collaborator

Any updates on this?

@mguterl

We're still running an old version of Spree and we're working around it. I believe everything that I say above still holds true.

@radar
Collaborator

Right, could you submit a patch to fix this issue though? Keep in mind that some people will have preferences stored in their database that still have the "incorrect" key, so you'll need to migrate those.

@radar
Collaborator

I will be closing this issue in a week's time as no patch has been provided. If you would like to see this issue fixed, please submit a patch.

@radar radar closed this
@radar radar reopened this
@radar radar closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.