Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,9 @@ is `User`.
1. userSignUp
1. userUpdatePassword
1. userSendPasswordReset
1. `userResendConfirmation(email: String!, redirectUrl: String!): UserResendConfirmationPayload`
Comment thread
aarona marked this conversation as resolved.

The `UserResendConfirmationPayload` will return the `authenticable` resource that was sent the confirmation instructions but also has a `message: String!` that can be used to notify a user what to do after the instructions were sent to them and a `success: Boolean!` to indicate success.
#### Queries
1. userConfirmAccount
1. userCheckPasswordToken
Expand Down
32 changes: 32 additions & 0 deletions app/graphql/graphql_devise/mutations/resend_confirmation.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
module GraphqlDevise
module Mutations
class ResendConfirmation < Base
argument :email, String, required: true, prepare: ->(email, _) { email.downcase }
argument :redirect_url, String, required: true

field :message, String, null: false
Comment thread
aarona marked this conversation as resolved.

def resolve(email:, redirect_url:)
resource = controller.find_resource(:uid, email)

if resource
yield resource if block_given?

raise_user_error("Email #{I18n.t('errors.messages.already_confirmed')}") if resource.confirmed?
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why interpolate a localized string here? Just put the entire string in the locales file please

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The reason why I did this is because it seemed you wanted to fall back on Devise locales. The errors.message.already_confirmed value is: "was already confirmed, please try signing in". I can make this whole thing as a locale entry but I'll have to put it into the graphql_devise locale. Would you prefer that instead?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes please, move the entire message to our own locales file. I think the rule for localized messages should be that ANY message we use directly in OUR code, should go in our locales file. Otherwise we depend on the other gems to keep those keys on their locales files, if for any reason a version of those gems changes the messages, it could affect our gem. I'll have to check the rest of the code for the same thing, but for this PR, please make sure any localized message YOU use in the code has an entry in our locales file (the entire message always as some languages might have a completely different way to say something)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes check the /app/views/graphql_devise/mailer/reset_password_instructions.html.erb file because it also relies on Devise's locale file. I can create a seperate PR for that and fix that.

I'll fix mine and update this PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually it looks like both mailers do this. I'll create a PR for both later.


resource.send_confirmation_instructions({
redirect_url: redirect_url,
template_path: ['graphql_devise/mailer']
})

{
authenticable: resource,
message: I18n.t('devise.confirmations.send_instructions', email: email)
}
Comment thread
aarona marked this conversation as resolved.
else
raise_user_error(I18n.t('graphql_devise.confirmations.user_not_found', email: email))
end
end
end
end
end
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<p><%= t(:welcome).capitalize + ' ' + @email %>!</p>

<p><%= t '.confirm_link_msg' %> </p>
<p><%= t '.confirm_link_msg' %></p>

<p><%= link_to t('.confirm_account_link'), url_for(controller: 'graphql_devise/graphql', action: :auth, **confirmation_query(resource_name: @resource.class.to_s, redirect_url: message['redirect-url'], token: @token)) %></p>
4 changes: 4 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ en:
not_confirmed: "A confirmation email was sent to your account at '%{email}'. You must follow the instructions in the email before your account can be activated"
confirmations:
invalid_token: "Invalid confirmation token. Please try again"
user_not_found: "Unable to find user with email '%{email}'."
Comment thread
aarona marked this conversation as resolved.
mailer:
confirmation_instructions:
confirm_link_msg: "You can confirm your account email through the link below:"
confirm_account_link: "Confirm my account"
unlock_instructions:
account_lock_msg: "Your account has been locked due to an excessive number of unsuccessful sign in attempts."
3 changes: 2 additions & 1 deletion lib/graphql_devise/rails/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ def mount_graphql_devise_for(resource, opts = {})
logout: GraphqlDevise::Mutations::Logout,
sign_up: GraphqlDevise::Mutations::SignUp,
update_password: GraphqlDevise::Mutations::UpdatePassword,
send_password_reset: GraphqlDevise::Mutations::SendPasswordReset
send_password_reset: GraphqlDevise::Mutations::SendPasswordReset,
resend_confirmation: GraphqlDevise::Mutations::ResendConfirmation
}.freeze
default_queries = {
confirm_account: GraphqlDevise::Resolvers::ConfirmAccount,
Expand Down
83 changes: 83 additions & 0 deletions spec/requests/mutations/resend_confirmation_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
require 'rails_helper'

RSpec.describe 'Resend confirmation' do
include_context 'with graphql query request'

let(:user) { create(:user, confirmed_at: nil) }
let(:email) { user.email }
let(:id) { user.id }
let(:redirect) { Faker::Internet.url }
let(:query) do
<<-GRAPHQL
mutation {
userResendConfirmation(
email:"#{email}",
redirectUrl:"#{redirect}"
) {
message
authenticable {
id
email
}
}
}
GRAPHQL
end

context 'when params are correct' do
it 'sends an email to the user with confirmation url and returns a success message' do
expect { post_request }.to change(ActionMailer::Base.deliveries, :count).by 1
Comment thread
aarona marked this conversation as resolved.
Outdated
expect(json_response[:data][:userResendConfirmation]).to include(
authenticable: {
id: id,
email: email
},
message: "You will receive an email with instructions for how to confirm your email address in a few minutes."
)

email = Nokogiri::HTML(ActionMailer::Base.deliveries.last.body.encoded)
link = email.css('a').first
confirm_link_msg_text = email.css('p')[1].inner_html
confirm_account_link_text = link.inner_html

expect(confirm_link_msg_text).to eq("You can confirm your account email through the link below:")
expect(confirm_account_link_text).to eq("Confirm my account")

# TODO: Move to feature spec
expect do
get link['href']
user.reload
end.to change(user, :confirmed_at).from(NilClass).to(ActiveSupport::TimeWithZone)
end

context 'when the user has already been confirmed' do
before { user.confirm }

it 'does *NOT* send an email and raises an error' do
expect { post_request }.to change(ActionMailer::Base.deliveries, :count).by 0
Comment thread
aarona marked this conversation as resolved.
Outdated
expect(json_response[:data][:userResendConfirmation]).to be_nil
expect(json_response[:errors]).to contain_exactly(
hash_including(
message: "Email was already confirmed, please try signing in",
extensions: { code: 'USER_ERROR' }
)
)
end
end
end

context "when the email isn't in the system" do
let(:email) { 'nothere@gmail.com' }

it 'does *NOT* send an email and raises an error' do
expect { post_request }.to not_change(ActionMailer::Base.deliveries, :count)
expect(json_response[:data][:userResendConfirmation]).to be_nil
expect(json_response[:errors]).to contain_exactly(
hash_including(
message: "Unable to find user with email '#{email}'.",
extensions: { code: 'USER_ERROR' }
)
)
end
end
end