diff --git a/app/assets/javascripts/protocols/steps.js.erb b/app/assets/javascripts/protocols/steps.js.erb
index 8f96b177b..b6d58275c 100644
--- a/app/assets/javascripts/protocols/steps.js.erb
+++ b/app/assets/javascripts/protocols/steps.js.erb
@@ -162,29 +162,30 @@
// Set callback for click on move step
function applyMoveStepCallBack() {
- $("[data-action='move-step']").off("ajax:success");
- $("[data-action='move-step']")
- .on("ajax:success", function(e, data) {
- var $step = $(this).closest(".step");
- var stepUpPosition = data.step_up_position;
- var stepDownPosition = data.step_down_position;
- var $stepDown, $stepUp;
- switch (data.move_direction) {
- case "up":
- $stepDown = $step.prev(".step");
+ $('#steps').on('ajax:success', "[data-action='move-step']", function(e, data) {
+ if ($.isEmptyObject(data)) return;
+
+ let $step = $(this).closest('.step');
+ let stepUpPosition = data.step_up_position;
+ let stepDownPosition = data.step_down_position;
+ let $stepDown, $stepUp;
+
+ switch ($(this).data('direction')) {
+ case 'up':
+ $stepDown = $step.prev('.step');
$stepUp = $step;
break;
- case "down":
+ case 'down':
$stepDown = $step;
- $stepUp = $step.next(".step");
+ $stepUp = $step.next('.step');
}
// Switch position of top and bottom steps
if (!_.isUndefined($stepDown) && !_.isUndefined($stepUp)) {
$stepDown.insertAfter($stepUp);
- $stepDown.find(".step-number").html(stepDownPosition + 1);
- $stepUp.find(".step-number").html(stepUpPosition + 1);
- $("html, body").animate({ scrollTop: $step.offset().top - window.innerHeight / 2 });
+ $stepDown.find('.step-number').html(`${stepDownPosition + 1}.`);
+ $stepUp.find('.step-number').html(`${stepUpPosition + 1}.`);
+ $('html, body').animate({ scrollTop: $step.offset().top - window.innerHeight / 2 });
}
if (typeof refreshProtocolStatusBar === 'function') refreshProtocolStatusBar();
diff --git a/app/controllers/steps_controller.rb b/app/controllers/steps_controller.rb
index 58077aabf..ddec42b89 100644
--- a/app/controllers/steps_controller.rb
+++ b/app/controllers/steps_controller.rb
@@ -80,9 +80,6 @@ class StepsController < ApplicationController
else
log_activity(:add_step_to_protocol_repository, nil, protocol: @protocol.id)
end
-
- # Update protocol timestamp
- update_protocol_ts(@step)
end
respond_to do |format|
@@ -194,9 +191,6 @@ class StepsController < ApplicationController
log_activity(:edit_step_in_protocol_repository, nil, protocol: @protocol.id)
end
- # Update protocol timestamp
- update_protocol_ts(@step)
-
format.json {
render json: {
html: render_to_string({
@@ -226,11 +220,6 @@ class StepsController < ApplicationController
def destroy
if @step.can_destroy?
- # Update position on other steps of this module
- @protocol.steps.where('position > ?', @step.position).each do |step|
- step.position = step.position - 1
- step.save
- end
# Calculate space taken by this step
team = @protocol.team
@@ -250,9 +239,6 @@ class StepsController < ApplicationController
team.release_space(previous_size)
team.save
- # Update protocol timestamp
- update_protocol_ts(@step)
-
flash[:success] = t(
'protocols.steps.destroy.success_flash',
step: (@step.position_plus_one).to_s
@@ -367,57 +353,34 @@ class StepsController < ApplicationController
def move_up
respond_to do |format|
- if @step.position.positive?
- step_down = @step.protocol.steps.find_by(position: @step.position - 1)
- @step.position -= 1
- @step.save
+ format.json do
+ if @step.protocol.steps.minimum(:position) != @step.position
+ @step.update!(position: @step.position - 1)
- if step_down
- step_down.position += 1
- step_down.save
-
- # Update protocol timestamp
- update_protocol_ts(@step)
-
- format.json do
- render json: { move_direction: 'up',
- step_up_position: @step.position,
- step_down_position: step_down.position },
- status: :ok
- end
+ render json: {
+ step_up_position: @step.position,
+ step_down_position: @step.position + 1
+ }
else
- format.json { render json: {}, status: :forbidden }
+ render json: {}
end
- else
- format.json { render json: {}, status: :forbidden }
end
end
end
def move_down
respond_to do |format|
- if @step.position < @step.protocol.steps.count - 1
- step_up = @step.protocol.steps.find_by(position: @step.position + 1)
- @step.position += 1
- @step.save
+ format.json do
+ if @step.protocol.steps.maximum(:position) != @step.position
+ @step.update!(position: @step.position + 1)
- if step_up
- step_up.position -= 1
- step_up.save
-
- # Update protocol timestamp
- update_protocol_ts(@step)
-
- format.json do
- render json: { move_direction: 'down',
- step_up_position: step_up.position,
- step_down_position: @step.position }
- end
+ render json: {
+ step_up_position: @step.position - 1,
+ step_down_position: @step.position
+ }
else
- format.json { render json: {}, status: :forbidden }
+ render json: {}
end
- else
- format.json { render json: {}, status: :forbidden }
end
end
end
@@ -517,12 +480,6 @@ class StepsController < ApplicationController
end
end
- def update_protocol_ts(step)
- if step.present? && step.protocol.present?
- step.protocol.update(updated_at: Time.now)
- end
- end
-
def update_checklist_items_without_callback(params)
params.dig('checklists_attributes')&.values&.each do |cl|
ck = @step.checklists.find_by(id: cl[:id])
diff --git a/app/models/step.rb b/app/models/step.rb
index 86fa7002c..bf40e18c7 100644
--- a/app/models/step.rb
+++ b/app/models/step.rb
@@ -16,12 +16,13 @@ class Step < ApplicationRecord
before_validation :set_completed_on, if: :completed_changed?
before_save :set_last_modified_by
+ around_save :adjust_positions_on_save, if: :position_changed?
before_destroy :cascade_before_destroy
- before_destroy :adjust_positions_on_destroy
+ after_destroy :adjust_positions_after_destroy
belongs_to :user, inverse_of: :steps
belongs_to :last_modified_by, foreign_key: 'last_modified_by_id', class_name: 'User', optional: true
- belongs_to :protocol, inverse_of: :steps
+ belongs_to :protocol, inverse_of: :steps, touch: true
has_many :checklists, inverse_of: :step, dependent: :destroy
has_many :step_comments, foreign_key: :associated_id, dependent: :destroy
has_many :step_assets, inverse_of: :step, dependent: :destroy
@@ -125,9 +126,20 @@ class Step < ApplicationRecord
private
- def adjust_positions_on_destroy
+ def adjust_positions_on_save
+ step_to_swap = protocol.steps.find_by(position: position)
+
+ return yield unless step_to_swap
+
+ position_to_swap = position_was
+ step_to_swap.position = -1
+ yield
+ step_to_swap.update!(position: position_to_swap)
+ end
+
+ def adjust_positions_after_destroy
protocol.steps.where('position > ?', position).find_each do |step|
- step.update(position: step.position - 1)
+ step.update!(position: step.position - 1)
end
end
diff --git a/app/views/steps/_step.html.erb b/app/views/steps/_step.html.erb
index bfbfadd56..6374a6824 100644
--- a/app/views/steps/_step.html.erb
+++ b/app/views/steps/_step.html.erb
@@ -49,29 +49,28 @@
<% end %>
<% end %>
- <% if (can_manage_protocol_in_module?(@protocol) ||
- can_manage_protocol_in_repository?(@protocol)) && !(preview) %>
- "
- data-remote="true">
-
- "
- data-remote="true">
-
- "
- href="<%= edit_step_path(step, format: :json) %>"
- data-remote="true" >
-
-
- <%= link_to(step_path(step), title: t("protocols.steps.options.delete_title"), method: "delete", class: "btn btn-light icon-btn",
- data: {action: "delete-step", confirm: t("protocols.steps.destroy.confirm", step: step.name)}) do %>
+ <% if (can_manage_protocol_in_module?(@protocol) || can_manage_protocol_in_repository?(@protocol)) && !(preview) %>
+ <%= link_to(move_up_step_path(step),
+ class: 'btn btn-light icon-btn',
+ title: t('protocols.steps.options.up_arrow_title'),
+ remote: true,
+ method: :put,
+ data: { action: 'move-step', direction: :up }) do %>
+
+ <% end %>
+ <%= link_to(move_down_step_path(step),
+ class: 'btn btn-light icon-btn',
+ title: t('protocols.steps.options.down_arrow_title'),
+ remote: true,
+ method: :put,
+ data: { action: 'move-step', direction: :down }) do %>
+
+ <% end %>
+ <%= link_to(step_path(step),
+ title: t('protocols.steps.options.delete_title'),
+ method: :delete,
+ class: 'btn btn-light icon-btn',
+ data: { action: 'delete-step', confirm: t('protocols.steps.destroy.confirm', step: step.name) }) do %>
<% end %>
<% end %>
diff --git a/config/routes.rb b/config/routes.rb
index 298b2ead2..8e603000e 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -448,8 +448,8 @@ Rails.application.routes.draw do
member do
post 'checklistitem_state'
post 'toggle_step_state'
- get 'move_down'
- get 'move_up'
+ put 'move_down'
+ put 'move_up'
post 'update_view_state'
end
end
diff --git a/spec/factories/steps.rb b/spec/factories/steps.rb
index 94886a6b8..8c6677963 100644
--- a/spec/factories/steps.rb
+++ b/spec/factories/steps.rb
@@ -3,7 +3,7 @@
FactoryBot.define do
factory :step do
name { Faker::Name.unique.name }
- position { Faker::Number.between(from: 1, to: 10) }
+ position { protocol ? protocol.steps.count : Faker::Number.between(from: 1, to: 10) }
completed { true }
user
protocol