From 5e2622ecfcb8bd81235c4d9b0596d69094d87748 Mon Sep 17 00:00:00 2001 From: Luka Murn Date: Wed, 18 Apr 2018 13:21:30 +0200 Subject: [PATCH 1/8] Change default repository state to Ruby representation Also remove some dead code from repositories datatables --- .../repositories/repository_datatable.js.erb | 92 +++++-------------- .../user_repositories_controller.rb | 3 +- app/models/repository_table_state.rb | 46 ++++++---- app/services/repository_datatable_service.rb | 5 +- app/utilities/hash_util.rb | 18 ++++ config/initializers/constants.rb | 40 ++++---- 6 files changed, 97 insertions(+), 107 deletions(-) create mode 100644 app/utilities/hash_util.rb diff --git a/app/assets/javascripts/repositories/repository_datatable.js.erb b/app/assets/javascripts/repositories/repository_datatable.js.erb index 875f825b2..63a92ba63 100644 --- a/app/assets/javascripts/repositories/repository_datatable.js.erb +++ b/app/assets/javascripts/repositories/repository_datatable.js.erb @@ -47,7 +47,6 @@ var RepositoryDatatable = (function(global) { originalHeader = $(TABLE_ID + ' thead').children().clone(); viewAssigned = 'assigned'; TABLE = $(TABLE_ID).DataTable({ - order: [[3, 'desc']], dom: "R<'row'<'col-sm-9-custom toolbar'l><'col-sm-3-custom'f>>tpi", stateSave: true, processing: true, @@ -70,6 +69,7 @@ var RepositoryDatatable = (function(global) { type: 'POST' }, columnDefs: [{ + // Checkbox column needs special handling targets: 0, searchable: false, orderable: false, @@ -79,22 +79,19 @@ var RepositoryDatatable = (function(global) { return ""; } }, { + // Assigned column is not searchable targets: 1, searchable: false, orderable: true, sWidth: '1%' }, { - targets: 2, - searchable: true, - orderable: true, - sWidth: '1%' - }, { - targets: 3, - render: function(data, type, row) { - return "" + data + ''; - } - }], + // Name column is clickable + targets: 3, + render: function(data, type, row) { + return "" + data + ''; + } + }], rowCallback: function(row, data) { // Get row ID var rowId = data.DT_RowId; @@ -104,18 +101,23 @@ var RepositoryDatatable = (function(global) { $(row).addClass('selected'); } }, + // Next 2 options are shared with server-side default state + // (and get overriden once state load from server kicks in) + order: <%= Constants::REPOSITORY_TABLE_DEFAULT_STATE[:order][0].to_s %>, columns: (function() { var numOfColumns = $(TABLE_ID).data('num-columns'); - var columns = []; + var columns = + <%= Constants::REPOSITORY_TABLE_DEFAULT_STATE[:columns].keys.sort.map do |k| + col = Constants::REPOSITORY_TABLE_DEFAULT_STATE[:columns][k] + col.slice(:visible, :searchable) + end.to_json %>; for (var i = 0; i < numOfColumns; i++) { - var visible = (i <= 4); - var searchable = (i > 0 && i <= 4); - columns.push({ - data: String(i), - defaultContent: '', - visible: visible, - searchable: searchable - }); + if (columns[i] == undefined) { + // This should only occur for custom columns + columns[i] = { visible: true, searchable: true }; + } + columns[i].data = String(i); + columns[i].defaultContent = ''; } return columns; })(), @@ -133,7 +135,6 @@ var RepositoryDatatable = (function(global) { rowsSelected.length + ' entries selected)'); initRowSelection(); - initHeaderTooltip(); }, preDrawCallback: function() { animateSpinner(this); @@ -170,7 +171,6 @@ var RepositoryDatatable = (function(global) { type: 'POST' }); loadFirstTime = false; - initHeaderTooltip(); }, fnInitComplete: function(oSettings) { // Reload correct column order and visibility (if you refresh page) @@ -179,7 +179,7 @@ var RepositoryDatatable = (function(global) { TABLE.column(1).visible(true); for (var i = 2; i < TABLE.columns()[0].length; i++) { var visibility = false; - if (myData.columns[i]) { + if (myData.columns && myData.columns[i]) { visibility = myData.columns[i].visible; } if (typeof (visibility) === 'string') { @@ -196,10 +196,6 @@ var RepositoryDatatable = (function(global) { if( oSettings._colReorder ) { oSettings._colReorder.fnOrder(myData.ColReorder); } - TABLE.on('mousedown', function() { - $('#repository-columns-dropdown').removeClass('open'); - }); - initHeaderTooltip(); initRowSelection(); bindExportActions(); disableCheckboxToggleOnAssetDownload(); @@ -384,44 +380,6 @@ var RepositoryDatatable = (function(global) { } } - function initHeaderTooltip() { - // Fix compatibility of fixed table header and column names modal-tooltip - $('.modal-tooltip').off(); - $('.modal-tooltip').hover(function() { - var $tooltip = $(this).find('.modal-tooltiptext'); - var offsetLeft = $tooltip.offset().left; - if ((offsetLeft + 200) > $(window).width()) { - offsetLeft -= 150; - } - var offsetTop = $tooltip.offset().top; - var width = 200; - - // set tooltip params in the table body - if ($(this).parents(TABLE_ID).length) { - offsetLeft = $(TABLE_ID).offset().left + 100; - width = $(TABLE_ID).width() - 200; - } - $('body').append($tooltip); - $tooltip.css('background-color', '#d2d2d2'); - $tooltip.css('border-radius', '6px'); - $tooltip.css('color', '#333'); - $tooltip.css('display', 'block'); - $tooltip.css('left', offsetLeft + 'px'); - $tooltip.css('padding', '5px'); - $tooltip.css('position', 'absolute'); - $tooltip.css('text-align', 'center'); - $tooltip.css('top', offsetTop + 'px'); - $tooltip.css('visibility', 'visible'); - $tooltip.css('width', width + 'px'); - $tooltip.css('word-wrap', 'break-word'); - $tooltip.css('z-index', '4'); - $(this).data('dropdown-tooltip', $tooltip); - }, function() { - $(this).append($(this).data('dropdown-tooltip')); - $(this).data('dropdown-tooltip').removeAttr('style'); - }); - } - global.onClickAddRecord = function() { changeToEditMode(); updateButtons(); @@ -1060,7 +1018,6 @@ var RepositoryDatatable = (function(global) { currentMode = 'editMode'; // Table specific stuff TABLE.button(0).enable(false); - initHeaderTooltip(); } /* @@ -1146,7 +1103,6 @@ var RepositoryDatatable = (function(global) { li.removeClass('col-invisible'); column.visible(true); TABLE.setColumnSearchable(column.index(), true); - initHeaderTooltip(); } // Re-filter/search if neccesary diff --git a/app/controllers/user_repositories_controller.rb b/app/controllers/user_repositories_controller.rb index a028ce7c7..d0c1d6cde 100644 --- a/app/controllers/user_repositories_controller.rb +++ b/app/controllers/user_repositories_controller.rb @@ -22,7 +22,8 @@ class UserRepositoriesController < ApplicationController def load_table_state table_state = RepositoryTableState.load_state(current_user, - @repository).first + @repository) + .state respond_to do |format| if table_state format.json do diff --git a/app/models/repository_table_state.rb b/app/models/repository_table_state.rb index dc2f3c6d2..3b12446e0 100644 --- a/app/models/repository_table_state.rb +++ b/app/models/repository_table_state.rb @@ -4,13 +4,17 @@ class RepositoryTableState < ApplicationRecord validates :user, :repository, presence: true + # We're using Constants::REPOSITORY_TABLE_DEFAULT_STATE as a reference for + # default table state; this Ruby Hash makes heavy use of Ruby symbols + # notation; HOWEVER, the state that is saved on the RepositoryTableState + # record, has EVERYTHING (booleans, symbols, keys, ...) saved as Strings. + def self.load_state(user, repository) - table_state = where(user: user, repository: repository).pluck(:state) - if table_state.blank? - RepositoryTableState.create_state(user, repository) - table_state = where(user: user, repository: repository).pluck(:state) + state = where(user: user, repository: repository).take + if state.blank? + state = RepositoryTableState.create_state(user, repository) end - table_state + state end def self.update_state(custom_column, column_index, user) @@ -43,8 +47,8 @@ class RepositoryTableState < ApplicationRecord else # add column index = repository_state['columns'].count - repository_state['columns'][index] = Constants:: - REPOSITORY_TABLE_DEFAULT_STATE['columns'].first + repository_state['columns'][index] = + Constants::REPOSITORY_TABLE_DEFAULT_STATE[:columns].first repository_state['ColReorder'].insert(2, index.to_s) end table_state.update(state: repository_state) @@ -52,17 +56,25 @@ class RepositoryTableState < ApplicationRecord end def self.create_state(user, repository) - default_columns_num = Constants:: - REPOSITORY_TABLE_DEFAULT_STATE['columns'].count - repository_state = - Constants::REPOSITORY_TABLE_DEFAULT_STATE.deep_dup + default_columns_num = + Constants::REPOSITORY_TABLE_DEFAULT_STATE[:length] + + # This state should be strings-only + state = HashUtil.deep_stringify_keys_and_values( + Constants::REPOSITORY_TABLE_DEFAULT_STATE + ) repository.repository_columns.each_with_index do |_, index| - repository_state['columns'] << Constants:: - REPOSITORY_TABLE_DEFAULT_STATE['columns'].first - repository_state['ColReorder'] << (default_columns_num + index) + real_index = default_columns_num + index + state['columns'][real_index.to_s] = + HashUtil.deep_stringify_keys_and_values( + Constants::REPOSITORY_TABLE_STATE_CUSTOM_COLUMN_TEMPLATE + ) + state['ColReorder'] << real_index.to_s end - RepositoryTableState.create(user: user, - repository: repository, - state: repository_state) + state['length'] = state['columns'].length.to_s + state['time'] = Time.new.to_i.to_s + return RepositoryTableState.create(user: user, + repository: repository, + state: state) end end diff --git a/app/services/repository_datatable_service.rb b/app/services/repository_datatable_service.rb index 892236e32..705975eac 100644 --- a/app/services/repository_datatable_service.rb +++ b/app/services/repository_datatable_service.rb @@ -95,9 +95,8 @@ class RepositoryDatatableService direction == column_obj[:dir].upcase end || 'ASC' column_index = column_obj[:column] - col_order = @repository.repository_table_states - .find_by_user_id(@user.id) - .state['ColReorder'] + col_order = RepositoryTableState.load_state(@user, @repository) + .state['ColReorder'] column_id = col_order[column_index].to_i if sortable_columns[column_id - 1] == 'assigned' diff --git a/app/utilities/hash_util.rb b/app/utilities/hash_util.rb new file mode 100644 index 000000000..294f2bef1 --- /dev/null +++ b/app/utilities/hash_util.rb @@ -0,0 +1,18 @@ +module HashUtil + + def deep_stringify_values(obj, include_arrays = true) + if obj.is_a?(Hash) + obj.map { |k, v| [k, deep_stringify_values(v, include_arrays)] }.to_h + elsif include_arrays && obj.is_a?(Array) + obj.map { |i| deep_stringify_values(i, include_arrays) } + else + obj.to_s + end + end + module_function :deep_stringify_values + + def deep_stringify_keys_and_values(obj, include_arrays = true) + deep_stringify_values(obj, include_arrays).deep_stringify_keys + end + module_function :deep_stringify_keys_and_values +end \ No newline at end of file diff --git a/config/initializers/constants.rb b/config/initializers/constants.rb index 5ee3c87b4..e2f85e9e1 100644 --- a/config/initializers/constants.rb +++ b/config/initializers/constants.rb @@ -843,28 +843,32 @@ class Constants # Repository default table state REPOSITORY_TABLE_DEFAULT_STATE = { - 'time' => 0, - 'start' => 0, - 'length' => 6, - 'order' => [[3, 'desc']], - 'search' => { 'search' => '', - 'smart' => true, - 'regex' => false, - 'caseInsensitive' => true }, - 'columns' => [], - 'assigned' => 'assigned', - 'ColReorder' => [*0..5] + time: 0, + start: 0, + length: 6, + order: { 0 => [[2, 'asc']] }, # Default sorting by 'ID' column + search: { search: '', + smart: true, + regex: false, + caseInsensitive: true }, + columns: {}, + assigned: 'assigned', + ColReorder: [*0..5] } - 6.times do - REPOSITORY_TABLE_DEFAULT_STATE['columns'] << { - 'visible' => true, - 'search' => { 'search' => '', - 'smart' => true, - 'regex' => false, - 'caseInsensitive' => true } + 6.times do |i| + REPOSITORY_TABLE_DEFAULT_STATE[:columns][i] = { + visible: true, + searchable: i >= 1, # Checkboxes column is not searchable + search: { search: '', + smart: true, + regex: false, + caseInsensitive: true } } end REPOSITORY_TABLE_DEFAULT_STATE.freeze + REPOSITORY_TABLE_STATE_CUSTOM_COLUMN_TEMPLATE = + REPOSITORY_TABLE_DEFAULT_STATE[:columns][1].deep_dup + .freeze EXPORTABLE_ZIP_EXPIRATION_DAYS = 7 From d8ca42aa0c4d0cd9d180a25343891012272ea027 Mon Sep 17 00:00:00 2001 From: Luka Murn Date: Wed, 18 Apr 2018 15:20:58 +0200 Subject: [PATCH 2/8] Refactor repository_table_state and its class methods --- .../repository_columns_controller.rb | 5 +- .../user_repositories_controller.rb | 10 +- app/models/repository_column.rb | 6 +- app/models/repository_table_state.rb | 100 +++++++++++------- 4 files changed, 66 insertions(+), 55 deletions(-) diff --git a/app/controllers/repository_columns_controller.rb b/app/controllers/repository_columns_controller.rb index 899ae2207..6d9910dcc 100644 --- a/app/controllers/repository_columns_controller.rb +++ b/app/controllers/repository_columns_controller.rb @@ -124,10 +124,9 @@ class RepositoryColumnsController < ApplicationController respond_to do |format| format.json do if @repository_column.destroy - RepositoryTableState.update_state( + RepositoryTableState.update_states_with_removed_column( @del_repository_column, - params[:repository_column][:column_index], - current_user + params[:repository_column][:column_index] ) render json: { message: t('libraries.repository_columns.destroy.success_flash', diff --git a/app/controllers/user_repositories_controller.rb b/app/controllers/user_repositories_controller.rb index d0c1d6cde..e416f9b25 100644 --- a/app/controllers/user_repositories_controller.rb +++ b/app/controllers/user_repositories_controller.rb @@ -2,15 +2,7 @@ class UserRepositoriesController < ApplicationController before_action :load_vars def save_table_state - table_state = RepositoryTableState.where(user: current_user, - repository: @repository).first - if table_state - table_state.update(state: params[:state]) - else - RepositoryTableState.create(user: current_user, - repository: @repository, - state: params[:state]) - end + RepositoryTableState.update_state(current_user, @repository, params[:state]) respond_to do |format| format.json do render json: { diff --git a/app/models/repository_column.rb b/app/models/repository_column.rb index a8cb64c25..80c1fb440 100644 --- a/app/models/repository_column.rb +++ b/app/models/repository_column.rb @@ -19,12 +19,12 @@ class RepositoryColumn < ApplicationRecord validates :repository, presence: true validates :data_type, presence: true - after_create :update_repository_table_state + after_create :update_repository_table_states scope :list_type, -> { where(data_type: 'RepositoryListValue') } - def update_repository_table_state - RepositoryTableState.update_state(self, nil, created_by) + def update_repository_table_states + RepositoryTableState.update_states_with_new_column(self) end def importable? diff --git a/app/models/repository_table_state.rb b/app/models/repository_table_state.rb index 3b12446e0..d2f5e8ab4 100644 --- a/app/models/repository_table_state.rb +++ b/app/models/repository_table_state.rb @@ -12,50 +12,20 @@ class RepositoryTableState < ApplicationRecord def self.load_state(user, repository) state = where(user: user, repository: repository).take if state.blank? - state = RepositoryTableState.create_state(user, repository) + state = RepositoryTableState.create_default_state(user, repository) end state end - def self.update_state(custom_column, column_index, user) - # table state of every user having access to this repository needs udpating - table_states = RepositoryTableState.where( - repository: custom_column.repository - ) - table_states.each do |table_state| - repository_state = table_state['state'] - if column_index - # delete column - repository_state['columns'].delete(column_index) - repository_state['columns'].keys.each do |index| - if index.to_i > column_index.to_i - repository_state['columns'][(index.to_i - 1).to_s] = - repository_state['columns'].delete(index) - else - index - end - end - - repository_state['ColReorder'].delete(column_index) - repository_state['ColReorder'].map! do |index| - if index.to_i > column_index.to_i - (index.to_i - 1).to_s - else - index - end - end - else - # add column - index = repository_state['columns'].count - repository_state['columns'][index] = - Constants::REPOSITORY_TABLE_DEFAULT_STATE[:columns].first - repository_state['ColReorder'].insert(2, index.to_s) - end - table_state.update(state: repository_state) - end + def self.update_state(user, repository, state) + RepositoryTableState.load_state(user, repository) + .update(state: state) end - def self.create_state(user, repository) + def self.create_default_state(user, repository) + # Destroy any state object before recreating a new one + RepositoryTableState.where(user: user, repository: repository).destroy + default_columns_num = Constants::REPOSITORY_TABLE_DEFAULT_STATE[:length] @@ -74,7 +44,57 @@ class RepositoryTableState < ApplicationRecord state['length'] = state['columns'].length.to_s state['time'] = Time.new.to_i.to_s return RepositoryTableState.create(user: user, - repository: repository, - state: state) + repository: repository, + state: state) + end + + def self.update_states_with_new_column(new_column) + RepositoryTableState.where( + repository: new_column.repository + ).find_each do |table_state| + state = table_state.state + index = state['columns'].count + + # Add new columns, ColReorder, length entries + state['columns'][index.to_s] = + HashUtil.deep_stringify_keys_and_values( + Constants::REPOSITORY_TABLE_STATE_CUSTOM_COLUMN_TEMPLATE + ) + state['ColReorder'] << index.to_s + state['length'] = (index + 1).to_s + state['time'] = Time.new.to_i.to_s + table_state.save + end + end + + def self.update_states_with_removed_column(repository, old_column_index) + RepositoryTableState.where( + repository: repository + ).find_each do |table_state| + state = table_state.state + + # old_column_index is a String! + + # Remove column from ColReorder, columns, length entries + state['columns'].delete(old_column_index) + state['columns'].keys.each do |index| + if index.to_i > old_column_index.to_i + state['columns'][(index.to_i - 1).to_s] = + state['columns'].delete(index.to_s) + end + end + + state['ColReorder'].delete(old_column_index) + state['ColReorder'].map! do |index| + if index.to_i > old_column_index.to_i + (index.to_i - 1).to_s + else + index + end + end + state['length'] = (state['length'].to_i - 1).to_s + state['time'] = Time.new.to_i.to_s + table_state.save + end end end From 21e7e7057e8e0ca73592d6463290c00fd4cf9909 Mon Sep 17 00:00:00 2001 From: Luka Murn Date: Wed, 18 Apr 2018 15:53:48 +0200 Subject: [PATCH 3/8] Refactor all the methods into two new services --- .../repositories/repository_datatable.js.erb | 2 +- .../repository_columns_controller.rb | 5 +- .../user_repositories_controller.rb | 12 +-- app/models/repository_column.rb | 3 +- app/models/repository_table_state.rb | 94 ------------------- app/services/repository_datatable_service.rb | 4 +- ...itory_table_state_column_update_service.rb | 56 +++++++++++ .../repository_table_state_service.rb | 61 ++++++++++++ config/initializers/constants.rb | 2 +- 9 files changed, 132 insertions(+), 107 deletions(-) create mode 100644 app/services/repository_table_state_column_update_service.rb create mode 100644 app/services/repository_table_state_service.rb diff --git a/app/assets/javascripts/repositories/repository_datatable.js.erb b/app/assets/javascripts/repositories/repository_datatable.js.erb index 63a92ba63..3310146c0 100644 --- a/app/assets/javascripts/repositories/repository_datatable.js.erb +++ b/app/assets/javascripts/repositories/repository_datatable.js.erb @@ -101,7 +101,7 @@ var RepositoryDatatable = (function(global) { $(row).addClass('selected'); } }, - // Next 2 options are shared with server-side default state + // Next 2 options are provided by server-side default state // (and get overriden once state load from server kicks in) order: <%= Constants::REPOSITORY_TABLE_DEFAULT_STATE[:order][0].to_s %>, columns: (function() { diff --git a/app/controllers/repository_columns_controller.rb b/app/controllers/repository_columns_controller.rb index 6d9910dcc..bd3018856 100644 --- a/app/controllers/repository_columns_controller.rb +++ b/app/controllers/repository_columns_controller.rb @@ -124,8 +124,9 @@ class RepositoryColumnsController < ApplicationController respond_to do |format| format.json do if @repository_column.destroy - RepositoryTableState.update_states_with_removed_column( - @del_repository_column, + service = RepositoryTableStateColumnUpdateService.new + service.update_states_with_removed_column( + @del_repository_column.repository, params[:repository_column][:column_index] ) render json: { diff --git a/app/controllers/user_repositories_controller.rb b/app/controllers/user_repositories_controller.rb index e416f9b25..2a6562c8f 100644 --- a/app/controllers/user_repositories_controller.rb +++ b/app/controllers/user_repositories_controller.rb @@ -2,7 +2,8 @@ class UserRepositoriesController < ApplicationController before_action :load_vars def save_table_state - RepositoryTableState.update_state(current_user, @repository, params[:state]) + service = RepositoryTableStateService.new(current_user, @repository) + service.update_state(params[:state]) respond_to do |format| format.json do render json: { @@ -13,14 +14,13 @@ class UserRepositoriesController < ApplicationController end def load_table_state - table_state = RepositoryTableState.load_state(current_user, - @repository) - .state + service = RepositoryTableStateService.new(current_user, @repository) + state = service.load_state.state respond_to do |format| - if table_state + if state format.json do render json: { - state: table_state + state: state } end end diff --git a/app/models/repository_column.rb b/app/models/repository_column.rb index 80c1fb440..9f92ec08d 100644 --- a/app/models/repository_column.rb +++ b/app/models/repository_column.rb @@ -24,7 +24,8 @@ class RepositoryColumn < ApplicationRecord scope :list_type, -> { where(data_type: 'RepositoryListValue') } def update_repository_table_states - RepositoryTableState.update_states_with_new_column(self) + service = RepositoryTableStateColumnUpdateService.new + service.update_states_with_new_column(self) end def importable? diff --git a/app/models/repository_table_state.rb b/app/models/repository_table_state.rb index d2f5e8ab4..0b83269e8 100644 --- a/app/models/repository_table_state.rb +++ b/app/models/repository_table_state.rb @@ -3,98 +3,4 @@ class RepositoryTableState < ApplicationRecord belongs_to :repository, inverse_of: :repository_table_states, optional: true validates :user, :repository, presence: true - - # We're using Constants::REPOSITORY_TABLE_DEFAULT_STATE as a reference for - # default table state; this Ruby Hash makes heavy use of Ruby symbols - # notation; HOWEVER, the state that is saved on the RepositoryTableState - # record, has EVERYTHING (booleans, symbols, keys, ...) saved as Strings. - - def self.load_state(user, repository) - state = where(user: user, repository: repository).take - if state.blank? - state = RepositoryTableState.create_default_state(user, repository) - end - state - end - - def self.update_state(user, repository, state) - RepositoryTableState.load_state(user, repository) - .update(state: state) - end - - def self.create_default_state(user, repository) - # Destroy any state object before recreating a new one - RepositoryTableState.where(user: user, repository: repository).destroy - - default_columns_num = - Constants::REPOSITORY_TABLE_DEFAULT_STATE[:length] - - # This state should be strings-only - state = HashUtil.deep_stringify_keys_and_values( - Constants::REPOSITORY_TABLE_DEFAULT_STATE - ) - repository.repository_columns.each_with_index do |_, index| - real_index = default_columns_num + index - state['columns'][real_index.to_s] = - HashUtil.deep_stringify_keys_and_values( - Constants::REPOSITORY_TABLE_STATE_CUSTOM_COLUMN_TEMPLATE - ) - state['ColReorder'] << real_index.to_s - end - state['length'] = state['columns'].length.to_s - state['time'] = Time.new.to_i.to_s - return RepositoryTableState.create(user: user, - repository: repository, - state: state) - end - - def self.update_states_with_new_column(new_column) - RepositoryTableState.where( - repository: new_column.repository - ).find_each do |table_state| - state = table_state.state - index = state['columns'].count - - # Add new columns, ColReorder, length entries - state['columns'][index.to_s] = - HashUtil.deep_stringify_keys_and_values( - Constants::REPOSITORY_TABLE_STATE_CUSTOM_COLUMN_TEMPLATE - ) - state['ColReorder'] << index.to_s - state['length'] = (index + 1).to_s - state['time'] = Time.new.to_i.to_s - table_state.save - end - end - - def self.update_states_with_removed_column(repository, old_column_index) - RepositoryTableState.where( - repository: repository - ).find_each do |table_state| - state = table_state.state - - # old_column_index is a String! - - # Remove column from ColReorder, columns, length entries - state['columns'].delete(old_column_index) - state['columns'].keys.each do |index| - if index.to_i > old_column_index.to_i - state['columns'][(index.to_i - 1).to_s] = - state['columns'].delete(index.to_s) - end - end - - state['ColReorder'].delete(old_column_index) - state['ColReorder'].map! do |index| - if index.to_i > old_column_index.to_i - (index.to_i - 1).to_s - else - index - end - end - state['length'] = (state['length'].to_i - 1).to_s - state['time'] = Time.new.to_i.to_s - table_state.save - end - end end diff --git a/app/services/repository_datatable_service.rb b/app/services/repository_datatable_service.rb index 705975eac..37af12f2f 100644 --- a/app/services/repository_datatable_service.rb +++ b/app/services/repository_datatable_service.rb @@ -95,8 +95,8 @@ class RepositoryDatatableService direction == column_obj[:dir].upcase end || 'ASC' column_index = column_obj[:column] - col_order = RepositoryTableState.load_state(@user, @repository) - .state['ColReorder'] + service = RepositoryTableStateService.new(@user, @repository) + col_order = service.load_state.state['ColReorder'] column_id = col_order[column_index].to_i if sortable_columns[column_id - 1] == 'assigned' diff --git a/app/services/repository_table_state_column_update_service.rb b/app/services/repository_table_state_column_update_service.rb new file mode 100644 index 000000000..0c7d853dd --- /dev/null +++ b/app/services/repository_table_state_column_update_service.rb @@ -0,0 +1,56 @@ +class RepositoryTableStateColumnUpdateService + # We're using Constants::REPOSITORY_TABLE_DEFAULT_STATE as a reference for + # default table state; this Ruby Hash makes heavy use of Ruby symbols + # notation; HOWEVER, the state that is saved on the RepositoryTableState + # record, has EVERYTHING (booleans, symbols, keys, ...) saved as Strings. + + def update_states_with_new_column(new_column) + RepositoryTableState.where( + repository: new_column.repository + ).find_each do |table_state| + state = table_state.state + index = state['columns'].count + + # Add new columns, ColReorder, length entries + state['columns'][index.to_s] = + HashUtil.deep_stringify_keys_and_values( + Constants::REPOSITORY_TABLE_STATE_CUSTOM_COLUMN_TEMPLATE + ) + state['ColReorder'] << index.to_s + state['length'] = (index + 1).to_s + state['time'] = Time.new.to_i.to_s + table_state.save + end + end + + def update_states_with_removed_column(repository, old_column_index) + RepositoryTableState.where( + repository: repository + ).find_each do |table_state| + state = table_state.state + + # old_column_index is a String! + + # Remove column from ColReorder, columns, length entries + state['columns'].delete(old_column_index) + state['columns'].keys.each do |index| + if index.to_i > old_column_index.to_i + state['columns'][(index.to_i - 1).to_s] = + state['columns'].delete(index.to_s) + end + end + + state['ColReorder'].delete(old_column_index) + state['ColReorder'].map! do |index| + if index.to_i > old_column_index.to_i + (index.to_i - 1).to_s + else + index + end + end + state['length'] = (state['length'].to_i - 1).to_s + state['time'] = Time.new.to_i.to_s + table_state.save + end + end +end \ No newline at end of file diff --git a/app/services/repository_table_state_service.rb b/app/services/repository_table_state_service.rb new file mode 100644 index 000000000..4fc78334c --- /dev/null +++ b/app/services/repository_table_state_service.rb @@ -0,0 +1,61 @@ +class RepositoryTableStateService + + attr_reader :user, :repository + + def initialize(user, repository) + @user = user + @repository = repository + end + + # We're using Constants::REPOSITORY_TABLE_DEFAULT_STATE as a reference for + # default table state; this Ruby Hash makes heavy use of Ruby symbols + # notation; HOWEVER, the state that is saved on the RepositoryTableState + # record, has EVERYTHING (booleans, symbols, keys, ...) saved as Strings. + + def load_state + state = RepositoryTableState.where(user: @user, repository: @repository).take + if state.blank? + state = self.create_default_state + end + state + end + + def update_state(state) + self.load_state + .update(state: state) + end + + def create_default_state + # Destroy any state object before recreating a new one + RepositoryTableState.where(user: @user, repository: @repository).destroy_all + + return RepositoryTableState.create( + user: @user, + repository: @repository, + state: generate_default_state + ) + end + + private + + def generate_default_state + default_columns_num = + Constants::REPOSITORY_TABLE_DEFAULT_STATE[:length] + + # This state should be strings-only + state = HashUtil.deep_stringify_keys_and_values( + Constants::REPOSITORY_TABLE_DEFAULT_STATE + ) + repository.repository_columns.each_with_index do |_, index| + real_index = default_columns_num + index + state['columns'][real_index.to_s] = + HashUtil.deep_stringify_keys_and_values( + Constants::REPOSITORY_TABLE_STATE_CUSTOM_COLUMN_TEMPLATE + ) + state['ColReorder'] << real_index.to_s + end + state['length'] = state['columns'].length.to_s + state['time'] = Time.new.to_i.to_s + state + end +end diff --git a/config/initializers/constants.rb b/config/initializers/constants.rb index e2f85e9e1..8019da32a 100644 --- a/config/initializers/constants.rb +++ b/config/initializers/constants.rb @@ -846,7 +846,7 @@ class Constants time: 0, start: 0, length: 6, - order: { 0 => [[2, 'asc']] }, # Default sorting by 'ID' column + order: { 0 => [2, 'asc'] }, # Default sorting by 'ID' column search: { search: '', smart: true, regex: false, From caab0cf55c8157d410132bccc0c24f748f5b59f0 Mon Sep 17 00:00:00 2001 From: Luka Murn Date: Fri, 20 Apr 2018 09:22:23 +0200 Subject: [PATCH 4/8] Update the ordering upon table load --- .../repositories/repository_datatable.js.erb | 30 +++++++++++++------ app/helpers/repository_datatable_helper.rb | 17 ++++++++++- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/repositories/repository_datatable.js.erb b/app/assets/javascripts/repositories/repository_datatable.js.erb index 3310146c0..c0d8df842 100644 --- a/app/assets/javascripts/repositories/repository_datatable.js.erb +++ b/app/assets/javascripts/repositories/repository_datatable.js.erb @@ -1,5 +1,7 @@ //= require jquery-ui/widgets/sortable +<% environment.context_class.instance_eval { include RepositoryDatatableHelper } %> + var RepositoryDatatable = (function(global) { 'use strict'; @@ -101,16 +103,12 @@ var RepositoryDatatable = (function(global) { $(row).addClass('selected'); } }, - // Next 2 options are provided by server-side default state - // (and get overriden once state load from server kicks in) - order: <%= Constants::REPOSITORY_TABLE_DEFAULT_STATE[:order][0].to_s %>, + <% # Next 2 options are provided by server-side default state + # (and get overriden once state load from server kicks in) %> + order: <%= default_table_order_as_js_array %>, columns: (function() { var numOfColumns = $(TABLE_ID).data('num-columns'); - var columns = - <%= Constants::REPOSITORY_TABLE_DEFAULT_STATE[:columns].keys.sort.map do |k| - col = Constants::REPOSITORY_TABLE_DEFAULT_STATE[:columns][k] - col.slice(:visible, :searchable) - end.to_json %>; + var columns = <%= default_table_columns %>; for (var i = 0; i < numOfColumns; i++) { if (columns[i] == undefined) { // This should only occur for custom columns @@ -153,6 +151,12 @@ var RepositoryDatatable = (function(global) { type: 'POST', success: function(json) { myData = json.state; + + // Fix the order - convert it from index-keyed JS object that + // is returned from the server state into true JS array; + // e.g. {0: [2, 'asc'], 1: [3, 'desc']} + // is converted into [[2, 'asc'], [3, 'desc']] + myData.order = _.toArray(myData.order); } }); return myData; @@ -173,10 +177,11 @@ var RepositoryDatatable = (function(global) { loadFirstTime = false; }, fnInitComplete: function(oSettings) { - // Reload correct column order and visibility (if you refresh page) // First two columns are fixed TABLE.column(0).visible(true); TABLE.column(1).visible(true); + + // Reload correct column order and visibility (if you refresh page) for (var i = 2; i < TABLE.columns()[0].length; i++) { var visibility = false; if (myData.columns && myData.columns[i]) { @@ -188,6 +193,13 @@ var RepositoryDatatable = (function(global) { TABLE.column(i).visible(visibility); TABLE.setColumnSearchable(i, visibility); } + + // Re-order table as per loaded state + if (myData.order) { + TABLE.order(myData.order); + TABLE.draw(); + } + // Datatables triggers this action about 3 times // sometimes on the first iteration the oSettings._colReorder is null // and the fnOrder rises an error that breaks the table diff --git a/app/helpers/repository_datatable_helper.rb b/app/helpers/repository_datatable_helper.rb index d5e858d6c..70d135e48 100644 --- a/app/helpers/repository_datatable_helper.rb +++ b/app/helpers/repository_datatable_helper.rb @@ -65,4 +65,19 @@ module RepositoryDatatableHelper can_create_repositories?(team) || can_manage_repository_rows?(team) end -end + + # The order must be converted from Ruby Hash into a JS array - + # because arrays in JS are in truth regular JS objects with indexes as keys + def default_table_order_as_js_array + Constants::REPOSITORY_TABLE_DEFAULT_STATE[:order].keys.sort.map do |k| + Constants::REPOSITORY_TABLE_DEFAULT_STATE[:order][k] + end.to_s + end + + def default_table_columns + Constants::REPOSITORY_TABLE_DEFAULT_STATE[:columns].keys.sort.map do |k| + col = Constants::REPOSITORY_TABLE_DEFAULT_STATE[:columns][k] + col.slice(:visible, :searchable) + end.to_json + end +end \ No newline at end of file From 68caf56ec278914d4bd6d8d38cacedc4a0de84dd Mon Sep 17 00:00:00 2001 From: Luka Murn Date: Fri, 20 Apr 2018 09:47:59 +0200 Subject: [PATCH 5/8] Add comment to constants --- config/initializers/constants.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/initializers/constants.rb b/config/initializers/constants.rb index 8019da32a..d11d7ba72 100644 --- a/config/initializers/constants.rb +++ b/config/initializers/constants.rb @@ -866,6 +866,8 @@ class Constants } end REPOSITORY_TABLE_DEFAULT_STATE.freeze + # For default custom column template, any searchable default + # column can be reused REPOSITORY_TABLE_STATE_CUSTOM_COLUMN_TEMPLATE = REPOSITORY_TABLE_DEFAULT_STATE[:columns][1].deep_dup .freeze From d023db16af136a22ddeb2803421e7594308b7b0a Mon Sep 17 00:00:00 2001 From: Luka Murn Date: Fri, 20 Apr 2018 11:44:43 +0200 Subject: [PATCH 6/8] Update the destroy_column action so it properly updates all states --- .../repository_columns_controller.rb | 12 ++------ app/models/repository_column.rb | 28 +++++++++++++++++-- ...itory_table_state_column_update_service.rb | 16 +++++++++-- .../_delete_column_modal_body.html.erb | 3 +- 4 files changed, 43 insertions(+), 16 deletions(-) diff --git a/app/controllers/repository_columns_controller.rb b/app/controllers/repository_columns_controller.rb index bd3018856..d32bf1b34 100644 --- a/app/controllers/repository_columns_controller.rb +++ b/app/controllers/repository_columns_controller.rb @@ -110,8 +110,7 @@ class RepositoryColumnsController < ApplicationController format.json do render json: { html: render_to_string( - partial: 'repository_columns/delete_column_modal_body.html.erb', - locals: { column_index: params[:column_index] } + partial: 'repository_columns/delete_column_modal_body.html.erb' ) } end @@ -119,19 +118,14 @@ class RepositoryColumnsController < ApplicationController end def destroy - @del_repository_column = @repository_column.dup column_id = @repository_column.id + column_name = @repository_column.name respond_to do |format| format.json do if @repository_column.destroy - service = RepositoryTableStateColumnUpdateService.new - service.update_states_with_removed_column( - @del_repository_column.repository, - params[:repository_column][:column_index] - ) render json: { message: t('libraries.repository_columns.destroy.success_flash', - name: @del_repository_column.name), + name: column_name), id: column_id, status: :ok } diff --git a/app/models/repository_column.rb b/app/models/repository_column.rb index 9f92ec08d..fe4caca43 100644 --- a/app/models/repository_column.rb +++ b/app/models/repository_column.rb @@ -19,13 +19,35 @@ class RepositoryColumn < ApplicationRecord validates :repository, presence: true validates :data_type, presence: true - after_create :update_repository_table_states + after_create :update_repository_table_states_with_new_column + around_destroy :update_repository_table_states_with_removed_column scope :list_type, -> { where(data_type: 'RepositoryListValue') } - def update_repository_table_states + def update_repository_table_states_with_new_column service = RepositoryTableStateColumnUpdateService.new - service.update_states_with_new_column(self) + service.update_states_with_new_column(repository) + end + + def update_repository_table_states_with_removed_column + # Calculate old_column_index - this can only be done before + # record is deleted when we still have its index + old_column_index = ( + Constants::REPOSITORY_TABLE_DEFAULT_STATE[:length] + + repository.repository_columns + .order(id: :asc) + .pluck(:id) + .index(id) + ).to_s + + # Perform the destroy itself + yield + + # Update repository table states + service = RepositoryTableStateColumnUpdateService.new + service.update_states_with_removed_column( + repository, old_column_index + ) end def importable? diff --git a/app/services/repository_table_state_column_update_service.rb b/app/services/repository_table_state_column_update_service.rb index 0c7d853dd..f66b2a4b6 100644 --- a/app/services/repository_table_state_column_update_service.rb +++ b/app/services/repository_table_state_column_update_service.rb @@ -4,9 +4,9 @@ class RepositoryTableStateColumnUpdateService # notation; HOWEVER, the state that is saved on the RepositoryTableState # record, has EVERYTHING (booleans, symbols, keys, ...) saved as Strings. - def update_states_with_new_column(new_column) + def update_states_with_new_column(repository) RepositoryTableState.where( - repository: new_column.repository + repository: repository ).find_each do |table_state| state = table_state.state index = state['columns'].count @@ -24,6 +24,8 @@ class RepositoryTableStateColumnUpdateService end def update_states_with_removed_column(repository, old_column_index) + raise ArgumentError, 'old_column_index is empty' if old_column_index.blank? + RepositoryTableState.where( repository: repository ).find_each do |table_state| @@ -48,6 +50,16 @@ class RepositoryTableStateColumnUpdateService index end end + + state['order'].reject! { |k, v| v[0] == old_column_index } + if state['order'].empty? + # Fallback to default order if user had table ordered by + # the deleted column + state['order'] = HashUtil.deep_stringify_keys_and_values( + Constants::REPOSITORY_TABLE_DEFAULT_STATE[:order] + ) + end + state['length'] = (state['length'].to_i - 1).to_s state['time'] = Time.new.to_i.to_s table_state.save diff --git a/app/views/repository_columns/_delete_column_modal_body.html.erb b/app/views/repository_columns/_delete_column_modal_body.html.erb index ce8f42d73..9e12b4c01 100644 --- a/app/views/repository_columns/_delete_column_modal_body.html.erb +++ b/app/views/repository_columns/_delete_column_modal_body.html.erb @@ -1,11 +1,10 @@ <%= bootstrap_form_for @repository_column, url: repository_repository_column_path(@repository, @repository_column), - remote: true, + remote: true, method: :delete, data: { role: "destroy-repository-column-form", id: @repository_column.id} do |f| %> - <%= f.hidden_field :column_index, value: column_index %>

<%= t("repositories.modal_delete_column.message", column: @repository_column.name) %>