Merge pull request #2759 from okriuchykhin/ok_SCI_4817

Refactor task archive and move permissions [SCI-4817]

app/controllers/canvas_controller.rb

@@ -33,10 +33,7 @@ class CanvasController < ApplicationController
     to_archive = []
     if update_params[:remove].present?
       to_archive = update_params[:remove].split(',')
-      if to_archive.all? do |id|
+      if to_archive.all? { |id| can_archive_module?(MyModule.find_by(id: id)) }
-           is_int?(id) &&
-           can_manage_module?(MyModule.find_by_id(id))
-         end
         to_archive.collect!(&:to_i)
       else
         return render_403
@@ -117,16 +114,14 @@ class CanvasController < ApplicationController
         # Okay, JSON parsed!
         unless to_move.is_a?(Hash) &&
                to_move.keys.all? do |id|
-                 id.is_a?(String) &&
+                 !is_int?(id) || can_move_module?(MyModule.find_by(id: id))
-                 (!is_int?(id) || can_manage_module?(MyModule.find_by_id(id)))
                end &&
                to_move.values.all? do |exp_id|
-                 exp_id.is_a?(String) &&
+                 can_manage_experiment?(Experiment.find_by(id: exp_id))
-                 can_manage_experiment?(Experiment.find_by_id(exp_id))
                end
           return render_403
         end
-      rescue
+      rescue StandardError
         return render_403
       end
     end

app/controllers/my_modules_controller.rb

@@ -9,7 +9,7 @@ class MyModulesController < ApplicationController
 
   before_action :load_vars
   before_action :load_projects_tree, only: %i(protocols results activities archive)
-  before_action :check_manage_permissions_archive, only: %i(update)
+  before_action :check_archive_and_restore_permissions, only: %i(update)
   before_action :check_manage_permissions, only: %i(description due_date update_description update_protocol_description)
   before_action :check_view_permissions, except: %i(update update_description update_protocol_description
                                                     toggle_task_state)
@@ -372,11 +372,11 @@ class MyModulesController < ApplicationController
     render_403 && return unless can_manage_module?(@my_module)
   end
 
-  def check_manage_permissions_archive
+  def check_archive_and_restore_permissions
     render_403 && return unless if my_module_params[:archived] == 'false'
                                   can_restore_module?(@my_module)
                                 else
-                                  can_manage_module?(@my_module)
+                                  can_archive_module?(@my_module)
                                 end
   end
 

app/models/experiment.rb

@@ -138,13 +138,11 @@ class Experiment < ApplicationRecord
         archive_modules(to_archive, current_user) if to_archive.any?
 
         # Update only existing tasks positions to release positions for new tasks
-        existing_positions = positions
+        existing_positions = positions.slice(*positions.keys.map { |k| k unless k.to_s.start_with?('n') }.compact)
-                             .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
+        existing_to_move = to_move.slice(*to_move.keys.map { |k| k unless k.to_s.start_with?('n') }.compact)
-                            .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

app/permissions/experiment.rb

@@ -72,12 +72,17 @@ Canaid::Permissions.register_for(MyModule) do
     end
   end
 
-  # module: update, archive, move
+  # module: update
   # result: create, update
   can :manage_module do |user, my_module|
     can_manage_experiment?(user, my_module.experiment)
   end
 
+  # module: archive
+  can :archive_module do |user, my_module|
+    can_manage_experiment?(user, my_module.experiment)
+  end
+
   # NOTE: Must not be dependent on canaid parmision for which we check if it's
   # active
   # module: restore
@@ -86,6 +91,11 @@ Canaid::Permissions.register_for(MyModule) do
       my_module.archived?
   end
 
+  # module: move
+  can :move_module do |user, my_module|
+    can_manage_experiment?(user, my_module.experiment)
+  end
+
   # module: assign/reassign/unassign users
   can :manage_users_in_module do |user, my_module|
     user.is_owner_of_project?(my_module.experiment.project)

app/views/canvas/edit/_my_module.html.erb

@@ -7,7 +7,6 @@
   data-module-conns="<%= construct_module_connections(my_module) %>">
 
   <% module_group = my_module.my_module_group %>
-  <% can_manage_module_group = module_group && (module_group.new_record? || module_group.my_modules.all? { |my_module| can_manage_module?(my_module) }) %>
 
   <div class="panel-heading">
 
@@ -21,35 +20,35 @@
         <li class="dropdown-header"><%= t('projects.index.options_header') %></li>
         <% if can_manage_module?(my_module) %>
           <li>
-              <a class="edit-module" href="" data-module-id="<%= my_module.id %>"><%=t "experiments.canvas.edit.edit_module" %></a>
+            <a class="edit-module" href="" data-module-id="<%= my_module.id %>"><%= t('experiments.canvas.edit.edit_module') %></a>
           </li>
         <% end %>
         <% if can_manage_experiment?(my_module.experiment) %>
           <li>
-            <a class ="clone-module" href="" data-module-id="<%= my_module.id %>"><%=t "experiments.canvas.edit.clone_module" %></a>
+            <a class ="clone-module" href="" data-module-id="<%= my_module.id %>"><%= t('experiments.canvas.edit.clone_module') %></a>
           </li>
           <li <%= 'style=display:none;' if my_module.my_module_group.blank? %>>
-            <a class ="clone-module-group" href="" data-module-id="<%= my_module.id %>"><%=t "experiments.canvas.edit.clone_module_group" %></a>
+            <a class ="clone-module-group" href="" data-module-id="<%= my_module.id %>"><%= t('experiments.canvas.edit.clone_module_group') %></a>
           </li>
         <% end %>
-        <% if can_manage_module?(my_module) %>
+        <% if can_move_module?(my_module) %>
           <li>
-            <a class="move-module" href="" data-module-id="<%= my_module.id %>"><%=t "experiments.canvas.edit.move_module" %></a>
+            <a class="move-module" href="" data-module-id="<%= my_module.id %>"><%= t('experiments.canvas.edit.move_module') %></a>
           </li>
         <% end %>
-        <% if can_manage_module_group %>
+        <% if module_group.my_modules.all? { |my_module| can_move_module?(my_module) } %>
           <li>
-            <a class="move-module-group" href="" data-module-id="<%= my_module.id %>"><%=t "experiments.canvas.edit.move_module_group" %></a>
+            <a class="move-module-group" href="" data-module-id="<%= my_module.id %>"><%= t('experiments.canvas.edit.move_module_group') %></a>
           </li>
         <% end %>
-        <% if can_manage_module?(my_module) %>
+        <% if can_archive_module?(my_module) %>
           <li>
-            <a class="delete-module" href="" data-module-id="<%= my_module.id %>"><%=t "experiments.canvas.edit.delete_module" %></a>
+            <a class="delete-module" href="" data-module-id="<%= my_module.id %>"><%= t('experiments.canvas.edit.delete_module') %></a>
           </li>
         <% end %>
-        <% if can_manage_module_group %>
+        <% if module_group.my_modules.all? { |my_module| can_archive_module?(my_module) } %>
           <li data-hook="archive-module-group">
-            <a class ="delete-module-group" href="" data-module-id="<%= my_module.id %>"><%=t "experiments.canvas.edit.delete_module_group" %></a>
+            <a class ="delete-module-group" href="" data-module-id="<%= my_module.id %>"><%= t('experiments.canvas.edit.delete_module_group') %></a>
           </li>
         <% end %>
       </ul>
@@ -59,7 +58,7 @@
 
   <% if can_manage_experiment?(my_module.experiment) %>
     <div class="panel-body ep">
-      <%=t "experiments.canvas.edit.drag_connections" %>
+      <%= t('experiments.canvas.edit.drag_connections') %>
     </div>
   <% end %>