diff --git a/VERSION b/VERSION index 836ae4eda..1b92e588b 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.19.2 +1.19.3 diff --git a/app/assets/javascripts/repositories/renderers/edit_renderers.js b/app/assets/javascripts/repositories/renderers/edit_renderers.js index 3bb608bbe..067499ff5 100644 --- a/app/assets/javascripts/repositories/renderers/edit_renderers.js +++ b/app/assets/javascripts/repositories/renderers/edit_renderers.js @@ -27,7 +27,7 @@ $.fn.dataTable.render.editRepositoryAssetValue = function(formId, columnId, cell $.fn.dataTable.render.editRepositoryTextValue = function(formId, columnId, cell) { let $cell = $(cell.node()); - let text = $cell.text(); + let text = $cell.find('.text-value').data('edit-value'); $cell.html(`
diff --git a/app/assets/javascripts/repositories/renderers/view_renderers.js b/app/assets/javascripts/repositories/renderers/view_renderers.js index cfca42aeb..8d23fce1f 100644 --- a/app/assets/javascripts/repositories/renderers/view_renderers.js +++ b/app/assets/javascripts/repositories/renderers/view_renderers.js @@ -26,7 +26,8 @@ $.fn.dataTable.render.defaultRepositoryAssetValue = function() { }; $.fn.dataTable.render.RepositoryTextValue = function(data) { - return data.value; + var text = $(`${data.value.view}`); + return text.prop('outerHTML'); }; $.fn.dataTable.render.defaultRepositoryTextValue = function() { diff --git a/app/assets/javascripts/sitewide/marvinjs_editor.js b/app/assets/javascripts/sitewide/marvinjs_editor.js index f5300bb65..e5a7025f2 100644 --- a/app/assets/javascripts/sitewide/marvinjs_editor.js +++ b/app/assets/javascripts/sitewide/marvinjs_editor.js @@ -317,7 +317,7 @@ var MarvinJsEditorApi = (function() { $(document).on('turbolinks:load', function() { MarvinJsEditor = MarvinJsEditorApi(); if (MarvinJsEditor.enabled()) { - if ($('#marvinjs-editor')[0].dataset.marvinjsMode === 'remote') { + if ($('#marvinjs-editor')[0].dataset.marvinjsMode === 'remote' && typeof(ChemicalizeMarvinJs) !== 'undefined') { ChemicalizeMarvinJs.createEditor('#marvinjs-sketch').then(function(marvin) { marvin.setDisplaySettings({ toolbars: 'reporting' }); marvinJsRemoteEditor = marvin; diff --git a/app/assets/stylesheets/marvinjs.scss b/app/assets/stylesheets/marvinjs.scss index 319d100ed..3476b9ea1 100644 --- a/app/assets/stylesheets/marvinjs.scss +++ b/app/assets/stylesheets/marvinjs.scss @@ -10,7 +10,7 @@ // MarvinJs modal .modal-marvin-js { background: transparent; - font-size: $font-size-large; + font-size: $font-size-base; padding: 0 !important; .preview-close { @@ -40,19 +40,23 @@ .modal-header { background: $color-black; border: 0; + display: flex; height: 60px; line-height: 40px; padding: 10px 15px; text-align: center; .file-save-link { + flex-shrink: 0; margin: 0 20px 0 0; } .file-name { align-items: center; display: flex; + flex-shrink: 0; float: left; + margin-right: auto; input { border-radius: 5px; diff --git a/app/controllers/canvas_controller.rb b/app/controllers/canvas_controller.rb index 35582816b..52b9ae45f 100644 --- a/app/controllers/canvas_controller.rb +++ b/app/controllers/canvas_controller.rb @@ -60,7 +60,6 @@ class CanvasController < ApplicationController positions = {} if update_params[:positions].present? poss = update_params[:positions].split(';') - center = '' (poss.collect { |pos| pos.split(',') }).each_with_index do |pos, index| unless pos.length == 3 && pos[0].is_a?(String) && float?(pos[1]) && float?(pos[2]) @@ -68,8 +67,7 @@ class CanvasController < ApplicationController end x = pos[1].to_i y = pos[2].to_i - # Multiple modules cannot have same position - return render_403 if positions.any? { |_, v| v[:x] == x && v[:y] == y } + positions[pos[0]] = { x: x, y: y } end end diff --git a/app/models/experiment.rb b/app/models/experiment.rb index 2161052c1..58bb7c899 100644 --- a/app/models/experiment.rb +++ b/app/models/experiment.rb @@ -132,14 +132,25 @@ class Experiment < ApplicationRecord positions, current_user ) - cloned_modules = [] begin with_lock do - # First, add new modules + # Start with archiving to release positions for new tasks + archive_modules(to_archive, current_user) if to_archive.any? + + # Update only existing tasks positions to release positions for new tasks + existing_positions = positions + .slice(*positions.keys.map { |k| k unless k.to_s.start_with?('n') }.compact) + update_module_positions(existing_positions) if existing_positions.any? + + # Move only existing tasks to release positions for new tasks + existing_to_move = to_move + .slice(*to_move.keys.map { |k| k unless k.to_s.start_with?('n') }.compact) + move_modules(existing_to_move, current_user) if existing_to_move.any? + + # add new modules new_ids, cloned_pairs, originals = add_modules( to_add, to_clone, current_user ) - cloned_modules = cloned_pairs.collect { |mn, _| mn } # Rename modules rename_modules(to_rename, current_user) @@ -161,9 +172,6 @@ class Experiment < ApplicationRecord my_module_new: mn.id }) end - # Then, archive modules that need to be archived - archive_modules(to_archive, current_user) if to_archive.any? - # Update connections, positions & module group variables # with actual IDs retrieved from the new modules creation updated_to_move = {} @@ -325,9 +333,9 @@ class Experiment < ApplicationRecord to_move.each do |id, experiment_id| my_module = my_modules.find_by_id(id) experiment = project.experiments.find_by_id(experiment_id) - experiment_org = my_module.experiment next unless my_module.present? && experiment.present? + experiment_original = my_module.experiment my_module.experiment = experiment # Calculate new module position @@ -349,7 +357,7 @@ class Experiment < ApplicationRecord message_items: { my_module: my_module.id, experiment_original: - experiment_org.id, + experiment_original.id, experiment_new: experiment.id }) end diff --git a/app/serializers/repository_datatable/repository_text_value_serializer.rb b/app/serializers/repository_datatable/repository_text_value_serializer.rb index 60a788f97..c505abbc2 100644 --- a/app/serializers/repository_datatable/repository_text_value_serializer.rb +++ b/app/serializers/repository_datatable/repository_text_value_serializer.rb @@ -8,9 +8,10 @@ module RepositoryDatatable def value @user = scope[:user] - custom_auto_link(object.data, - simple_format: true, - team: scope[:team]) + { + view: custom_auto_link(object.data, simple_format: true, team: scope[:team]), + edit: sanitize_input(object.data) + } end end end diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 1256f75b9..0fa7b6fba 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -8,7 +8,7 @@ <%= stylesheet_link_tag 'application', media: 'all' %> <%= javascript_include_tag 'application' %> - <% if ENV['MARVINJS_API_KEY'] %> + <% if MarvinJsService.enabled? && ENV['MARVINJS_API_KEY'] %> <% end %> diff --git a/app/views/shared/_marvinjs_modal.html.erb b/app/views/shared/_marvinjs_modal.html.erb index d63b455aa..fb75bd6ef 100644 --- a/app/views/shared/_marvinjs_modal.html.erb +++ b/app/views/shared/_marvinjs_modal.html.erb @@ -9,12 +9,12 @@
- <%= t('marvinjs.modal_name_title') %> <%= text_field_tag :sketch_name, '', placeholder: t('marvinjs.structure_placeholder') %>

<%= t('SaveClose')%>

+
diff --git a/spec/factories/my_modules.rb b/spec/factories/my_modules.rb index dca7990f4..d002ed7db 100644 --- a/spec/factories/my_modules.rb +++ b/spec/factories/my_modules.rb @@ -3,8 +3,8 @@ FactoryBot.define do factory :my_module do sequence(:name) { |n| "Task-#{n}" } - x { Faker::Number.between(from: 1, to: 100) } sequence(:y) { |n| n } + sequence(:x) { |n| n } workflow_order { MyModule.where(experiment_id: experiment.id).count + 1 } experiment my_module_group { create :my_module_group, experiment: experiment } diff --git a/spec/models/experiment_spec.rb b/spec/models/experiment_spec.rb index 4552dcac6..c68ca534d 100644 --- a/spec/models/experiment_spec.rb +++ b/spec/models/experiment_spec.rb @@ -92,12 +92,8 @@ describe Experiment, type: :model do let(:user) { experiment.created_by } context 'when creating tasks' do - let(:to_add) do - [{ id: 'n0', name: 'new task name', x: 50, y: 50 }] - end - let(:function_call) do - experiment.update_canvas([], to_add, [], [], [], [], [], [], user) - end + let(:to_add) { [{ id: 'n0', name: 'new task name', x: 50, y: 50 }] } + let(:function_call) { experiment.update_canvas([], to_add, [], {}, [], [], [], {}, user) } it 'calls create activity for creating tasks' do expect(Activities::CreateActivityService) @@ -112,6 +108,57 @@ describe Experiment, type: :model do it 'adds activity in DB' do expect { function_call }.to(change { Activity.all.count }) end + + context 'when moving existing one and creating new one on the same position' do + let(:first_task) { experiment.my_modules.first } + let(:to_add) { [{ id: 'n0', name: 'new task name', x: 0, y: 0 }] } + let(:function_call) { experiment.update_canvas([], to_add, [], {}, [], [], [], positions, user) } + let(:positions) do + Hash[first_task.id.to_s, + { x: first_task.x + 1, y: first_task.y + 1 }, 'n0', { x: first_task.x, y: first_task.y }] + end + + before do + first_task.update(x: 0, y: 0) + end + + it 'returns true' do + expect(function_call).to be_truthy + end + end + + context 'when creating new one on position of "toBeArchived" task' do + let(:first_task) { experiment.my_modules.first } + let(:to_add) { [{ id: 'n0', name: 'new task name', x: 0, y: 0 }] } + let(:positions) { Hash['n0', { x: first_task.x, y: first_task.y }] } + let(:to_archive) { [first_task.id] } + let(:function_call) { experiment.update_canvas(to_archive, to_add, [], {}, [], [], [], positions, user) } + + before do + first_task.update(x: 0, y: 0) + end + + it 'returns true' do + expect(function_call).to be_truthy + end + end + + context 'when creating new one on position of "toBeMoved" task' do + let(:first_task) { experiment.my_modules.first } + let(:to_add) { [{ id: 'n0', name: 'new task name', x: 0, y: 0 }] } + let(:positions) { Hash['n0', { x: first_task.x, y: first_task.y }] } + let(:to_move) { Hash[first_task.id, second_experiment.id] } + let(:second_experiment) { create :experiment, project: experiment.project } + let(:function_call) { experiment.update_canvas([], to_add, [], to_move, [], [], [], positions, user) } + + before do + first_task.update(x: 0, y: 0) + end + + it 'returns true' do + expect(function_call).to be_truthy + end + end end context 'when cloning tasks' do @@ -131,7 +178,7 @@ describe Experiment, type: :model do .map.with_index { |t, i| { 'n' + i.to_s => t.id } }.reduce({}, :merge) end let(:function_call) do - experiment.update_canvas([], to_add, [], [], [], to_clone, [], [], user) + experiment.update_canvas([], to_add, [], {}, [], to_clone, [], {}, user) end it 'calls create activity for cloning tasks' do @@ -159,7 +206,7 @@ describe Experiment, type: :model do end let(:function_call) do - experiment.update_canvas([], [], to_rename, [], [], [], [], [], user) + experiment.update_canvas([], [], to_rename, {}, [], [], [], {}, user) end it 'calls create activity for renaming my_moudles' do @@ -180,7 +227,7 @@ describe Experiment, type: :model do let(:to_archive) { experiment.my_modules.pluck(:id) } let(:function_call) do - experiment.update_canvas(to_archive, [], [], [], [], [], [], [], user) + experiment.update_canvas(to_archive, [], [], {}, [], [], [], {}, user) end it 'calls create activity for archiving tasks' do @@ -205,7 +252,7 @@ describe Experiment, type: :model do .map { |t| { t.id => new_experiment.id } }.reduce({}, :merge) end let(:function_call) do - experiment.update_canvas([], [], [], to_move, [], [], [], [], user) + experiment.update_canvas([], [], [], to_move, [], [], [], {}, user) end it 'calls create activity for moving tasks to another experiment' do @@ -221,5 +268,21 @@ describe Experiment, type: :model do expect { function_call }.to change { Activity.all.count }.by(3) end end + + context 'when moving new task (not saved to DB yet)' do + let(:first_task) { experiment.my_modules.first } + let(:second_experiment) { create :experiment, project: experiment.project } + let(:to_add) { [{ id: 'n0', name: 'new task name', x: 0, y: 0 }] } + let(:to_move) { Hash['n0', second_experiment.id] } + let(:function_call) { experiment.update_canvas([], to_add, [], to_move, [], [], [], {}, user) } + + it 'returns true' do + expect(function_call).to be_truthy + end + + it 'assigns task to new experiment' do + expect { function_call }.to change { second_experiment.my_modules.count }.by(1) + end + end end end