From dc91893d7eee322e1b5a44ad47b69dd88a4bf96c Mon Sep 17 00:00:00 2001 From: aignatov-bio Date: Wed, 18 Dec 2019 14:10:41 +0100 Subject: [PATCH] Refactor checklist column data structure --- .../repository_cell_values_checklist_item.rb | 8 ++ app/models/repository_checklist_item.rb | 2 + app/models/repository_checklist_value.rb | 25 ++++-- app/services/repository_datatable_service.rb | 8 +- config/initializers/extends.rb | 2 + ...3522_create_repository_checklist_values.rb | 1 - ..._repository_cell_values_checklist_items.rb | 13 +++ db/structure.sql | 85 ++++++++++++++++-- .../repository_list_items_controller_spec.rb | 86 ------------------- .../models/repository_checklist_value_spec.rb | 1 - .../create_repository_row_service_spec.rb | 2 +- 11 files changed, 128 insertions(+), 105 deletions(-) create mode 100644 app/models/repository_cell_values_checklist_item.rb create mode 100644 db/migrate/20191217132401_create_repository_cell_values_checklist_items.rb delete mode 100644 spec/controllers/repository_list_items_controller_spec.rb diff --git a/app/models/repository_cell_values_checklist_item.rb b/app/models/repository_cell_values_checklist_item.rb new file mode 100644 index 000000000..167f5ee53 --- /dev/null +++ b/app/models/repository_cell_values_checklist_item.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class RepositoryCellValuesChecklistItem < ApplicationRecord + belongs_to :repository_checklist_item + belongs_to :repository_checklist_value + + validates :repository_checklist_item, :repository_checklist_value, presence: true +end diff --git a/app/models/repository_checklist_item.rb b/app/models/repository_checklist_item.rb index 10f54bb7a..d3d5947b8 100644 --- a/app/models/repository_checklist_item.rb +++ b/app/models/repository_checklist_item.rb @@ -11,4 +11,6 @@ class RepositoryChecklistItem < ApplicationRecord inverse_of: :created_repository_checklist_types belongs_to :last_modified_by, foreign_key: 'last_modified_by_id', class_name: 'User', inverse_of: :modified_repository_checklist_types + has_many :repository_cell_values_checklist_items, dependent: :destroy + has_many :repository_checklist_values, through: :repository_cell_values_checklist_items end diff --git a/app/models/repository_checklist_value.rb b/app/models/repository_checklist_value.rb index 8fd7f405d..0f370b8b1 100644 --- a/app/models/repository_checklist_value.rb +++ b/app/models/repository_checklist_value.rb @@ -6,9 +6,12 @@ class RepositoryChecklistValue < ApplicationRecord belongs_to :last_modified_by, foreign_key: 'last_modified_by_id', class_name: 'User', inverse_of: :modified_repository_checklist_values has_one :repository_cell, as: :value, dependent: :destroy, inverse_of: :value + has_many :repository_cell_values_checklist_items, dependent: :destroy + has_many :repository_checklist_items, through: :repository_cell_values_checklist_items + accepts_nested_attributes_for :repository_cell - SORTABLE_COLUMN_NAME = 'repository_checklistitems' + SORTABLE_COLUMN_NAME = 'repository_checklist_items.data' SORTABLE_VALUE_INCLUDE = { repository_checklist_value: :repository_checklist_items }.freeze def formatted @@ -16,24 +19,30 @@ class RepositoryChecklistValue < ApplicationRecord end def data - repository_cell.repository_column.repository_checklist_items - .where(id: repository_checklist_items).select(:id, :data) - .map { |i| { value: i.id, label: i.data } } + repository_checklist_items.order(created_at: :asc).map { |i| { value: i.id, label: i.data } } end def data_changed?(new_data) - JSON.parse(new_data) != repository_checklist_items + JSON.parse(new_data) != repository_checklist_items.pluck(:id) end def update_data!(new_data, user) - self.repository_checklist_items = JSON.parse(new_data) + repository_cell_values_checklist_items.destroy_all + repository_cell.repository_column + .repository_checklist_items.where(id: JSON.parse(new_data)).find_each do |item| + repository_cell_values_checklist_items.create(repository_checklist_item: item) + end self.last_modified_by = user save! end def self.new_with_payload(payload, attributes) - value = new(attributes) - value.repository_checklist_items = JSON.parse(payload) + value = create(attributes) + value.repository_cell + .repository_column + .repository_checklist_items.where(id: JSON.parse(payload)).find_each do |item| + value.repository_cell_values_checklist_items.create(repository_checklist_item: item) + end value end end diff --git a/app/services/repository_datatable_service.rb b/app/services/repository_datatable_service.rb index 1871c3ed4..f48067225 100644 --- a/app/services/repository_datatable_service.rb +++ b/app/services/repository_datatable_service.rb @@ -1,5 +1,6 @@ -class RepositoryDatatableService +# frozen_string_literal: true +class RepositoryDatatableService attr_reader :repository_rows, :assigned_rows, :mappings def initialize(repository, params, user, my_module = nil) @@ -100,6 +101,7 @@ class RepositoryDatatableService if sortable_columns[column_id - 1] == 'assigned' return records if @my_module && @params[:assigned] == 'assigned' + if @my_module # Depending on the sort, order nulls first or # nulls last on repository_cells association @@ -119,14 +121,14 @@ class RepositoryDatatableService end elsif sortable_columns[column_id - 1] == 'repository_cell.value' id = @mappings.key(column_id.to_s) - sorting_column = RepositoryColumn.find_by_id(id) + sorting_column = RepositoryColumn.find_by(id: id) return records unless sorting_column sorting_data_type = sorting_column.data_type.constantize cells = RepositoryCell.joins(sorting_data_type::SORTABLE_VALUE_INCLUDE) .where('repository_cells.repository_column_id': sorting_column.id) - .select("repository_cells.repository_row_id, + .select("DISTINCT ON (repository_cells.repository_row_id) repository_row_id, #{sorting_data_type::SORTABLE_COLUMN_NAME} AS value") records.joins("LEFT OUTER JOIN (#{cells.to_sql}) AS values ON values.repository_row_id = repository_rows.id") diff --git a/config/initializers/extends.rb b/config/initializers/extends.rb index 77e6aefd6..1a0343ec9 100644 --- a/config/initializers/extends.rb +++ b/config/initializers/extends.rb @@ -63,11 +63,13 @@ class Extends # are only supported REPOSITORY_EXTRA_SEARCH_ATTR = ['repository_text_values.data', 'repository_list_items.data', + 'repository_checklist_items.data', 'active_storage_blobs.filename'] # Array of includes used in search query for repository rows REPOSITORY_SEARCH_INCLUDES = [:repository_text_value, repository_list_value: :repository_list_item, + repository_checklist_value: :repository_checklist_items, repository_asset_value: { asset: { file_attachment: :blob } }] # List of implemented core API versions diff --git a/db/migrate/20191205133522_create_repository_checklist_values.rb b/db/migrate/20191205133522_create_repository_checklist_values.rb index b802583c5..9b2afe104 100644 --- a/db/migrate/20191205133522_create_repository_checklist_values.rb +++ b/db/migrate/20191205133522_create_repository_checklist_values.rb @@ -5,7 +5,6 @@ class CreateRepositoryChecklistValues < ActiveRecord::Migration[6.0] create_table :repository_checklist_values do |t| t.references :created_by, index: true, foreign_key: { to_table: :users }, null: true t.references :last_modified_by, index: true, foreign_key: { to_table: :users }, null: true - t.jsonb :repository_checklist_items, t.timestamps end diff --git a/db/migrate/20191217132401_create_repository_cell_values_checklist_items.rb b/db/migrate/20191217132401_create_repository_cell_values_checklist_items.rb new file mode 100644 index 000000000..c688b359e --- /dev/null +++ b/db/migrate/20191217132401_create_repository_cell_values_checklist_items.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class CreateRepositoryCellValuesChecklistItems < ActiveRecord::Migration[6.0] + def change + create_table :repository_cell_values_checklist_items do |t| + t.references :repository_checklist_item, null: false, foreign_key: true, + index: { name: :repository_cell_values_checklist_item_id } + t.references :repository_checklist_value, null: false, foreign_key: true, + index: { name: :repository_cell_values_checklist_value_id } + t.timestamps + end + end +end diff --git a/db/structure.sql b/db/structure.sql index 979ab0009..a62a0c480 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -1142,6 +1142,38 @@ CREATE SEQUENCE public.repository_asset_values_id_seq ALTER SEQUENCE public.repository_asset_values_id_seq OWNED BY public.repository_asset_values.id; +-- +-- Name: repository_cell_values_checklist_items; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE public.repository_cell_values_checklist_items ( + id bigint NOT NULL, + repository_checklist_item_id bigint NOT NULL, + repository_checklist_value_id bigint NOT NULL, + created_at timestamp(6) without time zone NOT NULL, + updated_at timestamp(6) without time zone NOT NULL +); + + +-- +-- Name: repository_cell_values_checklist_items_id_seq; Type: SEQUENCE; Schema: public; Owner: - +-- + +CREATE SEQUENCE public.repository_cell_values_checklist_items_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + + +-- +-- Name: repository_cell_values_checklist_items_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - +-- + +ALTER SEQUENCE public.repository_cell_values_checklist_items_id_seq OWNED BY public.repository_cell_values_checklist_items.id; + + -- -- Name: repository_cells; Type: TABLE; Schema: public; Owner: - -- @@ -1220,9 +1252,7 @@ CREATE TABLE public.repository_checklist_values ( created_by_id bigint, last_modified_by_id bigint, created_at timestamp(6) without time zone NOT NULL, - updated_at timestamp(6) without time zone NOT NULL, - repository_checklist_items jsonb, - "#