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

MNOE-1301 - modified the send-confirmation-email and reset-password-email methods… #797

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

Conversation

steven-chang-maestrano
Copy link

I was thinking this way, the macaw-enterprise( or whoever ) can define their own custom url as a method and mno-enterprise can use it if detected.

Let me know what you think,
had some other ideas but they were tricky to implement

… so that they have the option of using a custom url function if it's available
@steven-chang-maestrano steven-chang-maestrano changed the title modified the send-confirmation-email and reset-password-email methods… MNOE-1301 - modified the send-confirmation-email and reset-password-email methods… Nov 22, 2018
@adamaziz15
Copy link
Contributor

@ouranos @steven-chang-maestrano What if we overwrite the helpers in the mailer class? Then we could use the same url helpers, and allow the configuration to happen via env vars or settings. For example, maybe something like this:

# In settings.yml
auth:
  confirmation_path: 'auth/register/confirm'

# In system_notification_mailer.rb
def user_confirmation_url(opts = {})
  if Settings.auth.confirmation_path.present?
    "#{default_url_options[:protocol]}://#{default_url_options[:host]}/#{Settings.auth.confirmation_path}?#{opts.to_query}"
  else
    super
  end
end

@adamaziz15 adamaziz15 requested a review from ouranos November 26, 2018 18:05
@steven-chang-maestrano
Copy link
Author

@ouranos @steven-chang-maestrano What if we overwrite the helpers in the mailer class? Then we could use the same url helpers, and allow the configuration to happen via env vars or settings. For example, maybe something like this:

# In settings.yml
auth:
  confirmation_path: 'auth/register/confirm'

# In system_notification_mailer.rb
def user_confirmation_url(opts = {})
  if Settings.auth.confirmation_path.present?
    "#{default_url_options[:protocol]}://#{default_url_options[:host]}/#{Settings.auth.confirmation_path}?#{opts.to_query}"
  else
    super
  end
end

@adamaziz15 - It's not coupled to the 'mnoe/auth/users/confirmation' so I think your solution is better! Have you tested it to see if it works?

I was looking at a solution where we could overwrite the user_confirmation_url method as well but got stuck getting the root path! I like this defaul_url_options idea.

@adamaziz15
Copy link
Contributor

@steven-chang-maestrano I tested it a bit locally and it seems to be ok 👍

My first thought was to do this at a higher level since there are some other urls that will probably need to be configurable (password expiration page for sure), but I was worried about causing any unwanted side-effects with devise. 👮

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants