diff --git a/app/assets/javascripts/projects/canvas.js.erb b/app/assets/javascripts/projects/canvas.js.erb index 65375ca2a..bd070ad7a 100644 --- a/app/assets/javascripts/projects/canvas.js.erb +++ b/app/assets/javascripts/projects/canvas.js.erb @@ -163,7 +163,6 @@ function initializeEdit() { // Read permissions from the data attributes of the form var canEditModules = _.isEqual($("#update-canvas").data("can-edit-modules"), "yes"); - var canEditModuleGroups = _.isEqual($("#update-canvas").data("can-edit-module-groups"), "yes"); var canCreateModules = _.isEqual($("#update-canvas").data("can-create-modules"), "yes"); var canCloneModules = _.isEqual($("#update-canvas").data("can-clone-modules"), "yes"); var canMoveModules = _.isEqual($("#update-canvas").data("can-move-modules"), "yes"); @@ -211,11 +210,6 @@ function initializeEdit() { $(".edit-module").on("click touchstart", editModuleHandler); } - if (canEditModuleGroups) { - initEditModuleGroups(); - $(".edit-module-group").on("click touchstart", editModuleGroupHandler); - } - if (canCloneModules) { bindCloneModuleAction( $(".module-options a.clone-module"), @@ -1220,22 +1214,6 @@ function updateModuleHtml(module, id, name, gridDistX, gridDistY) { // Add click handler for the edit module $(editModuleLink).on("click touchstart", editModuleHandler); } - if (_.isEqual($("#update-canvas").data("can-edit-module-groups"), "yes")) { - var editModuleGroupItem = document.createElement("li"); - $(editModuleGroupItem).appendTo(dropdownMenu); - $(editModuleGroupItem).hide(); - - var editModuleGroupLink = document.createElement("a"); - $(editModuleGroupLink) - .attr("href", "") - .attr("data-module-id", id) - .addClass("edit-module-group") - .html($("#edit-group-link-placeholder").text().trim()) - .appendTo(editModuleGroupItem); - - // Add click handler for the edit module group - $(editModuleGroupLink).on("click touchstart", editModuleGroupHandler); - } // Add clone links if neccesary if (_.isEqual($("#update-canvas").data("can-clone-modules"), "yes")) { @@ -1636,95 +1614,11 @@ editModuleHandler = function(ev) { /** * Initialize editing of module groups. */ -function initEditModuleGroups() { - function handleRenameConfirm(modal, ev) { - var input = modal.find("#edit-module-group-name-input"); - // Validate module group name - var moduleNameValid = textValidator(ev, input, 1, - <%= Constants::NAME_MAX_LENGTH %>, true); - if (moduleNameValid) { - var newModuleGroupName = input.val(); - var moduleId = modal.attr("data-module-id"); - var moduleEl = $("#" + moduleId); - // Update the module group name for all modules - // currently in the module group - var ids = connectedComponents(graph, moduleEl.attr("data-module-id")); - _.each(ids, function(id) { - $("#" + id).attr("data-module-group-name", newModuleGroupName); - }); - - // Hide modal - modal.modal("hide"); - } - } - - $("#modal-edit-module-group") - .on("show.bs.modal", function (event) { - var modal = $(this); - var moduleId = modal.attr("data-module-id"); - var moduleEl = $("#" + moduleId); - var input = modal.find("#edit-module-group-name-input"); - - // Set the input to the current module's name - input - .attr("value", moduleEl.attr("data-module-group-name")); - input.val(moduleEl.attr("data-module-group-name")); - - // Bind on enter button - input.keydown(function(ev) { - if (ev.keyCode == 13) { - // "Submit" modal - handleRenameConfirm(modal, ev); - - // In any case, prevent form submission - ev.preventDefault(); - ev.stopPropagation(); - return false; - } - }); - }) - .on("shown.bs.modal", function (event) { - $(this).find("#edit-module-group-name-input").focus(); - }) - .on("hide.bs.modal", function (event) { - // Remove potential error classes - $(this).find("#edit-module-group-name-input").parent().removeClass("has-error"); - $(this).find("span.help-block").remove(); - - $(this).find("#edit-module-group-name-input").off("keydown"); - - // When hiding modal, re-enable events - toggleCanvasEvents(true); - }); - - // Bind the confirm button on modal - $("#modal-edit-module-group").find("button[data-action='confirm']").on("click", function(ev) { - var modal = $(this).closest(".modal"); - handleRenameConfirm(modal, ev); - }); -} /** * Handler when editing a module group. */ -editModuleGroupHandler = function(ev) { - var modal = $("#modal-edit-module-group"); - var moduleEl = $(this).closest(".module"); - - // Set modal's module id - modal.attr("data-module-id", moduleEl.attr("data-module-id")); - - // Disable dragging & zooming events on canvas temporarily - toggleCanvasEvents(false); - - // Show modal - modal.modal("show"); - - ev.preventDefault(); - ev.stopPropagation(); - return false; -}; function initMoveModules() { function handleMoveConfirm(modal) { @@ -1978,7 +1872,6 @@ function deleteModule(id, linkConnections) { _.each (ins, function(inEdge) { if (graph.degree(inEdge[0]) === 0) { tempModuleEl = $("#" + inEdge[0]); - tempModuleEl.find(".edit-module-group").parents("li").hide(); tempModuleEl.find(".clone-module-group").parents("li").hide(); tempModuleEl.find(".move-module-group").parents("li").hide(); tempModuleEl.find(".delete-module-group").parents("li").hide(); @@ -1989,7 +1882,6 @@ function deleteModule(id, linkConnections) { _.each (outs, function(outEdge) { if (graph.degree(outEdge[1]) === 0) { tempModuleEl = $("#" + outEdge[1]); - tempModuleEl.find(".edit-module-group").parents("li").hide(); tempModuleEl.find(".clone-module-group").parents("li").hide(); tempModuleEl.find(".move-module-group").parents("li").hide(); tempModuleEl.find(".delete-module-group").parents("li").hide(); @@ -2361,7 +2253,6 @@ cloneModuleGroupHandler = function(moduleId, modulesSel, gridDistX, gridDistY) { var nm = cloneModule(m, gridDistX, gridDistY, elLeft(m), elTop(m) + height + offset - gridDistY); //Show module group options - nm.find(".edit-module-group").parents("li").show(); nm.find(".clone-module-group").parents("li").show(); nm.find(".move-module-group").parents("li").show(); nm.find(".delete-module-group").parents("li").show(); @@ -2963,12 +2854,10 @@ function initJsPlumb(containerSel, containerChildSel, modulesSel, params) { //Modules should belong to module group now //Show module group options for target and source - srcModuleEl.find(".edit-module-group").parents("li").show(); srcModuleEl.find(".clone-module-group").parents("li").show(); srcModuleEl.find(".move-module-group").parents("li").show(); srcModuleEl.find(".delete-module-group").parents("li").show(); - targetModuleEl.find(".edit-module-group").parents("li").show(); targetModuleEl.find(".clone-module-group").parents("li").show(); targetModuleEl.find(".move-module-group").parents("li").show(); targetModuleEl.find(".delete-module-group").parents("li").show(); @@ -3001,13 +2890,11 @@ function initJsPlumb(containerSel, containerChildSel, modulesSel, params) { var targetModuleEl = $("#" + c.targetId); //First source if (graph.degree(c.sourceId) === 0) { - srcModuleEl.find(".edit-module-group").parents("li").hide(); srcModuleEl.find(".clone-module-group").parents("li").hide(); srcModuleEl.find(".move-module-group").parents("li").hide(); srcModuleEl.find(".delete-module-group").parents("li").hide(); } if (graph.degree(c.targetId) === 0) { - targetModuleEl.find(".edit-module-group").parents("li").hide(); targetModuleEl.find(".clone-module-group").parents("li").hide(); targetModuleEl.find(".move-module-group").parents("li").hide(); targetModuleEl.find(".delete-module-group").parents("li").hide(); diff --git a/app/controllers/canvas_controller.rb b/app/controllers/canvas_controller.rb index 4f0291a64..14728cc92 100644 --- a/app/controllers/canvas_controller.rb +++ b/app/controllers/canvas_controller.rb @@ -29,58 +29,41 @@ class CanvasController < ApplicationController end def update - error = false - # Make sure that remove parameter is valid to_archive = [] - if can_archive_modules(@experiment) and - update_params[:remove].present? then - to_archive = update_params[:remove].split(",") - unless to_archive.all? { |id| is_int? id } - error = true + if can_archive_modules(@experiment) && update_params[:remove].present? + to_archive = update_params[:remove].split(',') + if to_archive.all? { |id| is_int? id } + to_archive.collect!(&:to_i) else - to_archive.collect! { |id| id.to_i } + return render_403 end end - if error then - render_403 and return - end - # Make sure connections parameter is valid connections = [] - if can_edit_connections(@experiment) and - update_params[:connections].present? then - conns = update_params[:connections].split(",") - unless conns.length % 2 == 0 and - conns.all? { |c| c.is_a? String } then - error = true - else + if can_edit_connections(@experiment) && update_params[:connections].present? + conns = update_params[:connections].split(',') + if conns.length.even? && conns.all? { |c| c.is_a? String } conns.each_slice(2).each do |c| connections << [c[0], c[1]] end + else + return render_403 end end - if error then - render_403 and return - end - # Make sure positions parameter is valid - positions = Hash.new - if can_reposition_modules(@experiment) and - update_params[:positions].present? then - 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]) - error = true - break + positions = {} + if can_reposition_modules(@experiment) && 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]) + return render_403 end - if index == 0 + if index.zero? center = pos x = 0 y = 0 @@ -89,86 +72,59 @@ class CanvasController < ApplicationController y = pos[2].to_i - center[2].to_i end # Multiple modules cannot have same position - if positions.any? { |k,v| v[:x] == x and v[:y] == y} then - error = true - break - end + return render_403 if positions.any? { |_, v| v[:x] == x && v[:y] == y } positions[pos[0]] = { x: x, y: y } end end - if error then - render_403 and return - end - # Make sure that to_add is an array of strings, # as well as that positions for newly added modules exist to_add = [] - if can_create_modules(@experiment) and - update_params[:add].present? and - update_params["add-names"].present? then - ids = update_params[:add].split(",") - names = update_params["add-names"].split("|") - unless ids.length == names.length and - ids.all? { |id| id.is_a? String and positions.include? id } and - names.all? { |name| name.is_a? String } - error = true - else + if can_create_modules(@experiment) && update_params[:add].present? && + update_params['add-names'].present? + ids = update_params[:add].split(',') + names = update_params['add-names'].split('|') + if ids.length == names.length && + ids.all? { |id| id.is_a?(String) && positions.include?(id) } && + names.all? { |name| name.is_a? String } ids.each_with_index do |id, i| - to_add << { - id: id, - name: names[i], - x: positions[id][:x], - y: positions[id][:y] - } + to_add << { id: id, name: names[i], + x: positions[id][:x], y: positions[id][:y] } end + else + return render_403 end end - if error then - render_403 and return - end - # Make sure rename parameter is valid - to_rename = Hash.new - if can_edit_modules(@experiment) and - update_params[:rename].present? then + to_rename = {} + if can_edit_modules(@experiment) && update_params[:rename].present? begin to_rename = JSON.parse(update_params[:rename]) - # Okay, JSON parsed! - unless ( - to_rename.is_a? Hash and - to_rename.keys.all? { |k| k.is_a? String } and - to_rename.values.all? { |k| k.is_a? String } - ) - error = true + unless to_rename.is_a?(Hash) && + to_rename.keys.all? { |k| k.is_a? String } && + to_rename.values.all? { |k| k.is_a? String } + return render_403 end rescue - error = true + return render_403 end end - if error then - render_403 and return - end - # Make sure move parameter is valid to_move = {} if can_move_modules(@experiment) && update_params[:move].present? begin to_move = JSON.parse(update_params[:move]) - # Okay, JSON parsed! - unless ( - to_move.is_a? Hash and - to_move.keys.all? { |k| k.is_a? String } && - to_move.values.all? { |k| k.is_a? String } - ) - error = true + unless to_move.is_a?(Hash) && + to_move.keys.all? { |k| k.is_a? String } && + to_move.values.all? { |k| k.is_a? String } + return render_403 end rescue - error = true + return render_403 end end @@ -181,54 +137,21 @@ class CanvasController < ApplicationController end end - render_403 and return if error - # Make sure that to_clone is an array of pairs, # as well as that all IDs exist - to_clone = Hash.new - if can_clone_modules(@experiment) and - update_params[:cloned].present? then - clones = update_params[:cloned].split(";") - (clones.collect { |v| v.split(",") }).each do |val| - unless (val.length == 2 and - is_int? val[0] and - val[1].is_a? String and - to_add.any? { |m| m[:id] == val[1] }) - error = true - break - else + to_clone = {} + if can_clone_modules(@experiment) && update_params[:cloned].present? + clones = update_params[:cloned].split(';') + (clones.collect { |v| v.split(',') }).each do |val| + if val.length == 2 && is_int?(val[0]) && val[1].is_a?(String) && + to_add.any? { |m| m[:id] == val[1] } to_clone[val[1]] = val[0] + else + return render_403 end end end - if error then - render_403 and return - end - - module_groups = Hash.new - if can_edit_module_groups(@experiment) and - update_params["module-groups"].present? then - begin - module_groups = JSON.parse(update_params["module-groups"]) - - # Okay, JSON parsed! - unless ( - module_groups.is_a? Hash and - module_groups.keys.all? { |k| k.is_a? String } and - module_groups.values.all? { |k| k.is_a? String } - ) - error = true - end - rescue - error = true - end - end - - if error then - render_403 and return - end - # Call the "master" function to do all the updating for us unless @experiment.update_canvas( to_archive, @@ -239,36 +162,29 @@ class CanvasController < ApplicationController to_clone, connections, positions, - current_user, - module_groups + current_user ) - render_403 and return + return render_403 end - #Save activities that modules were archived + # Save activities that modules were archived to_archive.each do |module_id| my_module = MyModule.find_by_id(module_id) - unless my_module.blank? - Activity.create( - type_of: :archive_module, - project: my_module.experiment.project, - experiment: my_module.experiment, - my_module: my_module, - user: current_user, - message: t( - 'activities.archive_module', - user: current_user.full_name, - module: my_module.name - ) - ) - end + next if my_module.blank? + Activity.create(type_of: :archive_module, + project: my_module.experiment.project, + experiment: my_module.experiment, + my_module: my_module, + user: current_user, + message: t('activities.archive_module', + user: current_user.full_name, + module: my_module.name)) end # Create workflow image @experiment.delay.generate_workflow_img - flash[:success] = t( - "experiments.canvas.update.success_flash") + flash[:success] = t('experiments.canvas.update.success_flash') redirect_to canvas_experiment_path(@experiment) end diff --git a/app/controllers/my_modules_controller.rb b/app/controllers/my_modules_controller.rb index be9eec021..c0457c635 100644 --- a/app/controllers/my_modules_controller.rb +++ b/app/controllers/my_modules_controller.rb @@ -291,7 +291,7 @@ class MyModulesController < ApplicationController task_names = [] new_samples = [] - @my_module.get_downstream_modules.each do |my_module| + @my_module.downstream_modules.each do |my_module| new_samples = samples.select { |el| my_module.samples.exclude?(el) } my_module.samples.push(*new_samples) task_names << my_module.name @@ -330,7 +330,7 @@ class MyModulesController < ApplicationController end task_names = [] - @my_module.get_downstream_modules.each do |my_module| + @my_module.downstream_modules.each do |my_module| task_names << my_module.name my_module.samples.destroy(samples & my_module.samples) end diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index d809d39bd..2ff557292 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -8,7 +8,6 @@ class SearchController < ApplicationController search_projects if @search_category == :projects search_experiments if @search_category == :experiments - search_workflows if @search_category == :workflows search_modules if @search_category == :modules search_results if @search_category == :results search_tags if @search_category == :tags @@ -145,7 +144,6 @@ class SearchController < ApplicationController def count_search_results @project_search_count = count_by_name Project @experiment_search_count = count_by_name Experiment - @workflow_search_count = count_by_name MyModuleGroup @module_search_count = count_by_name MyModule @result_search_count = count_by_name Result @tag_search_count = count_by_name Tag @@ -161,7 +159,6 @@ class SearchController < ApplicationController @search_results_count = @project_search_count @search_results_count += @experiment_search_count - @search_results_count += @workflow_search_count @search_results_count += @module_search_count @search_results_count += @result_search_count @search_results_count += @tag_search_count @@ -190,14 +187,6 @@ class SearchController < ApplicationController @search_count = @experiment_search_count end - def search_workflows - @workflow_results = [] - if @workflow_search_count > 0 - @workflow_results = search_by_name(MyModuleGroup) - end - @search_count = @workflow_search_count - end - def search_modules @module_results = [] @module_results = search_by_name(MyModule) if @module_search_count > 0 diff --git a/app/helpers/permission_helper.rb b/app/helpers/permission_helper.rb index 907cb2ac8..ef403766f 100644 --- a/app/helpers/permission_helper.rb +++ b/app/helpers/permission_helper.rb @@ -64,7 +64,6 @@ module PermissionHelper :can_edit_connections, :can_create_modules, :can_edit_modules, - :can_edit_module_groups, :can_clone_modules, :can_archive_modules, :can_view_reports, @@ -149,7 +148,6 @@ module PermissionHelper :can_edit_connections, :can_create_modules, :can_edit_modules, - :can_edit_module_groups, :can_clone_modules, :can_archive_modules ] do |proxy, *args, &block| @@ -421,10 +419,6 @@ module PermissionHelper is_user_or_higher_of_project(experiment.project) end - def can_edit_module_groups(experiment) - is_user_or_higher_of_project(experiment.project) - end - def can_clone_modules(experiment) is_user_or_higher_of_project(experiment.project) end diff --git a/app/models/connection.rb b/app/models/connection.rb index e2c68652b..b6bcb7e68 100644 --- a/app/models/connection.rb +++ b/app/models/connection.rb @@ -1,4 +1,6 @@ class Connection < ActiveRecord::Base - belongs_to :to, :class_name => 'MyModule', :foreign_key => 'input_id', inverse_of: :inputs - belongs_to :from, :class_name => 'MyModule', :foreign_key => 'output_id', inverse_of: :outputs + belongs_to :to, class_name: 'MyModule', foreign_key: 'input_id', + inverse_of: :inputs + belongs_to :from, class_name: 'MyModule', foreign_key: 'output_id', + inverse_of: :outputs end diff --git a/app/models/experiment.rb b/app/models/experiment.rb index 3ad412694..9f2a7cfce 100644 --- a/app/models/experiment.rb +++ b/app/models/experiment.rb @@ -87,26 +87,27 @@ class Experiment < ActiveRecord::Base end def modules_without_group - MyModule.where(experiment_id: id).where(my_module_group: nil) - .where(archived: false) + MyModule.where(experiment_id: id) + .where(my_module_group: nil) + .where(archived: false) end def active_module_groups - self.my_module_groups.joins(:my_modules) - .where('my_modules.archived = ?', false) - .distinct + my_module_groups.joins(:my_modules) + .where('my_modules.archived = ?', false) + .distinct end def active_modules - my_modules.where(:archived => false) + my_modules.where(archived: false) end def archived_modules - my_modules.where(:archived => true) + my_modules.where(archived: true) end def assigned_samples - Sample.joins(:my_modules).where(my_modules: {id: my_modules} ) + Sample.joins(:my_modules).where(my_modules: { id: my_modules }) end def unassigned_samples(assigned_samples) @@ -122,15 +123,15 @@ class Experiment < ActiveRecord::Base to_clone, connections, positions, - current_user, - module_groups + current_user ) cloned_modules = [] begin - Experiment.transaction do + with_lock do # First, add new modules new_ids, cloned_pairs, originals = add_modules( - to_add, to_clone, current_user) + to_add, to_clone, current_user + ) cloned_modules = cloned_pairs.collect { |mn, _| mn } # Rename modules @@ -138,39 +139,31 @@ class Experiment < ActiveRecord::Base # Add activities that modules were created originals.each do |m| - Activity.create( - type_of: :create_module, + Activity.create(type_of: :create_module, user: current_user, - project: self.project, + project: project, experiment: m.experiment, my_module: m, - message: I18n.t( - "activities.create_module", - user: current_user.full_name, - module: m.name - ) - ) + message: I18n.t('activities.create_module', + user: current_user.full_name, + module: m.name)) end # Add activities that modules were cloned cloned_pairs.each do |mn, mo| - Activity.create( - type_of: :clone_module, + Activity.create(type_of: :clone_module, project: mn.experiment.project, experiment: mn.experiment, my_module: mn, user: current_user, - message: I18n.t( - "activities.clone_module", - user: current_user.full_name, - module_new: mn.name, - module_original: mo.name - ) - ) + message: I18n.t('activities.clone_module', + user: current_user.full_name, + module_new: mn.name, + module_original: mo.name)) end # Then, archive modules that need to be archived - archive_modules(to_archive, current_user) + 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 @@ -187,17 +180,13 @@ class Experiment < ActiveRecord::Base updated_to_move_groups[mapped] = value end updated_connections = [] - connections.each do |a,b| + connections.each do |a, b| updated_connections << [new_ids.fetch(a, a), new_ids.fetch(b, b)] end - updated_positions = Hash.new + updated_positions = {} positions.each do |id, pos| updated_positions[new_ids.fetch(id, id)] = pos end - updated_module_groups = {} - module_groups.each do |id, name| - updated_module_groups[new_ids.fetch(id, id)] = name - end # Update connections update_module_connections(updated_connections) @@ -209,7 +198,7 @@ class Experiment < ActiveRecord::Base normalize_module_positions # Finally, update module groups - update_module_groups(updated_module_groups, current_user) + update_module_groups(current_user) # Finally move any modules to another experiment move_modules(updated_to_move) @@ -217,11 +206,12 @@ class Experiment < ActiveRecord::Base # Everyhing is set, now we can move any module groups move_module_groups(updated_to_move_groups) end - rescue ActiveRecord::ActiveRecordError, ArgumentError, ActiveRecord::RecordNotSaved + rescue ActiveRecord::ActiveRecordError, + ArgumentError, + ActiveRecord::RecordNotSaved return false end - - return true + true end # This method generate the workflow image and saves it as @@ -422,22 +412,15 @@ class Experiment < ActiveRecord::Base # Archive all modules. Receives an array of module integer IDs. def archive_modules(module_ids) - module_ids.each do |m_id| - my_module = self.my_modules.find_by_id(m_id) - unless my_module.blank? - my_module.archive! - end - end - modules.reload + my_modules.where(id: module_ids).each(&:archive!) + my_modules.reload end - # Archive all modules. Receives an array of module integer IDs and current user. + # Archive all modules. Receives an array of module integer IDs + # and current user. def archive_modules(module_ids, current_user) - module_ids.each do |m_id| - my_module = self.my_modules.find_by_id(m_id) - unless my_module.blank? - my_module.archive!(current_user) - end + my_modules.where(id: module_ids).each do |m| + m.archive!(current_user) end my_modules.reload end @@ -451,15 +434,14 @@ class Experiment < ActiveRecord::Base def add_modules(to_add, to_clone, current_user) originals = [] cloned_pairs = {} - ids_map = Hash.new + ids_map = {} to_add.each do |m| original = MyModule.find_by_id(to_clone.fetch(m[:id], nil)) - if original.present? then + if original.present? my_module = original.deep_clone(current_user) cloned_pairs[my_module] = original else - my_module = MyModule.new( - experiment: self) + my_module = MyModule.new(experiment: self) originals << my_module end @@ -528,7 +510,7 @@ class Experiment < ActiveRecord::Base # to bottom left corner. def move_module_groups(to_move) to_move.each do |ids, experiment_id| - modules = my_modules.where(id: ids) + modules = my_modules.find(ids) groups = Set.new(modules.map(&:my_module_group)) experiment = project.experiments.find_by_id(experiment_id) @@ -568,10 +550,8 @@ class Experiment < ActiveRecord::Base # Generates workflow img when the workflow or module is moved # to other experiment def generate_workflow_img_for_moved_modules(to_move) - to_move.values.uniq.each do |id| - experiment = Experiment.find_by_id(id) - next unless experiment - experiment.delay.generate_workflow_img + Experiment.where(id: to_move.values.uniq).each do |exp| + exp.delay.generate_workflow_img end end @@ -583,42 +563,39 @@ class Experiment < ActiveRecord::Base require 'rgl/base' require 'rgl/adjacency' require 'rgl/topsort' - dg = RGL::DirectedAdjacencyGraph.new - connections.each do |a,b| + connections.each do |a, b| # Check if both vertices exist - if (my_modules.find_all {|m| [a.to_i, b.to_i].include? m.id }).count == 2 + if (my_modules.find_all { |m| [a.to_i, b.to_i].include? m.id }).count == 2 dg.add_edge(a, b) end end # Check if cycles exist! topsort = dg.topsort_iterator.to_a - if topsort.length == 0 and dg.edges.size > 1 - raise ArgumentError, "Cycles exist." + if topsort.length.zero? && dg.edges.size > 1 + raise ArgumentError, 'Cycles exist.' end # First, delete existing connections # but keep a copy of previous state previous_sources = {} previous_sources.default = [] - my_modules.each do |m| + + my_modules.includes(inputs: { from: [:inputs, outputs: :to] }).each do |m| previous_sources[m.id] = [] m.inputs.each do |c| previous_sources[m.id] << c.from end end - self.my_modules.each do |m| - unless m.outputs.destroy_all - raise ActiveRecord::ActiveRecordError - end - end + # There are no callbacks in Connection, so delete_all should be safe + Connection.delete_all(output_id: my_modules) # Add new connections filtered_edges = dg.edges.collect { |e| [e.source, e.target] } filtered_edges.each do |a, b| - Connection.create!(:input_id => b, :output_id => a) + Connection.create!(input_id: b, output_id: a) end # Unassign samples from former downstream modules @@ -628,47 +605,49 @@ class Experiment < ActiveRecord::Base visited = [] # Assign samples to all new downstream modules filtered_edges.each do |a, b| - source = self.my_modules.find(a.to_i) - target = self.my_modules.find(b.to_i) + source = my_modules.includes({ inputs: :from }, :samples).find(a.to_i) + target = my_modules.find(b.to_i) # Do this only for new edges - if previous_sources[target.id].exclude?(source) - # Go as high upstream as new edges take us - # and then assign samples to all downsteam samples - assign_samples_to_new_downstream_modules(previous_sources, visited, source) - end + next unless previous_sources[target.id].exclude?(source) + # Go as high upstream as new edges take us + # and then assign samples to all downsteam samples + assign_samples_to_new_downstream_modules(previous_sources, + visited, + source) end # Save topological order of modules (for modules without workflow, # leave them unordered) - self.my_modules.each do |m| - if topsort.include? m.id.to_s - m.workflow_order = topsort.find_index(m.id.to_s) - else - m.workflow_order = -1 - end + my_modules.includes(:my_module_group).each do |m| + m.workflow_order = + if topsort.include? m.id.to_s + topsort.find_index(m.id.to_s) + else + -1 + end m.save! end - # Make sure to reload my modules, which now have updated connections and samples - self.my_modules.reload + # Make sure to reload my modules, which now have updated connections + # and samples + my_modules.reload true end # When connections are deleted, unassign samples that # are not inherited anymore def unassign_samples_from_old_downstream_modules(sources) - self.my_modules.each do |my_module| - sources[my_module.id].each do |s| + my_modules.each do |my_module| + sources[my_module.id].each do |src| # Only do this for newly deleted connections - if s.outputs.map{|i| i.to}.exclude? my_module - my_module.get_downstream_modules.each do |dm| - # Get unique samples for all upstream modules - um = dm.get_upstream_modules - um.shift # remove current module - ums = um.map{|m| m.samples}.flatten.uniq - s.samples.each do |sample| - dm.samples.destroy(sample) if ums.exclude? sample - end + next unless src.outputs.map(&:to).exclude? my_module + my_module.downstream_modules.each do |dm| + # Get unique samples for all upstream modules + um = dm.upstream_modules + um.shift # remove current module + ums = um.map(&:samples).flatten.uniq + src.samples.each do |sample| + dm.samples.destroy(sample) if ums.exclude? sample end end end @@ -678,24 +657,21 @@ class Experiment < ActiveRecord::Base # Assign samples to new connections recursively def assign_samples_to_new_downstream_modules(sources, visited, my_module) # If samples are already assigned for this module, stop going upstream - if visited.include? (my_module) - return - end + return if visited.include?(my_module) visited << my_module - # Edge case, when module is source or it doesn't have any new input connections - if my_module.inputs.blank? or ( - my_module.inputs.map{|c| c.from} - - sources[my_module.id] - ).empty? - my_module.get_downstream_modules.each do |dm| - new_samples = my_module.samples.select { |el| dm.samples.exclude?(el) } - dm.samples.push(*new_samples) + # Edge case, when module is source or it doesn't have any new input + # connections + if my_module.inputs.blank? || + (my_module.inputs.map(&:from) - sources[my_module.id]).empty? + my_module.downstream_modules.each do |dm| + new_samples = my_module.samples.where.not(id: dm.samples) + dm.samples << new_samples end else my_module.inputs.each do |input| # Go upstream for new in connections if sources[my_module.id].exclude?(input.from) - assign_samples_to_new_downstream_modules(input.from) + assign_samples_to_new_downstream_modules(sources, visited, input.from) end end end @@ -705,104 +681,58 @@ class Experiment < ActiveRecord::Base # Input is a map where keys are module IDs, and values are # hashes like { x: , y: }. def update_module_positions(positions) - positions.each do |id, pos| - unless MyModule.update(id, x: pos[:x], y: pos[:y]) - raise ActiveRecord::ActiveRecordError - end + modules = my_modules.where(id: positions.keys) + modules.each do |m| + m.update_columns(x: positions[m.id.to_s][:x], y: positions[m.id.to_s][:y]) end - self.my_modules.reload + my_modules.reload end # Normalize module positions in this project. def normalize_module_positions # This method normalizes module positions so x-s and y-s # are all positive - x_diff = (self.my_modules.collect { |m| m.x }).min - y_diff = (self.my_modules.collect { |m| m.y }).min + x_diff = my_modules.collect(&:x).min + y_diff = my_modules.collect(&:y).min - self.my_modules.each do |m| - unless - m.update_attribute(:x, m.x - x_diff) and - m.update_attribute(:y, m.y - y_diff) - raise ActiveRecord::ActiveRecordError - end + my_modules.each do |m| + m.update_columns(x: m.x - x_diff, y: m.y - y_diff) end end # Recalculate module groups in this project. Input is # a hash of module ids and their corresponding module names. - def update_module_groups(module_groups, current_user) + def update_module_groups(current_user) require 'rgl/base' require 'rgl/adjacency' require 'rgl/connected_components' dg = RGL::DirectedAdjacencyGraph[] group_ids = Set.new - active_modules.each do |m| - unless m.my_module_group.blank? - group_ids << m.my_module_group.id - end - unless dg.has_vertex? m.id - dg.add_vertex m.id - end + active_modules.includes(:my_module_group, outputs: :to).each do |m| + group_ids << m.my_module_group.id unless m.my_module_group.blank? + dg.add_vertex m.id unless dg.has_vertex? m.id m.outputs.each do |o| dg.add_edge m.id, o.to.id end end workflows = [] - dg.to_undirected.each_connected_component { |w| workflows << w } - - # Retrieve maximum allowed module group name - max_length = (MyModuleGroup.validators_on(:name).select { |v| v.class == ActiveModel::Validations::LengthValidator }).first.options[:maximum] - # For each workflow, generate new names - new_index = 1 - wf_names = [] - suffix = I18n.t("my_module_groups.new.suffix") - cut_index = -(suffix.length + 1) - workflows.each do |w| - modules = MyModule.find(w) - - # Get an array of module names - names = [] - modules.each do |m| - names << module_groups.fetch(m.id.to_s, "") - end - names = names.uniq - name = (names.select { |v| v != "" }).join(", ") - - if w.length <= 1 - name = nil - elsif name.blank? - name = I18n.t("my_module_groups.new.name", index: new_index) - new_index += 1 - while MyModuleGroup.find_by(name: name).present? - name = I18n.t("my_module_groups.new.name", index: new_index) - new_index += 1 - end - elsif name.length > max_length - # If length is too long, shorten it - name = name[0..(max_length + cut_index)] + suffix - end - - wf_names << name + dg.to_undirected.each_connected_component do |w| + workflows << my_modules.find(w) end # Remove any existing module groups from modules - unless MyModuleGroup.destroy_all(:id => group_ids.to_a) + unless MyModuleGroup.destroy_all(id: group_ids.to_a) raise ActiveRecord::ActiveRecordError end # Second, create new groups - workflows.each_with_index do |w, i| + workflows.each do |modules| # Single modules are not considered part of any workflow - if w.length > 1 - group = MyModuleGroup.new( - name: wf_names[i], - experiment: self, - my_modules: MyModule.find(w)) - group.created_by = current_user - group.save! - end + next unless modules.length > 1 + MyModuleGroup.create!(experiment: self, + my_modules: modules, + created_by: current_user) end my_module_groups.reload diff --git a/app/models/my_module.rb b/app/models/my_module.rb index 27937bfeb..232dce7d2 100644 --- a/app/models/my_module.rb +++ b/app/models/my_module.rb @@ -252,28 +252,24 @@ class MyModule < ActiveRecord::Base end # Treat this module as root, get all modules of that subtree - def get_downstream_modules + def downstream_modules final = [] modules = [self] - while !modules.empty? + until modules.empty? my_module = modules.shift - if !final.include?(my_module) - final << my_module - end + final << my_module unless final.include?(my_module) modules.push(*my_module.my_modules.flatten) end final end # Treat this module as inversed root, get all modules of that inversed subtree - def get_upstream_modules + def upstream_modules final = [] modules = [self] - while !modules.empty? + until modules.empty? my_module = modules.shift - if !final.include?(my_module) - final << my_module - end + final << my_module unless final.include?(my_module) modules.push(*my_module.my_module_antecessors.flatten) end final diff --git a/app/models/my_module_group.rb b/app/models/my_module_group.rb index f965876c4..dc5b15ece 100644 --- a/app/models/my_module_group.rb +++ b/app/models/my_module_group.rb @@ -1,49 +1,18 @@ class MyModuleGroup < ActiveRecord::Base include SearchableModel - auto_strip_attributes :name, nullify: false - validates :name, - presence: true, - length: { maximum: Constants::NAME_MAX_LENGTH } validates :experiment, presence: true belongs_to :experiment, inverse_of: :my_module_groups belongs_to :created_by, foreign_key: 'created_by_id', class_name: 'User' has_many :my_modules, inverse_of: :my_module_group, dependent: :nullify - def self.search(user, - include_archived, - query = nil, - page = 1, - _current_team = nil, - options = {}) - exp_ids = - Experiment - .search(user, include_archived, nil, Constants::SEARCH_NO_LIMIT) - .pluck(:id) - - new_query = MyModuleGroup - .distinct - .where('my_module_groups.experiment_id IN (?)', exp_ids) - .where_attributes_like('my_module_groups.name', query, options) - - # Show all results if needed - if page == Constants::SEARCH_NO_LIMIT - new_query - else - new_query - .limit(Constants::SEARCH_LIMIT) - .offset((page - 1) * Constants::SEARCH_LIMIT) - end - end - def ordered_modules my_modules.order(workflow_order: :asc) end def deep_clone_to_experiment(current_user, experiment) clone = MyModuleGroup.new( - name: name, created_by: created_by, experiment: experiment ) diff --git a/app/utilities/first_time_data_generator.rb b/app/utilities/first_time_data_generator.rb index c77cf8ca0..c3a7933d2 100644 --- a/app/utilities/first_time_data_generator.rb +++ b/app/utilities/first_time_data_generator.rb @@ -150,7 +150,6 @@ module FirstTimeDataGenerator # Create a module group my_module_group = MyModuleGroup.create( - name: 'Potato qPCR workflow', experiment: experiment ) @@ -317,7 +316,7 @@ module FirstTimeDataGenerator samples_to_assign << sample end - my_modules[1].get_downstream_modules.each do |mm| + my_modules[1].downstream_modules.each do |mm| samples_to_assign.each do |s| SampleMyModule.create( sample: s, @@ -881,7 +880,7 @@ module FirstTimeDataGenerator ).sneaky_save # create thumbnail - experiment.generate_workflow_img + experiment.delay.generate_workflow_img # Lastly, create cookie with according ids # so tutorial steps can be properly positioned diff --git a/app/views/canvas/_edit.html.erb b/app/views/canvas/_edit.html.erb index 45b44cb37..a362c5066 100644 --- a/app/views/canvas/_edit.html.erb +++ b/app/views/canvas/_edit.html.erb @@ -1,7 +1,6 @@
" data-can-edit-modules="<%= can_edit_modules(@experiment) ? "yes" : "no" %>" - data-can-edit-module-groups="<%= can_edit_module_groups(@experiment) ? "yes" : "no" %>" data-can-clone-modules="<%= can_clone_modules(@experiment) ? "yes" : "no" %>" data-can-move-modules="<%= can_move_modules(@experiment) ? "yes" : "no" %>" data-can-delete-modules="<%= can_archive_modules(@experiment) ? "yes" : "no" %>" @@ -49,9 +48,6 @@ - @@ -87,9 +83,6 @@ <% if can_edit_modules(@experiment) %> <%= render partial: "canvas/edit/modal/edit_module", locals: {experiment: @experiment } %> <% end %> -<% if can_edit_module_groups(@experiment) %> - <%= render partial: "canvas/edit/modal/edit_module_group", locals: {experiment: @experiment } %> -<% end %> <% if can_move_modules(@experiment) %> <%= render partial: "canvas/edit/modal/move_module", locals: {experiment: @experiment } %> <%= render partial: "canvas/edit/modal/move_module_group", locals: {experiment: @experiment } %> diff --git a/app/views/canvas/edit/_my_module.html.erb b/app/views/canvas/edit/_my_module.html.erb index 54ee90c4e..d0a876290 100644 --- a/app/views/canvas/edit/_my_module.html.erb +++ b/app/views/canvas/edit/_my_module.html.erb @@ -2,11 +2,6 @@ id="<%= my_module.id %>" data-module-id="<%= my_module.id %>" data-module-name="<%= my_module.name %>" - <% if my_module.my_module_group.present? %> - data-module-group-name="<%= my_module.my_module_group.name %>" - <% else %> - data-module-group-name="" - <% end %> data-module-x="<%= my_module.x %>" data-module-y="<%= my_module.y %>" data-module-conns="<%= construct_module_connections(my_module) %>"> @@ -26,11 +21,6 @@ <%=t "experiments.canvas.edit.edit_module" %> <% end %> - <% if can_edit_module_groups(my_module.experiment) %> -
  • > - <%=t "experiments.canvas.edit.edit_module_group" %> -
  • - <% end %> <% if can_clone_modules(my_module.experiment) %>
  • <%=t "experiments.canvas.edit.clone_module" %> diff --git a/app/views/canvas/edit/modal/_edit_module_group.html.erb b/app/views/canvas/edit/modal/_edit_module_group.html.erb deleted file mode 100644 index d2e14a628..000000000 --- a/app/views/canvas/edit/modal/_edit_module_group.html.erb +++ /dev/null @@ -1,19 +0,0 @@ - diff --git a/app/views/search/index.html.erb b/app/views/search/index.html.erb index 92bfaea45..b00a61fcb 100644 --- a/app/views/search/index.html.erb +++ b/app/views/search/index.html.erb @@ -70,19 +70,6 @@ <%= t'Experiments' %>
  • -