From f1071575cd22ce21b40c9e4c8f1d975990bad82f Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Fri, 30 Jan 2026 10:56:50 +0000 Subject: [PATCH 01/10] Remove Angular from Enterprise > Settings > Users section --- .../enterprise_controller.js.coffee | 24 ---- .../admin/enterprises_controller.rb | 9 -- .../admin/user_invitations_controller.rb | 31 +++++ .../concerns/manager_invitations.rb | 19 --- app/forms/user_invitation.rb | 47 ++++++++ app/models/enterprise.rb | 10 +- app/models/spree/ability.rb | 2 +- app/models/spree/user.rb | 1 + app/reflexes/invite_manager_reflex.rb | 40 ------- .../permitted_attributes/enterprise.rb | 2 +- .../_add_new_unregistered_manager.html.haml | 21 ---- .../admin/enterprises/form/_users.html.haml | 112 +++++++----------- .../shared/_question_circle_tooltip.html.haml | 1 + .../user_invitations/create.turbo_stream.haml | 5 + .../user_invitations/new.turbo_stream.haml | 17 +++ app/views/spree/layouts/_admin_body.html.haml | 4 +- .../controllers/select_user_controller.js | 37 ++++++ app/webpacker/css/admin/enterprises.scss | 11 +- app/webpacker/css/admin/shared/layout.scss | 12 ++ config/locales/en.yml | 25 ++-- config/routes/admin.rb | 2 + .../admin/enterprises_controller_spec.rb | 6 +- spec/forms/user_invitation_spec.rb | 67 +++++++++++ .../enterprise_controller_spec.js.coffee | 51 +------- spec/models/enterprise_spec.rb | 34 ++++++ spec/requests/admin/user_invitations_spec.rb | 54 +++++++++ spec/support/tom_select_helper.rb | 9 ++ spec/system/admin/enterprise_roles_spec.rb | 57 ++------- spec/system/admin/enterprises_spec.rb | 28 ++--- 29 files changed, 431 insertions(+), 307 deletions(-) create mode 100644 app/controllers/admin/user_invitations_controller.rb delete mode 100644 app/controllers/concerns/manager_invitations.rb create mode 100644 app/forms/user_invitation.rb delete mode 100644 app/reflexes/invite_manager_reflex.rb delete mode 100644 app/views/admin/enterprises/form/_add_new_unregistered_manager.html.haml create mode 100644 app/views/admin/shared/_question_circle_tooltip.html.haml create mode 100644 app/views/admin/user_invitations/create.turbo_stream.haml create mode 100644 app/views/admin/user_invitations/new.turbo_stream.haml create mode 100644 app/webpacker/controllers/select_user_controller.js create mode 100644 spec/forms/user_invitation_spec.rb create mode 100644 spec/requests/admin/user_invitations_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 3209788640..164aea917d 100644 --- a/app/assets/javascripts/admin/enterprises/controllers/enterprise_controller.js.coffee +++ b/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)) diff --git a/app/controllers/admin/enterprises_controller.rb b/app/controllers/admin/enterprises_controller.rb index de96364445..2e50e68185 100644 --- a/app/controllers/admin/enterprises_controller.rb +++ b/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) diff --git a/app/controllers/admin/user_invitations_controller.rb b/app/controllers/admin/user_invitations_controller.rb new file mode 100644 index 0000000000..ccc8a18def --- /dev/null +++ b/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 diff --git a/app/controllers/concerns/manager_invitations.rb b/app/controllers/concerns/manager_invitations.rb deleted file mode 100644 index 587707cac9..0000000000 --- a/app/controllers/concerns/manager_invitations.rb +++ /dev/null @@ -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 diff --git a/app/forms/user_invitation.rb b/app/forms/user_invitation.rb new file mode 100644 index 0000000000..355266d1d6 --- /dev/null +++ b/app/forms/user_invitation.rb @@ -0,0 +1,47 @@ +# 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 + 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 diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index bf8e52d56d..edc872338f 100644 --- a/app/models/enterprise.rb +++ b/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 diff --git a/app/models/spree/ability.rb b/app/models/spree/ability.rb index 4695559945..69aeabf2cf 100644 --- a/app/models/spree/ability.rb +++ b/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 diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb index b32293dbf8..b31099752b 100644 --- a/app/models/spree/user.rb +++ b/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 diff --git a/app/reflexes/invite_manager_reflex.rb b/app/reflexes/invite_manager_reflex.rb deleted file mode 100644 index 51d7bc484b..0000000000 --- a/app/reflexes/invite_manager_reflex.rb +++ /dev/null @@ -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 diff --git a/app/services/permitted_attributes/enterprise.rb b/app/services/permitted_attributes/enterprise.rb index b7c8fa92e4..004454b79d 100644 --- a/app/services/permitted_attributes/enterprise.rb +++ b/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 diff --git a/app/views/admin/enterprises/form/_add_new_unregistered_manager.html.haml b/app/views/admin/enterprises/form/_add_new_unregistered_manager.html.haml deleted file mode 100644 index e933824b0e..0000000000 --- a/app/views/admin/enterprises/form/_add_new_unregistered_manager.html.haml +++ /dev/null @@ -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')}" - diff --git a/app/views/admin/enterprises/form/_users.html.haml b/app/views/admin/enterprises/form/_users.html.haml index 4951d09f8e..7b158289bc 100644 --- a/app/views/admin/enterprises/form/_users.html.haml +++ b/app/views/admin/enterprises/form/_users.html.haml @@ -1,75 +1,55 @@ -- 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 partial: 'admin/shared/question_circle_tooltip', locals: { tooltip_text: t('.owner_tip') } + %th.center + = t('.contact') + = render partial: 'admin/shared/question_circle_tooltip', locals: { tooltip_text: t('.contact_tip') } + %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 + .text-right + %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 diff --git a/app/views/admin/shared/_question_circle_tooltip.html.haml b/app/views/admin/shared/_question_circle_tooltip.html.haml new file mode 100644 index 0000000000..8af9e6e973 --- /dev/null +++ b/app/views/admin/shared/_question_circle_tooltip.html.haml @@ -0,0 +1 @@ += render AdminTooltipComponent.new(text: tooltip_text, link_text: %[].html_safe, link: nil) diff --git a/app/views/admin/user_invitations/create.turbo_stream.haml b/app/views/admin/user_invitations/create.turbo_stream.haml new file mode 100644 index 0000000000..a3ea860748 --- /dev/null +++ b/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 } diff --git a/app/views/admin/user_invitations/new.turbo_stream.haml b/app/views/admin/user_invitations/new.turbo_stream.haml new file mode 100644 index 0000000000..67212f7146 --- /dev/null +++ b/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.text_field :email, placeholder: t('.eg_email_address'), data: { controller: "select-user" } + = 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") diff --git a/app/views/spree/layouts/_admin_body.html.haml b/app/views/spree/layouts/_admin_body.html.haml index 63ed3a5284..74bc12ddf1 100644 --- a/app/views/spree/layouts/_admin_body.html.haml +++ b/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 diff --git a/app/webpacker/controllers/select_user_controller.js b/app/webpacker/controllers/select_user_controller.js new file mode 100644 index 0000000000..3a21520b86 --- /dev/null +++ b/app/webpacker/controllers/select_user_controller.js @@ -0,0 +1,37 @@ +import { Controller } from "stimulus"; +import TomSelect from "tom-select/dist/esm/tom-select.complete"; + +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) => response.json()) + .then((json) => { + callback({ items: json }); + }) + .catch((error) => { + console.log(error); + callback(); + }); + } +} diff --git a/app/webpacker/css/admin/enterprises.scss b/app/webpacker/css/admin/enterprises.scss index 85e115ec34..420c30fbee 100644 --- a/app/webpacker/css/admin/enterprises.scss +++ b/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; diff --git a/app/webpacker/css/admin/shared/layout.scss b/app/webpacker/css/admin/shared/layout.scss index ce532052de..5f754f9330 100644 --- a/app/webpacker/css/admin/shared/layout.scss +++ b/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; } diff --git a/config/locales/en.yml b/config/locales/en.yml index bf2406c867..0c0c376a7e 100644 --- a/config/locales/en.yml +++ b/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 diff --git a/config/routes/admin.rb b/config/routes/admin.rb index 305ef999f6..081aacfc35 100644 --- a/config/routes/admin.rb +++ b/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 diff --git a/spec/controllers/admin/enterprises_controller_spec.rb b/spec/controllers/admin/enterprises_controller_spec.rb index db2d06e9e4..645f62eed4 100644 --- a/spec/controllers/admin/enterprises_controller_spec.rb +++ b/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 }. diff --git a/spec/forms/user_invitation_spec.rb b/spec/forms/user_invitation_spec.rb new file mode 100644 index 0000000000..189a0a0cf0 --- /dev/null +++ b/spec/forms/user_invitation_spec.rb @@ -0,0 +1,67 @@ +# 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 and sends them an invitation email" do + existing_user = create(:user) + user_invitation = UserInvitation.new(defaults.merge(email: existing_user.email)) + + expect do + user_invitation.save + end.to have_enqueued_mail(EnterpriseMailer, :manager_invitation) + + expect(enterprise.users).to include(existing_user) + end + end +end diff --git a/spec/javascripts/unit/admin/enterprises/controllers/enterprise_controller_spec.js.coffee b/spec/javascripts/unit/admin/enterprises/controllers/enterprise_controller_spec.js.coffee index cf88065f10..75c01c3f70 100644 --- a/spec/javascripts/unit/admin/enterprises/controllers/enterprise_controller_spec.js.coffee +++ b/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] diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb index 9deee6ddff..f31721c589 100644 --- a/spec/models/enterprise_spec.rb +++ b/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) diff --git a/spec/requests/admin/user_invitations_spec.rb b/spec/requests/admin/user_invitations_spec.rb new file mode 100644 index 0000000000..48011ca5f0 --- /dev/null +++ b/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 '' + 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 '' + expect(response.body).to include '' + 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 diff --git a/spec/support/tom_select_helper.rb b/spec/support/tom_select_helper.rb index fa483f47f0..34a0179d98 100644 --- a/spec/support/tom_select_helper.rb +++ b/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 diff --git a/spec/system/admin/enterprise_roles_spec.rb b/spec/system/admin/enterprise_roles_spec.rb index 43b08b17f6..880a3493ec 100644 --- a/spec/system/admin/enterprise_roles_spec.rb +++ b/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 diff --git a/spec/system/admin/enterprises_spec.rb b/spec/system/admin/enterprises_spec.rb index 08dc187592..886cc9623b 100644 --- a/spec/system/admin/enterprises_spec.rb +++ b/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,44 @@ 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 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: "invalid_email" expect do click_button "Invite" - expect(page).to have_content "Email is invalid" + expect(page).to have_content "Email invalid_email 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 From 7433f6d183b8068fce41d10757829ba326a25110 Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Fri, 13 Feb 2026 09:58:43 +0000 Subject: [PATCH 02/10] Rename :save to save! on UserInvitation because it possibly could raise an exception --- app/controllers/admin/user_invitations_controller.rb | 2 +- app/forms/user_invitation.rb | 6 +++--- spec/forms/user_invitation_spec.rb | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/admin/user_invitations_controller.rb b/app/controllers/admin/user_invitations_controller.rb index ccc8a18def..46bdba4196 100644 --- a/app/controllers/admin/user_invitations_controller.rb +++ b/app/controllers/admin/user_invitations_controller.rb @@ -8,7 +8,7 @@ module Admin def create @user_invitation.attributes = permitted_resource_params - if @user_invitation.save + if @user_invitation.save! flash[:success] = I18n.t(:user_invited, email: @user_invitation.email) else render :new diff --git a/app/forms/user_invitation.rb b/app/forms/user_invitation.rb index 355266d1d6..db7fba54d4 100644 --- a/app/forms/user_invitation.rb +++ b/app/forms/user_invitation.rb @@ -14,17 +14,17 @@ class UserInvitation validates :enterprise, presence: true validate :not_existing_enterprise_user - def save + def save! return unless valid? - user = find_or_create_user + user = find_or_create_user! enterprise.users << user EnterpriseMailer.manager_invitation(enterprise, user).deliver_later end private - def find_or_create_user + def find_or_create_user! Spree::User.find_or_create_by!(email: email) do |user| user.email = email user.password = SecureRandom.base58(64) diff --git a/spec/forms/user_invitation_spec.rb b/spec/forms/user_invitation_spec.rb index 189a0a0cf0..71d69c60a2 100644 --- a/spec/forms/user_invitation_spec.rb +++ b/spec/forms/user_invitation_spec.rb @@ -42,7 +42,7 @@ RSpec.describe UserInvitation do user_invitation = UserInvitation.new(defaults.merge(email: "new_user@example.com")) expect do - user_invitation.save + user_invitation.save! end.to have_enqueued_mail(EnterpriseMailer, :manager_invitation) new_user = Spree::User.find_by(email: "new_user@example.com") @@ -58,7 +58,7 @@ RSpec.describe UserInvitation do user_invitation = UserInvitation.new(defaults.merge(email: existing_user.email)) expect do - user_invitation.save + user_invitation.save! end.to have_enqueued_mail(EnterpriseMailer, :manager_invitation) expect(enterprise.users).to include(existing_user) From 50265780cfaea414dd5cd2dd1fdefec37e9ebf65 Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Fri, 13 Feb 2026 10:05:59 +0000 Subject: [PATCH 03/10] Call AdminTooltipComponent directly and remove unnecessary partial --- app/views/admin/enterprises/form/_users.html.haml | 4 ++-- app/views/admin/shared/_question_circle_tooltip.html.haml | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) delete mode 100644 app/views/admin/shared/_question_circle_tooltip.html.haml diff --git a/app/views/admin/enterprises/form/_users.html.haml b/app/views/admin/enterprises/form/_users.html.haml index 7b158289bc..460112090a 100644 --- a/app/views/admin/enterprises/form/_users.html.haml +++ b/app/views/admin/enterprises/form/_users.html.haml @@ -12,10 +12,10 @@ %th= t('.manager') %th.center = t('.owner') - = render partial: 'admin/shared/question_circle_tooltip', locals: { tooltip_text: t('.owner_tip') } + = render AdminTooltipComponent.new(text: t('.owner_tip'), link_text: %[].html_safe, link: nil) %th.center = t('.contact') - = render partial: 'admin/shared/question_circle_tooltip', locals: { tooltip_text: t('.contact_tip') } + = render AdminTooltipComponent.new(text: t('.contact_tip'), link_text: %[].html_safe, link: nil) %tbody - @enterprise.users.each do |user| - contact = user.id == @enterprise.contact&.id diff --git a/app/views/admin/shared/_question_circle_tooltip.html.haml b/app/views/admin/shared/_question_circle_tooltip.html.haml deleted file mode 100644 index 8af9e6e973..0000000000 --- a/app/views/admin/shared/_question_circle_tooltip.html.haml +++ /dev/null @@ -1 +0,0 @@ -= render AdminTooltipComponent.new(text: tooltip_text, link_text: %[].html_safe, link: nil) From 1554459eb9552f611bfc4427ebc0bdff69650c2b Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Fri, 13 Feb 2026 10:14:07 +0000 Subject: [PATCH 04/10] Display a JS alert if /admin/search/known_users returns an error --- app/webpacker/controllers/select_user_controller.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/webpacker/controllers/select_user_controller.js b/app/webpacker/controllers/select_user_controller.js index 3a21520b86..a9eb6ea528 100644 --- a/app/webpacker/controllers/select_user_controller.js +++ b/app/webpacker/controllers/select_user_controller.js @@ -1,5 +1,6 @@ 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() { @@ -25,7 +26,13 @@ export default class extends Controller { #load(query, callback) { const url = "/admin/search/known_users.json?q=" + encodeURIComponent(query); fetch(url) - .then((response) => response.json()) + .then((response) => { + if (!response.ok) { + showHttpError(response.status); + throw response; + } + return response.json(); + }) .then((json) => { callback({ items: json }); }) From efae11e2af0a77c40470022200ac6738f51ee498 Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Fri, 13 Feb 2026 10:36:08 +0000 Subject: [PATCH 05/10] Change assertion for flakey test failure --- spec/system/admin/enterprises_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/system/admin/enterprises_spec.rb b/spec/system/admin/enterprises_spec.rb index 886cc9623b..0947ed5608 100644 --- a/spec/system/admin/enterprises_spec.rb +++ b/spec/system/admin/enterprises_spec.rb @@ -645,7 +645,7 @@ RSpec.describe ' expect do click_button "Invite" - expect(page).to have_content "Email invalid_email is invalid" + expect(page).to have_content "is invalid" end.not_to enqueue_job ActionMailer::MailDeliveryJob end end From d3eb887664d7052d110875ad1e7f1092d77c1b35 Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Sat, 28 Feb 2026 14:05:29 +0000 Subject: [PATCH 06/10] Align button --- app/views/admin/enterprises/form/_users.html.haml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/views/admin/enterprises/form/_users.html.haml b/app/views/admin/enterprises/form/_users.html.haml index 460112090a..98208fa2be 100644 --- a/app/views/admin/enterprises/form/_users.html.haml +++ b/app/views/admin/enterprises/form/_users.html.haml @@ -43,10 +43,9 @@ - else %a{ class: "icon-trash no-text disabled" } - .text-right - %a.button{ href: "#{new_admin_enterprise_user_invitation_path(@enterprise)}", data: { turbo_stream: true, turbo: true } } - %i.icon-plus - = t('.invite_manager') + %a.button{ href: "#{new_admin_enterprise_user_invitation_path(@enterprise)}", data: { turbo_stream: true, turbo: true } } + %i.icon-plus + = t('.invite_manager') %br - else From f063e2e8c643f511e2bfb4ee81a803e38ea4d66d Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Sat, 28 Feb 2026 14:14:12 +0000 Subject: [PATCH 07/10] Change to email field --- app/views/admin/user_invitations/new.turbo_stream.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/admin/user_invitations/new.turbo_stream.haml b/app/views/admin/user_invitations/new.turbo_stream.haml index 67212f7146..d016621b3e 100644 --- a/app/views/admin/user_invitations/new.turbo_stream.haml +++ b/app/views/admin/user_invitations/new.turbo_stream.haml @@ -8,7 +8,7 @@ %fieldset.no-border-top.no-border-bottom .row = f.label :email, t(:email) - = f.text_field :email, placeholder: t('.eg_email_address'), data: { controller: "select-user" } + = 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 From d57274fc4c8b3810a5a8eb92bb1f416f046649cc Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Sat, 28 Feb 2026 14:30:19 +0000 Subject: [PATCH 08/10] Manager invitation email is only for new users --- app/forms/user_invitation.rb | 5 ++++- spec/forms/user_invitation_spec.rb | 8 +++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/app/forms/user_invitation.rb b/app/forms/user_invitation.rb index db7fba54d4..f1da146f6e 100644 --- a/app/forms/user_invitation.rb +++ b/app/forms/user_invitation.rb @@ -19,7 +19,10 @@ class UserInvitation user = find_or_create_user! enterprise.users << user - EnterpriseMailer.manager_invitation(enterprise, user).deliver_later + + if user.previously_new_record? + EnterpriseMailer.manager_invitation(enterprise, user).deliver_later + end end private diff --git a/spec/forms/user_invitation_spec.rb b/spec/forms/user_invitation_spec.rb index 71d69c60a2..856f675790 100644 --- a/spec/forms/user_invitation_spec.rb +++ b/spec/forms/user_invitation_spec.rb @@ -53,13 +53,11 @@ RSpec.describe UserInvitation do end context "inviting a existing user who isn't a user on the enterprise" do - it "adds the user to the enterprise and sends them an invitation email" do + it "adds the user to the enterprise" do existing_user = create(:user) - user_invitation = UserInvitation.new(defaults.merge(email: existing_user.email)) - expect do - user_invitation.save! - end.to have_enqueued_mail(EnterpriseMailer, :manager_invitation) + user_invitation = UserInvitation.new(defaults.merge(email: existing_user.email)) + user_invitation.save! expect(enterprise.users).to include(existing_user) end From 67853bb97621c30b78b9f4a2a70b1294d8b94b1b Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Sat, 28 Feb 2026 14:48:43 +0000 Subject: [PATCH 09/10] Use guard --- app/forms/user_invitation.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/forms/user_invitation.rb b/app/forms/user_invitation.rb index f1da146f6e..2ff837a540 100644 --- a/app/forms/user_invitation.rb +++ b/app/forms/user_invitation.rb @@ -20,9 +20,9 @@ class UserInvitation user = find_or_create_user! enterprise.users << user - if user.previously_new_record? - EnterpriseMailer.manager_invitation(enterprise, user).deliver_later - end + return unless user.previously_new_record? + + EnterpriseMailer.manager_invitation(enterprise, user).deliver_later end private From 447d80c9605a5b93146c2ad6fe45510fa7b3a44d Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Sat, 28 Feb 2026 14:54:04 +0000 Subject: [PATCH 10/10] Fix spec due to switch to email type field --- spec/system/admin/enterprises_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/system/admin/enterprises_spec.rb b/spec/system/admin/enterprises_spec.rb index 0947ed5608..e831695c51 100644 --- a/spec/system/admin/enterprises_spec.rb +++ b/spec/system/admin/enterprises_spec.rb @@ -640,8 +640,10 @@ RSpec.describe ' 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 - tomselect_fill_in "user_invitation[email]", with: "invalid_email" + tomselect_fill_in "user_invitation[email]", with: "newuser@example.invaliddomain" expect do click_button "Invite"