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

feat(account invitation): accomplish sign up before accepting the invitation #30

Merged
merged 3 commits into from
Aug 27, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
13 changes: 10 additions & 3 deletions app/controllers/accept_account_invitations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,15 @@ class AcceptAccountInvitationsController < ApplicationController
def show
return if user_signed_in?

session[:after_sign_in_url] = request.fullpath
redirect_to new_user_session_url(email: AccountInvitation.find_by!(token: ps.fetch(:token)).email)
session[:after_authentication_url] = request.fullpath
Copy link
Member

Choose a reason for hiding this comment

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

Why changed the key :after_sign_in_url to :after_authentication_url? :after_authentication_url is longer, harder to write and spell.


invitee_email = AccountInvitation.find_by!(token: ps[:token]).email
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
invitee_email = AccountInvitation.find_by!(token: ps[:token]).email
invitee_email = AccountInvitation.find_by!(token: ps.fetch(:token)).email

If the ps.fetch(:token) bothers you, create a private method token_param or something and use it in all places.

user = User.find_by(email: invitee_email)
if user.present?
redirect_to new_user_session_url(email: invitee_email)
else
redirect_to new_user_registration_url(email: invitee_email)
end
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if user.present?
redirect_to new_user_session_url(email: invitee_email)
else
redirect_to new_user_registration_url(email: invitee_email)
end
redirect_url = user ? new_user_session_url(email: invitee_email) : new_user_registration_url(email: invitee_email)
redirect_to redirect_url

it's a bit more DRY

end

def update
Expand All @@ -16,6 +23,6 @@ def update
private

helper_method memoize def received_invitation
AccountInvitation.unaccepted.for(current_user).find_by!(token: ps.fetch(:token))
AccountInvitation.unaccepted.for(current_user).find_by!(token: ps[:token])
Copy link
Member

Choose a reason for hiding this comment

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

what was the problem with the previous implementation? revert it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

... ok, I've seen the main idea of thefetch for us: errors. Ok

end
end
17 changes: 17 additions & 0 deletions app/controllers/users/registrations_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
class Users::RegistrationsController < Devise::RegistrationsController
def new
if params.key?(:email)
self.resource = User.new(email: params[:email])
else
super
end
end

def create
super do |user|
user.create_account(name: user.email, email: user.email) if user.persisted?
Expand All @@ -10,4 +18,13 @@ def create
def devise_mapping
@devise_mapping ||= Devise.mappings[:user]
end

private

def after_sign_up_path_for(resource)
after_authentication_url = session.delete(:after_authentication_url)
return after_authentication_url if after_authentication_url.present?

super
end
end
4 changes: 2 additions & 2 deletions app/controllers/users/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ def new
private

def after_sign_in_path_for(_resource)
after_sign_in_url = session.delete(:after_sign_in_url)
return after_sign_in_url if after_sign_in_url.present?
after_authentication_url = session.delete(:after_authentication_url)
return after_authentication_url if after_authentication_url.present?

super
end
Expand Down
11 changes: 10 additions & 1 deletion spec/controllers/accept_account_invitations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
describe '#show' do
let(:account) { create(:account, :parent) }
let(:user) { create(:user, account: account) }
let(:second_user) { create(:user, account: account) }
let(:second_user) { create(:user) }

let!(:account_invitation) { create(:account_invitation, user: user, account: account, email: second_user.email) }

Expand All @@ -21,6 +21,15 @@
end
end

context 'when the user not signed up' do
let(:account_invitation) { create(:account_invitation, user: user, account: account) }

it 'redirects to registration url with email' do
subject
expect(response).to redirect_to(new_user_registration_url(email: account_invitation.email))
end
end

context 'when the user signed in' do
before { sign_in second_user }

Expand Down