From c955c235d256b68f7d4a2c975014a38dc56fed08 Mon Sep 17 00:00:00 2001 From: artoscinote <85488244+artoscinote@users.noreply.github.com> Date: Tue, 29 Aug 2023 10:32:39 +0200 Subject: [PATCH] Fix user assignment endpoints [SCI-9150] (#6094) * Fix project assignments API endpoint [SCI-9150] * Fix API endpoint for updating experiment assignments [SCI-9150] * Fix API endpoint for updating task assignments [SCI-9150] --- .../experiment_user_assignments_controller.rb | 47 +++++++--- .../v1/project_user_assignments_controller.rb | 94 +++++++++++++++---- .../v1/task_user_assignments_controller.rb | 41 ++++---- 3 files changed, 136 insertions(+), 46 deletions(-) diff --git a/app/controllers/api/v1/experiment_user_assignments_controller.rb b/app/controllers/api/v1/experiment_user_assignments_controller.rb index 6debe9632..146c1c6fa 100644 --- a/app/controllers/api/v1/experiment_user_assignments_controller.rb +++ b/app/controllers/api/v1/experiment_user_assignments_controller.rb @@ -28,22 +28,26 @@ module Api end def update - user_role = UserRole.find user_assignment_params[:user_role_id] - user = @user_assignment.user - experiment_member = ExperimentMember.new( - current_user, - @experiment, - @project, - user, - @user_assignment - ) + ActiveRecord::Base.transaction do + if @user_assignment.user_role_id == user_assignment_params[:user_role_id] + return render body: nil, status: :no_content + end - return render body: nil, status: :no_content if @user_assignment.user_role == user_role + @user_assignment.update!(user_assignment_params.merge(assigned: :manually)) - experiment_member.update(user_role_id: user_role.id, user_id: user.id) - render jsonapi: experiment_member.user_assignment.reload, - serializer: UserAssignmentSerializer, - status: :ok + UserAssignments::PropagateAssignmentJob.perform_later( + @experiment, + @user_assignment.user_id, + @user_assignment.user_role, + current_user.id + ) + + log_change_activity + + render jsonapi: @user_assignment.reload, + serializer: UserAssignmentSerializer, + status: :ok + end end private @@ -69,6 +73,21 @@ module Api def permitted_includes %w(user user_role assignable) end + + def log_change_activity + Activities::CreateActivityService.call( + activity_type: :change_user_role_on_experiment, + owner: current_user, + subject: @experiment, + team: @project.team, + project: @project, + message_items: { + experiment: @experiment.id, + user_target: @user_assignment.user_id, + role: @user_assignment.user_role.name + } + ) + end end end end diff --git a/app/controllers/api/v1/project_user_assignments_controller.rb b/app/controllers/api/v1/project_user_assignments_controller.rb index 32b502e1a..53a4c64a3 100644 --- a/app/controllers/api/v1/project_user_assignments_controller.rb +++ b/app/controllers/api/v1/project_user_assignments_controller.rb @@ -38,37 +38,99 @@ module Api def create raise PermissionError.new(Project, :manage) unless can_manage_project_users?(@project) - # internally we reuse the same logic as for user project assignment - user = @team.users.find(user_project_params[:user_id]) + ActiveRecord::Base.transaction do + user_assignment = UserAssignment.find_or_initialize_by( + assignable: @project, + user_id: user_project_params[:user_id], + team: @project.team + ) - project_member = ProjectMember.new(user, @project, current_user) - project_member.assign = true - project_member.user_role_id = user_project_params[:user_role_id] - project_member.save - render jsonapi: project_member.user_assignment.reload, - serializer: UserAssignmentSerializer, - status: :created + user_assignment.update!( + user_role_id: user_project_params[:user_role_id], + assigned_by: current_user, + assigned: :manually + ) + + log_activity(:assign_user_to_project, { user_target: user_assignment.user.id, + role: user_assignment.user_role.name }) + propagate_job(user_assignment) + + render jsonapi: user_assignment.reload, + serializer: UserAssignmentSerializer, + status: :created + end end def update - user_role = UserRole.find user_project_params[:user_role_id] - project_member = ProjectMember.new(@user_assignment.user, @project, current_user) + # prevent role change if it would result in no manually assigned users having the user management permission + new_user_role = UserRole.find(user_project_params[:user_role_id]) + if !new_user_role.has_permission?(ProjectPermissions::USERS_MANAGE) && + @user_assignment.last_with_permission?(ProjectPermissions::USERS_MANAGE, assigned: :manually) + raise ActiveRecord::RecordInvalid + end - return render body: nil, status: :no_content if project_member.user_assignment&.user_role == user_role + return render body: nil, status: :no_content if @user_assignment&.user_role == new_user_role + + ActiveRecord::Base.transaction do + @user_assignment.update!(user_role: new_user_role) + + log_activity(:change_user_role_on_project, { user_target: @user_assignment.user.id, + role: @user_assignment.user_role.name }) + + propagate_job(@user_assignment) + end - project_member.user_role_id = user_role.id - project_member.update render jsonapi: @user_assignment.reload, serializer: UserAssignmentSerializer, status: :ok end def destroy - project_member = ProjectMember.new(@user_assignment.user, @project, current_user) - project_member.destroy + # prevent deletion of last manually assigned user that can manage users + if @user_assignment.last_with_permission?(ProjectPermissions::USERS_MANAGE, assigned: :manually) + raise ActiveRecord::RecordInvalid + end + + ActiveRecord::Base.transaction do + if @project.visible? + @user_assignment.update!( + user_role: @project.default_public_user_role, + assigned: :automatically + ) + else + @user_assignment.destroy! + end + + propagate_job(@user_assignment, destroy: true) + log_activity(:unassign_user_from_project, { user_target: @user_assignment.user.id, + role: @user_assignment.user_role.name }) + end + render body: nil end private + def propagate_job(user_assignment, destroy: false) + UserAssignments::PropagateAssignmentJob.perform_later( + @project, + user_assignment.user.id, + user_assignment.user_role, + current_user.id, + destroy: destroy + ) + end + + def log_activity(type_of, message_items = {}) + message_items = { project: @project.id }.merge(message_items) + + Activities::CreateActivityService + .call(activity_type: type_of, + owner: current_user, + subject: @project, + team: @project.team, + project: @project, + message_items: message_items) + end + def check_read_permissions # team admins can always manage users, so they should also be able to read them unless can_read_project_users?(@project) || can_manage_project_users?(@project) diff --git a/app/controllers/api/v1/task_user_assignments_controller.rb b/app/controllers/api/v1/task_user_assignments_controller.rb index 129c186e8..e8da94be0 100644 --- a/app/controllers/api/v1/task_user_assignments_controller.rb +++ b/app/controllers/api/v1/task_user_assignments_controller.rb @@ -29,24 +29,18 @@ module Api end def update - user_role = UserRole.find user_assignment_params[:user_role_id] - user = @user_assignment.user - my_module_member = MyModuleMember.new( - current_user, - @task, - @experiment, - @project, - user, - @user_assignment - ) + ActiveRecord::Base.transaction do + if @user_assignment.user_role_id == user_assignment_params[:user_role_id] + return render body: nil, status: :no_content + end - return render body: nil, status: :no_content if @user_assignment.user_role == user_role + @user_assignment.update!(user_assignment_params.merge(assigned: :manually)) + log_change_activity - my_module_member.update(user_role_id: user_role.id, user_id: user.id) - - render jsonapi: my_module_member.user_assignment.reload, - serializer: UserAssignmentSerializer, - status: :ok + render jsonapi: @user_assignment.reload, + serializer: UserAssignmentSerializer, + status: :ok + end end private @@ -72,6 +66,21 @@ module Api def permitted_includes %w(user user_role assignable) end + + def log_change_activity + Activities::CreateActivityService.call( + activity_type: :change_user_role_on_my_module, + owner: current_user, + subject: @task, + team: @project.team, + project: @project, + message_items: { + my_module: @task.id, + user_target: @user_assignment.user_id, + role: @user_assignment.user_role.name + } + ) + end end end end