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

DeviseTokenAuth::Url.generate should accept relative path #1252

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mengqing
Copy link

When a relative url or empty string is passed to DeviseTokenAuth::Url.generate, it should parse and return the same relative url back.

@mengqing mengqing changed the title DeviseTokenAuth::Url.generate should accept relate path DeviseTokenAuth::Url.generate should accept relative path Jan 24, 2019
Copy link
Collaborator

@MaicolBen MaicolBen left a comment

Choose a reason for hiding this comment

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

@mengqing nice refactor! can you rebase and fix the failing specs ?

@mengqing
Copy link
Author

Hi @MaicolBen, I'm not exactly sure why the spec failed. I've tried testing them locally and they all passed. Do you have any idea on why the spec is failing?

@@ -4,15 +4,10 @@ module DeviseTokenAuth::Url

def self.generate(url, params = {})
uri = URI(url)
query_params = Hash[URI.decode_www_form(uri.query || '')].merge(params)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The problem is that expiry gets lost here in rails 4.2 (/controllers/devise_token_auth/passwords_controller_test.rb:207). I thought URI is from ruby, so it's weird

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