From b64327fbb3d83108a72ee4ccab1042e43c120b31 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Fri, 29 Sep 2017 15:16:02 +0100 Subject: [PATCH 1/6] Invite enterprise manager functionality --- .../enterprise_controller.js.coffee | 17 +++++++++-- .../admin/modals/invite_manager.html.haml | 19 +++++++++++++ .../admin/openfoodnetwork.css.scss | 4 +++ .../admin/enterprises_controller.rb | 28 +++++++++++++++++++ app/jobs/manager_invitation_job.rb | 7 +++++ app/mailers/enterprise_mailer.rb | 12 ++++++++ app/models/enterprise.rb | 4 +++ .../admin/enterprises/form/_users.html.haml | 13 +++++++++ .../manager_invitation.html.haml | 16 +++++++++++ config/locales/en.yml | 22 +++++++++++++-- config/routes.rb | 2 ++ 11 files changed, 140 insertions(+), 4 deletions(-) create mode 100644 app/assets/javascripts/templates/admin/modals/invite_manager.html.haml create mode 100644 app/jobs/manager_invitation_job.rb create mode 100644 app/views/enterprise_mailer/manager_invitation.html.haml diff --git a/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee b/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee index 98183a7c26..9fa74e78d3 100644 --- a/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee +++ b/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee @@ -1,5 +1,5 @@ angular.module("admin.enterprises") - .controller "enterpriseCtrl", ($scope, $window, NavigationCheck, enterprise, EnterprisePaymentMethods, EnterpriseShippingMethods, SideMenu, StatusMessage) -> + .controller "enterpriseCtrl", ($scope, $http, $window, NavigationCheck, enterprise, EnterprisePaymentMethods, EnterpriseShippingMethods, SideMenu, StatusMessage) -> $scope.Enterprise = enterprise $scope.PaymentMethods = EnterprisePaymentMethods.paymentMethods $scope.ShippingMethods = EnterpriseShippingMethods.shippingMethods @@ -26,7 +26,7 @@ angular.module("admin.enterprises") # from a directive "nav-check" in the page - if we pass it here it will be called in the test suite, # and on all new uses of this contoller, and we might not want that. enterpriseNavCallback = -> - if $scope.enterprise_form.$dirty + if $scope.enterprise_form != undefined && $scope.enterprise_form.$dirty t('admin.unsaved_confirm_leave') # Register the NavigationCheck callback @@ -51,3 +51,16 @@ angular.module("admin.enterprises") $scope.Enterprise.users.push manager else alert ("#{manager.email}" + " " + t("is_already_manager")) + + $scope.inviteUser = -> + $scope.invite_errors = $scope.invite_success = null + email = $scope.newUser + + $http.post('/admin/enterprises/invite_manager', {email: email, enterprise: $scope.Enterprise.id}).success (data)-> + $scope.addManager({id: data.user, email: email}) + $scope.invite_success = t('user_invited', email: email) + .error (data) -> + $scope.invite_errors = data.errors + + $scope.resetModal = -> + $scope.newUser = $scope.invite_errors = $scope.invite_success = null diff --git a/app/assets/javascripts/templates/admin/modals/invite_manager.html.haml b/app/assets/javascripts/templates/admin/modals/invite_manager.html.haml new file mode 100644 index 0000000000..618299cfb3 --- /dev/null +++ b/app/assets/javascripts/templates/admin/modals/invite_manager.html.haml @@ -0,0 +1,19 @@ +#invite-manager-modal{ng: {app: 'admin.enterprises', controller: 'enterpriseCtrl'}} + + .margin-bottom-30.text-center + .text-big + = t('js.admin.modals.invite_title') + + %p.alert-box.ok{ng: {show: 'invite_success'}} + {{invite_success}} + + %p.alert-box.error{ng: {show: 'invite_errors'}} + {{invite_errors}} + + %input#invite_email.fullwidth.margin-bottom-20{ng: {model: 'newUser'}} + + .margin-bottom-20.text-center + %button.text-center.margin-top-10{ng: {show: '!invite_success', click: 'inviteUser()'}} + = t('js.admin.modals.invite') + %button.text-center.margin-top-10{ng: {show: 'invite_success', click: 'resetModal(); close()'}} + = t('js.admin.modals.close') diff --git a/app/assets/stylesheets/admin/openfoodnetwork.css.scss b/app/assets/stylesheets/admin/openfoodnetwork.css.scss index 9a970c87d4..d10ca748d0 100644 --- a/app/assets/stylesheets/admin/openfoodnetwork.css.scss +++ b/app/assets/stylesheets/admin/openfoodnetwork.css.scss @@ -1,3 +1,7 @@ +input[type="submit"], input[type="button"], button, .button { + cursor: pointer; +} + .text-center { text-align: center; } diff --git a/app/controllers/admin/enterprises_controller.rb b/app/controllers/admin/enterprises_controller.rb index 11dd547042..fe6e8b8c44 100644 --- a/app/controllers/admin/enterprises_controller.rb +++ b/app/controllers/admin/enterprises_controller.rb @@ -114,8 +114,36 @@ module Admin end end + def invite_manager + existing_user = Spree::User.where("email = :email OR unconfirmed_email = :email", email: params[:email]).first + + if existing_user + render json: { errors: t('admin.enterprises.invite_manager.user_already_exists') }, status: :unprocessable_entity + return + end + + new_user = create_new_manager(params[:email], params[:enterprise]) + + if new_user + render json: { user: new_user.id }, status: :ok + else + render json: { errors: t('admin.enterprises.invite_manager.error') }, status: 500 + end + end + protected + def create_new_manager(email, enterprise_id) + enterprise = Enterprise.find(enterprise_id) + password = Devise.friendly_token.first(8) + new_user = Spree::User.create(email: email, unconfirmed_email: email, password: password, password_confirmation: password) + + enterprise.send_manager_invitation(new_user) + enterprise.users << new_user + + new_user + end + def build_resource_with_address enterprise = build_resource_without_address enterprise.address ||= Spree::Address.new diff --git a/app/jobs/manager_invitation_job.rb b/app/jobs/manager_invitation_job.rb new file mode 100644 index 0000000000..5a1e894f0e --- /dev/null +++ b/app/jobs/manager_invitation_job.rb @@ -0,0 +1,7 @@ +ManagerInvitationJob = Struct.new(:enterprise_id, :user_id) do + def perform + enterprise = Enterprise.find enterprise_id + user = Spree::User.find user_id + EnterpriseMailer.manager_invitation(enterprise, user).deliver + end +end diff --git a/app/mailers/enterprise_mailer.rb b/app/mailers/enterprise_mailer.rb index 350a247d73..a57e39535e 100644 --- a/app/mailers/enterprise_mailer.rb +++ b/app/mailers/enterprise_mailer.rb @@ -12,6 +12,18 @@ class EnterpriseMailer < Spree::BaseMailer :subject => subject) end + def manager_invitation(enterprise, user) + @enterprise = enterprise + @instance = Spree::Config[:site_name] + @instance_email = Spree::Config[:preferred_mails_from] + + subject = t('enterprise_mailer.invite_manager.subject', enterprise: @enterprise.name) + + mail(to: user.email, + from: from_address, + subject: subject) + end + private def find_enterprise(enterprise) diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index 8b0f3fe385..3333f8441a 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -348,6 +348,10 @@ class Enterprise < ActiveRecord::Base abn.present? end + def send_manager_invitation(user) + Delayed::Job.enqueue ManagerInvitationJob.new(self.id, user.id) + end + protected def devise_mailer diff --git a/app/views/admin/enterprises/form/_users.html.haml b/app/views/admin/enterprises/form/_users.html.haml index 45318ec114..6934f58931 100644 --- a/app/views/admin/enterprises/form/_users.html.haml +++ b/app/views/admin/enterprises/form/_users.html.haml @@ -60,3 +60,16 @@ - @enterprise.users.each do |manager| = manager.email %br + +- if full_permissions + %form + .row + .three.columns.alpha + %label + = t('.invite_manager') + %div{'ofn-with-tip' => t('.invite_manager_tip')} + %a= t('admin.whats_this') + .eight.columns.omega + .row + %a.button.help-modal{template: 'admin/modals/invite_manager.html'} + = t('.add_unregistered_user') diff --git a/app/views/enterprise_mailer/manager_invitation.html.haml b/app/views/enterprise_mailer/manager_invitation.html.haml new file mode 100644 index 0000000000..e7256b47c9 --- /dev/null +++ b/app/views/enterprise_mailer/manager_invitation.html.haml @@ -0,0 +1,16 @@ +%h3 + = t('invite_email.greeting') +%p.lead + = t('invite_email.invited_to_manage', enterprise: @enterprise.name, instance: @instance) + +%p + = t('invite_email.confirm_your_email') +%p + = t('invite_email.mistakenly_sent', owner_email: @enterprise.owner.email, instance: @instance, instance_email: @instance_email) + +%p + = t :email_help + += render 'shared/mailers/signoff' + += render 'shared/mailers/social_and_contact' diff --git a/config/locales/en.yml b/config/locales/en.yml index bedb5c6295..d020513384 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -114,6 +114,8 @@ en: subject: "Please confirm the email address for %{enterprise}" welcome: subject: "%{enterprise} is now on %{sitename}" + invite_manager: + subject: "%{enterprise} has invited you to be a manager" producer_mailer: order_cycle: subject: "Order cycle report for %{producer}" @@ -655,6 +657,9 @@ en: notifications_note: 'Note: A new email address may need to be confirmed prior to use' managers: Managers managers_tip: The other users with permission to manage this enterprise. + invite_manager: "Invite Manager" + invite_manager_tip: "Invite an unregistered user to sign up and become a manager of this enterprise." + add_unregistered_user: "Add an unregistered user" email_confirmed: "Email confirmed" email_not_confirmed: "Email not confirmed" actions: @@ -714,6 +719,9 @@ en: welcome_text: You have successfully created a next_step: Next step choose_starting_point: 'Choose your starting point:' + invite_manager: + user_already_exists: "User already exists" + error: "Something went wrong" order_cycles: edit: advanced_settings: Advanced Settings @@ -1309,6 +1317,13 @@ See the %{link} to find out more about %{sitename}'s features and to start using If you are a producer or food enterprise, we are excited to have you as a part of the network." email_signup_help_html: "We welcome all your questions and feedback; you can use the Send Feedback button on the site or email us at %{email}" + invite_email: + greeting: "Hello!" + invited_to_manage: "You have been invited to manage %{enterprise} on %{instance}." + confirm_your_email: "You will receive an email shortly to confirm your registration." + set_a_password: "You will then be prompted to set a password before you are able to administer the enterprise." + mistakenly_sent: "Not sure why you have received this email? Please contact %{owner_email} for more information, or you can contact %{instance} at %{instance_email}." + producer_mail_greeting: "Dear" producer_mail_text_before: "We now have all the consumer orders for the next food drop." producer_mail_order_text: "Here is a summary of the orders for your products:" @@ -2123,10 +2138,10 @@ See the %{link} to find out more about %{sitename}'s features and to start using unsaved_changes_confirmation: "Unsaved changes will be lost. Continue anyway?" one_product_unsaved: "Changes to one product remain unsaved." products_unsaved: "Changes to %{n} products remain unsaved." - add_manager: "Add a manager" is_already_manager: "is already a manager!" no_change_to_save: " No change to save" - add_manager: "Add a manager" + user_invited: "%{email} has been invited to manage this enterprise" + add_manager: "Add an existing user" users: "Users" about: "About" images: "Images" @@ -2209,6 +2224,9 @@ See the %{link} to find out more about %{sitename}'s features and to start using enterprise_limit_reached: "You have reached the standard limit of enterprises per account. Write to %{contact_email} if you need to increase it." modals: got_it: Got it + close: "Close" + invite: "Invite" + invite_title: "Invite an unregistered user" tag_rule_help: title: Tag Rules overview: Overview diff --git a/config/routes.rb b/config/routes.rb index 85276ad0b4..c46516a9a6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -85,6 +85,8 @@ Openfoodnetwork::Application.routes.draw do get "/enterprises/:permalink", to: redirect("/") # Legacy enterprise URL namespace :admin do + post '/enterprises/invite_manager', to: 'enterprises#invite_manager' + resources :order_cycles do post :bulk_update, on: :collection, as: :bulk_update From 128ca3b1ef376226280b4fe1db6b2e041e89d41c Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Fri, 29 Sep 2017 16:52:10 +0100 Subject: [PATCH 2/6] Invite manager specs --- app/mailers/enterprise_mailer.rb | 2 +- .../admin/enterprises_controller_spec.rb | 36 +++++++++++++++++++ spec/features/admin/enterprise_roles_spec.rb | 29 ++++++++++++++- spec/mailers/enterprise_mailer_spec.rb | 22 +++++++++--- 4 files changed, 82 insertions(+), 7 deletions(-) diff --git a/app/mailers/enterprise_mailer.rb b/app/mailers/enterprise_mailer.rb index a57e39535e..03d9279c93 100644 --- a/app/mailers/enterprise_mailer.rb +++ b/app/mailers/enterprise_mailer.rb @@ -15,7 +15,7 @@ class EnterpriseMailer < Spree::BaseMailer def manager_invitation(enterprise, user) @enterprise = enterprise @instance = Spree::Config[:site_name] - @instance_email = Spree::Config[:preferred_mails_from] + @instance_email = from_address subject = t('enterprise_mailer.invite_manager.subject', enterprise: @enterprise.name) diff --git a/spec/controllers/admin/enterprises_controller_spec.rb b/spec/controllers/admin/enterprises_controller_spec.rb index f95bc6e8da..f67902d5c6 100644 --- a/spec/controllers/admin/enterprises_controller_spec.rb +++ b/spec/controllers/admin/enterprises_controller_spec.rb @@ -634,5 +634,41 @@ module Admin end end end + + describe "#invite_manager" do + context "when given email matches an existing user" do + let!(:existing_user) { create(:user) } + let!(:enterprise) { create(:enterprise) } + let(:admin) { create(:admin_user) } + + before do + controller.stub spree_current_user: admin + end + + it "returns an error" do + spree_post :invite_manager, {email: user.email, enterprise: enterprise.id} + + expect(response.status).to eq 422 + expect(json_response['errors']).to eq I18n.t('admin.enterprises.invite_manager.user_already_exists') + end + end + + context "signing up a new user" do + let!(:enterprise) { create(:enterprise) } + let(:admin) { create(:admin_user) } + + before do + controller.stub spree_current_user: admin + end + + it "creates a new user, sends an invitation email, and returns the user id" do + spree_post :invite_manager, {email: 'un.registered@email.com', enterprise: enterprise.id} + + expect(Delayed::Job.last.payload_object.class.to_s).to eq('ManagerInvitationJob') + expect(response.status).to eq 200 + expect(json_response['user']).to eq Spree::User.find_by_email('un.registered@email.com').id + end + end + end end end diff --git a/spec/features/admin/enterprise_roles_spec.rb b/spec/features/admin/enterprise_roles_spec.rb index ee036b9d27..3c3957a9e4 100644 --- a/spec/features/admin/enterprise_roles_spec.rb +++ b/spec/features/admin/enterprise_roles_spec.rb @@ -114,7 +114,9 @@ feature %q{ # user3 has been added and has an unconfirmed email address expect(page).to have_css "tr#manager-#{user3.id}" - expect(page).to have_css 'i.unconfirmed' + within "tr#manager-#{user3.id}" do + expect(page).to have_css 'i.unconfirmed' + end end end @@ -133,6 +135,31 @@ feature %q{ end end end + + it "can invite unregistered users to be managers" do + new_email = 'new@manager.com' + + find('a.button.help-modal').click + expect(page).to have_css '#invite-manager-modal' + + within '#invite-manager-modal' do + fill_in 'invite_email', with: new_email + click_button I18n.t('js.admin.modals.invite') + expect(page).to have_content I18n.t('user_invited', email: new_email) + click_button I18n.t('js.admin.modals.close') + end + + new_user = Spree::User.find_by_email_and_confirmed_at(new_email, nil) + expect(Enterprise.managed_by(new_user)).to include enterprise + + within 'table.managers' do + expect(page).to have_content new_email + + within "tr#manager-#{new_user.id}" do + expect(page).to have_css 'i.unconfirmed' + end + end + end end end diff --git a/spec/mailers/enterprise_mailer_spec.rb b/spec/mailers/enterprise_mailer_spec.rb index 7eda5ff505..e5be9a6dc8 100644 --- a/spec/mailers/enterprise_mailer_spec.rb +++ b/spec/mailers/enterprise_mailer_spec.rb @@ -2,15 +2,27 @@ require 'spec_helper' describe EnterpriseMailer do let!(:enterprise) { create(:enterprise) } + let!(:user) { create(:user) } before do ActionMailer::Base.deliveries = [] end - it "should send a welcome email when given an enterprise" do - EnterpriseMailer.welcome(enterprise).deliver - ActionMailer::Base.deliveries.count.should == 1 - mail = ActionMailer::Base.deliveries.first - expect(mail.subject).to eq "#{enterprise.name} is now on #{Spree::Config[:site_name]}" + describe "#welcome" do + it "should send a welcome email when given an enterprise" do + EnterpriseMailer.welcome(enterprise).deliver + ActionMailer::Base.deliveries.count.should == 1 + mail = ActionMailer::Base.deliveries.first + expect(mail.subject).to eq "#{enterprise.name} is now on #{Spree::Config[:site_name]}" + end + end + + describe "#manager_invitation" do + it "should send a manager invitation email when given an enterprise and user" do + EnterpriseMailer.manager_invitation(enterprise, user).deliver + expect(ActionMailer::Base.deliveries.count).to eq 1 + mail = ActionMailer::Base.deliveries.first + expect(mail.subject).to eq "#{enterprise.name} has invited you to be a manager" + end end end From f3f18d833813d51985641f3fbc7ce3d382d68b6e Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Tue, 13 Mar 2018 22:04:04 +0000 Subject: [PATCH 3/6] Cleaner routes.rb entry --- config/routes.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index c46516a9a6..a5ab9673cd 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -85,8 +85,6 @@ Openfoodnetwork::Application.routes.draw do get "/enterprises/:permalink", to: redirect("/") # Legacy enterprise URL namespace :admin do - post '/enterprises/invite_manager', to: 'enterprises#invite_manager' - resources :order_cycles do post :bulk_update, on: :collection, as: :bulk_update @@ -101,6 +99,7 @@ Openfoodnetwork::Application.routes.draw do get :for_order_cycle get :visible post :bulk_update, as: :bulk_update + post :invite_manager end member do From 5f0075f8b77f7be79e5097ed3a10c42f72392810 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Thu, 15 Mar 2018 16:51:05 +0000 Subject: [PATCH 4/6] Refactor to use new REST resource --- .../enterprise_controller.js.coffee | 2 +- .../admin/enterprises_controller.rb | 28 ------------- .../admin/manager_invitations_controller.rb | 37 ++++++++++++++++ app/models/enterprise.rb | 4 -- config/routes.rb | 3 +- .../admin/enterprises_controller_spec.rb | 36 ---------------- .../manager_invitations_controller_spec.rb | 42 +++++++++++++++++++ spec/features/admin/enterprise_roles_spec.rb | 3 +- spec/mailers/enterprise_mailer_spec.rb | 2 +- 9 files changed, 84 insertions(+), 73 deletions(-) create mode 100644 app/controllers/admin/manager_invitations_controller.rb create mode 100644 spec/controllers/admin/manager_invitations_controller_spec.rb diff --git a/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee b/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee index 9fa74e78d3..749929c7f6 100644 --- a/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee +++ b/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee @@ -56,7 +56,7 @@ angular.module("admin.enterprises") $scope.invite_errors = $scope.invite_success = null email = $scope.newUser - $http.post('/admin/enterprises/invite_manager', {email: email, enterprise: $scope.Enterprise.id}).success (data)-> + $http.post("/admin/manager_invitations", {email: email, enterprise_id: $scope.Enterprise.id}).success (data)-> $scope.addManager({id: data.user, email: email}) $scope.invite_success = t('user_invited', email: email) .error (data) -> diff --git a/app/controllers/admin/enterprises_controller.rb b/app/controllers/admin/enterprises_controller.rb index fe6e8b8c44..11dd547042 100644 --- a/app/controllers/admin/enterprises_controller.rb +++ b/app/controllers/admin/enterprises_controller.rb @@ -114,36 +114,8 @@ module Admin end end - def invite_manager - existing_user = Spree::User.where("email = :email OR unconfirmed_email = :email", email: params[:email]).first - - if existing_user - render json: { errors: t('admin.enterprises.invite_manager.user_already_exists') }, status: :unprocessable_entity - return - end - - new_user = create_new_manager(params[:email], params[:enterprise]) - - if new_user - render json: { user: new_user.id }, status: :ok - else - render json: { errors: t('admin.enterprises.invite_manager.error') }, status: 500 - end - end - protected - def create_new_manager(email, enterprise_id) - enterprise = Enterprise.find(enterprise_id) - password = Devise.friendly_token.first(8) - new_user = Spree::User.create(email: email, unconfirmed_email: email, password: password, password_confirmation: password) - - enterprise.send_manager_invitation(new_user) - enterprise.users << new_user - - new_user - end - def build_resource_with_address enterprise = build_resource_without_address enterprise.address ||= Spree::Address.new diff --git a/app/controllers/admin/manager_invitations_controller.rb b/app/controllers/admin/manager_invitations_controller.rb new file mode 100644 index 0000000000..11c28b75c6 --- /dev/null +++ b/app/controllers/admin/manager_invitations_controller.rb @@ -0,0 +1,37 @@ +module Admin + class ManagerInvitationsController < Spree::Admin::BaseController + def create + @email = params[:email] + @enterprise = Enterprise.find(params[:enterprise_id]) + + authorize! :edit, @enterprise + + existing_user = Spree::User.find_by_email(@email) + + if existing_user + render json: { errors: t('admin.enterprises.invite_manager.user_already_exists') }, status: :unprocessable_entity + return + end + + new_user = create_new_manager + + if new_user + render json: { user: new_user.id }, status: :ok + else + render json: { errors: t('admin.enterprises.invite_manager.error') }, status: 500 + end + end + + private + + def create_new_manager + password = Devise.friendly_token.first(8) + new_user = Spree::User.create(email: @email, unconfirmed_email: @email, password: password) + + @enterprise.users << new_user + Delayed::Job.enqueue ManagerInvitationJob.new(@enterprise.id, new_user.id) + + new_user + end + end +end diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index 3333f8441a..8b0f3fe385 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -348,10 +348,6 @@ class Enterprise < ActiveRecord::Base abn.present? end - def send_manager_invitation(user) - Delayed::Job.enqueue ManagerInvitationJob.new(self.id, user.id) - end - protected def devise_mailer diff --git a/config/routes.rb b/config/routes.rb index a5ab9673cd..0ae125dfd9 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -99,7 +99,6 @@ Openfoodnetwork::Application.routes.draw do get :for_order_cycle get :visible post :bulk_update, as: :bulk_update - post :invite_manager end member do @@ -114,6 +113,8 @@ Openfoodnetwork::Application.routes.draw do resources :tag_rules, only: [:destroy] end + resources :manager_invitations, only: [:create] + resources :enterprise_relationships resources :enterprise_roles diff --git a/spec/controllers/admin/enterprises_controller_spec.rb b/spec/controllers/admin/enterprises_controller_spec.rb index f67902d5c6..f95bc6e8da 100644 --- a/spec/controllers/admin/enterprises_controller_spec.rb +++ b/spec/controllers/admin/enterprises_controller_spec.rb @@ -634,41 +634,5 @@ module Admin end end end - - describe "#invite_manager" do - context "when given email matches an existing user" do - let!(:existing_user) { create(:user) } - let!(:enterprise) { create(:enterprise) } - let(:admin) { create(:admin_user) } - - before do - controller.stub spree_current_user: admin - end - - it "returns an error" do - spree_post :invite_manager, {email: user.email, enterprise: enterprise.id} - - expect(response.status).to eq 422 - expect(json_response['errors']).to eq I18n.t('admin.enterprises.invite_manager.user_already_exists') - end - end - - context "signing up a new user" do - let!(:enterprise) { create(:enterprise) } - let(:admin) { create(:admin_user) } - - before do - controller.stub spree_current_user: admin - end - - it "creates a new user, sends an invitation email, and returns the user id" do - spree_post :invite_manager, {email: 'un.registered@email.com', enterprise: enterprise.id} - - expect(Delayed::Job.last.payload_object.class.to_s).to eq('ManagerInvitationJob') - expect(response.status).to eq 200 - expect(json_response['user']).to eq Spree::User.find_by_email('un.registered@email.com').id - end - end - end end end diff --git a/spec/controllers/admin/manager_invitations_controller_spec.rb b/spec/controllers/admin/manager_invitations_controller_spec.rb new file mode 100644 index 0000000000..01b7c984d1 --- /dev/null +++ b/spec/controllers/admin/manager_invitations_controller_spec.rb @@ -0,0 +1,42 @@ +require 'spec_helper' + +module Admin + describe ManagerInvitationsController, type: :controller do + let!(:existing_user) { create(:user) } + let!(:enterprise) { create(:enterprise) } + let(:admin) { create(:admin_user) } + + describe "#create" do + context "when given email matches an existing user" do + before do + controller.stub spree_current_user: admin + end + + it "returns an error" do + spree_post :create, {email: existing_user.email, enterprise_id: enterprise.id} + + expect(response.status).to eq 422 + expect(json_response['errors']).to eq I18n.t('admin.enterprises.invite_manager.user_already_exists') + end + end + + context "signing up a new user" do + before do + controller.stub spree_current_user: admin + end + + it "creates a new user, sends an invitation email, and returns the user id" do + expect do + spree_post :create, {email: 'un.registered@email.com', enterprise_id: enterprise.id} + end.to enqueue_job Delayed::PerformableMethod + + new_user = Spree::User.find_by_email('un.registered@email.com') + + expect(new_user.reset_password_token).to_not be_nil + expect(response.status).to eq 200 + expect(json_response['user']).to eq new_user.id + end + end + end + end +end diff --git a/spec/features/admin/enterprise_roles_spec.rb b/spec/features/admin/enterprise_roles_spec.rb index 3c3957a9e4..5029a8b054 100644 --- a/spec/features/admin/enterprise_roles_spec.rb +++ b/spec/features/admin/enterprise_roles_spec.rb @@ -83,6 +83,7 @@ feature %q{ let!(:user1) { create(:user, email: 'user1@example.com') } let!(:user2) { create(:user, email: 'user2@example.com') } let!(:user3) { create(:user, email: 'user3@example.com', confirmed_at: nil) } + let(:new_email) { 'new@manager.com' } let!(:enterprise) { create(:enterprise, name: 'Test Enterprise', owner: user1) } let!(:enterprise_role) { create(:enterprise_role, user_id: user2.id, enterprise_id: enterprise.id) } @@ -137,8 +138,6 @@ feature %q{ end it "can invite unregistered users to be managers" do - new_email = 'new@manager.com' - find('a.button.help-modal').click expect(page).to have_css '#invite-manager-modal' diff --git a/spec/mailers/enterprise_mailer_spec.rb b/spec/mailers/enterprise_mailer_spec.rb index e5be9a6dc8..fae627a0d8 100644 --- a/spec/mailers/enterprise_mailer_spec.rb +++ b/spec/mailers/enterprise_mailer_spec.rb @@ -11,7 +11,7 @@ describe EnterpriseMailer do describe "#welcome" do it "should send a welcome email when given an enterprise" do EnterpriseMailer.welcome(enterprise).deliver - ActionMailer::Base.deliveries.count.should == 1 + expect(ActionMailer::Base.deliveries.count).to eq 1 mail = ActionMailer::Base.deliveries.first expect(mail.subject).to eq "#{enterprise.name} is now on #{Spree::Config[:site_name]}" end From 3fad51d271755446531104bc526d7c1be6992325 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Thu, 15 Mar 2018 16:53:27 +0000 Subject: [PATCH 5/6] Clearer angular method naming --- .../enterprises/controllers/enterprise_controller.js.coffee | 2 +- .../javascripts/templates/admin/modals/invite_manager.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee b/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee index 749929c7f6..cd1da7b447 100644 --- a/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee +++ b/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee @@ -52,7 +52,7 @@ angular.module("admin.enterprises") else alert ("#{manager.email}" + " " + t("is_already_manager")) - $scope.inviteUser = -> + $scope.inviteManager = -> $scope.invite_errors = $scope.invite_success = null email = $scope.newUser diff --git a/app/assets/javascripts/templates/admin/modals/invite_manager.html.haml b/app/assets/javascripts/templates/admin/modals/invite_manager.html.haml index 618299cfb3..bbde3baa49 100644 --- a/app/assets/javascripts/templates/admin/modals/invite_manager.html.haml +++ b/app/assets/javascripts/templates/admin/modals/invite_manager.html.haml @@ -13,7 +13,7 @@ %input#invite_email.fullwidth.margin-bottom-20{ng: {model: 'newUser'}} .margin-bottom-20.text-center - %button.text-center.margin-top-10{ng: {show: '!invite_success', click: 'inviteUser()'}} + %button.text-center.margin-top-10{ng: {show: '!invite_success', click: 'inviteManager()'}} = t('js.admin.modals.invite') %button.text-center.margin-top-10{ng: {show: 'invite_success', click: 'resetModal(); close()'}} = t('js.admin.modals.close') From 1600067383f073ab81159bb73e773fe4ed505b0f Mon Sep 17 00:00:00 2001 From: Matt-Yorkley Date: Thu, 15 Mar 2018 18:47:25 +0000 Subject: [PATCH 6/6] Redirect to set password page after invite --- .../admin/manager_invitations_controller.rb | 4 +++- .../user_confirmations_controller.rb | 4 ++++ .../user_confirmations_controller_spec.rb | 19 +++++++++++++------ 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/app/controllers/admin/manager_invitations_controller.rb b/app/controllers/admin/manager_invitations_controller.rb index 11c28b75c6..ed70322f34 100644 --- a/app/controllers/admin/manager_invitations_controller.rb +++ b/app/controllers/admin/manager_invitations_controller.rb @@ -25,8 +25,10 @@ module Admin private def create_new_manager - password = Devise.friendly_token.first(8) + password = Devise.friendly_token new_user = Spree::User.create(email: @email, unconfirmed_email: @email, password: password) + new_user.reset_password_token = Devise.friendly_token + new_user.save! @enterprise.users << new_user Delayed::Job.enqueue ManagerInvitationJob.new(@enterprise.id, new_user.id) diff --git a/app/controllers/user_confirmations_controller.rb b/app/controllers/user_confirmations_controller.rb index f5ee6a10e5..8e3e504eaa 100644 --- a/app/controllers/user_confirmations_controller.rb +++ b/app/controllers/user_confirmations_controller.rb @@ -38,6 +38,10 @@ class UserConfirmationsController < DeviseController 'not_confirmed' end + if resource.reset_password_token.present? + return spree.edit_spree_user_password_path(reset_password_token: resource.reset_password_token) + end + path = (session[:confirmation_return_url] || login_path).to_s path += path.include?('?') ? '&' : '?' path + "validation=#{result}" diff --git a/spec/controllers/user_confirmations_controller_spec.rb b/spec/controllers/user_confirmations_controller_spec.rb index d264a011a0..678e7ccd86 100644 --- a/spec/controllers/user_confirmations_controller_spec.rb +++ b/spec/controllers/user_confirmations_controller_spec.rb @@ -25,17 +25,17 @@ describe UserConfirmationsController, type: :controller do end context "that has not been confirmed" do - it "redirects the user to login" do - spree_get :show, confirmation_token: unconfirmed_user.confirmation_token - expect(response).to redirect_to login_path(validation: 'confirmed') - end - it "confirms the user" do spree_get :show, confirmation_token: unconfirmed_user.confirmation_token expect(unconfirmed_user.reload.confirmed_at).not_to eq(nil) end - it "redirects to previous url" do + it "redirects the user to #/login by default" do + spree_get :show, confirmation_token: unconfirmed_user.confirmation_token + expect(response).to redirect_to login_path(validation: 'confirmed') + end + + it "redirects to previous url, if present" do session[:confirmation_return_url] = producers_path + '#/login' spree_get :show, confirmation_token: unconfirmed_user.confirmation_token expect(response).to redirect_to producers_path + '#/login?validation=confirmed' @@ -46,6 +46,13 @@ describe UserConfirmationsController, type: :controller do spree_get :show, confirmation_token: unconfirmed_user.confirmation_token expect(response).to redirect_to registration_path + '#/signup?after_login=%2Fregister&validation=confirmed' end + + it "redirects to set password page, if user needs to reset their password" do + unconfirmed_user.reset_password_token = Devise.friendly_token + unconfirmed_user.save! + spree_get :show, confirmation_token: unconfirmed_user.confirmation_token + expect(response).to redirect_to spree.edit_spree_user_password_path(reset_password_token: unconfirmed_user.reset_password_token) + end end end