From f480c0eb8d02a161ebf0b35c0494499eacc0cb34 Mon Sep 17 00:00:00 2001 From: Anton Ignatov Date: Fri, 14 Jun 2019 16:15:30 +0200 Subject: [PATCH 1/5] Adding new activtiies for image editing --- .../javascripts/sitewide/file_preview.js | 4 +- app/controllers/assets_controller.rb | 68 +++++++++++++++++-- config/initializers/extends.rb | 11 +-- config/locales/en.yml | 3 + config/locales/global_activities/en.yml | 6 ++ config/routes.rb | 1 + db/schema.rb | 12 ++-- spec/controllers/assets_controller_spec.rb | 67 ++++++++++++++++++ 8 files changed, 155 insertions(+), 17 deletions(-) create mode 100644 spec/controllers/assets_controller_spec.rb diff --git a/app/assets/javascripts/sitewide/file_preview.js b/app/assets/javascripts/sitewide/file_preview.js index c0b4c1d17..90ef54df1 100644 --- a/app/assets/javascripts/sitewide/file_preview.js +++ b/app/assets/javascripts/sitewide/file_preview.js @@ -1,7 +1,8 @@ /* eslint no-underscore-dangle: ["error", { "allowAfterThis": true }]*/ /* eslint no-use-before-define: ["error", { "functions": false }]*/ /* eslint-disable no-underscore-dangle */ -/* global Uint8Array fabric tui animateSpinner Assets I18n PerfectScrollbar*/ +/* global Uint8Array fabric tui animateSpinner */ +/* global Assets I18n PerfectScrollbar refreshProtocolStatusBar */ //= require assets var FilePreviewModal = (function() { @@ -448,6 +449,7 @@ var FilePreviewModal = (function() { if (!readOnly && data.editable) { modal.find('.file-edit-link').css('display', ''); modal.find('.file-edit-link').off().click(function(ev) { + $.post('/files/' + data.id + '/start_edit'); ev.preventDefault(); ev.stopPropagation(); modal.modal('hide'); diff --git a/app/controllers/assets_controller.rb b/app/controllers/assets_controller.rb index 007e4344e..de07ee88a 100644 --- a/app/controllers/assets_controller.rb +++ b/app/controllers/assets_controller.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class AssetsController < ApplicationController include WopiUtil # include ActionView::Helpers @@ -71,12 +73,10 @@ class AssetsController < ApplicationController 'processing-img' => image_tag('medium/processing.gif') ) else - response_json.merge!( - 'processing' => @asset.file.processing?, - 'preview-icon' => render_to_string( - partial: 'shared/file_preview_icon.html.erb', - locals: { asset: @asset } - ) + response_json['processing'] = @asset.file.processing? + response_json['preview-icon'] = render_to_string( + partial: 'shared/file_preview_icon.html.erb', + locals: { asset: @asset } ) end @@ -155,6 +155,13 @@ class AssetsController < ApplicationController render layout: false end + def start_edit + asset = Asset.find_by_id(params[:id]) + return render_403 unless asset + + create_edit_image_activity(asset, current_user, true) + end + def update_image @asset = Asset.find(params[:id]) orig_file_size = @asset.file_file_size @@ -164,6 +171,7 @@ class AssetsController < ApplicationController @asset.file = params[:image] @asset.file_file_name = orig_file_name @asset.save! + create_edit_image_activity(@asset, current_user, false) # release previous image space @asset.team.release_space(orig_file_size) # Post process file here @@ -307,4 +315,52 @@ class AssetsController < ApplicationController 'file' end + + def create_edit_image_activity(asset, current_user, started_editing) + action = if started_editing + t('activities.file_editing.started') + else + t('activities.file_editing.finished') + end + if asset.step.class == Step + protocol = asset.step.protocol + default_step_items = + { step: asset.step.id, + step_position: { id: asset.step.id, value_for: 'position_plus_one' }, + asset_name: { id: asset.id, value_for: 'file_file_name' }, + action: action } + if protocol.in_module? + project = protocol.my_module.experiment.project + team = project.team + type_of = :edit_image_on_step + message_items = { my_module: protocol.my_module.id } + else + type_of = :edit_image_on_step_in_repository + project = nil + team = protocol.team + message_items = { protocol: protocol.id } + end + message_items = default_step_items.merge(message_items) + Activities::CreateActivityService + .call(activity_type: type_of, + owner: current_user, + subject: protocol, + team: team, + project: project, + message_items: message_items) + elsif asset.result.class == Result + my_module = asset.result.my_module + Activities::CreateActivityService + .call(activity_type: :edit_image_on_result, + owner: current_user, + subject: asset.result, + team: my_module.experiment.project.team, + project: my_module.experiment.project, + message_items: { + result: asset.result.id, + asset_name: { id: asset.id, value_for: 'file_file_name' }, + action: action + }) + end + end end diff --git a/config/initializers/extends.rb b/config/initializers/extends.rb index 2e79014d1..16b1be747 100644 --- a/config/initializers/extends.rb +++ b/config/initializers/extends.rb @@ -218,19 +218,22 @@ class Extends export_protocol_from_task: 106, import_inventory_items: 107, create_tag: 108, - delete_tag: 109 + delete_tag: 109, + edit_image_on_result: 110, + edit_image_on_step: 111, + edit_image_on_step_in_repository: 112, } ACTIVITY_GROUPS = { projects: [*0..7, 32, 33, 34, 95, 108, 65, 109], - task_results: [23, 26, 25, 42, 24, 40, 41, 99], + task_results: [23, 26, 25, 42, 24, 40, 41, 99, 110], task: [8, 58, 9, 59, 10, 11, 12, 13, 14, 35, 36, 37, 53, 54, *60..64, *66..69, 106], - task_protocol: [15, 22, 16, 18, 19, 20, 21, 17, 38, 39, 100, 45, 46, 47], + task_protocol: [15, 22, 16, 18, 19, 20, 21, 17, 38, 39, 100, 111, 45, 46, 47], task_inventory: [55, 56], experiment: [*27..31, 57], reports: [48, 50, 49], inventories: [70, 71, 105, 72, 73, 74, 102, 75, 76, 77, 78, 96, 107], - protocol_repository: [80, 103, 89, 87, 79, 90, 91, 88, 85, 86, 84, 81, 82, 83, 101], + protocol_repository: [80, 103, 89, 87, 79, 90, 91, 88, 85, 86, 84, 81, 82, 83, 101, 102], team: [92, 94, 93, 97, 104] }.freeze end diff --git a/config/locales/en.yml b/config/locales/en.yml index 0ab703562..5c7ba58cf 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1333,6 +1333,9 @@ en: wupi_file_editing: started: "editing started" finished: "editing finished" + file_editing: + started: "editing started" + finished: "editing finished" protocols: my_to_team_message: 'My protocols to Team protocols' team_to_my_message: 'Team protocols to My protocols' diff --git a/config/locales/global_activities/en.yml b/config/locales/global_activities/en.yml index 14e5b2b4f..b2afa22be 100644 --- a/config/locales/global_activities/en.yml +++ b/config/locales/global_activities/en.yml @@ -131,6 +131,9 @@ en: edit_tag_html: "%{user} edited tag %{tag} in project %{project}." delete_tag_html: "%{user} deleted tag %{tag} in project %{project}." import_inventory_items_html: "%{user} imported %{num_of_items} inventory item(s) to %{repository}." + edit_image_on_result_html: "%{user} edited image %{asset_name} on result %{result}: %{action}." + edit_image_on_step_html: "%{user} edited image %{asset_name} on protocol's step %{step_position} %{step} on task %{my_module}: %{action}." + edit_image_on_step_in_repository_html: "%{user} edited image %{asset_name} on protocol %{protocol}'s step %{step_position} %{step} in Protocol repository: %{action}." activity_name: create_project: "Project created" @@ -233,6 +236,9 @@ en: edit_tag: "Tag edited" delete_tag: "Tag deleted" import_inventory_items: "Inventory items imported" + edit_image_on_result: "Image on result edited" + edit_image_on_step: "Image on task step edited" + edit_image_on_step_in_repository: "Image on step edited" activity_group: projects: "Projects" diff --git a/config/routes.rb b/config/routes.rb index 260961435..4e2c39d31 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -593,6 +593,7 @@ Rails.application.routes.draw do post 'files/create_wopi_file', to: 'assets#create_wopi_file', as: 'create_wopi_file' + post 'files/:id/start_edit', to: 'assets#start_edit', as: 'start_edit_asset' devise_scope :user do get 'avatar/:id/:style' => 'users/registrations#avatar', as: 'avatar' diff --git a/db/schema.rb b/db/schema.rb index 42cf7ba6d..ba5dc8029 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -56,7 +56,7 @@ ActiveRecord::Schema.define(version: 20190520135317) do t.datetime "updated_at", null: false t.string "file_file_name" t.string "file_content_type" - t.integer "file_file_size" + t.bigint "file_file_size" t.datetime "file_updated_at" t.bigint "created_by_id" t.bigint "last_modified_by_id" @@ -169,7 +169,7 @@ ActiveRecord::Schema.define(version: 20190520135317) do t.datetime "updated_at", null: false t.string "workflowimg_file_name" t.string "workflowimg_content_type" - t.integer "workflowimg_file_size" + t.bigint "workflowimg_file_size" t.datetime "workflowimg_updated_at" t.uuid "uuid" t.index ["archived_by_id"], name: "index_experiments_on_archived_by_id" @@ -737,14 +737,14 @@ ActiveRecord::Schema.define(version: 20190520135317) do t.datetime "updated_at", null: false t.string "file_file_name" t.string "file_content_type" - t.integer "file_file_size" + t.bigint "file_file_size" t.datetime "file_updated_at" end create_table "tiny_mce_assets", force: :cascade do |t| t.string "image_file_name" t.string "image_content_type" - t.integer "image_file_size" + t.bigint "image_file_size" t.datetime "image_updated_at" t.integer "estimated_size", default: 0, null: false t.integer "step_id" @@ -857,7 +857,7 @@ ActiveRecord::Schema.define(version: 20190520135317) do t.datetime "updated_at", null: false t.string "avatar_file_name" t.string "avatar_content_type" - t.integer "avatar_file_size" + t.bigint "avatar_file_size" t.datetime "avatar_updated_at" t.string "confirmation_token" t.datetime "confirmed_at" @@ -924,7 +924,7 @@ ActiveRecord::Schema.define(version: 20190520135317) do t.datetime "updated_at", null: false t.string "zip_file_file_name" t.string "zip_file_content_type" - t.integer "zip_file_file_size" + t.bigint "zip_file_file_size" t.datetime "zip_file_updated_at" t.string "type" t.index ["user_id"], name: "index_zip_exports_on_user_id" diff --git a/spec/controllers/assets_controller_spec.rb b/spec/controllers/assets_controller_spec.rb new file mode 100644 index 000000000..6644e043d --- /dev/null +++ b/spec/controllers/assets_controller_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe AssetsController, type: :controller do + login_user + + let(:user) { subject.current_user } + let!(:team) { create :team, created_by: user } + let(:user_team) { create :user_team, :admin, user: user, team: team } + 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(:my_module) { create :my_module, name: 'test task', experiment: experiment } + let(:protocol) do + create :protocol, my_module: my_module, team: team, added_by: user + end + let(:step) { create :step, protocol: protocol, user: user } + let(:step_asset_task) { create :step_asset, step: step } + + let(:result) do + create :result, name: 'test result', my_module: my_module, user: user + end + let(:result_asset) { create :result_asset, result: result } + + let(:protocol_in_repository) { create :protocol, :in_public_repository, team: team } + let(:step_in_repository) { create :step, protocol: protocol_in_repository, user: user } + + let!(:asset) { create :asset } + let(:step_asset_in_repository) { create :step_asset, step: step_in_repository, asset: asset } + + describe 'POST start_edit' do + let(:action) { post :start_edit, params: params, format: :json } + let!(:params) do + { id: nil } + end + it 'calls create activity service (start edit image on step)' do + params[:id] = step_asset_task.asset.id + expect(Activities::CreateActivityService).to receive(:call) + .with(hash_including(activity_type: :edit_image_on_step)) + action + end + + it 'calls create activity service (start edit image on result)' do + params[:id] = result_asset.asset.id + expect(Activities::CreateActivityService).to receive(:call) + .with(hash_including(activity_type: :edit_image_on_result)) + action + end + + it 'calls create activity service (start edit image on step in repository)' do + params[:id] = step_asset_in_repository.asset.id + user_team + expect(Activities::CreateActivityService).to receive(:call) + .with(hash_including(activity_type: :edit_image_on_step_in_repository)) + action + end + + it 'adds activity in DB' do + params[:id] = step_asset_task.asset.id + expect { action } + .to(change { Activity.count }) + end + end +end From a843a016a0efcd5e7418b59c91e2f1e58433b806 Mon Sep 17 00:00:00 2001 From: Anton Ignatov Date: Wed, 19 Jun 2019 15:19:47 +0200 Subject: [PATCH 2/5] Fix markup and naming --- .../javascripts/sitewide/file_preview.js | 2 +- app/controllers/assets_controller.rb | 58 ++----------------- app/controllers/concerns/assets_actions.rb | 53 +++++++++++++++++ config/initializers/extends.rb | 2 +- config/routes.rb | 2 +- 5 files changed, 60 insertions(+), 57 deletions(-) create mode 100644 app/controllers/concerns/assets_actions.rb diff --git a/app/assets/javascripts/sitewide/file_preview.js b/app/assets/javascripts/sitewide/file_preview.js index 90ef54df1..48f2ee033 100644 --- a/app/assets/javascripts/sitewide/file_preview.js +++ b/app/assets/javascripts/sitewide/file_preview.js @@ -449,7 +449,7 @@ var FilePreviewModal = (function() { if (!readOnly && data.editable) { modal.find('.file-edit-link').css('display', ''); modal.find('.file-edit-link').off().click(function(ev) { - $.post('/files/' + data.id + '/start_edit'); + $.post('/files/' + data.id + '/start_edit_image'); ev.preventDefault(); ev.stopPropagation(); modal.modal('hide'); diff --git a/app/controllers/assets_controller.rb b/app/controllers/assets_controller.rb index de07ee88a..a7dca2507 100644 --- a/app/controllers/assets_controller.rb +++ b/app/controllers/assets_controller.rb @@ -2,6 +2,7 @@ class AssetsController < ApplicationController include WopiUtil + include AssetsActions # include ActionView::Helpers include ActionView::Helpers::AssetTagHelper include ActionView::Helpers::TextHelper @@ -155,11 +156,8 @@ class AssetsController < ApplicationController render layout: false end - def start_edit - asset = Asset.find_by_id(params[:id]) - return render_403 unless asset - - create_edit_image_activity(asset, current_user, true) + def create_start_edit_image_activity + create_edit_image_activity(@asset, current_user, :start_editing) end def update_image @@ -171,7 +169,7 @@ class AssetsController < ApplicationController @asset.file = params[:image] @asset.file_file_name = orig_file_name @asset.save! - create_edit_image_activity(@asset, current_user, false) + create_edit_image_activity(@asset, current_user, :finish_editing) # release previous image space @asset.team.release_space(orig_file_size) # Post process file here @@ -315,52 +313,4 @@ class AssetsController < ApplicationController 'file' end - - def create_edit_image_activity(asset, current_user, started_editing) - action = if started_editing - t('activities.file_editing.started') - else - t('activities.file_editing.finished') - end - if asset.step.class == Step - protocol = asset.step.protocol - default_step_items = - { step: asset.step.id, - step_position: { id: asset.step.id, value_for: 'position_plus_one' }, - asset_name: { id: asset.id, value_for: 'file_file_name' }, - action: action } - if protocol.in_module? - project = protocol.my_module.experiment.project - team = project.team - type_of = :edit_image_on_step - message_items = { my_module: protocol.my_module.id } - else - type_of = :edit_image_on_step_in_repository - project = nil - team = protocol.team - message_items = { protocol: protocol.id } - end - message_items = default_step_items.merge(message_items) - Activities::CreateActivityService - .call(activity_type: type_of, - owner: current_user, - subject: protocol, - team: team, - project: project, - message_items: message_items) - elsif asset.result.class == Result - my_module = asset.result.my_module - Activities::CreateActivityService - .call(activity_type: :edit_image_on_result, - owner: current_user, - subject: asset.result, - team: my_module.experiment.project.team, - project: my_module.experiment.project, - message_items: { - result: asset.result.id, - asset_name: { id: asset.id, value_for: 'file_file_name' }, - action: action - }) - end - end end diff --git a/app/controllers/concerns/assets_actions.rb b/app/controllers/concerns/assets_actions.rb new file mode 100644 index 000000000..de32d2f6b --- /dev/null +++ b/app/controllers/concerns/assets_actions.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +module AssetsActions + extend ActiveSupport::Concern + + def create_edit_image_activity(asset, current_user, started_editing) + action = if started_editing == :start_editing + t('activities.file_editing.started') + elsif started_editing == :finish_editing + t('activities.file_editing.finished') + end + if asset.step.class == Step + protocol = asset.step.protocol + default_step_items = + { step: asset.step.id, + step_position: { id: asset.step.id, value_for: 'position_plus_one' }, + asset_name: { id: asset.id, value_for: 'file_file_name' }, + action: action } + if protocol.in_module? + project = protocol.my_module.experiment.project + team = project.team + type_of = :edit_image_on_step + message_items = { my_module: protocol.my_module.id } + else + type_of = :edit_image_on_step_in_repository + project = nil + team = protocol.team + message_items = { protocol: protocol.id } + end + message_items = default_step_items.merge(message_items) + Activities::CreateActivityService + .call(activity_type: type_of, + owner: current_user, + subject: protocol, + team: team, + project: project, + message_items: message_items) + elsif asset.result.class == Result + my_module = asset.result.my_module + Activities::CreateActivityService + .call(activity_type: :edit_image_on_result, + owner: current_user, + subject: asset.result, + team: my_module.experiment.project.team, + project: my_module.experiment.project, + message_items: { + result: asset.result.id, + asset_name: { id: asset.id, value_for: 'file_file_name' }, + action: action + }) + end + end +end diff --git a/config/initializers/extends.rb b/config/initializers/extends.rb index 16b1be747..3a85ede69 100644 --- a/config/initializers/extends.rb +++ b/config/initializers/extends.rb @@ -233,7 +233,7 @@ class Extends experiment: [*27..31, 57], reports: [48, 50, 49], inventories: [70, 71, 105, 72, 73, 74, 102, 75, 76, 77, 78, 96, 107], - protocol_repository: [80, 103, 89, 87, 79, 90, 91, 88, 85, 86, 84, 81, 82, 83, 101, 102], + protocol_repository: [80, 103, 89, 87, 79, 90, 91, 88, 85, 86, 84, 81, 82, 83, 101, 112], team: [92, 94, 93, 97, 104] }.freeze end diff --git a/config/routes.rb b/config/routes.rb index 4e2c39d31..22407fa77 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -593,7 +593,7 @@ Rails.application.routes.draw do post 'files/create_wopi_file', to: 'assets#create_wopi_file', as: 'create_wopi_file' - post 'files/:id/start_edit', to: 'assets#start_edit', as: 'start_edit_asset' + post 'files/:id/start_edit_image', to: 'assets#create_start_edit_image_activity', as: 'start_edit_image' devise_scope :user do get 'avatar/:id/:style' => 'users/registrations#avatar', as: 'avatar' From 6f1f6e297c1d2014f52b9e8e8902c6b89d024282 Mon Sep 17 00:00:00 2001 From: Anton Ignatov Date: Wed, 19 Jun 2019 15:40:27 +0200 Subject: [PATCH 3/5] Fix tests --- spec/controllers/assets_controller_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/controllers/assets_controller_spec.rb b/spec/controllers/assets_controller_spec.rb index 6644e043d..fa8fdfddf 100644 --- a/spec/controllers/assets_controller_spec.rb +++ b/spec/controllers/assets_controller_spec.rb @@ -32,7 +32,7 @@ describe AssetsController, type: :controller do let(:step_asset_in_repository) { create :step_asset, step: step_in_repository, asset: asset } describe 'POST start_edit' do - let(:action) { post :start_edit, params: params, format: :json } + let(:action) { post :create_start_edit_image_activity, params: params, format: :json } let!(:params) do { id: nil } end From 3069ec8dd227d74edf5731dbac5c50a4326992f8 Mon Sep 17 00:00:00 2001 From: Anton Ignatov Date: Thu, 20 Jun 2019 09:43:23 +0200 Subject: [PATCH 4/5] Add check for edit permission --- app/controllers/assets_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/assets_controller.rb b/app/controllers/assets_controller.rb index a7dca2507..9aecf53eb 100644 --- a/app/controllers/assets_controller.rb +++ b/app/controllers/assets_controller.rb @@ -14,7 +14,7 @@ class AssetsController < ApplicationController before_action :load_vars, except: :create_wopi_file before_action :check_read_permission, except: :file_present - before_action :check_edit_permission, only: :edit + before_action :check_edit_permission, only: %i(edit create_start_edit_image_activity) def file_present return render_403 unless @asset.team == current_team From 6530786f178dc7bc7380455a1bc88b61604ddee4 Mon Sep 17 00:00:00 2001 From: Anton Ignatov Date: Thu, 20 Jun 2019 16:05:19 +0200 Subject: [PATCH 5/5] Fix tests --- spec/controllers/assets_controller_spec.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spec/controllers/assets_controller_spec.rb b/spec/controllers/assets_controller_spec.rb index fa8fdfddf..a4a853bb8 100644 --- a/spec/controllers/assets_controller_spec.rb +++ b/spec/controllers/assets_controller_spec.rb @@ -32,6 +32,9 @@ describe AssetsController, type: :controller do let(:step_asset_in_repository) { create :step_asset, step: step_in_repository, asset: asset } describe 'POST start_edit' do + before do + allow(controller).to receive(:check_edit_permission).and_return(true) + end let(:action) { post :create_start_edit_image_activity, params: params, format: :json } let!(:params) do { id: nil }