From 25bf8a071154718e6b23403788f8046fe61bb85f Mon Sep 17 00:00:00 2001 From: Suzanne Aitchison Date: Tue, 3 May 2022 09:33:12 +0100 Subject: [PATCH] Remove name from invitation flow (#17438) * remove name from invite user flow * remove name from invitation instructions * update invitations spec * update specs and invitation actions overflow menu name * allow users to set name when accepting invite --- app/controllers/admin/invitations_controller.rb | 4 +--- app/controllers/application_controller.rb | 1 + app/controllers/invitations_controller.rb | 2 +- app/views/admin/invitations/index.html.erb | 6 +++--- app/views/admin/invitations/new.html.erb | 7 ------- .../users/index/_invitation_actions_dropdown.html.erb | 2 +- app/views/devise/invitations/edit.html.erb | 7 ++++++- app/views/devise/mailer/invitation_instructions.html.erb | 2 +- app/views/devise/mailer/invitation_instructions.text.erb | 2 +- config/locales/devise_invitable.en.yml | 2 +- config/locales/devise_invitable.fr.yml | 2 +- .../seededFlows/adminFlows/users/invitedUsers.spec.js | 4 ++-- spec/requests/admin/invitations_spec.rb | 7 +++---- spec/system/admin/admin_invites_user_spec.rb | 4 +--- 14 files changed, 23 insertions(+), 29 deletions(-) diff --git a/app/controllers/admin/invitations_controller.rb b/app/controllers/admin/invitations_controller.rb index d55e70eb8..701af3aa9 100644 --- a/app/controllers/admin/invitations_controller.rb +++ b/app/controllers/admin/invitations_controller.rb @@ -13,7 +13,6 @@ module Admin def create email = params.dig(:user, :email) - name = params.dig(:user, :name) if User.exists?(email: email.downcase, registered: true) flash[:error] = I18n.t("admin.invitations_controller.duplicate", email: email) @@ -21,9 +20,8 @@ module Admin return end - username = "#{name.downcase.tr(' ', '_').gsub(/[^0-9a-z ]/i, '')}_#{rand(1000)}" + username = "#{email.split('@').first.gsub(/[^0-9a-z ]/i, '')}_#{rand(1000)}" User.invite!(email: email, - name: name, username: username, remote_profile_image_url: ::Users::ProfileImageGenerator.call, registered: false) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 1fdc3e4fd..3e8089fd6 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -276,6 +276,7 @@ class ApplicationController < ActionController::Base def configure_permitted_parameters devise_parameter_sanitizer.permit(:sign_up, keys: %i[username name profile_image profile_image_url]) + devise_parameter_sanitizer.permit(:accept_invitation, keys: %i[name]) end def internal_nav_param diff --git a/app/controllers/invitations_controller.rb b/app/controllers/invitations_controller.rb index 3d6cbb8e1..5accc1a11 100644 --- a/app/controllers/invitations_controller.rb +++ b/app/controllers/invitations_controller.rb @@ -11,7 +11,7 @@ class InvitationsController < Devise::InvitationsController yield resource if block_given? if invitation_accepted - resource.update!(registered_at: Time.current, registered: true) + resource.update!(registered_at: Time.current, registered: true, name: params[:user][:name]) if resource.class.allow_insecure_sign_in_after_accept flash_message = resource.active_for_authentication? ? :updated : :updated_not_active set_flash_message :notice, flash_message if is_flashing_format? diff --git a/app/views/admin/invitations/index.html.erb b/app/views/admin/invitations/index.html.erb index 617432ecd..e1ffff655 100644 --- a/app/views/admin/invitations/index.html.erb +++ b/app/views/admin/invitations/index.html.erb @@ -28,7 +28,7 @@
<%= render "admin/users/index/member_image", user: user %>
- <%= render "admin/users/index/member_data", user: user %> + @<%= user.username %> <%= user.email %>
Invited on
@@ -69,9 +69,9 @@ <% @invitations.each do |user| %> -
+
<%= render "admin/users/index/member_image", user: user %> - <%= render "admin/users/index/member_data", user: user %> + @<%= user.username %>
<%= user.email %> diff --git a/app/views/admin/invitations/new.html.erb b/app/views/admin/invitations/new.html.erb index ea5dc2fc5..5558e9d0b 100644 --- a/app/views/admin/invitations/new.html.erb +++ b/app/views/admin/invitations/new.html.erb @@ -30,13 +30,6 @@ placeholder: "Email of invitee", required: true %>
-
- <%= f.label :name, class: "crayons-field__label" %> - <%= f.text_field :name, - class: "crayons-textfield", - placeholder: "Name of invitee", - required: true %> -
<%= f.submit "Invite User", class: "crayons-btn" %>
diff --git a/app/views/admin/users/index/_invitation_actions_dropdown.html.erb b/app/views/admin/users/index/_invitation_actions_dropdown.html.erb index f23c5a659..513c81d2a 100644 --- a/app/views/admin/users/index/_invitation_actions_dropdown.html.erb +++ b/app/views/admin/users/index/_invitation_actions_dropdown.html.erb @@ -1,6 +1,6 @@
diff --git a/app/views/devise/invitations/edit.html.erb b/app/views/devise/invitations/edit.html.erb index abf31eb71..177e67aa9 100644 --- a/app/views/devise/invitations/edit.html.erb +++ b/app/views/devise/invitations/edit.html.erb @@ -7,12 +7,17 @@ <% end %>
-

Set a password to access your account

+

Finish setting up your account

<%= form_for(resource, as: resource_name, url: invitation_path(resource_name), html: { method: :put }) do |f| %> <%= render "devise/shared/error_messages", resource: resource %> <%= f.hidden_field :invitation_token, readonly: true %> +
+ <%= f.label :name, class: "crayons-field__label" %> + <%= f.text_field :name, class: "crayons-textfield" %> +
+ <% if f.object.class.require_password_on_accepting %>
<%= f.label :password, class: "crayons-field__label" %> diff --git a/app/views/devise/mailer/invitation_instructions.html.erb b/app/views/devise/mailer/invitation_instructions.html.erb index 86dfb4db8..5439d8a80 100644 --- a/app/views/devise/mailer/invitation_instructions.html.erb +++ b/app/views/devise/mailer/invitation_instructions.html.erb @@ -1,4 +1,4 @@ -

<%= t("devise.mailer.invitation_instructions.hello", name: @resource.name) %>

+

<%= t("devise.mailer.invitation_instructions.hello") %>

<%= t("devise.mailer.invitation_instructions.someone_invited_you", community_name: Settings::Community.community_name, url: root_url) %>

diff --git a/app/views/devise/mailer/invitation_instructions.text.erb b/app/views/devise/mailer/invitation_instructions.text.erb index 7c93bfd73..1763af0ad 100644 --- a/app/views/devise/mailer/invitation_instructions.text.erb +++ b/app/views/devise/mailer/invitation_instructions.text.erb @@ -1,4 +1,4 @@ -<%= t("devise.mailer.invitation_instructions.hello", name: @resource.name) %> +<%= t("devise.mailer.invitation_instructions.hello") %> <%= t("devise.mailer.invitation_instructions.someone_invited_you", community_name: Settings::Community.community_name, url: root_url) %> diff --git a/config/locales/devise_invitable.en.yml b/config/locales/devise_invitable.en.yml index 92b137390..b0e2cac2a 100644 --- a/config/locales/devise_invitable.en.yml +++ b/config/locales/devise_invitable.en.yml @@ -21,7 +21,7 @@ en: accept_instructions: You can accept the invitation by clicking this link accept: Accept Invitation accept_until: This invitation will expire after %{due_date}. - hello: Hello %{name} + hello: Hello ignore: If you don't want to accept the invitation, please ignore this email. Your account won't be created until you access the link above and set your password. someone_invited_you: You’ve been invited to join the %{community_name} community at %{url}. subject: Invitation instructions diff --git a/config/locales/devise_invitable.fr.yml b/config/locales/devise_invitable.fr.yml index 5e6d7f3e8..c8b6dc981 100644 --- a/config/locales/devise_invitable.fr.yml +++ b/config/locales/devise_invitable.fr.yml @@ -24,7 +24,7 @@ fr: accept_instructions: Vous pouvez accepter l'invitation en cliquant sur ce lien accept: Accepter l'invitation accept_until: Cette invitation expirera après %{due_date}. - hello: Bonjour %{name} + hello: Bonjour ignore: Si vous ne souhaitez pas accepter cette invitation, veuillez ignorer cet e-mail.
Votre compte ne sera pas créé tant que vous n'accéderez pas au lien ci-dessous et que vous ayez défini votre mot de passe. someone_invited_you: Vous avez été invité à rejoindre la communauté %{community_name} sur %{url}. subject: Vous avez reçu une invitation diff --git a/cypress/integration/seededFlows/adminFlows/users/invitedUsers.spec.js b/cypress/integration/seededFlows/adminFlows/users/invitedUsers.spec.js index 561a2590c..72556cab2 100644 --- a/cypress/integration/seededFlows/adminFlows/users/invitedUsers.spec.js +++ b/cypress/integration/seededFlows/adminFlows/users/invitedUsers.spec.js @@ -116,7 +116,7 @@ describe('Invited users', () => { }; const resendInviteForTestMember = () => { - cy.findByRole('button', { name: 'Invitation actions: Test user' }) + cy.findByRole('button', { name: 'Invitation actions: test@test.com' }) .pipe(click) .should('have.attr', 'aria-expanded', 'true'); @@ -124,7 +124,7 @@ describe('Invited users', () => { }; const cancelInviteForTestMember = () => { - cy.findByRole('button', { name: 'Invitation actions: Test user' }) + cy.findByRole('button', { name: 'Invitation actions: test@test.com' }) .pipe(click) .should('have.attr', 'aria-expanded', 'true'); diff --git a/spec/requests/admin/invitations_spec.rb b/spec/requests/admin/invitations_spec.rb index 52aae12e7..b1308ff82 100644 --- a/spec/requests/admin/invitations_spec.rb +++ b/spec/requests/admin/invitations_spec.rb @@ -21,21 +21,20 @@ RSpec.describe "/admin/invitations", type: :request do it "renders to appropriate page" do get new_admin_invitation_path expect(response.body).to include("Email") - expect(response.body).to include("Name") end end describe "POST /admin/invitations" do it "creates new invitation" do post admin_invitations_path, - params: { user: { email: "hey#{rand(1000)}@email.co", name: "Roger #{rand(1000)}" } } + params: { user: { email: "hey#{rand(1000)}@email.co" } } expect(User.last.registered).to be false end it "enqueues an invitation email to be sent", :aggregate_failures do assert_enqueued_with(job: Devise.mailer.delivery_job) do post admin_invitations_path, - params: { user: { email: "hey#{rand(1000)}@email.co", name: "Roger #{rand(1000)}" } } + params: { user: { email: "hey#{rand(1000)}@email.co" } } end expect(enqueued_jobs.first[:args]).to match(array_including("invitation_instructions")) @@ -44,7 +43,7 @@ RSpec.describe "/admin/invitations", type: :request do it "does not create an invitation if a user with that email exists" do expect do post admin_invitations_path, - params: { user: { email: admin.email, name: "Roger #{rand(1000)}" } } + params: { user: { email: admin.email } } end.not_to change { User.all.count } expect(admin.reload.registered).to be true expect(flash[:error].present?).to be true diff --git a/spec/system/admin/admin_invites_user_spec.rb b/spec/system/admin/admin_invites_user_spec.rb index 923c873e4..ac07165ae 100644 --- a/spec/system/admin/admin_invites_user_spec.rb +++ b/spec/system/admin/admin_invites_user_spec.rb @@ -35,7 +35,6 @@ RSpec.describe "Admin invites user", type: :system do it "does not contain any for fields" do expect(page).not_to have_field "Email" - expect(page).not_to have_field "Name" end it "does not contain any submit buttons" do @@ -49,9 +48,8 @@ RSpec.describe "Admin invites user", type: :system do visit new_admin_invitation_path end - it "shows the input fields" do + it "shows the input field" do expect(page).to have_field "Email" - expect(page).to have_field "Name" end it "shows the submit button" do