Project activities refactoring

app/controllers/project_comments_controller.rb

@@ -51,17 +51,7 @@ class ProjectCommentsController < ApplicationController
     respond_to do |format|
       if @comment.save
         project_comment_annotation_notification
-        # Generate activity
+        log_activity(:add_comment_to_project)
-        Activity.create(
-          type_of: :add_comment_to_project,
-          user: current_user,
-          project: @project,
-          message: t(
-            'activities.add_comment_to_project',
-            user: current_user.full_name,
-            project: @project.name
-          )
-        )
 
         format.json {
           render json: {
@@ -107,19 +97,9 @@ class ProjectCommentsController < ApplicationController
     respond_to do |format|
       format.json do
         if @comment.save
-
           project_comment_annotation_notification(old_text)
-          # Generate activity
+          log_activity(:edit_project_comment)
-          Activity.create(
+
-            type_of: :edit_project_comment,
-            user: current_user,
-            project: @project,
-            message: t(
-              'activities.edit_project_comment',
-              user: current_user.full_name,
-              project: @project.name
-            )
-          )
           message = custom_auto_link(@comment.message, team: current_team)
           render json: { comment: message }, status: :ok
         else
@@ -134,17 +114,8 @@ class ProjectCommentsController < ApplicationController
     respond_to do |format|
       format.json do
         if @comment.destroy
-          # Generate activity
+          log_activity(:delete_project_comment)
-          Activity.create(
+
-            type_of: :delete_project_comment,
-            user: current_user,
-            project: @project,
-            message: t(
-              'activities.delete_project_comment',
-              user: current_user.full_name,
-              project: @project.name
-            )
-          )
           # 'counter' and 'linked_id' are used for counter badge
           render json: { linked_id: @project.id,
                          counter: @project.project_comments.count },
@@ -198,4 +169,14 @@ class ProjectCommentsController < ApplicationController
                  project: link_to(@project.name, project_url(@project)))
     )
   end
+
+  def log_activity(type_of)
+    Activities::CreateActivityService
+      .call(activity_type: type_of,
+            owner: current_user,
+            subject: @project,
+            team: @project.team,
+            project: @project,
+            message_items: { project: @project.id })
+  end
 end

app/controllers/projects_controller.rb

@@ -102,14 +102,7 @@ class ProjectsController < ApplicationController
         project: @project
       )
       up.save
-
+      log_activity(:create_project)
-      Activities::CreateActivityService
-        .call(activity_type: :create_project,
-              owner: current_user,
-              subject: @project,
-              team: @project.team,
-              project: @project,
-              message_items: { project: @project.id })
 
       message = t('projects.create.success_flash', name: @project.name)
       respond_to do |format|
@@ -164,43 +157,32 @@ class ProjectsController < ApplicationController
     message_visibility = nil
     if (project_params.include? :name) &&
        (project_params[:name] != @project.name)
-      message_renamed = t(
+      message_renamed = true
-        'activities.rename_project',
-        user: current_user.full_name,
-        project_old: @project.name,
-        project_new: project_params[:name]
-      )
     end
     if (project_params.include? :visibility) &&
        (project_params[:visibility] != @project.visibility)
-      message_visibility = t(
+      message_visibility = if project_params[:visibility] == 'visible'
-        'activities.change_project_visibility',
+                             t('projects.activity.visibility_visible')
-        user: current_user.full_name,
+                           else
-        project: @project.name,
+                             t('projects.activity.visibility_hidden')
-        visibility: project_params[:visibility] == "visible" ?
+                           end
-          t("general.public") :
-          t("general.private")
-      )
     end
 
     @project.last_modified_by = current_user
     if !return_error && @project.update(project_params)
       # Add activities if needed
       if message_visibility.present?
-        Activity.create(
+        Activities::CreateActivityService
-          type_of: :change_project_visibility,
+          .call(activity_type: :change_project_visibility,
-          user: current_user,
+                owner: current_user,
+                subject: @project,
+                team: @project.team,
                 project: @project,
-          message: message_visibility
+                message_items: { project: @project.id,
-        )
+                                 visibility: message_visibility })
       end
       if message_renamed.present?
-        Activity.create(
+        log_activity(:rename_project)
-          type_of: :rename_project,
-          owner: current_user,
-          project: @project,
-          message: message_renamed
-        )
       end
 
       flash_success = t('projects.update.success_flash', name: @project.name)
@@ -216,34 +198,12 @@ class ProjectsController < ApplicationController
             # The project should be restored
             unless @project.archived
               @project.restore(current_user)
-
+              log_activity(:restore_project)
-              # "Restore project" activity
-              Activity.create(
-                type_of: :restore_project,
-                user: current_user,
-                project: @project,
-                message: t(
-                  'activities.restore_project',
-                  user: current_user.full_name,
-                  project: @project.name
-                )
-              )
             end
           elsif @project.archived
             # The project should be archived
             @project.archive(current_user)
-
+            log_activity(:archive_project)
-            # "Archive project" activity
-            Activity.create(
-              type_of: :archive_project,
-              user: current_user,
-              project: @project,
-              message: t(
-                'activities.archive_project',
-                user: current_user.full_name,
-                project: @project.name
-              )
-            )
           end
           redirect_to projects_path
           flash[:success] = flash_success
@@ -388,4 +348,14 @@ class ProjectsController < ApplicationController
   def check_manage_permissions
     render_403 unless can_manage_project?(@project)
   end
+
+  def log_activity(type_of)
+    Activities::CreateActivityService
+      .call(activity_type: type_of,
+            owner: current_user,
+            subject: @project,
+            team: @project.team,
+            project: @project,
+            message_items: { project: @project.id })
+  end
 end

app/controllers/user_projects_controller.rb

@@ -52,19 +52,7 @@ class UserProjectsController < ApplicationController
     @up.assigned_by = current_user
 
     if @up.save
-      # Generate activity
+      log_activity(:assign_user_to_project)
-      Activity.create(
-        type_of: :assign_user_to_project,
-        user: current_user,
-        project: @project,
-        message: t(
-          "activities.assign_user_to_project",
-          assigned_user: @up.user.full_name,
-          role: @up.role_str,
-          project: @project.name,
-          assigned_by_user: current_user.full_name
-        )
-      )
 
       respond_to do |format|
         format.json do
@@ -90,19 +78,7 @@ class UserProjectsController < ApplicationController
     @up.role = up_params[:role]
 
     if @up.save
-      # Generate activity
+      log_activity(:change_user_role_on_project)
-      Activity.create(
-        type_of: :change_user_role_on_project,
-        user: current_user,
-        project: @project,
-        message: t(
-          "activities.change_user_role_on_project",
-          actor: current_user.full_name,
-          user: @up.user.full_name,
-          project: @project.name,
-          role: @up.role_str
-        )
-      )
 
       respond_to do |format|
         format.json do
@@ -123,20 +99,12 @@ class UserProjectsController < ApplicationController
 
   def destroy
     if @up.destroy
-      # Generate activity
+      log_activity(:unassign_user_from_project)
-      Activity.create(
+      generate_notification(current_user,
-        type_of: :unassign_user_from_project,
+                            @up.user,
-        user: current_user,
+                            false,
-        project: @project,
+                            @up.role_str,
-        message: t(
+                            @project)
-          "activities.unassign_user_from_project",
-          unassigned_user: @up.user.full_name,
-          project: @project.name,
-          unassigned_by_user: current_user.full_name
-        )
-      )
-      generate_notification(current_user, @up.user, false, false, @project)
-
       respond_to do |format|
         format.json do
           redirect_to project_users_edit_path(format: :json),
@@ -191,4 +159,16 @@ class UserProjectsController < ApplicationController
   def up_params
     params.require(:user_project).permit(:user_id, :project_id, :role)
   end
+
+  def log_activity(type_of)
+    Activities::CreateActivityService
+      .call(activity_type: type_of,
+            owner: current_user,
+            subject: @project,
+            team: @project.team,
+            project: @project,
+            message_items: { project: @project.id,
+                             user_target: @up.user.id,
+                             role: @up.role_str })
+  end
 end

app/helpers/notifications_helper.rb

@@ -32,9 +32,10 @@ module NotificationsHelper
       message = "#{I18n.t('search.index.team')} #{team.name}"
     elsif project
       title = I18n.t('activities.unassign_user_from_project',
-                     unassigned_user: target_user.full_name,
+                     user_target: target_user.full_name,
                      project: project.name,
-                     unassigned_by_user: user.full_name)
+                     user: user.full_name,
+                     role: role)
       message = "#{I18n.t('search.index.project')} #{@project.name}"
     end
 

config/locales/en.yml

@@ -319,6 +319,9 @@ en:
       users: "Users"
       experiments: "Experiments"
       tasks: "Tasks"
+    activity:
+      visibility_hidden: "Project members only"
+      visibility_visible: "All team members"
     create:
       success_flash: "Project <strong>%{name}</strong> successfully created."
     update:
@@ -1314,13 +1317,13 @@ en:
     modal:
       modal_title: "Activities"
     create_project: "<i>%{user}</i> created project <strong>%{project}</strong>."
-    rename_project: "<i>%{user}</i> renamed project <strong>%{project_old}</strong> to <strong>%{project_new}</strong>."
+    rename_project: "<i>%{user}</i> renamed project <strong>%{project}</strong>."
     change_project_visibility: "<i>%{user}</i> changed project <strong>%{project}</strong>'s visibility to %{visibility}."
     archive_project: "<i>%{user}</i> moved project <strong>%{project}</strong> to archive."
     restore_project: "<i>%{user}</i> restored project <strong>%{project}</strong> from archive."
-    assign_user_to_project: "<i>%{assigned_user}</i> was added as %{role} to project <strong>%{project}</strong> by <i>%{assigned_by_user}</i>."
+    assign_user_to_project: "<i>%{user}</i> assigned user <i>%{user_target}</i> with user role %{role} to project <strong>%{project}</strong>."
-    change_user_role_on_project: "<i>%{actor}</i> changed <i>%{user}</i>'s role on project <strong>%{project}</strong> to %{role}."
+    change_user_role_on_project: "<i>%{user}</i> changed <i>%{user_target}</i>'s role on project <strong>%{project}</strong> to %{role}."
-    unassign_user_from_project: "<i>%{unassigned_user}</i> was removed from project <strong>%{project}</strong> by <i>%{unassigned_by_user}</i>."
+    unassign_user_from_project: "<i>%{user}</i> removed user <i>%{user_target}</i> with user role %{role} from project <strong>%{project}</strong>."
     create_module: "<i>%{user}</i> created task <strong>%{module}</strong>."
     assign_user_to_module: "<i>%{assigned_user}</i> was added to task <strong>%{module}</strong> by <i>%{assigned_by_user}</i>."
     unassign_user_from_module: "<i>%{unassigned_user}</i> was removed from task <strong>%{module}</strong> by <i>%{unassigned_by_user}</i>."

spec/controllers/project_comments_controller_spec.rb

@@ -0,0 +1,67 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe ProjectCommentsController, type: :controller do
+  login_user
+
+  let(:user) { User.first }
+  let(:team) { create :team, created_by: user }
+  let(:user_team) { create :user_team, team: team, user: user }
+  let(:user_project) { create :user_project, :owner, user: user }
+  let(:project) do
+    create :project, team: team, user_projects: [user_project]
+  end
+  let(:project_comment) do
+    create :project_comment, project: project, user: user
+  end
+
+  describe '#create' do
+    context 'in JSON format' do
+      let(:params) do
+        { project_id: project.id,
+          comment: { message: 'test message' } }
+      end
+
+      it 'calls create activity service' do
+        expect(Activities::CreateActivityService).to receive(:call)
+          .with(hash_including(activity_type: :add_comment_to_project))
+
+        post :create, params: params, format: :json
+      end
+    end
+  end
+
+  describe '#update' do
+    context 'in JSON format' do
+      let(:params) do
+        { project_id: project.id,
+          id: project_comment.id,
+          comment: { message: 'test message updated' } }
+      end
+
+      it 'calls create activity service' do
+        expect(Activities::CreateActivityService).to receive(:call)
+          .with(hash_including(activity_type: :edit_project_comment))
+
+        put :update, params: params, format: :json
+      end
+    end
+  end
+
+  describe '#destroy' do
+    context 'in JSON format' do
+      let(:params) do
+        { project_id: project.id,
+          id: project_comment.id }
+      end
+
+      it 'calls create activity service' do
+        expect(Activities::CreateActivityService).to receive(:call)
+          .with(hash_including(activity_type: :delete_project_comment))
+
+        delete :destroy, params: params, format: :json
+      end
+    end
+  end
+end

spec/controllers/projects_controller_spec.rb

@@ -150,8 +150,8 @@ describe ProjectsController, type: :controller do
     context 'in HTML format' do
       let(:params) do
         { id: project_1.id,
-          project: { name: 'test project A1', team_id: team.id,
+          project: { name: project_1.name, team_id: project_1.team.id,
-                     visibility: 'visible' } }
+                     visibility: project_1.visibility } }
       end
 
       it 'returns redirect response' do
@@ -159,6 +159,39 @@ describe ProjectsController, type: :controller do
         expect(response).to have_http_status(:redirect)
         expect(response.content_type).to eq 'text/html'
       end
+
+      it 'calls create activity service (change_project_visibility)' do
+        params[:project][:visibility] = 'hidden'
+        expect(Activities::CreateActivityService).to receive(:call)
+          .with(hash_including(activity_type: :change_project_visibility))
+
+        put :update, params: params
+      end
+
+      it 'calls create activity service (rename_project)' do
+        params[:project][:name] = 'test project changed'
+        expect(Activities::CreateActivityService).to receive(:call)
+          .with(hash_including(activity_type: :rename_project))
+
+        put :update, params: params
+      end
+
+      it 'calls create activity service (restore_project)' do
+        project_1.update(archived: true)
+        params[:project][:archived] = false
+        expect(Activities::CreateActivityService).to receive(:call)
+          .with(hash_including(activity_type: :restore_project))
+
+        put :update, params: params
+      end
+
+      it 'calls create activity service (archive_project)' do
+        params[:project][:archived] = true
+        expect(Activities::CreateActivityService).to receive(:call)
+          .with(hash_including(activity_type: :archive_project))
+
+        put :update, params: params
+      end
     end
   end
 

spec/controllers/user_projects_controller_spec.rb

@@ -0,0 +1,74 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe UserProjectsController, type: :controller do
+  login_user
+
+  let(:user) { User.first }
+  let(:user_two) { create :user }
+  let(:target_user) { create :user }
+  let!(:team) { create :team, created_by: user }
+  let!(:user_team) { create :user_team, team: team, user: user }
+  let(:user_project) { create :user_project, :owner, user: user }
+  let(:target_user_project) do
+    create :user_project, :normal_user, user: target_user
+  end
+  let(:project) do
+    create :project, team: team, user_projects: [user_project,
+                                                 target_user_project]
+  end
+
+  describe '#create' do
+    context 'in JSON format' do
+      let(:params) do
+        { project_id: project.id,
+          user_project: { user_id: user_two.id,
+                          project_id: project.id,
+                          role: :owner } }
+      end
+
+      it 'calls create activity service' do
+        expect(Activities::CreateActivityService).to receive(:call)
+          .with(hash_including(activity_type: :assign_user_to_project))
+
+        post :create, params: params, format: :json
+      end
+    end
+  end
+
+  describe '#update' do
+    context 'in JSON format' do
+      let(:params) do
+        { project_id: project.id,
+          id: target_user_project.id,
+          user_project: { user_id: target_user.id,
+                          project_id: project.id,
+                          role: :viewer } }
+      end
+
+      it 'calls create activity service' do
+        expect(Activities::CreateActivityService).to receive(:call)
+          .with(hash_including(activity_type: :change_user_role_on_project))
+
+        put :update, params: params, format: :json
+      end
+    end
+  end
+
+  describe '#destroy' do
+    context 'in JSON format' do
+      let(:params) do
+        { project_id: project.id,
+          id: target_user_project.id }
+      end
+
+      it 'calls create activity service' do
+        expect(Activities::CreateActivityService).to receive(:call)
+          .with(hash_including(activity_type: :unassign_user_from_project))
+
+        delete :destroy, params: params, format: :json
+      end
+    end
+  end
+end

spec/factories/project_comments.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :project_comment do
+    user
+    project
+    message { Faker::Lorem.sentence }
+  end
+end