Update logic for linked protocol templates with task [SCI-7973] (#5058)

This commit is contained in:
Alex Kriuchykhin 2023-03-07 13:03:01 +01:00 committed by GitHub
parent 3cd4d9f760
commit f8a4e7ee65
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 23 additions and 163 deletions

View file

@ -49,10 +49,6 @@ class ProtocolsController < ApplicationController
update_from_parent_modal
delete_steps
)
before_action :check_manage_parent_in_repository_permissions, only: %i(
update_parent
update_parent_modal
)
before_action :check_manage_all_in_repository_permissions, only: :make_private
before_action :check_restore_all_in_repository_permissions, only: :restore
before_action :check_archive_all_in_repository_permissions, only: :archive
@ -477,47 +473,6 @@ class ProtocolsController < ApplicationController
end
end
def update_parent
respond_to do |format|
if @protocol.parent.can_destroy?
transaction_error = false
Protocol.transaction do
@protocol.update_parent(current_user)
rescue StandardError
transaction_error = true
raise ActiveRecord::Rollback
end
if transaction_error
# Bad request error
format.json do
render json: {
message: t('my_modules.protocols.update_parent_error')
},
status: :bad_request
end
else
# Everything good, record activity, display flash & render 200
log_activity(:update_protocol_in_repository_from_task,
@protocol.my_module.experiment.project,
my_module: @protocol.my_module.id,
protocol_repository: @protocol.parent.id)
flash[:success] = t(
'my_modules.protocols.update_parent_flash'
)
flash.keep(:success)
format.json { render json: {}, status: :ok }
end
else
format.json do
render json: {
message: t('my_modules.protocols.update_parent_error_locked')
}, status: :bad_request
end
end
end
end
def update_from_parent
protocol_can_destroy = @protocol.can_destroy?
respond_to do |format|
@ -912,19 +867,6 @@ class ProtocolsController < ApplicationController
end
end
def update_parent_modal
respond_to do |format|
format.json do
render json: {
title: t('my_modules.protocols.confirm_link_update_modal.update_parent_title'),
message: t('my_modules.protocols.confirm_link_update_modal.update_parent_message'),
btn_text: t('general.update'),
url: update_parent_protocol_path(@protocol)
}
end
end
end
def update_from_parent_modal
respond_to do |format|
format.json do
@ -1202,13 +1144,6 @@ class ProtocolsController < ApplicationController
can_delete_protocol_draft_in_repository?(@protocol)
end
def check_manage_parent_in_repository_permissions
@protocol = Protocol.find_by_id(params[:id])
render_403 unless @protocol.present? &&
can_read_protocol_in_module?(@protocol) &&
can_manage_protocol_in_repository?(@protocol.parent)
end
def check_manage_all_in_repository_permissions
@protocols = Protocol.where(id: params[:protocol_ids])
@protocols.find_each do |protocol|

View file

@ -404,14 +404,6 @@ class MyModule < ApplicationRecord
clone_tinymce_assets(target_my_module, target_my_module.experiment.project.team)
target_my_module.protocols << protocol.deep_clone_my_module(self, current_user)
target_my_module.reload
# fixes linked protocols
target_my_module.protocols.each do |protocol|
next unless protocol.linked?
protocol.updated_at = protocol.parent_updated_at
protocol.save
end
end
# Find an empty position for the restored module. It's

View file

@ -54,23 +54,22 @@ class Protocol < ApplicationRecord
validate :linked_parent_type_constrain
validates :added_by, presence: true
validates :parent, presence: true
validates :parent_updated_at, presence: true
end
with_options if: :in_repository? do
validates :name, presence: true
validates :added_by, presence: true
validates :my_module, absence: true
validates :parent_updated_at, absence: true
validate :version_number_constraint
end
with_options if: :in_repository_published_version? do
validates :parent, presence: true
validate :versions_same_name_constrain
validate :parent_type_constraint
validate :versions_same_name_constraint
end
with_options if: :in_repository_draft? do
# Only one draft can exist for each protocol
validate :ensure_single_draft
validate :versions_same_name_constrain
validate :versions_same_name_constraint
end
with_options if: -> { in_repository? && !parent } do |protocol|
# Active protocol must have unique name inside its team
@ -223,11 +222,8 @@ class Protocol < ApplicationRecord
end
def self.viewable_by_user(user, teams)
where(my_module: MyModule.viewable_by_user(user, teams))
.or(where(team: teams)
.where('protocol_type = 3 OR '\
'(protocol_type = 2 AND added_by_id = :user_id)',
user_id: user.id))
where(team: teams, protocol_type: REPOSITORY_TYPES).with_granted_permissions(user, ProtocolPermissions::READ)
.or(where(my_module: MyModule.viewable_by_user(user, teams)))
end
def self.filter_by_teams(teams = [])
@ -261,6 +257,16 @@ class Protocol < ApplicationRecord
in_repository_draft? && parent.blank?
end
def newer_published_version_present?
if in_repository_published_original?
published_versions.any?
elsif in_repository_published_version?
parent.published_versions.where('version_number > ?', version_number).any?
else
false
end
end
def latest_published_version_or_self
latest_published_version || self
end
@ -341,27 +347,12 @@ class Protocol < ApplicationRecord
unlinked? || linked?
end
def linked_no_diff?
linked? &&
updated_at == parent_updated_at &&
parent.updated_at == parent_updated_at
end
def newer_than_parent?
linked? && parent.updated_at == parent_updated_at &&
updated_at > parent_updated_at
linked? && updated_at > parent.published_on
end
def parent_newer?
linked? &&
updated_at == parent_updated_at &&
parent.updated_at > parent_updated_at
end
def parent_and_self_newer?
linked? &&
parent.updated_at > parent_updated_at &&
updated_at > parent_updated_at
linked? && parent.newer_published_version_present?
end
def number_of_steps
@ -385,9 +376,6 @@ class Protocol < ApplicationRecord
end
def make_private(user)
# Don't update "updated_at" timestamp
self.record_timestamps = false
self.added_by = user
self.published_on = nil
self.archived_by = nil
@ -488,8 +476,6 @@ class Protocol < ApplicationRecord
result = true
begin
Protocol.transaction do
self.record_timestamps = false
# First, destroy all keywords
protocol_protocol_keywords.destroy_all
if keywords.present?
@ -508,31 +494,11 @@ class Protocol < ApplicationRecord
def unlink
self.parent = nil
self.parent_updated_at = nil
self.linked_at = nil
self.protocol_type = Protocol.protocol_types[:unlinked]
save!
end
def update_parent(current_user)
ActiveRecord::Base.no_touching do
# First, destroy parent's step contents
parent.destroy_contents
parent.reload
# Now, clone step contents
Protocol.clone_contents(self, parent, current_user, false)
end
# Lastly, update the metadata
parent.reload
parent.record_timestamps = false
parent.updated_at = updated_at
parent.save!
self.record_timestamps = false
self.parent_updated_at = updated_at
save!
end
def update_from_parent(current_user, source)
ActiveRecord::Base.no_touching do
# First, destroy step contents
@ -546,7 +512,6 @@ class Protocol < ApplicationRecord
reload
self.record_timestamps = false
self.updated_at = source.published_on
self.parent_updated_at = source.published_on
self.added_by = current_user
self.last_modified_by = current_user
self.parent = source
@ -569,7 +534,6 @@ class Protocol < ApplicationRecord
self.record_timestamps = false
self.updated_at = source.published_on
self.parent = source
self.parent_updated_at = source.published_on
self.added_by = current_user
self.last_modified_by = current_user
self.linked_at = Time.zone.now
@ -627,7 +591,6 @@ class Protocol < ApplicationRecord
if linked?
clone.added_by = current_user
clone.parent = parent
clone.parent_updated_at = parent_updated_at
end
deep_clone(clone, current_user)
@ -804,19 +767,13 @@ class Protocol < ApplicationRecord
end
end
def version_parent_type_constrain
def parent_type_constraint
unless parent.in_repository_published_original?
errors.add(:base, I18n.t('activerecord.errors.models.protocol.wrong_parent_type'))
end
end
def draft_parent_type_constrain
unless parent.in_repository_published_original?
errors.add(:base, I18n.t('activerecord.errors.models.protocol.wrong_parent_type'))
end
end
def versions_same_name_constrain
def versions_same_name_constraint
if parent.present? && !parent.name.eql?(name)
errors.add(:base, I18n.t('activerecord.errors.models.protocol.wrong_version_name'))
end

View file

@ -140,8 +140,7 @@ class ProtocolSerializer < ActiveModel::Serializer
end
def update_protocol_url
return unless can_read_protocol_in_module?(object) && object.linked? &&
(object.parent_newer? || object.parent_and_self_newer?)
return unless can_read_protocol_in_module?(object) && object.linked? && object.parent_newer?
update_from_parent_modal_protocol_path(object, format: :json)
end

View file

@ -9,7 +9,7 @@
</span>
<% end %>
<a href="#" id="my-module-protocol-info-button" class="status-info
<%= 'parent-newer' if @protocol.parent_newer? || @protocol.parent_and_self_newer? %>
<%= 'parent-newer' if @protocol.parent_newer? %>
<%= 'protocol-newer' if @protocol.newer_than_parent? %>
" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
<i class="fas fa-info-circle"></i>
@ -47,7 +47,7 @@
<div class="value"><%= I18n.l(@protocol&.parent&.published_on, format: :full) %></div>
</div>
<div class="dropdown-footer">
<% if @protocol.parent_newer? || @protocol.parent_and_self_newer? %>
<% if @protocol.parent_newer? %>
<div class="notification-line new-parent-version">
<i class="fas fa-info-circle"></i>
<div class="description"><%= t("my_modules.protocols.protocol_status_bar.messages.template_updated_html") %></div>

View file

@ -1087,8 +1087,6 @@ en:
revert_title: "Revert protocol template"
revert_message: "This will override any changes you made. We cant recover them once you revert."
revert_btn_text: "Yes, revert it"
update_parent_title: "Update templates version"
update_parent_message: "Are you sure you want to update the templates protocol with this version? This will override any other changes made in the templates version."
update_self_title: "Update from protocol templates"
update_self_message: "This will override any changes you made. We cant recover them once you update this protocol with the new version."
update_self_btn_text: "Yes, update it"

View file

@ -180,23 +180,6 @@ describe ProtocolsController, type: :controller do
.to(change { Activity.count })
end
end
describe 'POST update_parent' do
let(:action) { put :update_parent, params: params, format: :json }
it 'calls create activity for updating protocol in repository from task' do
expect(Activities::CreateActivityService)
.to(receive(:call)
.with(hash_including(activity_type:
:update_protocol_in_repository_from_task)))
action
end
it 'adds activity in DB' do
expect { action }
.to(change { Activity.count })
end
end
end
describe 'POST load_from_repository' do

View file

@ -15,7 +15,6 @@ FactoryBot.define do
protocol_type { :linked }
parent { create :protocol }
added_by { create :user }
parent_updated_at { Time.now }
end
end
end

View file

@ -187,9 +187,6 @@ describe TeamImporter do
expect(db_protocol.restored_on).to be_nil
expect(db_protocol.authors).to eq json_protocol['authors']
expect(db_protocol.parent_id).to eq json_protocol['parent_id']
expect(db_protocol.parent_updated_at).to eq(
json_protocol['parent_updated_at']
)
expect(db_protocol.protocol_type).to eq(
json_protocol['protocol_type']
)