From cf599a82775d7e63b9a82d7fb21ceb26e8a1a016 Mon Sep 17 00:00:00 2001 From: Oleksii Kriuchykhin Date: Tue, 21 Mar 2023 10:59:31 +0100 Subject: [PATCH] Fix visibility of save as draft button, fix permissions for new drafts [SCI-8145][SCI-8157] --- app/controllers/protocols_controller.rb | 5 +- ...oad_from_repository_protocols_datatable.rb | 2 +- app/datatables/protocols_datatable.rb | 6 +- .../vue/protocol/protocolMetadata.vue | 3 +- .../protocol_group_assignment_job.rb | 28 --------- app/models/protocol.rb | 61 ++++++------------- app/permissions/team.rb | 9 ++- app/serializers/protocol_serializer.rb | 13 ++-- .../_linked_children_modal_body.html.erb | 3 +- .../index/_protocol_versions_modal.html.erb | 4 +- 10 files changed, 40 insertions(+), 94 deletions(-) delete mode 100644 app/jobs/user_assignments/protocol_group_assignment_job.rb diff --git a/app/controllers/protocols_controller.rb b/app/controllers/protocols_controller.rb index a925565dc..5d9677249 100644 --- a/app/controllers/protocols_controller.rb +++ b/app/controllers/protocols_controller.rb @@ -111,7 +111,7 @@ class ProtocolsController < ApplicationController title: I18n.t('protocols.index.linked_children.title', protocol: escape_input(@protocol.name)), html: render_to_string(partial: 'protocols/index/linked_children_modal_body.html.erb', - locals: { protocol: @protocol }) + locals: { protocol: @protocol }) } end end @@ -1016,8 +1016,7 @@ class ProtocolsController < ApplicationController def check_save_as_draft_permissions @protocol = Protocol.find_by(id: params[:id]) - render_403 unless @protocol.present? && - can_save_protocol_as_draft_in_repository?(@protocol) + render_403 unless @protocol.present? && can_save_protocol_version_as_draft?(@protocol) end def check_delete_draft_permissions diff --git a/app/datatables/load_from_repository_protocols_datatable.rb b/app/datatables/load_from_repository_protocols_datatable.rb index da8f7bd55..12a3f38f5 100644 --- a/app/datatables/load_from_repository_protocols_datatable.rb +++ b/app/datatables/load_from_repository_protocols_datatable.rb @@ -53,7 +53,7 @@ class LoadFromRepositoryProtocolsDatatable < CustomDatatable '1': record.version_number, '2': parent.code, '3': keywords_html(record), - '4': escape_input(record.published_by.full_name), + '4': escape_input(record.published_by&.full_name), '5': I18n.l(record.published_on, format: :full) } end diff --git a/app/datatables/protocols_datatable.rb b/app/datatables/protocols_datatable.rb index df54cf35d..ab784c756 100644 --- a/app/datatables/protocols_datatable.rb +++ b/app/datatables/protocols_datatable.rb @@ -183,9 +183,9 @@ class ProtocolsDatatable < CustomDatatable 'ON "protocol_protocol_keywords"."protocol_keyword_id" = "protocol_keywords"."id"') .joins('LEFT OUTER JOIN "users" "archived_users" ON "archived_users"."id" = "protocols"."archived_by_id"') .joins('LEFT OUTER JOIN "users" ON "users"."id" = "protocols"."published_by_id"') - .joins('LEFT OUTER JOIN "user_assignments" "all_user_assignments" '\ - 'ON "all_user_assignments"."assignable_type" = \'Protocol\' '\ - 'AND "all_user_assignments"."assignable_id" = "protocols"."id"') + .joins('LEFT OUTER JOIN "user_assignments" "all_user_assignments" ' \ + 'ON "all_user_assignments"."assignable_type" = \'Protocol\' ' \ + 'AND "all_user_assignments"."assignable_id" = "protocols"."id"') .group('"protocols"."id"') records = filter_protocols_records(records) diff --git a/app/javascript/vue/protocol/protocolMetadata.vue b/app/javascript/vue/protocol/protocolMetadata.vue index 01afab5a4..8ff0b688e 100644 --- a/app/javascript/vue/protocol/protocolMetadata.vue +++ b/app/javascript/vue/protocol/protocolMetadata.vue @@ -18,8 +18,7 @@ - - +
diff --git a/app/jobs/user_assignments/protocol_group_assignment_job.rb b/app/jobs/user_assignments/protocol_group_assignment_job.rb deleted file mode 100644 index ab12a7b32..000000000 --- a/app/jobs/user_assignments/protocol_group_assignment_job.rb +++ /dev/null @@ -1,28 +0,0 @@ -# frozen_string_literal: true - -module UserAssignments - class ProtocolGroupAssignmentJob < ApplicationJob - queue_as :high_priority - - def perform(team, protocol, assigned_by) - @team = team - @assigned_by = assigned_by - - ActiveRecord::Base.transaction do - team.users.where.not(id: assigned_by).find_each do |user| - user_assignment = UserAssignment.find_or_initialize_by( - user: user, - assignable: protocol - ) - - next if user_assignment.manually_assigned? - - user_assignment.update!( - user_role: protocol.default_public_user_role || UserRole.find_predefined_viewer_role, - assigned_by: @assigned_by - ) - end - end - end - end -end diff --git a/app/models/protocol.rb b/app/models/protocol.rb index b6f5e67fa..890335247 100644 --- a/app/models/protocol.rb +++ b/app/models/protocol.rb @@ -16,13 +16,10 @@ class Protocol < ApplicationRecord include PermissionCheckableModel include TinyMceImages - after_create :auto_assign_protocol_members, if: :visible? - after_destroy :decrement_linked_children - after_save :update_user_assignments, if: -> { saved_change_to_visibility? && in_repository? } - after_save :update_linked_children - skip_callback :create, :after, :create_users_assignments, if: -> { in_module? } + after_create :update_automatic_user_assignments, if: -> { visible? && in_repository? && parent.blank? } before_update :change_visibility, if: :default_public_user_role_id_changed? - after_update :sync_protocol_assignments, if: :visibility_changed? + after_update :update_automatic_user_assignments, if: -> { saved_change_to_visibility? && in_repository? } + skip_callback :create, :after, :create_users_assignments, if: -> { in_module? } enum visibility: { hidden: 0, visible: 1 } enum protocol_type: { @@ -238,13 +235,23 @@ class Protocol < ApplicationRecord in_module? ? my_module.created_by : added_by end + # Only for original published protocol def published_versions_with_original + return Protocol.none unless in_repository_published_original? + team.protocols .in_repository_published_version .where(parent: self) .or(team.protocols.in_repository_published_original.where(id: id)) end + # Only for original published protocol + def all_linked_children + return Protocol.none unless in_repository_published_original? + + Protocol.linked.where(parent: published_versions_with_original) + end + def initial_draft? in_repository_draft? && parent.blank? end @@ -539,6 +546,7 @@ class Protocol < ApplicationRecord draft.version_comment = nil draft.previous_version = self draft.last_modified_by = current_user + draft.skip_user_assignments = true return draft if draft.invalid? @@ -546,6 +554,10 @@ class Protocol < ApplicationRecord draft = deep_clone(draft, current_user) end + parent_protocol.user_assignments.each do |parent_user_assignment| + parent_protocol.sync_child_protocol_user_assignment(parent_user_assignment, draft.id) + end + draft end @@ -671,15 +683,7 @@ class Protocol < ApplicationRecord sync_child_protocol_user_assignment(user_assignment) end - def auto_assign_protocol_members - UserAssignments::ProtocolGroupAssignmentJob.perform_now( - team, - self, - last_modified_by || created_by - ) - end - - def update_user_assignments + def update_automatic_user_assignments case visibility when 'visible' create_public_user_assignments!(added_by) @@ -706,25 +710,6 @@ class Protocol < ApplicationRecord clone end - def update_linked_children - # Increment/decrement the parent's nr of linked children - if saved_change_to_parent_id? - unless parent_id_before_last_save.nil? - p = Protocol.find_by_id(parent_id_before_last_save) - p.record_timestamps = false - p.decrement!(:nr_of_linked_children) - end - unless parent_id.nil? - parent.record_timestamps = false - parent.increment!(:nr_of_linked_children) - end - end - end - - def decrement_linked_children - parent.decrement!(:nr_of_linked_children) if parent.present? - end - def prevent_update errors.add(:base, I18n.t('activerecord.errors.models.protocol.unchangable')) end @@ -765,12 +750,4 @@ class Protocol < ApplicationRecord def change_visibility self.visibility = default_public_user_role_id.present? ? :visible : :hidden end - - def sync_protocol_assignments - if visible? - auto_assign_protocol_members - else - user_assignments.where(assigned: :automatically).destroy_all - end - end end diff --git a/app/permissions/team.rb b/app/permissions/team.rb index c0a26ed99..524b3ac5f 100644 --- a/app/permissions/team.rb +++ b/app/permissions/team.rb @@ -76,7 +76,7 @@ Canaid::Permissions.register_for(Protocol) do clone_protocol_in_repository publish_protocol_in_repository delete_protocol_draft_in_repository - save_protocol_as_draft_in_repository) + save_protocol_version_as_draft) .each do |perm| can perm do |_, protocol| protocol.active? @@ -129,11 +129,10 @@ Canaid::Permissions.register_for(Protocol) do can_manage_protocol_draft_in_repository?(user, protocol) end - can :save_protocol_as_draft_in_repository do |user, protocol| - next false unless can_create_protocols_in_repository?(user, protocol.team) + can :save_protocol_version_as_draft do |user, protocol| + next false unless protocol.in_repository_published? - %(in_repository_published_original in_repository_published_version).include?(protocol.protocol_type) && - (protocol.parent || protocol).draft.blank? + protocol.permission_granted?(user, ProtocolPermissions::MANAGE_DRAFT) end end diff --git a/app/serializers/protocol_serializer.rb b/app/serializers/protocol_serializer.rb index 52c21f4b4..c4f058c0e 100644 --- a/app/serializers/protocol_serializer.rb +++ b/app/serializers/protocol_serializer.rb @@ -9,7 +9,7 @@ class ProtocolSerializer < ActiveModel::Serializer attributes :name, :id, :urls, :description, :description_view, :updated_at, :in_repository, :created_at_formatted, :updated_at_formatted, :added_by, :authors, :keywords, :version, :code, - :published, :version_comment, :archived, :linked, :disabled_drafting + :published, :version_comment, :archived, :linked, :has_draft def updated_at object.updated_at.to_i @@ -85,12 +85,13 @@ class ProtocolSerializer < ActiveModel::Serializer !object.in_module? end - def disabled_drafting - return false unless can_create_protocols_in_repository?(object.team) + # rubocop:disable Naming/PredicateName + def has_draft + return false unless object.in_repository_published? - %(in_repository_published_original in_repository_published_version).include?(object.protocol_type) && - (object.parent || object).draft.present? && !object.archived + (object.parent || object).draft.present? end + # rubocop:enable Naming/PredicateName def linked object.linked? @@ -201,7 +202,7 @@ class ProtocolSerializer < ActiveModel::Serializer end def save_as_draft_url - return unless can_save_protocol_as_draft_in_repository?(object) + return unless can_save_protocol_version_as_draft?(object) save_as_draft_protocol_path(object) end diff --git a/app/views/protocols/index/_linked_children_modal_body.html.erb b/app/views/protocols/index/_linked_children_modal_body.html.erb index bd0675aeb..9671fd3fc 100644 --- a/app/views/protocols/index/_linked_children_modal_body.html.erb +++ b/app/views/protocols/index/_linked_children_modal_body.html.erb @@ -1,7 +1,6 @@ -<% linked_children = protocol.linked_children %> <% published_versions = protocol.published_versions_with_original.order(version_number: :desc)%> -<% if protocol.nr_of_linked_children > 0 %> +<% if protocol.all_linked_children.any? %>
<%= label_tag "version-selector".to_sym, t("protocols.index.linked_children.show_version") %> <%= select_tag "version", diff --git a/app/views/protocols/index/_protocol_versions_modal.html.erb b/app/views/protocols/index/_protocol_versions_modal.html.erb index ee7060128..a21e4ae10 100644 --- a/app/views/protocols/index/_protocol_versions_modal.html.erb +++ b/app/views/protocols/index/_protocol_versions_modal.html.erb @@ -82,11 +82,11 @@ published_by: protocol_version.published_by&.full_name) %>
- <% if @protocol.active? %> + <% if can_save_protocol_version_as_draft?(@protocol) %> <%= button_to save_as_draft_protocol_path(protocol_version), class: "btn btn-light icon-btn save-as-draft", title: t('protocols.index.versions.save_as_draft'), - disabled: !can_save_protocol_as_draft_in_repository?(protocol_version), + disabled: @protocol.draft.present?, data: { placement: :left, toggle: :tooltip