Merge pull request #1420 from okriuchykhin/ok_SCI_2894

Replace reports materialized view with normal queries and re-enable fragment caching [SCI-2894]
This commit is contained in:
Alex Kriuchykhin 2018-12-16 19:44:50 +01:00 committed by GitHub
commit abdf64a8c7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
28 changed files with 73 additions and 290 deletions

View file

@ -40,7 +40,7 @@ class ReportsController < ApplicationController
render json: ::ReportDatatable.new(
view_context,
current_user,
current_team.datatables_reports.visible_by(current_user, current_team)
Report.visible_by(current_user, current_team)
)
end
end

View file

@ -4,12 +4,12 @@ class ReportDatatable < CustomDatatable
include InputSanitizeHelper
TABLE_COLUMNS = %w(
Views::Datatables::DatatablesReport.project_name
Views::Datatables::DatatablesReport.name
Views::Datatables::DatatablesReport.created_by
Views::Datatables::DatatablesReport.last_modified_by
Views::Datatables::DatatablesReport.created_at
Views::Datatables::DatatablesReport.updated_at
Report.project_name
Report.name
Report.created_by
Report.modified_by
Report.created_at
Report.updated_at
).freeze
def_delegator :@view, :edit_project_report_path
@ -32,20 +32,32 @@ class ReportDatatable < CustomDatatable
def data
records.map do |record|
{
'0' => record.id,
'1' => sanitize_input(record.project_name),
'2' => sanitize_input(record.name),
'3' => sanitize_input(record.created_by),
'4' => sanitize_input(record.last_modified_by),
'5' => I18n.l(record.created_at, format: :full),
'6' => I18n.l(record.updated_at, format: :full),
'0' => record.id,
'1' => sanitize_input(record.project_name),
'2' => sanitize_input(record.name),
'3' => sanitize_input(record.created_by),
'4' => sanitize_input(record.modified_by),
'5' => I18n.l(record.created_at, format: :full),
'6' => I18n.l(record.updated_at, format: :full),
'edit' => edit_project_report_path(record.project_id, record.id)
}
end
end
def get_raw_records
@reports
res = @reports.joins(:project)
.joins(
'LEFT OUTER JOIN users AS creators ' \
'ON reports.user_id = creators.id'
).joins(
'LEFT OUTER JOIN users AS modifiers '\
'ON reports.last_modified_by_id = modifiers.id'
)
.select('reports.* AS reports')
.select('projects.name AS project_name')
.select('creators.full_name AS created_by')
.select('modifiers.full_name AS modified_by')
Report.from(res, :reports)
end
# ==== Insert 'presenter'-like methods below if necessary

View file

@ -7,7 +7,7 @@ class Checklist < ApplicationRecord
length: { maximum: Constants::TEXT_MAX_LENGTH }
validates :step, presence: true
belongs_to :step, inverse_of: :checklists, optional: true
belongs_to :step, inverse_of: :checklists, touch: true, optional: true
belongs_to :created_by,
foreign_key: 'created_by_id',
class_name: 'User',

View file

@ -2,7 +2,7 @@ class Experiment < ApplicationRecord
include ArchivableModel
include SearchableModel
belongs_to :project, inverse_of: :experiments, optional: true
belongs_to :project, inverse_of: :experiments, touch: true, optional: true
belongs_to :created_by,
foreign_key: :created_by_id,
class_name: 'User',

View file

@ -31,7 +31,7 @@ class MyModule < ApplicationRecord
foreign_key: 'restored_by_id',
class_name: 'User',
optional: true
belongs_to :experiment, inverse_of: :my_modules, optional: true
belongs_to :experiment, inverse_of: :my_modules, touch: true, optional: true
belongs_to :my_module_group, inverse_of: :my_modules, optional: true
has_many :results, inverse_of: :my_module, dependent: :destroy
has_many :my_module_tags, inverse_of: :my_module, dependent: :destroy

View file

@ -49,10 +49,6 @@ class Project < ApplicationRecord
end
end)
after_commit do
Views::Datatables::DatatablesReport.refresh_materialized_view
end
def self.visible_from_user_by_name(user, team, name)
if user.is_admin_of_team? team
return where('projects.archived IS FALSE AND projects.name ILIKE ?',

View file

@ -2,6 +2,7 @@ class ProjectComment < Comment
belongs_to :project,
foreign_key: :associated_id,
inverse_of: :project_comments,
touch: true,
optional: true
validates :project, presence: true

View file

@ -22,10 +22,6 @@ class Report < ApplicationRecord
# or many module elements (if grouped by module)
has_many :report_elements, inverse_of: :report, dependent: :delete_all
after_commit do
Views::Datatables::DatatablesReport.refresh_materialized_view
end
def self.search(
user,
include_archived,
@ -56,6 +52,20 @@ class Report < ApplicationRecord
end
end
def self.visible_by(user, team)
projects = team.projects.joins(
'LEFT OUTER JOIN user_projects ON user_projects.project_id = projects.id'
).where(archived: false)
# Only admins see all projects of the team
unless user.is_admin_of_team?(team)
projects = projects.where(
'visibility = 1 OR user_projects.user_id = :user_id', user_id: user.id
)
end
where(project: projects)
end
def root_elements
(report_elements.order(:position)).select { |el| el.parent.blank? }
end

View file

@ -1,7 +1,7 @@
class ResultAsset < ApplicationRecord
validates :result, :asset, presence: true
belongs_to :result, inverse_of: :result_asset, optional: true
belongs_to :result, inverse_of: :result_asset, touch: true, optional: true
belongs_to :asset,
inverse_of: :result_asset,
dependent: :destroy,

View file

@ -2,6 +2,7 @@ class ResultComment < Comment
belongs_to :result,
foreign_key: :associated_id,
inverse_of: :result_comments,
touch: true,
optional: true
validates :result, presence: true

View file

@ -5,5 +5,6 @@ class ResultTable < ApplicationRecord
belongs_to :table,
inverse_of: :result_table,
dependent: :destroy,
touch: true,
optional: true
end

View file

@ -4,6 +4,6 @@ class ResultText < ApplicationRecord
presence: true,
length: { maximum: Constants::RICH_TEXT_MAX_LENGTH }
validates :result, presence: true
belongs_to :result, inverse_of: :result_text, optional: true
belongs_to :result, inverse_of: :result_text, touch: true, optional: true
has_many :tiny_mce_assets, inverse_of: :result_text, dependent: :destroy
end

View file

@ -3,6 +3,7 @@ class StepAsset < ApplicationRecord
belongs_to :step,
inverse_of: :step_assets,
touch: true,
optional: true
belongs_to :asset,
inverse_of: :step_asset,

View file

@ -2,6 +2,7 @@ class StepComment < Comment
belongs_to :step,
foreign_key: :associated_id,
inverse_of: :step_comments,
touch: true,
optional: true
validates :step, presence: true

View file

@ -1,6 +1,6 @@
class StepTable < ApplicationRecord
validates :step, :table, presence: true
belongs_to :step, inverse_of: :step_tables, optional: true
belongs_to :step, inverse_of: :step_tables, touch: true, optional: true
belongs_to :table, inverse_of: :step_table, optional: true
end

View file

@ -34,12 +34,6 @@ class Team < ApplicationRecord
has_many :tiny_mce_assets, inverse_of: :team, dependent: :destroy
has_many :repositories, dependent: :destroy
has_many :reports, inverse_of: :team, dependent: :destroy
has_many :datatables_reports,
class_name: 'Views::Datatables::DatatablesReport'
after_commit do
Views::Datatables::DatatablesReport.refresh_materialized_view
end
def default_view_state
{ 'projects' =>

View file

@ -5,9 +5,10 @@ class TinyMceAsset < ApplicationRecord
after_destroy :release_team_space
belongs_to :team, inverse_of: :tiny_mce_assets, optional: true
belongs_to :step, inverse_of: :tiny_mce_assets, optional: true
belongs_to :step, inverse_of: :tiny_mce_assets, touch: true, optional: true
belongs_to :result_text,
inverse_of: :tiny_mce_assets,
touch: true,
optional: true
has_attached_file :image,
styles: { large: [Constants::LARGE_PIC_FORMAT, :jpg] },

View file

@ -2,11 +2,12 @@ class UserMyModule < ApplicationRecord
validates :user, presence: true, uniqueness: { scope: :my_module }
validates :my_module, presence: true
belongs_to :user, inverse_of: :user_my_modules, optional: true
belongs_to :user, inverse_of: :user_my_modules, touch: true, optional: true
belongs_to :assigned_by,
foreign_key: 'assigned_by_id',
class_name: 'User',
optional: true
belongs_to :my_module, inverse_of: :user_my_modules,
touch: true,
optional: true
end

View file

@ -5,19 +5,15 @@ class UserProject < ApplicationRecord
validates :user, presence: true, uniqueness: { scope: :project }
validates :project, presence: true
belongs_to :user, inverse_of: :user_projects, optional: true
belongs_to :user, inverse_of: :user_projects, touch: true, optional: true
belongs_to :assigned_by,
foreign_key: 'assigned_by_id',
class_name: 'User',
optional: true
belongs_to :project, inverse_of: :user_projects, optional: true
belongs_to :project, inverse_of: :user_projects, touch: true, optional: true
before_destroy :destroy_associations
after_commit do
Views::Datatables::DatatablesReport.refresh_materialized_view
end
def role_str
I18n.t("user_projects.enums.role.#{role.to_s}")
end

View file

@ -5,7 +5,7 @@ class UserTeam < ApplicationRecord
validates :user, presence: true
validates :team, presence: true
belongs_to :user, inverse_of: :user_teams, optional: true
belongs_to :user, inverse_of: :user_teams, touch: true, optional: true
belongs_to :assigned_by,
foreign_key: 'assigned_by_id',
class_name: 'User',
@ -15,10 +15,6 @@ class UserTeam < ApplicationRecord
before_destroy :destroy_associations
after_create :create_samples_table_state
after_commit do
Views::Datatables::DatatablesReport.refresh_materialized_view
end
def role_str
I18n.t("user_teams.enums.role.#{role}")
end

View file

@ -1,83 +0,0 @@
# frozen_string_literal: true
module Views
module Datatables
class DatatablesReport < ApplicationRecord
belongs_to :team
# this isn't strictly necessary, but it will prevent
# rails from calling save, which would fail anyway.
def readonly?
true
end
class << self
def visible_by(user, team)
permitted_by_team = get_permitted_by_team_tokenized
permitted_by_project = get_permitted_by_project_tokenized
if user.is_admin_of_team? team
allowed_ids = for_admin(user, permitted_by_team)
else
allowed_ids = for_non_admin(
user, permitted_by_team, permitted_by_project
)
end
where(id: allowed_ids).where(project_archived: false)
end
def refresh_materialized_view
Scenic.database.refresh_materialized_view(:datatables_reports,
concurrently: true,
cascade: false)
end
private
PermissionItem = Struct.new(:report_id, :users_ids, :visibility)
def tokenize(items)
items.collect do |item|
PermissionItem.new(item[0], item[1], item[2])
end
end
def get_permitted_by_team_tokenized
tokenize(pluck(:id, :users_with_team_read_permissions, :project_visibility))
end
def get_permitted_by_project_tokenized
tokenize(pluck(:id, :users_with_project_read_permissions, :project_visibility))
end
def get_by_project_item(permitted_by_project, item)
permitted_by_project.select { |el| el.report_id == item.report_id }
.first
end
def for_admin(user, permitted_by_team)
allowed_ids = []
permitted_by_team.each do |item|
next unless user.id.in? item.users_ids
allowed_ids << item.report_id
end
allowed_ids
end
def for_non_admin(user, permitted_by_team, permitted_by_project)
allowed_ids = []
permitted_by_team.each do |item|
next unless user.id.in? item.users_ids
by_project = get_by_project_item(permitted_by_project, item)
next unless user_can_view?(user, by_project)
allowed_ids << item.report_id
end
allowed_ids
end
def user_can_view?(user, by_project)
user.id.in?(by_project.users_ids) || by_project.visibility == 1
end
end
end
end
end

View file

@ -1,3 +1,4 @@
<% cache [current_user, project] do %>
<% active = !project.archived %>
<% if (active && (can_manage_project?(project) || can_archive_project?(project))) || (!active && can_restore_project?(project)) %>
<div class="dropdown pull-right">
@ -35,3 +36,4 @@
</ul>
</div>
<% end %>
<% end %>

View file

@ -1,7 +1,9 @@
<div class="row">
<% projects.each_with_index do |project, i| %>
<div class="col-lg-3 col-md-4 col-sm-6 col-xs-12">
<% cache [current_user, project] do %>
<%= render partial: "projects/index/project.html.erb", locals: { project: project } %>
<% end %>
</div>
<% if (i + 1) % 4 == 0 %>
<div class="clearfix visible-lg-block"></div>

View file

@ -1,7 +0,0 @@
class CreateDatatablesReports < ActiveRecord::Migration[5.1]
def change
create_view :datatables_reports, materialized: true
add_index :datatables_reports, :id, unique: true
add_index :datatables_reports, :team_id
end
end

View file

@ -0,0 +1,9 @@
# frozen_string_literal: true
class RemoveDatatablesReportsView < ActiveRecord::Migration[5.1]
def up
ActiveRecord::Base.connection.execute(
'DROP MATERIALIZED VIEW IF EXISTS datatables_reports'
)
end
end

View file

@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20181008130519) do
ActiveRecord::Schema.define(version: 20181212162649) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@ -1038,36 +1038,4 @@ ActiveRecord::Schema.define(version: 20181008130519) do
JOIN user_teams ON ((teams.id = user_teams.team_id)));
SQL
create_view "datatables_reports", materialized: true, sql_definition: <<-SQL
SELECT DISTINCT ON (reports.id) reports.id,
reports.name,
projects.name AS project_name,
( SELECT users.full_name
FROM users
WHERE (users.id = reports.user_id)) AS created_by,
( SELECT users.full_name
FROM users
WHERE (users.id = reports.last_modified_by_id)) AS last_modified_by,
reports.created_at,
reports.updated_at,
projects.archived AS project_archived,
projects.visibility AS project_visibility,
projects.id AS project_id,
reports.team_id,
ARRAY( SELECT DISTINCT user_teams_1.user_id
FROM user_teams user_teams_1
WHERE (user_teams_1.team_id = teams.id)) AS users_with_team_read_permissions,
ARRAY( SELECT DISTINCT user_projects_1.user_id
FROM user_projects user_projects_1
WHERE (user_projects_1.project_id = projects.id)) AS users_with_project_read_permissions
FROM ((((reports
JOIN projects ON ((projects.id = reports.project_id)))
JOIN user_projects ON ((user_projects.project_id = projects.id)))
JOIN teams ON ((teams.id = projects.team_id)))
JOIN user_teams ON ((user_teams.team_id = teams.id)));
SQL
add_index "datatables_reports", ["id"], name: "index_datatables_reports_on_id", unique: true
add_index "datatables_reports", ["team_id"], name: "index_datatables_reports_on_team_id"
end

View file

@ -1,39 +0,0 @@
SELECT DISTINCT ON (id)
reports.id AS id,
reports.name AS name,
projects.name AS project_name,
(
SELECT users.full_name
FROM users
WHERE users.id = reports.user_id
) AS created_by,
(
SELECT users.full_name
FROM users
WHERE users.id = reports.last_modified_by_id
) AS last_modified_by,
reports.created_at AS created_at,
reports.updated_at AS updated_at,
projects.archived AS project_archived,
projects.visibility AS project_visibility,
projects.id AS project_id,
reports.team_id AS team_id,
ARRAY(
SELECT DISTINCT user_teams.user_id
FROM user_teams
WHERE user_teams.team_id = teams.id
) AS users_with_team_read_permissions,
ARRAY(
SELECT DISTINCT user_projects.user_id
FROM user_projects
WHERE user_projects.project_id = projects.id
) AS users_with_project_read_permissions
FROM reports
INNER JOIN projects
ON projects.id = reports.project_id
INNER JOIN user_projects
ON user_projects.project_id = projects.id
INNER JOIN teams
ON teams.id = projects.team_id
INNER JOIN user_teams
ON user_teams.team_id = teams.id

View file

@ -1,80 +0,0 @@
require 'rails_helper'
RSpec.describe Views::Datatables::DatatablesReport, type: :model do
describe 'Database table' do
it { should have_db_column :id }
it { should have_db_column :name }
it { should have_db_column :project_name }
it { should have_db_column :created_by }
it { should have_db_column :last_modified_by }
it { should have_db_column :created_at }
it { should have_db_column :updated_at }
it { should have_db_column :project_archived }
it { should have_db_column :project_visibility }
it { should have_db_column :project_id }
it { should have_db_column :team_id }
it { should have_db_column :users_with_team_read_permissions }
it { should have_db_column :users_with_project_read_permissions }
end
describe 'is readonly' do
let(:team) { create :team }
it do
expect {
Views::Datatables::DatatablesReport.create!(team: team)
}.to raise_error(
ActiveRecord::ReadOnlyRecord,
'Views::Datatables::DatatablesReport is marked as readonly'
)
end
end
describe '#tokenize/1' do
it 'returns an array of permission items' do
items = [[1, [1, 2]], [2, [3, 4]]]
tokenized = described_class.send('tokenize', items)
expect(tokenized.first).to have_attributes(report_id: 1,
users_ids: [1, 2])
expect(tokenized.last).to have_attributes(report_id: 2, users_ids: [3, 4])
end
end
describe '#for_admin/3' do
let!(:user) { create :user }
let!(:user_two) { create :user, email: 'user.two@asdf.com' }
let!(:team) { create :team }
let!(:project) do
create :project, created_by: user, last_modified_by: user, team: team
end
let!(:user_project) { create :user_project, project: project, user: user }
let!(:team_two) { create :team }
let!(:user_team) do
create :user_team, team: team_two, user: user_two, role: 0
end
let!(:user_team) { create :user_team, team: team, user: user, role: 2 }
let!(:project_two) do
create :project, created_by: user_two,
last_modified_by: user_two,
team: team_two
end
let!(:report_one) do
create :report, user: user,
team: team,
name: 'report one',
project: project
end
let!(:report_two) do
create :report, user: user,
team: team_two,
project: project_two,
name: 'report two'
end
it 'returns the reports' do
reports = team.datatables_reports.visible_by(user, team)
expect(reports.length).to eq 1
expect(reports.first.id).to eq report_one.id
end
end
end