From 9908b31d1128ea1b63d4b3e8478ef54e3d20fff2 Mon Sep 17 00:00:00 2001 From: Alex Kriuchykhin Date: Wed, 1 Mar 2023 15:04:53 +0100 Subject: [PATCH] Fix protocols datatable [SCI-7984] (#5046) --- app/controllers/protocols_controller.rb | 2 +- app/datatables/protocols_datatable.rb | 143 ++++++++++-------- app/models/protocol.rb | 32 ++-- .../index/_protocol_versions.html.erb | 16 +- 4 files changed, 112 insertions(+), 81 deletions(-) diff --git a/app/controllers/protocols_controller.rb b/app/controllers/protocols_controller.rb index b27525c2c..181e76a37 100644 --- a/app/controllers/protocols_controller.rb +++ b/app/controllers/protocols_controller.rb @@ -123,7 +123,7 @@ class ProtocolsController < ApplicationController return render_403 unless @protocol.in_repository_published_original? || (@protocol.in_repository_draft? && @protocol.parent_id.blank?) - @published_versions = @protocol.published_versions.order(version_number: :desc) + @published_versions = @protocol.published_versions_with_original.order(version_number: :desc) render json: { html: render_to_string(partial: 'protocols/index/protocol_versions_modal.html.erb') } diff --git a/app/datatables/protocols_datatable.rb b/app/datatables/protocols_datatable.rb index ff9765aa7..8cfb621a0 100644 --- a/app/datatables/protocols_datatable.rb +++ b/app/datatables/protocols_datatable.rb @@ -1,9 +1,13 @@ +# frozen_string_literal: true + class ProtocolsDatatable < CustomDatatable # Needed for sanitize_sql_like method include ActiveRecord::Sanitization::ClassMethods include InputSanitizeHelper include Rails.application.routes.url_helpers + PREFIXED_ID_SQL = "('#{Protocol::ID_PREFIX}' || COALESCE(\"protocols\".\"parent_id\", \"protocols\".\"id\"))" + def_delegator :@view, :can_read_protocol_in_repository? def_delegator :@view, :can_manage_protocol_in_repository? def_delegator :@view, :edit_protocol_path @@ -16,42 +20,38 @@ class ProtocolsDatatable < CustomDatatable def initialize(view, team, type, user) super(view) @team = team - # :public, :private or :archive - @type = type + @type = type # :active or :archived @user = user end def sortable_columns @sortable_columns ||= [ - 'Protocol.name', - 'Protocol.id', + 'name', + 'adjusted_parent_id', 'nr_of_versions', 'protocol_keywords_str', - 'Protocol.nr_of_linked_children', + 'nr_of_linked_children', 'nr_of_assigned_users', 'full_username_str', - 'Protocol.published_on', - 'Protocol.updated_at', + 'published_on', + 'updated_at', 'archived_full_username_str', - 'Protocol.archived_on' + 'archived_on' ] end def searchable_columns @searchable_columns ||= [ - "Protocol.name", + 'Protocol.name', 'Protocol.archived_on', 'Protocol.published_on', - "Protocol.#{Protocol::PREFIXED_ID_SQL}", - "Protocol.updated_at" + "Protocol.#{PREFIXED_ID_SQL}", + 'Protocol.updated_at', + 'ProtocolKeyword.name' ] end - # This hack is needed to display a correct amount of - # searched entries (needed for pagination). - # This is needed because of usage of GROUP operator in SQL. - # See https://github.com/antillas21/ajax-datatables-rails/issues/112 - def as_json(options = {}) + def as_json(_options = {}) { draw: dt_params[:draw].to_i, recordsTotal: get_raw_records.length, @@ -67,8 +67,8 @@ class ProtocolsDatatable < CustomDatatable model, column = column.split('.', 2) model = model.constantize case column - when Protocol::PREFIXED_ID_SQL - casted_column = ::Arel::Nodes::SqlLiteral.new(Protocol::PREFIXED_ID_SQL) + when PREFIXED_ID_SQL + casted_column = ::Arel::Nodes::SqlLiteral.new(PREFIXED_ID_SQL) when 'published_on' casted_column = ::Arel::Nodes::NamedFunction.new('CAST', [ Arel.sql("to_char( protocols.created_at, '#{ formated_date }' ) AS VARCHAR") ] ) @@ -87,20 +87,21 @@ class ProtocolsDatatable < CustomDatatable # Returns json of current protocols (already paginated) def data records.map do |record| + parent = record.parent || record { DT_RowId: record.id, DT_RowAttr: { - 'data-permissions-url': permissions_protocol_path(record) + 'data-permissions-url': permissions_protocol_path(parent) }, - '1': name_html(record), - '2': record.code, + '1': name_html(parent), + '2': parent.code, '3': versions_html(record), '4': keywords_html(record), '5': modules_html(record), - '6': access_html(record), - '7': escape_input(record.full_username_str), - '8': I18n.l(record.published_on || record.created_at, format: :full), - '9': I18n.l(record.updated_at, format: :full), + '6': access_html(parent), + '7': published_by(record), + '8': published_timestamp(record), + '9': modified_timestamp(record), '10': escape_input(record.archived_full_username_str), '11': (I18n.l(record.archived_on, format: :full) if record.archived_on) } @@ -136,27 +137,37 @@ class ProtocolsDatatable < CustomDatatable end def get_raw_records_base + original_without_versions = @team.protocols + .left_outer_joins(:published_versions) + .where(protocol_type: Protocol.protocol_types[:in_repository_published_original]) + .where(published_versions: { id: nil }) + .select(:id) + published_versions = @team.protocols + .where(protocol_type: Protocol.protocol_types[:in_repository_published_version]) + .select('DISTINCT ON (parent_id) id') + new_drafts = @team.protocols + .where(protocol_type: Protocol.protocol_types[:in_repository_draft], parent_id: nil) + .select(:id) + + records = Protocol.where( + "protocols.id IN ((#{original_without_versions.to_sql}) " \ + "UNION " \ + "(#{published_versions.to_sql}) " \ + "UNION " \ + "(#{new_drafts.to_sql}))" + ) + records = - Protocol - .where(team: @team) - .where('protocols.protocol_type = ? OR protocols.protocol_type = ? AND protocols.parent_id IS NULL', - Protocol.protocol_types[:in_repository_published_original], - Protocol.protocol_types[:in_repository_draft]) - .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 protocols protocol_versions "\ - "ON protocol_versions.protocol_type = #{Protocol.protocol_types[:in_repository_published_version]} "\ - "AND protocol_versions.parent_id = protocols.id") - .joins("LEFT OUTER JOIN protocols protocol_drafts "\ - "ON protocol_drafts.protocol_type = #{Protocol.protocol_types[:in_repository_draft]} "\ - "AND protocol_drafts.parent_id = protocols.id") - .joins('LEFT OUTER JOIN "protocol_protocol_keywords" '\ + records + .preload(:parent, :latest_published_version, :draft, :protocol_keywords, user_assignments: %i(user user_role)) + .joins("LEFT OUTER JOIN protocols protocol_versions " \ + "ON protocol_versions.protocol_type = #{Protocol.protocol_types[:in_repository_published_version]} " \ + "AND protocol_versions.parent_id = protocols.parent_id") + .joins('LEFT OUTER JOIN "protocol_protocol_keywords" ' \ 'ON "protocol_protocol_keywords"."protocol_id" = "protocols"."id"') - .joins('LEFT OUTER JOIN "protocol_keywords" '\ + .joins('LEFT OUTER JOIN "protocol_keywords" ' \ 'ON "protocol_protocol_keywords"."protocol_keyword_id" = "protocol_keywords"."id"') .with_granted_permissions(@user, ProtocolPermissions::READ) - .preload(user_assignments: %i(user user_role)) records = records.joins('LEFT OUTER JOIN "users" "archived_users" ON "archived_users"."id" = "protocols"."archived_by_id"') @@ -173,9 +184,11 @@ class ProtocolsDatatable < CustomDatatable def get_raw_records get_raw_records_base.select( '"protocols".*', + 'COALESCE("protocols"."parent_id", "protocols"."id") AS adjusted_parent_id', 'STRING_AGG(DISTINCT("protocol_keywords"."name"), \', \') AS "protocol_keywords_str"', - 'COUNT(DISTINCT("protocol_versions"."id")) + 1 AS "nr_of_versions"', # User assignments generate duplicates - 'COUNT("protocol_drafts"."id") AS "nr_of_drafts"', + "CASE WHEN protocols.protocol_type = #{Protocol.protocol_types[:in_repository_draft]}" \ + "THEN COUNT(DISTINCT(\"protocol_versions\".\"id\")) ELSE COUNT(DISTINCT(\"protocol_versions\".\"id\")) + 1 " \ + "END AS nr_of_versions", 'COUNT("user_assignments"."id") AS "nr_of_assigned_users"', 'MAX("users"."full_name") AS "full_username_str"', # "Hack" to get single username 'MAX("archived_users"."full_name") AS "archived_full_username_str"' @@ -185,19 +198,23 @@ class ProtocolsDatatable < CustomDatatable # Various helper methods def name_html(record) - "#{escape_input(record.name)}" + path = + if record.in_repository_published_original? && record.latest_published_version.present? + protocol_path(record.latest_published_version) + else + protocol_path(record) + end + "#{escape_input(record.name)}" end def keywords_html(record) - if !record.protocol_keywords_str || record.protocol_keywords_str.blank? - "#{I18n.t("protocols.no_keywords")}" + if record.protocol_keywords.blank? + "#{I18n.t('protocols.no_keywords')}" else - kws = record.protocol_keywords_str.split(", ") res = [] - kws.sort_by{ |word| word.downcase }.each do |kw| - sanitized_kw = sanitize_input(kw) - res << "#{sanitized_kw}" + record.protocol_keywords.sort_by { |kw| kw.name.downcase }.each do |kw| + sanitized_kw = sanitize_input(kw.name) + res << "#{sanitized_kw}" end res.join(', ') end @@ -206,7 +223,7 @@ class ProtocolsDatatable < CustomDatatable def modules_html(record) "" \ - "#{record.nr_of_linked_children}" \ + "#{record.nr_of_linked_children}" \ "" end @@ -219,12 +236,20 @@ class ProtocolsDatatable < CustomDatatable @view.controller.render_to_string(partial: 'protocols/index/protocol_access.html.erb', locals: { protocol: record }) end - def timestamp_column_html(record) - if @type == :archived - I18n.l(record.archived_on, format: :full) - else - I18n.l(record.published_on || record.created_at, format: :full) - end + def published_by(record) + return '' if record.published_by.blank? + + escape_input(record.published_by.full_name) + end + + def published_timestamp(record) + return '' if record.published_on.blank? + + I18n.l(record.published_on, format: :full) + end + + def modified_timestamp(record) + I18n.l(record.updated_at, format: :full) end # OVERRIDE - This is only called when filtering results; diff --git a/app/models/protocol.rb b/app/models/protocol.rb index e6e7c03c7..f5e382888 100644 --- a/app/models/protocol.rb +++ b/app/models/protocol.rb @@ -129,6 +129,21 @@ class Protocol < ApplicationRecord foreign_key: 'previous_version_id', inverse_of: :previous_version, dependent: :destroy + has_one :latest_published_version, + lambda { + in_repository_published_version.select('DISTINCT ON (parent_id) *') + .order(:parent_id, version_number: :desc) + }, + class_name: 'Protocol', + foreign_key: 'parent_id' + has_one :draft, + -> { in_repository_draft.select('DISTINCT ON (parent_id) *').order(:parent_id) }, + class_name: 'Protocol', + foreign_key: 'parent_id' + has_many :published_versions, + -> { in_repository_published_version }, + class_name: 'Protocol', + foreign_key: 'parent_id' has_many :protocol_protocol_keywords, inverse_of: :protocol, dependent: :destroy @@ -213,7 +228,6 @@ class Protocol < ApplicationRecord user_id: user.id)) end - def self.filter_by_teams(teams = []) teams.blank? ? self : where(team: teams) end @@ -234,27 +248,13 @@ class Protocol < ApplicationRecord in_module? ? my_module.created_by : added_by end - def draft - return self if in_repository_draft? && parent_id.blank? - - return nil unless in_repository_published_original? - - team.protocols.in_repository_draft.find_by(parent: self) - end - - def published_versions - return self.class.none unless in_repository_published_original? - + def published_versions_with_original team.protocols .in_repository_published_version .where(parent: self) .or(team.protocols.in_repository_published_original.where(id: id)) end - def latest_published_version - published_versions.order(version_number: :desc).first - end - def permission_parent in_module? ? my_module : team end diff --git a/app/views/protocols/index/_protocol_versions.html.erb b/app/views/protocols/index/_protocol_versions.html.erb index ef6071615..96915d131 100644 --- a/app/views/protocols/index/_protocol_versions.html.erb +++ b/app/views/protocols/index/_protocol_versions.html.erb @@ -1,10 +1,16 @@ -<%= link_to versions_modal_protocol_path(protocol), class: 'protocol-versions-link', remote: true do %> - <% if protocol.in_repository_published_original? %> +<% if protocol.in_repository_published_original? || protocol.in_repository_published_version? %> + <% parent = protocol.parent || protocol %> + <%= link_to versions_modal_protocol_path(parent), class: 'protocol-versions-link', remote: true do %> <%= protocol.nr_of_versions %> - <% if protocol.nr_of_drafts.positive? %> - / <%= t("protocols.index.table.draft") %> + <% end %> + <% if protocol.draft.present? %> + / + <%= link_to protocol_path(parent.draft) do %> + <%= t("protocols.index.table.draft") %> <% end %> - <% elsif protocol.in_repository_draft? %> + <% end %> +<% elsif protocol.in_repository_draft? %> + <%= link_to protocol_path(protocol) do %> <%= t("protocols.index.table.draft") %> <% end %> <% end %>