diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index 5d3a537bd..0610d5e36 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -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 diff --git a/app/datatables/report_datatable.rb b/app/datatables/report_datatable.rb index 5427af0e3..09aad71d8 100644 --- a/app/datatables/report_datatable.rb +++ b/app/datatables/report_datatable.rb @@ -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 diff --git a/app/models/checklist.rb b/app/models/checklist.rb index ba4e2e433..321ed514b 100644 --- a/app/models/checklist.rb +++ b/app/models/checklist.rb @@ -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', diff --git a/app/models/experiment.rb b/app/models/experiment.rb index 6bd1ce1e1..69d2926af 100644 --- a/app/models/experiment.rb +++ b/app/models/experiment.rb @@ -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', diff --git a/app/models/my_module.rb b/app/models/my_module.rb index 012899d3e..c96de8944 100644 --- a/app/models/my_module.rb +++ b/app/models/my_module.rb @@ -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 diff --git a/app/models/project.rb b/app/models/project.rb index 8c15fb9a4..73ffb8f4a 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -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 ?', diff --git a/app/models/project_comment.rb b/app/models/project_comment.rb index c625176b0..682025769 100644 --- a/app/models/project_comment.rb +++ b/app/models/project_comment.rb @@ -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 diff --git a/app/models/report.rb b/app/models/report.rb index 84620045a..b7d4354af 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -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 diff --git a/app/models/result_asset.rb b/app/models/result_asset.rb index 53f2bcfb7..55ab7f51e 100644 --- a/app/models/result_asset.rb +++ b/app/models/result_asset.rb @@ -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, diff --git a/app/models/result_comment.rb b/app/models/result_comment.rb index d3d858aa2..955d4ab29 100644 --- a/app/models/result_comment.rb +++ b/app/models/result_comment.rb @@ -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 diff --git a/app/models/result_table.rb b/app/models/result_table.rb index 26902412d..1fd0ede0d 100644 --- a/app/models/result_table.rb +++ b/app/models/result_table.rb @@ -5,5 +5,6 @@ class ResultTable < ApplicationRecord belongs_to :table, inverse_of: :result_table, dependent: :destroy, + touch: true, optional: true end diff --git a/app/models/result_text.rb b/app/models/result_text.rb index 719c2c026..de40b186e 100644 --- a/app/models/result_text.rb +++ b/app/models/result_text.rb @@ -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 diff --git a/app/models/step_asset.rb b/app/models/step_asset.rb index 60dfdbd33..46aa8622e 100644 --- a/app/models/step_asset.rb +++ b/app/models/step_asset.rb @@ -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, diff --git a/app/models/step_comment.rb b/app/models/step_comment.rb index bc13fd337..889b2495f 100644 --- a/app/models/step_comment.rb +++ b/app/models/step_comment.rb @@ -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 diff --git a/app/models/step_table.rb b/app/models/step_table.rb index b6e4d5cbd..103d192fe 100644 --- a/app/models/step_table.rb +++ b/app/models/step_table.rb @@ -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 diff --git a/app/models/team.rb b/app/models/team.rb index 4231d6bc9..3ade33d47 100644 --- a/app/models/team.rb +++ b/app/models/team.rb @@ -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' => diff --git a/app/models/tiny_mce_asset.rb b/app/models/tiny_mce_asset.rb index e4546c0a4..03b7c3afe 100644 --- a/app/models/tiny_mce_asset.rb +++ b/app/models/tiny_mce_asset.rb @@ -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] }, diff --git a/app/models/user_my_module.rb b/app/models/user_my_module.rb index 3371d7fd3..5b3edabb4 100644 --- a/app/models/user_my_module.rb +++ b/app/models/user_my_module.rb @@ -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 diff --git a/app/models/user_project.rb b/app/models/user_project.rb index 1702b76eb..36d3c4ce0 100644 --- a/app/models/user_project.rb +++ b/app/models/user_project.rb @@ -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 diff --git a/app/models/user_team.rb b/app/models/user_team.rb index f3547e091..fdcd89453 100644 --- a/app/models/user_team.rb +++ b/app/models/user_team.rb @@ -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 diff --git a/app/models/views/datatables/datatables_report.rb b/app/models/views/datatables/datatables_report.rb deleted file mode 100644 index db27050e8..000000000 --- a/app/models/views/datatables/datatables_report.rb +++ /dev/null @@ -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 diff --git a/app/views/projects/index/_project_actions_dropdown.html.erb b/app/views/projects/index/_project_actions_dropdown.html.erb index 2b88de79d..a9ed02db9 100644 --- a/app/views/projects/index/_project_actions_dropdown.html.erb +++ b/app/views/projects/index/_project_actions_dropdown.html.erb @@ -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)) %> <% end %> +<% end %> diff --git a/app/views/projects/index/_team_projects.html.erb b/app/views/projects/index/_team_projects.html.erb index 5e6df13bc..778763a20 100644 --- a/app/views/projects/index/_team_projects.html.erb +++ b/app/views/projects/index/_team_projects.html.erb @@ -1,7 +1,9 @@
<% projects.each_with_index do |project, i| %>
+ <% cache [current_user, project] do %> <%= render partial: "projects/index/project.html.erb", locals: { project: project } %> + <% end %>
<% if (i + 1) % 4 == 0 %>
diff --git a/db/migrate/20180417062042_create_datatables_reports.rb b/db/migrate/20180417062042_create_datatables_reports.rb deleted file mode 100644 index 3edf73c65..000000000 --- a/db/migrate/20180417062042_create_datatables_reports.rb +++ /dev/null @@ -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 diff --git a/db/migrate/20181212162649_remove_datatables_reports_view.rb b/db/migrate/20181212162649_remove_datatables_reports_view.rb new file mode 100644 index 000000000..ebf119b41 --- /dev/null +++ b/db/migrate/20181212162649_remove_datatables_reports_view.rb @@ -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 diff --git a/db/schema.rb b/db/schema.rb index 5b0a2b409..ab055cf53 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -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 diff --git a/db/views/datatables_reports_v01.sql b/db/views/datatables_reports_v01.sql deleted file mode 100644 index 224c84403..000000000 --- a/db/views/datatables_reports_v01.sql +++ /dev/null @@ -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 diff --git a/spec/models/views/datatables/datatables_report_spec.rb b/spec/models/views/datatables/datatables_report_spec.rb deleted file mode 100644 index 711420f24..000000000 --- a/spec/models/views/datatables/datatables_report_spec.rb +++ /dev/null @@ -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