From 86e9044a83363553a3df5f28789b1d42c4d7bbfd Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Thu, 9 Apr 2026 14:36:25 +0200 Subject: [PATCH] Implement adding users with ensured membership in one department only --- .../departments/add_user_component.html.erb | 8 +- .../admin/departments/add_user_component.rb | 14 ++ .../move_user_dialog_component.html.erb | 59 ++++++++ .../departments/move_user_dialog_component.rb | 49 +++++++ .../admin/departments_controller.rb | 35 +++-- app/services/departments/add_user_service.rb | 97 +++++++++++++ config/locales/en.yml | 7 + spec/factories/group_factory.rb | 8 ++ .../departments/add_user_service_spec.rb | 136 ++++++++++++++++++ 9 files changed, 403 insertions(+), 10 deletions(-) create mode 100644 app/components/admin/departments/move_user_dialog_component.html.erb create mode 100644 app/components/admin/departments/move_user_dialog_component.rb create mode 100644 app/services/departments/add_user_service.rb create mode 100644 spec/services/departments/add_user_service_spec.rb diff --git a/app/components/admin/departments/add_user_component.html.erb b/app/components/admin/departments/add_user_component.html.erb index 2933f42446b..bbaffadb8d9 100644 --- a/app/components/admin/departments/add_user_component.html.erb +++ b/app/components/admin/departments/add_user_component.html.erb @@ -39,6 +39,8 @@ See COPYRIGHT and LICENSE files for more details. helpers.admin_departments_path end + autocompleter_filters = filters + render_inline_form(f) do |form| form.group(layout: :horizontal) do |row| row.autocompleter( @@ -47,8 +49,12 @@ See COPYRIGHT and LICENSE files for more details. visually_hide_label: true, autocomplete_options: { resource: "principals", + component: "opce-user-autocompleter", url: ::API::V3::Utilities::PathHelper::ApiV3Path.principals, - filters: [{ name: "type", operator: "=", values: ["User"] }].to_json, + searchKey: "any_name_attribute", + focusDirectly: true, + multiple: false, + filters: autocompleter_filters, inputName: "user_id" } ) diff --git a/app/components/admin/departments/add_user_component.rb b/app/components/admin/departments/add_user_component.rb index 948ead9486f..61dddf5ae57 100644 --- a/app/components/admin/departments/add_user_component.rb +++ b/app/components/admin/departments/add_user_component.rb @@ -41,6 +41,20 @@ module Admin super() @group = group end + + def filters + filters = [ + { name: "type", operator: "=", values: %w[User] }, + { name: "status", operator: "=", values: [Principal.statuses[:active].to_s, Principal.statuses[:invited].to_s] } + ] + + existing_user_ids = group.user_ids.map(&:to_s) + if existing_user_ids.any? + filters << { name: "id", operator: "!", values: existing_user_ids } + end + + filters + end end end end diff --git a/app/components/admin/departments/move_user_dialog_component.html.erb b/app/components/admin/departments/move_user_dialog_component.html.erb new file mode 100644 index 00000000000..88cbef2812a --- /dev/null +++ b/app/components/admin/departments/move_user_dialog_component.html.erb @@ -0,0 +1,59 @@ +<%#-- copyright +OpenProject is an open source project management software. +Copyright (C) the OpenProject GmbH + +This program is free software; you can redistribute it and/or +modify it under the terms of the GNU General Public License version 3. + +OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +Copyright (C) 2006-2013 Jean-Philippe Lang +Copyright (C) 2010-2013 the ChiliProject Team + +This program is free software; you can redistribute it and/or +modify it under the terms of the GNU General Public License +as published by the Free Software Foundation; either version 2 +of the License, or (at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program; if not, write to the Free Software +Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +See COPYRIGHT and LICENSE files for more details. + +++#%> + +<%= + render( + Primer::OpenProject::DangerDialog.new( + id: DIALOG_ID, + title: t("departments.move_user_dialog.title"), + confirm_button_text: t("departments.move_user_dialog.confirm"), + size: :medium_portrait, + form_arguments: { + action: add_user_admin_department_path(to_department), + method: :post + } + ) + ) do |dialog| + dialog.with_confirmation_message do |message| + message.with_heading(tag: :h2) { t("departments.move_user_dialog.heading") } + message.with_description do + t( + "departments.move_user_dialog.description", + user: moved_user.name, + from_department: from_department.name + ) + end + end + + dialog.with_additional_details do + concat(hidden_field_tag(:user_id, moved_user.id)) + concat(hidden_field_tag(:remove_from_previous_department, "true")) + end + end +%> diff --git a/app/components/admin/departments/move_user_dialog_component.rb b/app/components/admin/departments/move_user_dialog_component.rb new file mode 100644 index 00000000000..0aa4bcbb041 --- /dev/null +++ b/app/components/admin/departments/move_user_dialog_component.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module Admin + module Departments + class MoveUserDialogComponent < ApplicationComponent + include ApplicationHelper + include OpTurbo::Streamable + + DIALOG_ID = "move-user-department-dialog" + + attr_reader :moved_user, :from_department, :to_department + + def initialize(user:, from_department:, to_department:) + super() + @moved_user = user + @from_department = from_department + @to_department = to_department + end + end + end +end diff --git a/app/controllers/admin/departments_controller.rb b/app/controllers/admin/departments_controller.rb index d36c8caed05..4100604d456 100644 --- a/app/controllers/admin/departments_controller.rb +++ b/app/controllers/admin/departments_controller.rb @@ -40,7 +40,8 @@ module Admin # TODO: We will check for users permission here before_action :require_admin before_action :find_group, - only: %i[show edit new_user update add_users remove_user create_memberships edit_membership destroy_membership] + only: %i[show edit new_user add_user update add_users remove_user create_memberships edit_membership + destroy_membership] def index @groups = Group.with_detail.organizational_units.visible.order(:lastname) @@ -51,8 +52,28 @@ module Admin @ancestors = @group.ancestors(order: :asc) end - def add_user - # TODO: Implement + def add_user # rubocop:disable Metrics/AbcSize + result = ::Departments::AddUserService + .new(@group, user: current_user) + .call( + user_id: params[:user_id], + remove_from_previous_department: params[:remove_from_previous_department] == "true" + ) + + if result.success? + redirect_to admin_department_path(@group), status: :see_other + elsif result.result.is_a?(Group) + respond_with_dialog( + Admin::Departments::MoveUserDialogComponent.new( + user: User.find(params[:user_id]), + from_department: result.result, + to_department: @group + ) + ) + else + flash[:error] = result.errors.full_messages.join("\n") + redirect_to admin_department_path(@group), status: :see_other + end end def new_department @@ -96,9 +117,7 @@ module Admin render action: :index end - def edit - @group = Group.includes(:members, :users, :group_detail).find(params[:id]) - end + def edit; end def update service_call = ::Groups::UpdateService @@ -122,8 +141,6 @@ module Admin end def remove_user - @group = Group.includes(:group_users).find(params[:id]) - service_call = ::Groups::UpdateService .new(user: current_user, model: @group) .call(remove_user_ids: Array(params[:user_id])) @@ -172,7 +189,7 @@ module Admin end def find_group - @group = Group.visible.find(params[:id]) + @group = Group.visible.organizational_units.includes(:members, :users, :group_detail).find(params[:id]) end def respond_membership_altered(service_call) diff --git a/app/services/departments/add_user_service.rb b/app/services/departments/add_user_service.rb new file mode 100644 index 00000000000..2e4cc76ad7f --- /dev/null +++ b/app/services/departments/add_user_service.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module Departments + class AddUserService < ::BaseServices::BaseContracted + def initialize(department, user:, contract_class: AdminOnlyContract) + self.model = department + super(user:, contract_class:) + end + + private + + def persist(call) + user_id = params[:user_id].to_i + existing_department = find_existing_department(user_id) + + if existing_department.nil? || existing_department.id == model.id + add_user_to_department(model, user_id, call) + else + handle_existing_membership(existing_department, user_id, call) + end + + call + end + + def handle_existing_membership(existing_department, user_id, call) + if params[:remove_from_previous_department] + move_user(from: existing_department, to: model, user_id:, call:) + else + call.success = false + call.result = existing_department + end + end + + def find_existing_department(user_id) + GroupUser + .joins(:group) + .merge(Group.organizational_units) + .where(user_id:) + .first + &.group + end + + def add_user_to_department(department, user_id, call) + result = Groups::UpdateService + .new(user:, model: department) + .call(add_user_ids: [user_id]) + + call.add_dependent!(result) + end + + def remove_user_from_department(department, user_id, call) + result = Groups::UpdateService + .new(user:, model: department) + .call(remove_user_ids: [user_id]) + + call.add_dependent!(result) + end + + def move_user(from:, to:, user_id:, call:) + Group.transaction do + remove_user_from_department(from, user_id, call) + raise ActiveRecord::Rollback unless call.success? + + add_user_to_department(to, user_id, call) + raise ActiveRecord::Rollback unless call.success? + end + end + end +end diff --git a/config/locales/en.yml b/config/locales/en.yml index 016a9d998ae..40644013711 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -737,6 +737,13 @@ en: add_department_form: name_label: "Department name" name_placeholder: "Enter department name" + move_user_dialog: + title: "User already in a department" + heading: "Move user to this department?" + description: "%{user} is currently a member of %{from_department}. Moving them will remove them from that department." + confirm: "Move user" + errors: + move_user_failed: "Failed to move user between departments." pagination: label: "Pagination" diff --git a/spec/factories/group_factory.rb b/spec/factories/group_factory.rb index 881464df3e2..09d0af5b05a 100644 --- a/spec/factories/group_factory.rb +++ b/spec/factories/group_factory.rb @@ -49,6 +49,14 @@ FactoryBot.define do end end + factory :department do + sequence(:lastname) { |n| "Department #{n}" } + + callback(:after_create) do |group| + group.detail.update!(organizational_unit: true) + end + end + factory :group_marked_for_deletion do lastname { "DeletedGroup" } status { Group.statuses[:deleted] } diff --git a/spec/services/departments/add_user_service_spec.rb b/spec/services/departments/add_user_service_spec.rb new file mode 100644 index 00000000000..2815b489cd8 --- /dev/null +++ b/spec/services/departments/add_user_service_spec.rb @@ -0,0 +1,136 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" + +RSpec.describe Departments::AddUserService do + let(:admin) { create(:admin) } + let(:user_to_add) { create(:user) } + + before do + allow(Notifications::GroupMemberAlteredJob).to receive(:perform_later) + end + + describe "#call" do + context "when user is not in any department" do + let!(:department) { create(:department) } + + it "adds the user successfully" do + result = described_class.new(department, user: admin).call(user_id: user_to_add.id) + + expect(result).to be_success + expect(department.reload.users).to include(user_to_add) + end + end + + context "when user is already in the same department" do + let!(:department) { create(:department, members: [user_to_add]) } + + it "succeeds (idempotent)" do + result = described_class.new(department, user: admin).call(user_id: user_to_add.id) + + expect(result).to be_success + expect(department.reload.users).to include(user_to_add) + end + end + + context "when user is in a different department" do + let!(:department_a) { create(:department, members: [user_to_add]) } + let!(:department_b) { create(:department) } + + context "without remove_from_previous_department flag" do + it "returns failure with the existing department" do + result = described_class.new(department_b, user: admin).call(user_id: user_to_add.id) + + expect(result).to be_failure + expect(result.result).to eq(department_a) + end + + it "does not add the user to the new department" do + described_class.new(department_b, user: admin).call(user_id: user_to_add.id) + + expect(department_b.reload.users).not_to include(user_to_add) + end + + it "does not remove the user from the old department" do + described_class.new(department_b, user: admin).call(user_id: user_to_add.id) + + expect(department_a.reload.users).to include(user_to_add) + end + end + + context "with remove_from_previous_department flag" do + it "moves the user successfully" do + result = described_class.new(department_b, user: admin).call(user_id: user_to_add.id, + remove_from_previous_department: true) + + expect(result).to be_success + expect(department_b.reload.users).to include(user_to_add) + expect(department_a.reload.users).not_to include(user_to_add) + end + end + end + + context "when user is in a regular (non-department) group" do + let!(:regular_group) { create(:group, members: [user_to_add]) } + let!(:department) { create(:department) } + + it "adds the user without conflict" do + result = described_class.new(department, user: admin).call(user_id: user_to_add.id) + + expect(result).to be_success + expect(department.reload.users).to include(user_to_add) + expect(regular_group.reload.users).to include(user_to_add) + end + end + + context "when user_id is passed as a string" do + let!(:department) { create(:department) } + + it "handles string user_id" do + result = described_class.new(department, user: admin).call(user_id: user_to_add.id.to_s) + + expect(result).to be_success + expect(department.reload.users).to include(user_to_add) + end + end + + context "when called by a non-admin user" do + let(:regular_user) { create(:user) } + let!(:department) { create(:department) } + + it "returns failure" do + result = described_class.new(department, user: regular_user).call(user_id: user_to_add.id) + + expect(result).to be_failure + end + end + end +end