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]

app/controllers/api/v1/experiment_user_assignments_controller.rb

@@ -28,23 +28,27 @@ module Api
       end
 
       def update
-        user_role = UserRole.find user_assignment_params[:user_role_id]
+        ActiveRecord::Base.transaction do
-        user = @user_assignment.user
+          if @user_assignment.user_role_id == user_assignment_params[:user_role_id]
-        experiment_member = ExperimentMember.new(
+            return render body: nil, status: :no_content
-          current_user,
+          end
+
+          @user_assignment.update!(user_assignment_params.merge(assigned: :manually))
+
+          UserAssignments::PropagateAssignmentJob.perform_later(
             @experiment,
-          @project,
+            @user_assignment.user_id,
-          user,
+            @user_assignment.user_role,
-          @user_assignment
+            current_user.id
           )
 
-        return render body: nil, status: :no_content if @user_assignment.user_role == user_role
+          log_change_activity
 
-        experiment_member.update(user_role_id: user_role.id, user_id: user.id)
+          render jsonapi: @user_assignment.reload,
-        render jsonapi: experiment_member.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

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
+        ActiveRecord::Base.transaction do
-        user = @team.users.find(user_project_params[:user_id])
+          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)
+          user_assignment.update!(
-        project_member.assign = true
+            user_role_id: user_project_params[:user_role_id],
-        project_member.user_role_id = user_project_params[:user_role_id]
+            assigned_by: current_user,
-        project_member.save
+            assigned: :manually
-        render jsonapi: project_member.user_assignment.reload,
+          )
+
+          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]
+        # prevent role change if it would result in no manually assigned users having the user management permission
-        project_member = ProjectMember.new(@user_assignment.user, @project, current_user)
+        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)
+        # prevent deletion of last manually assigned user that can manage users
-        project_member.destroy
+        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)

app/controllers/api/v1/task_user_assignments_controller.rb

@@ -29,25 +29,19 @@ module Api
       end
 
       def update
-        user_role = UserRole.find user_assignment_params[:user_role_id]
+        ActiveRecord::Base.transaction do
-        user = @user_assignment.user
+          if @user_assignment.user_role_id == user_assignment_params[:user_role_id]
-        my_module_member = MyModuleMember.new(
+            return render body: nil, status: :no_content
-          current_user,
+          end
-          @task,
-          @experiment,
-          @project,
-          user,
-          @user_assignment
-        )
 
-        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: @user_assignment.reload,
-
-        render jsonapi: my_module_member.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