From f8a4e7ee6535d6eaabdaf7642098366776435307 Mon Sep 17 00:00:00 2001 From: Alex Kriuchykhin Date: Tue, 7 Mar 2023 13:03:01 +0100 Subject: [PATCH] Update logic for linked protocol templates with task [SCI-7973] (#5058) --- app/controllers/protocols_controller.rb | 65 --------------- app/models/my_module.rb | 8 -- app/models/protocol.rb | 83 +++++-------------- app/serializers/protocol_serializer.rb | 3 +- .../protocols/_protocol_status_bar.html.erb | 4 +- config/locales/en.yml | 2 - spec/controllers/protocols_controller_spec.rb | 17 ---- spec/factories/protocols.rb | 1 - .../model_importers/team_importer_spec.rb | 3 - 9 files changed, 23 insertions(+), 163 deletions(-) diff --git a/app/controllers/protocols_controller.rb b/app/controllers/protocols_controller.rb index eb1eda20e..80c6110e7 100644 --- a/app/controllers/protocols_controller.rb +++ b/app/controllers/protocols_controller.rb @@ -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| diff --git a/app/models/my_module.rb b/app/models/my_module.rb index ed883bf4e..829c21d9b 100644 --- a/app/models/my_module.rb +++ b/app/models/my_module.rb @@ -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 diff --git a/app/models/protocol.rb b/app/models/protocol.rb index 94a39c27b..dd252bc68 100644 --- a/app/models/protocol.rb +++ b/app/models/protocol.rb @@ -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 diff --git a/app/serializers/protocol_serializer.rb b/app/serializers/protocol_serializer.rb index 38d0fd0d6..e8350eed2 100644 --- a/app/serializers/protocol_serializer.rb +++ b/app/serializers/protocol_serializer.rb @@ -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 diff --git a/app/views/my_modules/protocols/_protocol_status_bar.html.erb b/app/views/my_modules/protocols/_protocol_status_bar.html.erb index a933316e0..1cb450313 100644 --- a/app/views/my_modules/protocols/_protocol_status_bar.html.erb +++ b/app/views/my_modules/protocols/_protocol_status_bar.html.erb @@ -9,7 +9,7 @@ <% end %>