Merge pull request #13895 from cillian/remove-angular-from-enterprise-settings-users

Remove Angular from Enterprise > Settings > Users section

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

@@ -4,16 +4,12 @@ angular.module("admin.enterprises")
     $scope.Enterprises = Enterprises
     $scope.navClear = NavigationCheck.clear
     $scope.menu = SideMenu
-    $scope.newManager = { id: null, email: (t('add_manager')) }
     $scope.StatusMessage = StatusMessage
     $scope.RequestMonitor = RequestMonitor
 
     $scope.$watch 'enterprise_form.$dirty', (newValue) ->
       StatusMessage.display 'notice', t('admin.unsaved_changes') if newValue
 
-    $scope.$watch 'newManager', (newValue) ->
-      $scope.addManager($scope.newManager) if newValue
-
     $scope.setFormDirty = ->
       $scope.$apply ->
         $scope.enterprise_form.$setDirty()
@@ -35,26 +31,6 @@ angular.module("admin.enterprises")
     # Register the NavigationCheck callback
     NavigationCheck.register(enterpriseNavCallback)
 
-    $scope.removeManager = (manager) ->
-      if manager.id?
-        if manager.id == $scope.Enterprise.owner.id or manager.id == parseInt($scope.receivesNotifications)
-          return
-        for i, user of $scope.Enterprise.users when user.id == manager.id
-          $scope.Enterprise.users.splice i, 1
-          $scope.enterprise_form?.$setDirty()
-
-    $scope.addManager = (manager) ->
-      if manager.id? and angular.isNumber(manager.id) and manager.email?
-        manager =
-          id: manager.id
-          email: manager.email
-          confirmed: manager.confirmed
-        if (user for user in $scope.Enterprise.users when user.id == manager.id).length == 0
-          $scope.Enterprise.users.unshift(manager)
-          $scope.enterprise_form?.$setDirty()
-        else
-          alert ("#{manager.email}" + " " + t("is_already_manager"))
-
     $scope.performEnterpriseAction = (enterpriseActionName, warning_message_key, success_message_key) ->
       return unless confirm($scope.translation(warning_message_key))
 

app/controllers/admin/enterprises_controller.rb

@@ -67,7 +67,6 @@ module Admin
     def update
       tag_rules_attributes = params[object_name].delete :tag_rules_attributes
       update_tag_rules(tag_rules_attributes) if tag_rules_attributes.present?
-      update_enterprise_notifications
       update_vouchers
 
       delete_custom_tab if params[:custom_tab] == 'false'
@@ -314,14 +313,6 @@ module Admin
       end
     end
 
-    def update_enterprise_notifications
-      user_id = params[:receives_notifications].to_i
-
-      return unless user_id.positive? && @enterprise.user_ids.include?(user_id)
-
-      @enterprise.update_contact(user_id)
-    end
-
     def update_vouchers
       params_voucher_ids = params[:enterprise][:voucher_ids].to_a.map(&:to_i)
       voucher_ids = @enterprise.vouchers.map(&:id)

app/controllers/admin/user_invitations_controller.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+module Admin
+  class UserInvitationsController < ResourceController
+    before_action :load_enterprise
+
+    def new; end
+
+    def create
+      @user_invitation.attributes = permitted_resource_params
+      if @user_invitation.save!
+        flash[:success] = I18n.t(:user_invited, email: @user_invitation.email)
+      else
+        render :new
+      end
+    end
+
+    private
+
+    def load_enterprise
+      @enterprise = OpenFoodNetwork::Permissions
+        .new(spree_current_user)
+        .editable_enterprises
+        .find_by(permalink: params[:enterprise_id])
+    end
+
+    def permitted_resource_params
+      params.require(:user_invitation).permit(:email).merge(enterprise: @enterprise)
+    end
+  end
+end

app/controllers/concerns/manager_invitations.rb

@@ -1,19 +0,0 @@
-# frozen_string_literal: true
-
-module ManagerInvitations
-  extend ActiveSupport::Concern
-
-  def create_new_manager(email, enterprise)
-    password = SecureRandom.base58(64)
-    new_user = Spree::User.create(email:, unconfirmed_email: email, 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
-    if new_user.save
-      enterprise.users << new_user
-      EnterpriseMailer.manager_invitation(enterprise, new_user).deliver_later
-    end
-
-    new_user
-  end
-end

app/forms/user_invitation.rb

@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+class UserInvitation
+  include ActiveModel::Model
+  include ActiveModel::Attributes
+  include ActiveModel::Validations::Callbacks
+
+  attribute :enterprise
+  attribute :email
+
+  before_validation :normalize_email
+
+  validates :email, presence: true, 'valid_email_2/email': { mx: true }
+  validates :enterprise, presence: true
+  validate :not_existing_enterprise_user
+
+  def save!
+    return unless valid?
+
+    user = find_or_create_user!
+    enterprise.users << user
+
+    return unless user.previously_new_record?
+
+    EnterpriseMailer.manager_invitation(enterprise, user).deliver_later
+  end
+
+  private
+
+  def find_or_create_user!
+    Spree::User.find_or_create_by!(email: email) do |user|
+      user.email = email
+      user.password = SecureRandom.base58(64)
+      user.unconfirmed_email = email
+      user.reset_password_token = Devise.friendly_token
+      # Same time as used in Devise's lib/devise/models/recoverable.rb.
+      user.reset_password_sent_at = Time.now.utc
+    end
+  end
+
+  def normalize_email
+    self.email = email.strip if email.present?
+  end
+
+  def not_existing_enterprise_user
+    return unless email.present? && enterprise.users.where(email: email).exists?
+
+    errors.add(:email, :is_already_manager)
+  end
+end

app/models/enterprise.rb

@@ -294,7 +294,13 @@ class Enterprise < ApplicationRecord
     contact || owner
   end
 
-  def update_contact(user_id)
+  def contact_id
+    contact&.id
+  end
+
+  def contact_id=(user_id)
+    return unless user_id.to_i.positive? && users.confirmed.exists?(user_id.to_i)
+
     enterprise_roles.update_all(["receives_notifications=(user_id=?)", user_id])
   end
 
@@ -576,7 +582,7 @@ class Enterprise < ApplicationRecord
   end
 
   def set_default_contact
-    update_contact owner_id
+    self.contact_id = owner_id
   end
 
   def relate_to_owners_enterprises

app/models/spree/ability.rb

@@ -191,7 +191,7 @@ module Spree
         user.enterprises.include? stripe_account.enterprise
       end
 
-      can [:admin, :create], :manager_invitation
+      can [:admin, :create], UserInvitation
 
       can [:admin, :index, :destroy], :oidc_setting
 

app/models/spree/user.rb

@@ -23,6 +23,7 @@ module Spree
     before_destroy :check_completed_orders
 
     scope :admin, -> { where(admin: true) }
+    scope :confirmed, -> { where.not(confirmed_at: nil) }
 
     has_many :enterprise_roles, dependent: :destroy
     has_many :enterprises, through: :enterprise_roles

app/reflexes/invite_manager_reflex.rb

@@ -1,40 +0,0 @@
-# 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:)
-
-    locals = { error: nil, success: nil, email:, 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.errors.empty?
-      locals[:success] = true
-    else
-      locals[:error] = new_user.errors.full_messages.to_sentence
-    end
-
-    return_morph(locals)
-  end
-
-  private
-
-  def return_morph(locals)
-    morph "#add_manager_modal",
-          render(partial: "admin/enterprises/form/add_new_unregistered_manager", locals:)
-  end
-end

app/services/permitted_attributes/enterprise.rb

@@ -40,7 +40,7 @@ module PermittedAttributes
         :hide_ofn_navigation, :white_label_logo, :white_label_logo_link,
         :hide_groups_tab, :external_billing_id,
         :enable_producers_to_edit_orders,
-        :remove_logo, :remove_promo_image, :remove_white_label_logo
+        :remove_logo, :remove_promo_image, :remove_white_label_logo, :contact_id
       ]
     end
   end

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

@@ -1,21 +0,0 @@
-%form#add_manager_modal{ '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

@@ -1,75 +1,54 @@
-- owner_email = @enterprise&.owner&.email || ""
 - full_permissions = (spree_current_user.admin? || spree_current_user == @enterprise&.owner)
 
 .row
-  .three.columns.alpha
-    =f.label :owner_id, t('.owner')
-    - if full_permissions
-      %span.required *
-    = render partial: 'admin/shared/whats_this_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'
-    - else
-      = owner_email
+  = t '.description'
 
-.row
-  .three.columns.alpha
-    =f.label :user_ids, t('.notifications')
-    - if full_permissions
-      %span.required *
-    = render partial: 'admin/shared/whats_this_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', "ng-init": "receivesNotifications = '#{@enterprise.contact.id}'" }
-        %option{ value: '{{user.id}}', "ng-repeat": 'user in Enterprise.users', "ng-selected": "user.id == #{@enterprise.contact.id}", "ng-hide": '!user.confirmed' }
-          {{user.email}}
-    - else
-      = @enterprise.contact.email
+- if full_permissions && @enterprise.users.count > 0
+  - enterprise_role_ids_by_user_id = @enterprise.enterprise_roles.pluck(:user_id, :id).to_h
 
-.row
-  .three.columns.alpha
-    =f.label :user_ids, t('.managers')
-    - if full_permissions
-      %span.required *
-    = render partial: 'admin/shared/whats_this_tooltip', locals: {tooltip_text: t('.managers_tip')}
-  .eight.columns.omega
-    - if full_permissions
-      %table.managers
-        %tr
+  %table.managers
+    %thead
+      %tr
+        %th= t('.manager')
+        %th.center
+          = t('.owner')
+          = render AdminTooltipComponent.new(text: t('.owner_tip'), link_text: %[<i class="fa fa-question-circle"></i>].html_safe, link: nil)
+        %th.center
+          = t('.contact')
+          = render AdminTooltipComponent.new(text: t('.contact_tip'), link_text: %[<i class="fa fa-question-circle"></i>].html_safe, link: nil)
+    %tbody
+      - @enterprise.users.each do |user|
+        - contact = user.id == @enterprise.contact&.id
+        - owner = user.id == @enterprise.owner&.id
+        %tr{ id: "manager-#{user.id}" }
           %td
-            - # Ignore this input in the submit
-            = hidden_field_tag :ignored, nil, class: "select2 fullwidth", 'user-select' => 'newManager', 'ng-model' => 'newManager'
+            = user.email
+
+            - if user.confirmed?
+              %i.confirmation.confirmed.fa.fa-check-circle{ "ofn-with-tip": t('.email_confirmed') }
+            - else
+              %i.confirmation.unconfirmed.fa.fa-exclamation-triangle{ "ofn-with-tip": t('.email_not_confirmed') }
+
+          %td.center
+            - if user.confirmed?
+              = f.label :owner_id, t(".set_as_owner", email: user.email), class: "sr-only", value: user.id
+              = f.radio_button :owner_id, user.id
+          %td.center
+            - if user.confirmed?
+              = f.label :owner_id, t(".set_as_contact", email: user.email), class: "sr-only", value: user.id
+              = f.radio_button :contact_id, user.id
           %td.actions
-        %tr.animate-repeat{ id: "manager-{{manager.id}}", "ng-repeat": 'manager in Enterprise.users' }
-          %td
-            = hidden_field_tag "enterprise[user_ids][]", nil, multiple: true, 'ng-value' => 'manager.id'
-            {{ manager.email }}
-            %i.confirmation.confirmed.fa.fa-check-circle{ "ofn-with-tip": t('.email_confirmed'), "ng-show": 'manager.confirmed' }
-            %i.confirmation.unconfirmed.fa.fa-exclamation-triangle{ "ofn-with-tip": t('.email_not_confirmed'), "ng-show": '!manager.confirmed' }
-            %i.role.contact.fa.fa-envelope-o{ "ofn-with-tip": t('.contact'), "ng-show": 'manager.id == receivesNotifications' }
-            %i.role.owner.fa.fa-star{ "ofn-with-tip": t('.owner'), "ng-show": 'manager.id == Enterprise.owner.id' }
-          %td.actions
-            %a{ class: "icon-trash no-text", "ng-click": 'removeManager(manager)', "ng-class": "{disabled: manager.id == Enterprise.owner.id || manager.id == receivesNotifications}" }
+            - if !owner && !contact
+              = link_to_delete user, no_text: true, url: admin_enterprise_role_path(id: enterprise_role_ids_by_user_id[user.id])
+            - else
+              %a{ class: "icon-trash no-text disabled" }
 
-    - else
-      - @enterprise.users.each do |manager|
-        = manager.email
-        %br
+  %a.button{ href: "#{new_admin_enterprise_user_invitation_path(@enterprise)}", data: { turbo_stream: true, turbo: true } }
+    %i.icon-plus
+    = t('.invite_manager')
+  %br
 
-- if full_permissions
-  %form
-    .row
-      .three.columns.alpha
-        %label
-          = t('.invite_manager')
-        = render partial: 'admin/shared/whats_this_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" }
-            = t('.add_unregistered_user')
-
--# 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
-    = render partial: 'admin/enterprises/form/add_new_unregistered_manager', locals: { error: nil, success: nil }
+- else
+  - @enterprise.users.each do |manager|
+    = manager.email
+    %br

app/views/admin/user_invitations/create.turbo_stream.haml

@@ -0,0 +1,5 @@
+= turbo_stream.update "remote_modal", ""
+= turbo_stream.update "users_panel" do
+  = render partial: "admin/enterprises/form/users", locals: { f: ActionView::Helpers::FormBuilder.new(:enterprise, @enterprise, self, {}) }
+= turbo_stream.append "flashes" do
+  = render partial: 'admin/shared/flashes', locals: { flashes: flash }

app/views/admin/user_invitations/new.turbo_stream.haml

@@ -0,0 +1,17 @@
+= turbo_stream.update "remote_modal" do
+  = render ModalComponent.new id: "#modal_new_user_invitation", instant: true, close_button: false, modal_class: :fit do
+    = form_with model: @user_invitation, url: admin_enterprise_user_invitations_path(@enterprise), html: { name: "user_invitation", data: { turbo: true } } do |f|
+      %h2= t ".invite_new_user"
+
+      %p= t ".description"
+
+      %fieldset.no-border-top.no-border-bottom
+        .row
+          = f.label :email, t(:email)
+          = f.email_field :email, placeholder: t('.eg_email_address'), data: { controller: "select-user" }, inputmode: "email", autocomplete: "off"
+          = f.error_message_on :email
+
+      .modal-actions.justify-end.filter-actions
+        %input{ class: "secondary relaxed", type: 'button', value: t('.back'), "data-action": "click->modal#close" }
+        %button.button.primary.relaxed.icon-envelope{ type: "submit" }
+          = t(".invite")

app/views/spree/layouts/_admin_body.html.haml

@@ -60,10 +60,12 @@
             = yield :sidebar
 
   = render "admin/terms_of_service_banner" if tos_need_accepting?
-    
+
 %script
   = raw "Spree.api_key = \"#{spree_current_user.try(:spree_api_key).to_s}\";"
 
 = render "layouts/matomo_tag"
 
 = yield :admin_footer
+
+#remote_modal

app/webpacker/controllers/select_user_controller.js

@@ -0,0 +1,44 @@
+import { Controller } from "stimulus";
+import TomSelect from "tom-select/dist/esm/tom-select.complete";
+import showHttpError from "js/services/show_http_error";
+
+export default class extends Controller {
+  connect() {
+    this.control = new TomSelect(this.element, {
+      create: true,
+      plugins: ["dropdown_input"],
+      labelField: "email",
+      load: this.#load.bind(this),
+      maxItems: 1,
+      persist: false,
+      searchField: ["email"],
+      shouldLoad: (query) => query.length > 2,
+      valueField: "email",
+    });
+  }
+
+  disconnect() {
+    if (this.control) this.control.destroy();
+  }
+
+  // private
+
+  #load(query, callback) {
+    const url = "/admin/search/known_users.json?q=" + encodeURIComponent(query);
+    fetch(url)
+      .then((response) => {
+        if (!response.ok) {
+          showHttpError(response.status);
+          throw response;
+        }
+        return response.json();
+      })
+      .then((json) => {
+        callback({ items: json });
+      })
+      .catch((error) => {
+        console.log(error);
+        callback();
+      });
+  }
+}

app/webpacker/css/admin/enterprises.scss

@@ -4,8 +4,17 @@ form[name="enterprise_form"] {
   }
 
   table.managers {
+    th {
+      div[data-controller="tooltip"] {
+        display: inline-block;
+      }
+    }
+
+    .center {
+      text-align: center;
+    }
+
     i.role {
-      float: right;
       margin-left: 0.5em;
       font-size: 1.5em;
       cursor: pointer;

app/webpacker/css/admin/shared/layout.scss

@@ -68,6 +68,18 @@
   display: none;
 }
 
+// Only display for screen readers
+.sr-only {
+  clip: rect(1px, 1px, 1px, 1px);
+  clip-path: inset(50%);
+  height: 1px;
+  width: 1px;
+  margin: -1px;
+  overflow: hidden;
+  padding: 0;
+  position: absolute;
+}
+
 .float-right {
   float: right;
 }

config/locales/en.yml

@@ -314,6 +314,10 @@ en:
               no_default_card: "^No default card available for this customer"
             shipping_method:
               not_available_to_shop: "is not available to %{shop}"
+        user_invitation:
+          attributes:
+            email:
+              is_already_manager: is already a manager
   card_details: "Card details"
   card_type: "Card type"
   card_type_is: "Card type is"
@@ -1485,24 +1489,24 @@ en:
           show_hide_payment: 'Show or Hide payment methods at checkout'
           show_hide_order_cycles: 'Show or Hide order cycles in my shopfront'
         users:
+          description: The users with permission to manage this enterprise.
           legend: "Users"
           email_confirmation_notice_html: "Email confirmation is pending. We've sent a confirmation email to %{email}."
           resend: Resend
-          owner: 'Owner'
           contact: "Contact"
+          manager: "Manager"
+          owner: 'Owner'
           contact_tip: "The manager who will receive enterprise emails for orders and notifications. Must have a confirmed email adress."
           owner_tip: The primary user responsible for this enterprise.
           notifications: Notifications
           notifications_tip: Notifications about orders will be send to this email address.
           notifications_placeholder: eg. gustav@truffles.com
           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"
+          set_as_contact: "Set %{email} as contact"
+          set_as_owner: "Set %{email} as owner"
         vouchers:
           legend: Vouchers
           voucher_code: Voucher Code
@@ -1633,9 +1637,6 @@ en:
         choose_starting_point: 'Choose your package:'
         profile: 'Profile'
         producer_profile: 'Producer Profile'
-      invite_manager:
-        user_already_exists: "User already exists"
-        error: "Something went wrong"
     tag_rules:
       not_supported_type: Tag rule type not supported
       confirm_delete: Are you sure you want to delete this rule ?
@@ -2084,6 +2085,14 @@ en:
     schedules:
       destroy:
         associated_subscriptions_error: This schedule cannot be deleted because it has associated subscriptions
+    user_invitations:
+      new:
+        back: Back
+        description: "Invite a user to sign up and become a manager of this enterprise."
+        eg_email_address: e.g. email address of a new or existing user
+        email: Email
+        invite_new_user: Invite a new user
+        invite: Invite
     vouchers:
       new:
         legend: New Voucher

config/routes/admin.rb

@@ -38,6 +38,8 @@ Openfoodnetwork::Application.routes.draw do
 
       resources :connected_apps, only: [:create, :destroy]
 
+      resources :user_invitations, only: [:new, :create]
+
       resources :producer_properties do
         post :update_positions, on: :collection
       end

spec/controllers/admin/enterprises_controller_spec.rb

@@ -174,18 +174,18 @@ RSpec.describe Admin::EnterprisesController do
         allow(controller).to receive_messages spree_current_user: distributor_manager
         params = {
           id: distributor,
-          receives_notifications: distributor_manager.id,
+          enterprise: { contact_id: distributor_manager.id },
         }
 
         expect { spree_post :update, params }.
           to change { distributor.contact }.to(distributor_manager)
       end
 
-      it "updates the contact for notifications" do
+      it "doesn't update the contact for notifications if the :contact_id parameter is invalid" do
         allow(controller).to receive_messages spree_current_user: distributor_manager
         params = {
           id: distributor,
-          receives_notifications: "? object:null ?",
+          enterprise: { contact_id: "? object:null ?" },
         }
 
         expect { spree_post :update, params }.

spec/forms/user_invitation_spec.rb

@@ -0,0 +1,65 @@
+# frozen_string_literal: true
+
+RSpec.describe UserInvitation do
+  let(:enterprise) { create(:distributor_enterprise) }
+  let(:defaults) { { enterprise: enterprise } }
+
+  describe "#validations" do
+    it "validates the presence of :email and :enterprise" do
+      user_invitation = UserInvitation.new(defaults.merge(email: nil, enterprise: nil))
+      user_invitation.valid?
+
+      expect(user_invitation.errors[:email]).to eq ["can't be blank"]
+      expect(user_invitation.errors[:enterprise]).to eq ["can't be blank"]
+    end
+
+    it "validates the email format" do
+      user_invitation = UserInvitation.new(defaults.merge(email: "invalid_email"))
+      user_invitation.valid?
+
+      expect(user_invitation.errors[:email]).to eq ["is invalid"]
+    end
+
+    it "validates the email is not already a user on the enterprise" do
+      user_invitation = UserInvitation.new(defaults.merge(email: enterprise.owner.email))
+      user_invitation.valid?
+
+      expect(user_invitation.errors[:email]).to eq ["is already a manager"]
+    end
+
+    it "validates the email domain has a MX record" do
+      user_invitation = UserInvitation.new(defaults.merge(email: "newuser@example.invaliddomain"))
+      expect_any_instance_of(ValidEmail2::Address).to receive(:valid_mx?).and_return(false)
+      user_invitation.valid?
+
+      expect(user_invitation.errors[:email]).to eq ["is invalid"]
+    end
+  end
+
+  context "inviting a new user" do
+    it "creates a new unconfirmed user, adds thems to the enterprise and sends them an invitation
+        email" do
+      user_invitation = UserInvitation.new(defaults.merge(email: "new_user@example.com"))
+
+      expect do
+        user_invitation.save!
+      end.to have_enqueued_mail(EnterpriseMailer, :manager_invitation)
+
+      new_user = Spree::User.find_by(email: "new_user@example.com")
+      expect(new_user).not_to be_confirmed
+      expect(new_user.unconfirmed_email).to eq("new_user@example.com")
+      expect(enterprise.users).to include(new_user)
+    end
+  end
+
+  context "inviting a existing user who isn't a user on the enterprise" do
+    it "adds the user to the enterprise" do
+      existing_user = create(:user)
+
+      user_invitation = UserInvitation.new(defaults.merge(email: existing_user.email))
+      user_invitation.save!
+
+      expect(enterprise.users).to include(existing_user)
+    end
+  end
+end

spec/javascripts/unit/admin/enterprises/controllers/enterprise_controller_spec.js.coffee

@@ -13,62 +13,13 @@ describe "enterpriseCtrl", ->
       sells: "none"
       owner:
         id: 98
-    receivesNotifications = 99
 
     inject ($rootScope, $controller, _Enterprises_, _StatusMessage_) ->
       scope = $rootScope
       Enterprises = _Enterprises_
       StatusMessage = _StatusMessage_
-      ctrl = $controller "enterpriseCtrl", {$scope: scope, enterprise: enterprise, EnterprisePaymentMethods: PaymentMethods, Enterprises: Enterprises, StatusMessage: StatusMessage, receivesNotifications: receivesNotifications}
+      ctrl = $controller "enterpriseCtrl", {$scope: scope, enterprise: enterprise, EnterprisePaymentMethods: PaymentMethods, Enterprises: Enterprises, StatusMessage: StatusMessage }
 
   describe "initialisation", ->
     it "stores enterprise", ->
       expect(scope.Enterprise).toEqual enterprise
-
-  describe "adding managers", ->
-    u1 = u2 = u3 = null
-    beforeEach ->
-      u1 = { id: 1, email: 'name1@email.com', confirmed: true }
-      u2 = { id: 2, email: 'name2@email.com', confirmed: true }
-      u3 = { id: 3, email: 'name3@email.com', confirmed: true }
-      enterprise.users = [u1, u2 ,u3]
-
-    it "adds a user to the list", ->
-      u4 = { id: 4, email: "name4@email.com", confirmed: true }
-      scope.addManager u4
-      expect(enterprise.users).toContain u4
-
-    it "ignores object without an id", ->
-      u4 = { not_id: 4, email: "name4@email.com", confirmed: true }
-      scope.addManager u4
-      expect(enterprise.users).not.toContain u4
-
-    it "it ignores objects without an email", ->
-      u4 = { id: 4, not_email: "name4@email.com", confirmed: true }
-      scope.addManager u4
-      expect(enterprise.users).not.toContain u4
-
-    it "ignores objects that are already in the list, and alerts the user", ->
-      spyOn(window, "alert").and.callThrough()
-      u4 = { id: 3, email: "email-doesn't-matter.com", confirmed: true }
-      scope.addManager u4
-      expect(enterprise.users).not.toContain u4
-      expect(window.alert).toHaveBeenCalledWith "email-doesn't-matter.com is already a manager!"
-
-
-  describe "removing managers", ->
-    u1 = u2 = u3 = null
-    beforeEach ->
-      u1 = { id: 1, email: 'name1@email.com', confirmed: true }
-      u2 = { id: 2, email: 'name2@email.com', confirmed: true }
-      u3 = { id: 3, email: 'name3@email.com', confirmed: true }
-      enterprise.users = [u1, u2 ,u3]
-
-
-    it "removes a user with the given id", ->
-      scope.removeManager {id: 2}
-      expect(enterprise.users).not.toContain u2
-
-    it "does nothing when given object has no id attribute", ->
-      scope.removeManager {not_id: 2}
-      expect(enterprise.users).toEqual [u1,u2,u3]

spec/models/enterprise_spec.rb

@@ -1072,6 +1072,40 @@ RSpec.describe Enterprise do
       end
     end
   end
+
+  describe "#contact_id" do
+    it "returns the ID of the enterprise's contact" do
+      enterprise = build(:enterprise)
+      expect(enterprise.contact_id).to eq(enterprise.contact.id)
+    end
+  end
+
+  describe "#contact_id=" do
+    let(:enterprise) { create(:enterprise) }
+
+    it "accepts confirmed users that belongs to the enterprise" do
+      user = create(:user, confirmed_at: Time.now.utc)
+      enterprise.enterprise_roles.create!(user: user)
+      enterprise.contact_id = user.id
+
+      expect(enterprise.contact).to eq(user)
+    end
+
+    it "rejects users that don't belong to the enterprise" do
+      user = create(:user, confirmed_at: Time.now.utc)
+      enterprise.contact_id = user.id
+
+      expect(enterprise.contact).not_to eq(user)
+    end
+
+    it "rejects unconfirmed users" do
+      user = create(:user, confirmed_at: nil)
+      enterprise.enterprise_roles.create!(user: user)
+      enterprise.contact_id = user.id
+
+      expect(enterprise.contact).not_to eq(user)
+    end
+  end
 end
 
 def enterprise_name_error(owner_email)

spec/requests/admin/user_invitations_spec.rb

@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+RSpec.describe "/admin/user_invitations" do
+  let(:enterprise) { create(:distributor_enterprise) }
+  let(:params) { { enterprise_id: enterprise.permalink } }
+  let(:user) { create(:user) }
+
+  describe "#new" do
+    it "renders the user invitation modal via turbo" do
+      login_as enterprise.owner
+
+      get new_admin_enterprise_user_invitation_path(enterprise, format: :turbo_stream)
+
+      expect(response.body).to include '<turbo-stream action="update" target="remote_modal">'
+    end
+
+    it "redirects the user to the unauthorized path if they are not authorised" do
+      login_as user
+
+      get new_admin_enterprise_user_invitation_path(enterprise, format: :turbo_stream)
+
+      expect(response).to redirect_to unauthorized_path
+    end
+  end
+
+  describe "#create" do
+    it "creates the invitation, displays a success flash, closes the modal and updates the users " \
+       "panel via turbo if the user is authorised" do
+      login_as enterprise.owner
+
+      post admin_enterprise_user_invitations_path(
+        enterprise,
+        user_invitation: { email: "invitee@example.com" },
+        format: :turbo_stream
+      )
+
+      expect(flash[:success]).to be_present
+      expect(response.body).to include '<turbo-stream action="update" target="remote_modal">'
+      expect(response.body).to include '<turbo-stream action="update" target="users_panel">'
+    end
+
+    it "redirects the user to the unauthorized path if they are not authorised" do
+      login_as user
+
+      post admin_enterprise_user_invitations_path(
+        enterprise,
+        user_invitation: { email: "invitee@example.com" },
+        format: :turbo_stream
+      )
+
+      expect(response).to redirect_to unauthorized_path
+    end
+  end
+end

spec/support/tom_select_helper.rb

@@ -10,6 +10,15 @@ module TomSelectHelper
     page.find("body").click
   end
 
+  # Allows adding new values that are not included in the list of possible options
+  def tomselect_fill_in(selector, with:)
+    tomselect_wrapper = page.find_field(selector).sibling(".ts-wrapper")
+    tomselect_wrapper.find(".ts-control").click
+    # Use send_keys as setting the value directly doesn't trigger the search
+    tomselect_wrapper.find(:css, '.ts-dropdown input.dropdown-input').send_keys(with)
+    tomselect_wrapper.find(:css, '.ts-dropdown div.create').click
+  end
+
   def tomselect_search_and_select(value, options)
     tomselect_wrapper = page.find_field(options[:from]).sibling(".ts-wrapper")
     tomselect_wrapper.find(".ts-control").click

spec/system/admin/enterprise_roles_spec.rb

@@ -106,71 +106,36 @@ create(:enterprise)
         expect(page).to have_selector "table.managers"
       end
 
-      it "lists managers and shows icons for owner, contact, and email confirmation" do
+      it "lists managers, with radio button fields for changing the owner and contact and an icon" \
+         "showing email confirmation" do
         within 'table.managers' do
           expect(page).to have_content user1.email
           expect(page).to have_content user2.email
 
           within "tr#manager-#{user1.id}" do
             # user1 is both the enterprise owner and contact, and has email confirmed
-            expect(page).to have_css 'i.owner'
-            expect(page).to have_css 'i.contact'
+            expect(page).to have_checked_field "Set #{user1.email} as owner"
+            expect(page).to have_checked_field "Set #{user1.email} as contact"
             expect(page).to have_css 'i.confirmed'
           end
         end
       end
 
-      xit "allows adding new managers" do
-        within 'table.managers' do
-          select2_select user3.email, from: 'ignored', search: true
-
-          # user3 has been added and has an unconfirmed email address
-          expect(page).to have_css "tr#manager-#{user3.id}"
-          within "tr#manager-#{user3.id}" do
-            expect(page).to have_css 'i.unconfirmed'
-          end
-        end
-      end
-
-      xit "shows changes to enterprise contact or owner" do
-        select2_select user2.email, from: 'receives_notifications_dropdown'
+      it "changing the enterprise's contact and owner" do
+        choose "enterprise_owner_id_#{user2.id}"
+        choose "enterprise_contact_id_#{user2.id}"
         within('#save-bar') { click_button 'Update' }
         navigate_to_enterprise_users
         expect(page).to have_selector "table.managers"
 
         within 'table.managers' do
           within "tr#manager-#{user1.id}" do
-            expect(page).to have_css 'i.owner'
-            expect(page).not_to have_css 'i.contact'
+            expect(page).not_to have_checked_field "Set #{user1.email} as owner"
+            expect(page).not_to have_checked_field "Set #{user1.email} as contact"
           end
           within "tr#manager-#{user2.id}" do
-            expect(page).to have_css 'i.contact'
-          end
-        end
-      end
-
-      xit "can invite unregistered users to be managers" do
-        find('a.button.modal').click
-        expect(page).to have_css '#invite-manager-modal'
-
-        within '#invite-manager-modal' do
-          fill_in 'invite_email', with: new_email
-          click_button 'Invite'
-          expect(page).to have_content "#{new_email} has been invited to manage this enterprise"
-          click_button 'Close'
-        end
-
-        expect(page).not_to have_selector "#invite-manager-modal"
-        expect(page).to have_selector "table.managers"
-
-        new_user = Spree::User.find_by(email: new_email, confirmed_at: 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'
+            expect(page).to have_checked_field "Set #{user2.email} as owner"
+            expect(page).to have_checked_field "Set #{user2.email} as contact"
           end
         end
       end

spec/system/admin/enterprises_spec.rb

@@ -118,7 +118,7 @@ RSpec.describe '
     payment_method = create(:payment_method, distributors: [e2])
     shipping_method = create(:shipping_method, distributors: [e2])
     enterprise_fee = create(:enterprise_fee, enterprise: @enterprise )
-    user = create(:user)
+    user = create(:user, enterprises: [@enterprise])
 
     admin = login_as_admin
 
@@ -151,8 +151,7 @@ RSpec.describe '
       scroll_to(:bottom)
       within(".side_menu") { click_link "Users" }
     end
-    select2_select user.email, from: 'enterprise_owner_id'
-    expect(page).not_to have_selector '.select2-drop-mask' # Ensure select2 has finished
+    choose "Set #{user.email} as owner"
 
     accept_alert do
       click_link "About"
@@ -635,45 +634,46 @@ RSpec.describe '
 
       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
+          expect(page).to have_selector('a', text: /Invite Manager/i)
+          page.find('a', text: /Invite Manager/i).click
+          expect(page).to have_content "Invite a new user"
         end
 
         it "shows an error message if the email is invalid" do
+          expect_any_instance_of(ValidEmail2::Address).to receive(:valid_mx?).and_return(false)
+
           within ".reveal-modal" do
-            expect(page).to have_content "Invite an unregistered user"
-            fill_in "email", with: "invalid_email"
+            tomselect_fill_in "user_invitation[email]", with: "newuser@example.invaliddomain"
 
             expect do
               click_button "Invite"
-              expect(page).to have_content "Email is invalid"
+              expect(page).to have_content "is invalid"
             end.not_to 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
+            tomselect_search_and_select distributor1.owner.email, from: "user_invitation[email]"
 
             expect do
               click_button "Invite"
-              expect(page).to have_content "User already exists"
+              expect(page).to have_content "is already a manager"
             end.not_to 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"
+            tomselect_fill_in "user_invitation[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
+
+          expect(page)
+            .to have_content "email@email.com has been invited to manage this enterprise"
         end
       end
     end