From 7757096693f49418cd1463102a942f233a6fc6ea Mon Sep 17 00:00:00 2001 From: Mojca Lorber Date: Wed, 13 Mar 2019 19:05:29 +0100 Subject: [PATCH] Results activities refactoring --- app/controllers/reports_controller.rb | 30 ++++---- app/controllers/result_assets_controller.rb | 57 +++++++------- app/controllers/result_comments_controller.rb | 54 ++++---------- app/controllers/result_tables_controller.rb | 53 ++++--------- app/controllers/result_texts_controller.rb | 53 ++++--------- app/controllers/results_controller.rb | 36 ++++----- config/locales/en.yml | 20 ++--- .../result_assets_controller_spec.rb | 58 +++++++++++++++ .../result_comments_controller_spec.rb | 74 +++++++++++++++++++ .../result_tables_controller_spec.rb | 59 +++++++++++++++ .../result_texts_controller_spec.rb | 59 +++++++++++++++ spec/controllers/results_controller_spec.rb | 35 +++++++++ spec/factories/result_comment.rb | 9 +++ spec/factories/result_table.rb | 1 + 14 files changed, 402 insertions(+), 196 deletions(-) create mode 100644 spec/controllers/result_assets_controller_spec.rb create mode 100644 spec/controllers/result_comments_controller_spec.rb create mode 100644 spec/controllers/result_tables_controller_spec.rb create mode 100644 spec/controllers/result_texts_controller_spec.rb create mode 100644 spec/controllers/results_controller_spec.rb create mode 100644 spec/factories/result_comment.rb diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index 075c7eb79..fac9e03e9 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -67,14 +67,8 @@ class ReportsController < ApplicationController @report.last_modified_by = current_user if continue && @report.save_with_contents(report_contents) - # record an activity - Activities::CreateActivityService - .call(activity_type: :create_report, - owner: current_user, - subject: @report, - team: @report.team, - project: @report.project, - message_items: { report: @report.id }) + log_activity(:create_report) + respond_to do |format| format.json do render json: { url: reports_path }, status: :ok @@ -109,14 +103,8 @@ class ReportsController < ApplicationController @report.assign_attributes(report_params) if continue && @report.save_with_contents(report_contents) - # record an activity - Activities::CreateActivityService - .call(activity_type: :edit_report, - owner: current_user, - subject: @report, - team: @report.team, - project: @report.project, - message_items: { report: @report.id }) + log_activity(:edit_report) + respond_to do |format| format.json do render json: { url: reports_path }, status: :ok @@ -504,4 +492,14 @@ class ReportsController < ApplicationController :repository_item_id, :html) end + + def log_activity(type_of) + Activities::CreateActivityService + .call(activity_type: type_of, + owner: current_user, + subject: @report, + team: @report.team, + project: @report.project, + message_items: { report: @report.id }) + end end diff --git a/app/controllers/result_assets_controller.rb b/app/controllers/result_assets_controller.rb index 7b958bb15..db7e5ff68 100644 --- a/app/controllers/result_assets_controller.rb +++ b/app/controllers/result_assets_controller.rb @@ -93,18 +93,7 @@ class ResultAssetsController < ApplicationController success_flash = t('result_assets.archive.success_flash', module: @my_module.name) if saved - Activity.create( - type_of: :archive_result, - project: @my_module.experiment.project, - experiment: @my_module.experiment, - my_module: @my_module, - user: current_user, - message: t( - 'activities.archive_asset_result', - user: current_user.full_name, - result: @result.name - ) - ) + log_activity(:archive_result) end elsif @result.archived_changed?(from: true, to: false) render_403 @@ -135,16 +124,7 @@ class ResultAssetsController < ApplicationController # Post process new file if neccesary @result.asset.post_process_file(team) if @result.asset.present? - Activity.create( - type_of: :edit_result, - user: current_user, - project: @my_module.experiment.project, - experiment: @my_module.experiment, - my_module: @my_module, - message: t('activities.edit_asset_result', - user: current_user.full_name, - result: @result.name) - ) + log_activity(:edit_result) end end @@ -229,20 +209,33 @@ class ResultAssetsController < ApplicationController asset.post_process_file(@my_module.experiment.project.team) # Generate activity - Activity.create( - type_of: :add_result, - user: current_user, - project: @my_module.experiment.project, - experiment: @my_module.experiment, - my_module: @my_module, - message: t('activities.add_asset_result', - user: current_user.full_name, - result: result.name) - ) + Activities::CreateActivityService + .call(activity_type: :add_result, + owner: current_user, + subject: result, + team: @my_module.experiment.project.team, + project: @my_module.experiment.project, + message_items: { + result: result.id, + result_type: t('activities.result_type.asset') + }) else success = false end end { status: success, results: results } end + + def log_activity(type_of) + Activities::CreateActivityService + .call(activity_type: type_of, + owner: current_user, + subject: @result, + team: @my_module.experiment.project.team, + project: @my_module.experiment.project, + message_items: { + result: @result.id, + result_type: t('activities.result_type.asset') + }) + end end diff --git a/app/controllers/result_comments_controller.rb b/app/controllers/result_comments_controller.rb index 153a1ea0a..45881341c 100644 --- a/app/controllers/result_comments_controller.rb +++ b/app/controllers/result_comments_controller.rb @@ -50,21 +50,8 @@ class ResultCommentsController < ApplicationController respond_to do |format| if @comment.save - result_comment_annotation_notification - # Generate activity - Activity.create( - type_of: :add_comment_to_result, - user: current_user, - project: @result.my_module.experiment.project, - experiment: @result.my_module.experiment, - my_module: @result.my_module, - message: t( - 'activities.add_comment_to_result', - user: current_user.full_name, - result: @result.name - ) - ) + log_activity(:add_comment_to_result) format.json { render json: { @@ -109,21 +96,8 @@ class ResultCommentsController < ApplicationController respond_to do |format| format.json do if @comment.save - result_comment_annotation_notification(old_text) - # Generate activity - Activity.create( - type_of: :edit_result_comment, - user: current_user, - project: @result.my_module.experiment.project, - experiment: @result.my_module.experiment, - my_module: @result.my_module, - message: t( - 'activities.edit_result_comment', - user: current_user.full_name, - result: @result.name - ) - ) + log_activity(:edit_result_comment) message = custom_auto_link(@comment.message, team: current_team) render json: { comment: message }, status: :ok else @@ -138,19 +112,7 @@ class ResultCommentsController < ApplicationController respond_to do |format| format.json do if @comment.destroy - # Generate activity - Activity.create( - type_of: :delete_result_comment, - user: current_user, - project: @result.my_module.experiment.project, - experiment: @result.my_module.experiment, - my_module: @result.my_module, - message: t( - 'activities.delete_result_comment', - user: current_user.full_name, - result: @result.name - ) - ) + log_activity(:delete_result_comment) render json: {}, status: :ok else render json: { message: I18n.t('comments.delete_error') }, @@ -212,4 +174,14 @@ class ResultCommentsController < ApplicationController ))) ) end + + def log_activity(type_of) + Activities::CreateActivityService + .call(activity_type: type_of, + owner: current_user, + subject: @result, + team: @result.my_module.experiment.project.team, + project: @result.my_module.experiment.project, + message_items: { result: @result.id }) + end end diff --git a/app/controllers/result_tables_controller.rb b/app/controllers/result_tables_controller.rb index 9485c6728..dce570bcb 100644 --- a/app/controllers/result_tables_controller.rb +++ b/app/controllers/result_tables_controller.rb @@ -43,19 +43,7 @@ class ResultTablesController < ApplicationController respond_to do |format| if (@result.save and @table.save) then - # Generate activity - Activity.create( - type_of: :add_result, - user: current_user, - project: @my_module.experiment.project, - experiment: @my_module.experiment, - my_module: @my_module, - message: t( - "activities.add_table_result", - user: current_user.full_name, - result: @result.name - ) - ) + log_activity(:add_result) format.html { flash[:success] = t( @@ -103,18 +91,7 @@ class ResultTablesController < ApplicationController flash_success = t("result_tables.archive.success_flash", module: @my_module.name) if saved - Activity.create( - type_of: :archive_result, - project: @my_module.experiment.project, - experiment: @my_module.experiment, - my_module: @my_module, - user: current_user, - message: t( - 'activities.archive_table_result', - user: current_user.full_name, - result: @result.name - ) - ) + log_activity(:archive_result) end elsif @result.archived_changed?(from: true, to: false) render_403 @@ -122,18 +99,7 @@ class ResultTablesController < ApplicationController saved = @result.save if saved then - Activity.create( - type_of: :edit_result, - user: current_user, - project: @my_module.experiment.project, - experiment: @my_module.experiment, - my_module: @my_module, - message: t( - "activities.edit_table_result", - user: current_user.full_name, - result: @result.name - ) - ) + log_activity(:edit_result) end end respond_to do |format| @@ -214,4 +180,17 @@ class ResultTablesController < ApplicationController ] ) end + + def log_activity(type_of) + Activities::CreateActivityService + .call(activity_type: type_of, + owner: current_user, + subject: @result, + team: @my_module.experiment.project.team, + project: @my_module.experiment.project, + message_items: { + result: @result.id, + result_type: t('activities.result_type.table') + }) + end end diff --git a/app/controllers/result_texts_controller.rb b/app/controllers/result_texts_controller.rb index 95bf3ac48..5a6d78551 100644 --- a/app/controllers/result_texts_controller.rb +++ b/app/controllers/result_texts_controller.rb @@ -49,19 +49,7 @@ class ResultTextsController < ApplicationController link_tiny_mce_assets(@result_text.text, @result_text) result_annotation_notification - # Generate activity - Activity.create( - type_of: :add_result, - user: current_user, - project: @my_module.experiment.project, - experiment: @my_module.experiment, - my_module: @my_module, - message: t( - "activities.add_text_result", - user: current_user.full_name, - result: @result.name - ) - ) + log_activity(:add_result) format.html { flash[:success] = t( @@ -115,18 +103,7 @@ class ResultTextsController < ApplicationController success_flash = t("result_texts.archive.success_flash", module: @my_module.name) if saved - Activity.create( - type_of: :archive_result, - project: @my_module.experiment.project, - experiment: @my_module.experiment, - my_module: @my_module, - user: current_user, - message: t( - 'activities.archive_text_result', - user: current_user.full_name, - result: @result.name - ) - ) + log_activity(:archive_result) end elsif @result.archived_changed?(from: true, to: false) render_403 @@ -134,18 +111,7 @@ class ResultTextsController < ApplicationController saved = @result.save if saved then - Activity.create( - type_of: :edit_result, - user: current_user, - project: @my_module.experiment.project, - experiment: @my_module.experiment, - my_module: @my_module, - message: t( - "activities.edit_text_result", - user: current_user.full_name, - result: @result.name - ) - ) + log_activity(:edit_result) end end @@ -242,4 +208,17 @@ class ResultTextsController < ApplicationController ))) ) end + + def log_activity(type_of) + Activities::CreateActivityService + .call(activity_type: type_of, + owner: current_user, + subject: @result, + team: @my_module.experiment.project.team, + project: @my_module.experiment.project, + message_items: { + result: @result.id, + result_type: t('activities.result_type.text') + }) + end end diff --git a/app/controllers/results_controller.rb b/app/controllers/results_controller.rb index ae9dd51f2..45272730e 100644 --- a/app/controllers/results_controller.rb +++ b/app/controllers/results_controller.rb @@ -3,27 +3,21 @@ class ResultsController < ApplicationController before_action :check_destroy_permissions def destroy - act_log = t('my_modules.module_archive.table_log', - user: current_user.name, - result: @result.name, - date: l(Time.current, format: :full_date)) - act_log = t('my_modules.module_archive.text_log', - user: current_user.name, - result: @result.name, - date: l(Time.current, format: :full_date)) if @result.is_text - act_log = t('my_modules.module_archive.asset_log', - user: current_user.name, - result: @result.name, - date: l(Time.current, format: :full_date)) if @result.is_asset - - Activity.create( - type_of: :destroy_result, - user: current_user, - project: @my_module.experiment.project, - experiment: @my_module.experiment, - my_module: @my_module, - message: act_log - ) + result_type = if @result.is_text + t('activities.result_type.text') + elsif @result.is_table + t('activities.result_type.table') + elsif @result.is_asset + t('activities.result_type.asset') + end + Activities::CreateActivityService + .call(activity_type: :destroy_result, + owner: current_user, + subject: @result, + team: @my_module.experiment.project.team, + project: @my_module.experiment.project, + message_items: { result: @result.id, + result_type: result_type }) flash[:success] = t('my_modules.module_archive.delete_flash', result: @result.name, module: @my_module.name) diff --git a/config/locales/en.yml b/config/locales/en.yml index 85e0902cf..9a4c6b58f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -587,9 +587,6 @@ en: option_delete: "Delete" confirm_delete: "Are you sure you want to permanently delete result?" delete_flash: "Sucessfully removed result %{result} from task %{module}." - table_log: "%{user} deleted table result %{result} on %{date}" - text_log: "%{user} deleted text result %{result} on %{date}" - asset_log: "%{user} deleted file result %{result} on %{date}" archived_on: "Archived on" archived_on_title: "Result archived on %{date} at %{time}." option_download: "Download" @@ -1316,6 +1313,10 @@ en: collapse_all: "Collapse all" modal: modal_title: "Activities" + result_type: + text: "text" + table: "table" + asset: "file" create_project: "%{user} created project %{project}." rename_project: "%{user} renamed project %{project}." change_project_visibility: "%{user} changed project %{project}'s visibility to %{visibility}." @@ -1345,16 +1346,11 @@ en: check_step_checklist_item: "%{user} completed checklist item %{checkbox} (%{completed}/%{all} completed) in Step %{step} %{step_name}." uncheck_step_checklist_item: "%{user} uncompleted checklist item %{checkbox} (%{completed}/%{all} completed) in Step %{step} %{step_name}." edit_step: "%{user} edited Step %{step} %{step_name}." - add_asset_result: "%{user} added file result %{result}." - add_text_result: "%{user} added text result %{result}." - add_table_result: "%{user} added table result %{result}." + add_result: "%{user} added %{result_type} result %{result}." add_comment_to_result: "%{user} commented on result %{result}." - archive_asset_result: "%{user} archived file result %{result}." - archive_text_result: "%{user} archived text result %{result}." - archive_table_result: "%{user} archived table result %{result}." - edit_asset_result: "%{user} edited file result %{result}." - edit_text_result: "%{user} edited text result %{result}." - edit_table_result: "%{user} edited table result %{result}." + archive_result: "%{user} archived %{result_type} result %{result}." + destroy_result: "%{user} deleted %{result_type} result %{result}." + edit_result: "%{user} edited %{result_type} result %{result}." archive_experiment: "%{user} archived experiment %{experiment}." create_experiment: "%{user} created experiment %{experiment}." edit_experiment: "%{user} edited experiment %{experiment}." diff --git a/spec/controllers/result_assets_controller_spec.rb b/spec/controllers/result_assets_controller_spec.rb new file mode 100644 index 000000000..0748675a4 --- /dev/null +++ b/spec/controllers/result_assets_controller_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe ResultAssetsController, type: :controller do + login_user + + let(:user) { User.first } + let!(:team) { create :team, :with_members } + let!(:user_project) { create :user_project, :owner, user: user } + let(:project) do + create :project, team: team, user_projects: [user_project] + end + let(:experiment) { create :experiment, project: project } + let(:task) { create :my_module, name: 'test task', experiment: experiment } + let(:result) do + create :result, name: 'test result', my_module: task, user: user + end + let(:result_asset) { create :result_asset, result: result } + + describe '#create' do + let(:params) do + { my_module_id: task.id, + results_names: { '0': 'result name created' }, + results_files: + { '0': fixture_file_upload('files/export.csv', 'text/csv') } } + end + + it 'calls create activity service' do + expect(Activities::CreateActivityService).to receive(:call) + .with(hash_including(activity_type: :add_result)) + + post :create, params: params, format: :json + end + end + + describe '#update' do + let(:params) do + { id: result_asset.id, + result: { name: result.name } } + end + it 'calls create activity service (edit_result)' do + params[:result][:name] = 'test result changed' + expect(Activities::CreateActivityService).to receive(:call) + .with(hash_including(activity_type: :edit_result)) + + put :update, params: params, format: :json + end + + it 'calls create activity service (archive_result)' do + params[:result][:archived] = 'true' + expect(Activities::CreateActivityService).to receive(:call) + .with(hash_including(activity_type: :archive_result)) + + put :update, params: params, format: :json + end + end +end diff --git a/spec/controllers/result_comments_controller_spec.rb b/spec/controllers/result_comments_controller_spec.rb new file mode 100644 index 000000000..339b8ba19 --- /dev/null +++ b/spec/controllers/result_comments_controller_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe ResultCommentsController, type: :controller do + login_user + + let(:user) { User.first } + let!(:team) { create :team, :with_members } + let!(:user_project) { create :user_project, :owner, user: user } + let(:project) do + create :project, team: team, user_projects: [user_project] + end + let(:experiment) { create :experiment, project: project } + let(:task) { create :my_module, name: 'test task', experiment: experiment } + let(:result) do + create :result, name: 'test result', my_module: task, user: user + end + let!(:result_text) do + create :result_text, text: 'test text result', result: result + end + let(:result_comment) do + create :result_comment, message: 'test comment result', + result: result, + user: user + end + + describe '#create' do + context 'in JSON format' do + let(:params) do + { result_id: result.id, + comment: { message: 'test comment' } } + end + + it 'calls create activity service' do + expect(Activities::CreateActivityService).to receive(:call) + .with(hash_including(activity_type: :add_comment_to_result)) + + post :create, params: params, format: :json + end + end + end + + describe '#update' do + context 'in JSON format' do + let(:params) do + { result_id: result.id, + id: result_comment.id, + comment: { message: 'test comment updated' } } + end + + it 'calls create activity service' do + expect(Activities::CreateActivityService).to receive(:call) + .with(hash_including(activity_type: :edit_result_comment)) + + put :update, params: params, format: :json + end + end + end + + describe '#destroy' do + let(:params) do + { result_id: result.id, + id: result_comment.id } + end + + it 'calls create activity service' do + expect(Activities::CreateActivityService).to receive(:call) + .with(hash_including(activity_type: :delete_result_comment)) + + delete :destroy, params: params, format: :json + end + end +end diff --git a/spec/controllers/result_tables_controller_spec.rb b/spec/controllers/result_tables_controller_spec.rb new file mode 100644 index 000000000..4675255d2 --- /dev/null +++ b/spec/controllers/result_tables_controller_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe ResultTablesController, type: :controller do + login_user + + let(:user) { User.first } + let!(:team) { create :team, :with_members } + let!(:user_project) { create :user_project, :owner, user: user } + let(:project) do + create :project, team: team, user_projects: [user_project] + end + let(:experiment) { create :experiment, project: project } + let(:task) { create :my_module, name: 'test task', experiment: experiment } + let(:result) do + create :result, name: 'test result', my_module: task, user: user + end + let(:result_table) { create :result_table, result: result } + + describe '#create' do + let(:params) do + { my_module_id: task.id, + result: + { name: 'result name created', + table_attributes: + { contents: '{\"data\":[[\"a\",\"b\",\"1\",null,null]]}' } } } + end + + it 'calls create activity service' do + expect(Activities::CreateActivityService).to receive(:call) + .with(hash_including(activity_type: :add_result)) + + post :create, params: params, format: :json + end + end + + describe '#update' do + let(:params) do + { id: result_table.id, + result: { name: result.name } } + end + it 'calls create activity service (edit_result)' do + params[:result][:name] = 'test result changed' + expect(Activities::CreateActivityService).to receive(:call) + .with(hash_including(activity_type: :edit_result)) + + put :update, params: params, format: :json + end + + it 'calls create activity service (archive_result)' do + params[:result][:archived] = 'true' + expect(Activities::CreateActivityService).to receive(:call) + .with(hash_including(activity_type: :archive_result)) + + put :update, params: params, format: :json + end + end +end diff --git a/spec/controllers/result_texts_controller_spec.rb b/spec/controllers/result_texts_controller_spec.rb new file mode 100644 index 000000000..761981403 --- /dev/null +++ b/spec/controllers/result_texts_controller_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe ResultTextsController, type: :controller do + login_user + + let(:user) { User.first } + let!(:team) { create :team, :with_members } + let!(:user_project) { create :user_project, :owner, user: user } + let(:project) do + create :project, team: team, user_projects: [user_project] + end + let(:experiment) { create :experiment, project: project } + let(:task) { create :my_module, name: 'test task', experiment: experiment } + let(:result) do + create :result, name: 'test result', my_module: task, user: user + end + let(:result_text) do + create :result_text, text: 'test text result', result: result + end + + describe '#create' do + let(:params) do + { my_module_id: task.id, + result: { name: 'result name created', + result_text_attributes: { text: 'result text created' } } } + end + + it 'calls create activity service' do + expect(Activities::CreateActivityService).to receive(:call) + .with(hash_including(activity_type: :add_result)) + + post :create, params: params, format: :json + end + end + + describe '#update' do + let(:params) do + { id: result_text.id, + result: { name: result.name } } + end + it 'calls create activity service (edit_result)' do + params[:result][:name] = 'test result changed' + expect(Activities::CreateActivityService).to receive(:call) + .with(hash_including(activity_type: :edit_result)) + + put :update, params: params, format: :json + end + + it 'calls create activity service (archive_result)' do + params[:result][:archived] = 'true' + expect(Activities::CreateActivityService).to receive(:call) + .with(hash_including(activity_type: :archive_result)) + + put :update, params: params, format: :json + end + end +end diff --git a/spec/controllers/results_controller_spec.rb b/spec/controllers/results_controller_spec.rb new file mode 100644 index 000000000..d0e39f5f3 --- /dev/null +++ b/spec/controllers/results_controller_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe ResultsController, type: :controller do + login_user + + let(:user) { User.first } + let!(:team) { create :team, :with_members } + let!(:user_project) { create :user_project, :owner, user: user } + let(:project) do + create :project, team: team, user_projects: [user_project] + end + let(:experiment) { create :experiment, project: project } + let(:task) { create :my_module, name: 'test task', experiment: experiment } + let(:result) do + create :result, name: 'test result', my_module: task, user: user + end + let!(:result_text) do + create :result_text, text: 'test text result', result: result + end + + describe '#destroy' do + let(:params) do + { id: result.id } + end + + it 'calls create activity service' do + expect(Activities::CreateActivityService).to receive(:call) + .with(hash_including(activity_type: :destroy_result)) + + delete :destroy, params: params + end + end +end diff --git a/spec/factories/result_comment.rb b/spec/factories/result_comment.rb new file mode 100644 index 000000000..a49246425 --- /dev/null +++ b/spec/factories/result_comment.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :result_comment do + user + message { Faker::Lorem.sentence } + result { create(:result) } + end +end diff --git a/spec/factories/result_table.rb b/spec/factories/result_table.rb index 47849dbe4..759ed3b0a 100644 --- a/spec/factories/result_table.rb +++ b/spec/factories/result_table.rb @@ -3,5 +3,6 @@ FactoryBot.define do factory :result_table do table { create(:table) } + result { create(:result) } end end