Skip to content

Add register mutation and alternate confirmation flow#185

Merged
mcelicalderon merged 3 commits intomasterfrom
add-alternate-account-confirmation-flow
Jun 6, 2021
Merged

Add register mutation and alternate confirmation flow#185
mcelicalderon merged 3 commits intomasterfrom
add-alternate-account-confirmation-flow

Conversation

@mcelicalderon
Copy link
Copy Markdown
Member

Implements a new register mutation as an alternative to sign_up which will be deprecated in the future. Also implements a confirm_account_with_token mutation which confirms a resource by providing a token.

register changes the account confirmation flow. It now takes a confirm_url argument which should be the url of a client application. The confirmation email will include a link to confirm_url with confirmationToken as part of the query string. Then, the client application should use the confirm_account_with_token mutation to confirm the resource if required.

Resolves #184

@mcelicalderon mcelicalderon added the enhancement New feature or request label May 30, 2021
@mcelicalderon mcelicalderon requested a review from 00dav00 May 30, 2021 04:16
@mcelicalderon mcelicalderon force-pushed the add-alternate-account-confirmation-flow branch from a476e74 to c7ca858 Compare May 31, 2021 01:49
if resource.save
yield resource if block_given?

unless resource.confirmed?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Will this consider skip_confirmation_notification ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not really related, skip_confirmation_notification simply prevents regular devise email from going out. DTA does the same

unless resource.confirmed?
resource.send_confirmation_instructions(
redirect_url: redirect_url,
template_path: ['graphql_devise/mailer']
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Starting to feel we can have a common place for this path


<p><%= link_to t('.confirm_account_link'), "#{message['schema_url']}?#{confirmation_query(resource_name: @resource.class.to_s, redirect_url: message['redirect-url'], token: @token).to_query}" %></p>
<p>
<% if message['schema_url'].present? %>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this path trigger a deprecation warning?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

not sure we should do that here. Plus, any value can be passed to message, not sure it's necessary

@mcelicalderon mcelicalderon enabled auto-merge June 6, 2021 21:04
@mcelicalderon mcelicalderon merged commit 2566c94 into master Jun 6, 2021
@mcelicalderon mcelicalderon deleted the add-alternate-account-confirmation-flow branch June 6, 2021 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Another click in confirm account results in error

2 participants