Avoid using exception but simply errors attribute contained in object

and then expect the right values.

.rubocop_todo.yml

@@ -119,7 +119,6 @@ Layout/LineLength:
     - 'spec/controllers/admin/bulk_line_items_controller_spec.rb'
     - 'spec/controllers/admin/column_preferences_controller_spec.rb'
     - 'spec/controllers/admin/enterprises_controller_spec.rb'
-    - 'spec/controllers/admin/manager_invitations_controller_spec.rb'
     - 'spec/controllers/admin/order_cycles_controller_spec.rb'
     - 'spec/controllers/admin/schedules_controller_spec.rb'
     - 'spec/controllers/admin/stripe_accounts_controller_spec.rb'

app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee

@@ -55,19 +55,6 @@ angular.module("admin.enterprises")
         else
           alert ("#{manager.email}" + " " + t("is_already_manager"))
 
-    $scope.inviteManager = ->
-      $scope.invite_errors = $scope.invite_success = null
-      email = $scope.newUser
-
-      $http.post("/admin/manager_invitations", {email: email, enterprise_id: $scope.Enterprise.id}).then (response)->
-          $scope.addManager({id: response.data.user, email: email})
-          $scope.invite_success = t('user_invited', email: email)
-        .catch (response) ->
-          $scope.invite_errors = response.data.errors
-
-    $scope.resetModal = ->
-      $scope.newUser = $scope.invite_errors = $scope.invite_success = null
-
     $scope.removeLogo = ->
       $scope.performEnterpriseAction("removeLogo", "immediate_logo_removal_warning", "removed_logo_successfully")
 

app/assets/javascripts/admin/utils/utils.js.coffee

@@ -1,2 +1,3 @@
-angular.module("admin.utils", ["templates", "ngSanitize"]).config ($httpProvider) ->
+angular.module("admin.utils", ["templates", "ngSanitize"]).config ($httpProvider, $locationProvider) ->
+ $locationProvider.hashPrefix('')
  $httpProvider.defaults.headers.common["Accept"] = "application/json, text/javascript, */*"

app/controllers/admin/manager_invitations_controller.rb

@@ -1,47 +0,0 @@
-# frozen_string_literal: true
-
-module Admin
-  class ManagerInvitationsController < Spree::Admin::BaseController
-    authorize_resource class: false
-
-    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: :internal_server_error
-      end
-    end
-
-    private
-
-    def create_new_manager
-      password = Devise.friendly_token
-      new_user = Spree::User.create(email: @email, unconfirmed_email: @email, password: password)
-      new_user.reset_password_token = Devise.friendly_token
-      # Same time as used in Devise's lib/devise/models/recoverable.rb.
-      new_user.reset_password_sent_at = Time.now.utc
-      new_user.save!
-
-      @enterprise.users << new_user
-      EnterpriseMailer.manager_invitation(@enterprise, new_user).deliver_later
-
-      new_user
-    end
-  end
-end

app/controllers/concerns/manager_invitations.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module ManagerInvitations
+  extend ActiveSupport::Concern
+
+  def create_new_manager(email, enterprise)
+    password = Devise.friendly_token
+    new_user = Spree::User.create(email: email, unconfirmed_email: email, password: password)
+    new_user.reset_password_token = Devise.friendly_token
+    # Same time as used in Devise's lib/devise/models/recoverable.rb.
+    new_user.reset_password_sent_at = Time.now.utc
+    new_user.save
+
+    return new_user unless new_user.valid? # Return early if user is invalid.
+
+    enterprise.users << new_user
+    EnterpriseMailer.manager_invitation(@enterprise, new_user).deliver_later
+
+    new_user
+  end
+end

app/reflexes/application_reflex.rb

@@ -23,4 +23,8 @@ class ApplicationReflex < StimulusReflex::Reflex
   def current_ability
     Spree::Ability.new(current_user)
   end
+
+  def with_locale(&block)
+    I18n.with_locale(current_user.locale, &block)
+  end
 end

app/reflexes/invite_manager_reflex.rb

@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+class InviteManagerReflex < ApplicationReflex
+  include ManagerInvitations
+
+  def invite
+    email = params[:email]
+    enterprise = Enterprise.find(params[:enterprise_id])
+
+    authorize! :edit, enterprise
+
+    existing_user = Spree::User.find_by(email: email)
+
+    locals = { error: nil, success: nil, email: email, enterprise: enterprise }
+
+    if existing_user
+      locals[:error] = I18n.t('admin.enterprises.invite_manager.user_already_exists')
+
+      return_morph(locals)
+      return
+    end
+
+    new_user = create_new_manager(email, enterprise)
+
+    if new_user.valid?
+      locals[:success] = true
+    else
+      locals[:error] = new_user.errors.full_messages.to_sentence ||
+                       I18n.t('admin.enterprises.invite_manager.error')
+    end
+
+    return_morph(locals)
+  end
+
+  private
+
+  def return_morph(locals)
+    morph "#add_manager_modal",
+          with_locale {
+            render(partial: "admin/enterprises/form/add_new_unregistered_manager", locals: locals)
+          }
+  end
+end

app/views/admin/enterprises/form/_add_new_unregistered_manager.html.haml

@@ -0,0 +1,22 @@
+%div#add_manager_modal
+  %form{ "data-reflex": "submit->InviteManager#invite", "data-reflex-serialize-form": true }
+    .margin-bottom-30.text-center
+      .text-big
+        = t('js.admin.modals.invite_title')
+
+    - if success
+      %p.alert-box.ok= t('user_invited', email: email)
+
+    - if error
+      %p.alert-box.error= error
+
+    = text_field_tag :email, nil, class: 'fullwidth margin-bottom-20'
+    = hidden_field_tag :enterprise_id, @enterprise&.id || enterprise.id
+
+    .modal-actions
+      - if success
+        %input{ class: "button icon-plus secondary", type: 'button', value: t('js.admin.modals.close'), "data-action": "click->help-modal#close" }
+      - else
+        %input{ class: "button icon-plus secondary", type: 'button', value: t('js.admin.modals.cancel'), "data-action": "click->help-modal#close" }
+        = submit_tag "#{t('js.admin.modals.invite')}"
+      

app/views/admin/enterprises/form/_users.html.haml

@@ -6,8 +6,7 @@
     =f.label :owner_id, t('.owner')
     - if full_permissions
       %span.required *
-    %div{'ofn-with-tip' => t('.owner_tip')}
-      %a= t('admin.whats_this')
+    = render partial: 'admin/shared/tooltip', locals: {tooltip_text: t('.owner_tip')}
   .eight.columns.omega
     - if full_permissions
       = f.hidden_field :owner_id, class: "select2 fullwidth", 'user-select' => 'Enterprise.owner', 'ng-model' => 'Enterprise.owner'
@@ -19,8 +18,7 @@
     =f.label :user_ids, t('.notifications')
     - if full_permissions
       %span.required *
-    %div{'ofn-with-tip' => t('.contact_tip')}
-      %a= t('admin.whats_this')
+    = render partial: 'admin/shared/tooltip', locals: {tooltip_text: t('.contact_tip')}
   .eight.columns.omega
     - if full_permissions
       %select.select2.fullwidth{id: 'receives_notifications_dropdown', name: 'receives_notifications', ng: {model: 'receivesNotifications', init: "receivesNotifications = '#{@enterprise.contact.id}'"}}
@@ -34,8 +32,7 @@
     =f.label :user_ids, t('.managers')
     - if full_permissions
       %span.required *
-    %div{'ofn-with-tip' => t('.managers_tip')}
-      %a= t('admin.whats_this')
+    = render partial: 'admin/shared/tooltip', locals: {tooltip_text: t('.managers_tip')}
   .eight.columns.omega
     - if full_permissions
       %table.managers
@@ -66,8 +63,7 @@
       .three.columns.alpha
         %label
           = t('.invite_manager')
-        %div{'ofn-with-tip' => t('.invite_manager_tip')}
-          %a= t('admin.whats_this')
+        = render partial: 'admin/shared/tooltip', locals: {tooltip_text: t('.invite_manager_tip')}
       .eight.columns.omega
         .row
           %a.button{ "data-controller": "help-modal-link", "data-action": "click->help-modal-link#open", "data-help-modal-link-target-value": "invite-manager-modal" }
@@ -76,22 +72,4 @@
 -# add to admin footer to avoid nesting invitation form inside enterprise form
 - content_for :admin_footer do
   = render HelpModalComponent.new(id: "invite-manager-modal", close_button: false) do
-    %div{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: 'inviteManager()'}}
-          = t('js.admin.modals.invite')
-        %button.text-center.margin-top-10{"data-action": "click->help-modal#close", ng: {show: 'invite_success', click: 'resetModal();'}}
-          = t('js.admin.modals.close')
+    = render partial: 'admin/enterprises/form/add_new_unregistered_manager', locals: { error: nil, success: nil }

config/routes/admin.rb

@@ -42,8 +42,6 @@ Openfoodnetwork::Application.routes.draw do
       resources :tag_rules, only: [:destroy]
     end
 
-    resources :manager_invitations, only: [:create]
-
     resources :enterprise_relationships
     resources :enterprise_roles
 

spec/controllers/admin/manager_invitations_controller_spec.rb

@@ -1,89 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-module Admin
-  describe ManagerInvitationsController, type: :controller do
-    include OpenFoodNetwork::EmailHelper
-
-    let!(:enterprise_owner) { create(:user) }
-    let!(:other_enterprise_user) { create(:user) }
-    let!(:existing_user) { create(:user) }
-    let!(:enterprise) { create(:enterprise, owner: enterprise_owner ) }
-    let!(:enterprise2) { create(:enterprise, owner: other_enterprise_user ) }
-    let(:admin) { create(:admin_user) }
-
-    describe "#create" do
-      context "when given email matches an existing user" do
-        before do
-          allow(controller).to receive_messages 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 'User already exists'
-        end
-      end
-
-      context "signing up a new user" do
-        let(:mail_mock) { double(:mailer, deliver_later: true) }
-
-        before do
-          allow(EnterpriseMailer).to receive(:manager_invitation).
-            with(enterprise, kind_of(Spree::User)) { mail_mock }
-
-          allow(controller).to receive_messages spree_current_user: admin
-        end
-
-        it 'enqueues an invitation email' do
-          expect(mail_mock).to receive(:deliver_later)
-
-          spree_post :create, email: 'un.registered@email.com', enterprise_id: enterprise.id
-        end
-
-        it "returns the user id" do
-          spree_post :create, email: 'un.registered@email.com', enterprise_id: enterprise.id
-
-          new_user = Spree::User.find_by(email: 'un.registered@email.com')
-          expect(json_response['user']).to eq new_user.id
-        end
-      end
-    end
-
-    describe "with enterprise permissions" do
-      context "as user with proper enterprise permissions" do
-        before do
-          setup_email
-          allow(controller).to receive_messages spree_current_user: enterprise_owner
-        end
-
-        it "returns success code" do
-          spree_post :create, email: 'an@email.com', enterprise_id: enterprise.id
-
-          new_user = Spree::User.find_by(email: 'an@email.com')
-
-          expect(new_user.reset_password_token).to_not be_nil
-          expect(json_response['user']).to eq new_user.id
-          expect(response.status).to eq 200
-        end
-      end
-
-      context "as another enterprise user without permissions for this enterprise" do
-        before do
-          allow(controller).to receive_messages spree_current_user: other_enterprise_user
-        end
-
-        it "returns unauthorized response" do
-          spree_post :create, email: 'another@email.com', enterprise_id: enterprise.id
-
-          new_user = Spree::User.find_by(email: 'another@email.com')
-
-          expect(new_user).to be_nil
-          expect(response.status).to eq 302
-        end
-      end
-    end
-  end
-end

spec/services/order_cycle_form_spec.rb

@@ -288,8 +288,10 @@ describe OrderCycleForm do
                 distributor.users.first
               )
 
-              expect{ form.save }.to change{ order_cycle.distributor_payment_methods.pluck(:id) }
-                .from([distributor_payment_method, distributor_payment_method2])
+              expect{ form.save }.to change{
+                                       order_cycle.distributor_payment_methods.pluck(:id).sort
+                                     }
+                .from([distributor_payment_method, distributor_payment_method2].sort)
                 .to([distributor_payment_method])
             end
           end

spec/system/admin/enterprises_spec.rb

@@ -532,5 +532,57 @@ describe '
         end
       end
     end
+
+    describe "check users tab" do
+      before do
+        login_as_admin_and_visit edit_admin_enterprise_path(distributor1)
+        within ".side_menu" do
+          click_link 'Users'
+        end
+      end
+
+      context "invite user as manager" do
+        before do
+          expect(page).to have_selector('a', text: /Add an unregistered user/i)
+          page.find('a', text: /Add an unregistered user/i).click
+        end
+
+        it "shows an error message if the email is invalid" do
+          within ".reveal-modal" do
+            expect(page).to have_content "Invite an unregistered user"
+            fill_in "email", with: "invalid_email"
+
+            expect do
+              click_button "Invite"
+              expect(page).to have_content "Email is invalid"
+            end.to_not enqueue_job ActionMailer::MailDeliveryJob
+          end
+        end
+
+        it "shows an error message if the email is already linked to an existing user" do
+          within ".reveal-modal" do
+            expect(page).to have_content "Invite an unregistered user"
+            fill_in "email", with: distributor1.owner.email
+
+            expect do
+              click_button "Invite"
+              expect(page).to have_content "User already exists"
+            end.to_not enqueue_job ActionMailer::MailDeliveryJob
+          end
+        end
+
+        it "finally, can invite unregistered users" do
+          within ".reveal-modal" do
+            expect(page).to have_content "Invite an unregistered user"
+            fill_in "email", with: "email@email.com"
+
+            expect do
+              click_button "Invite"
+              expect(page).to have_content "email@email.com has been invited to manage this enterprise"
+            end.to enqueue_job(ActionMailer::MailDeliveryJob).exactly(:twice)
+          end
+        end
+      end
+    end
   end
 end