Merge pull request #1111 from Ducz0r/lm-sci-2276-fix

Update the code for repository table states so it (hopefully) causes less errors [SCI-2276]

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 "<input class='repository-row-selector' type='checkbox'>";
         }
       }, {
+        // Assigned column is not searchable
         targets: 1,
         searchable: false,
         orderable: true,
         sWidth: '1%'
       }, {
-        targets: 2,
+        // Name column is clickable
-        searchable: true,
+        targets: 3,
-        orderable: true,
+        render: function(data, type, row) {
-        sWidth: '1%'
+          return "<a href='" + row.recordInfoUrl + "'" +
-      }, {
+                 "class='record-info-link'>" + data + '</a>';
-       targets: 3,
+        }
-       render: function(data, type, row) {
+      }],
-         return "<a href='" + row.recordInfoUrl + "'" +
-                "class='record-info-link'>" + data + '</a>';
-       }
-     }],
       rowCallback: function(row, data) {
         // Get row ID
         var rowId = data.DT_RowId;
@@ -104,18 +101,19 @@ 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: <%= default_table_order_as_js_array %>,
       columns: (function() {
         var numOfColumns = $(TABLE_ID).data('num-columns');
-        var columns = [];
+        var columns = <%= default_table_columns %>;
         for (var i = 0; i < numOfColumns; i++) {
-          var visible = (i <= 4);
+          if (columns[i] == undefined) {
-          var searchable = (i > 0 && i <= 4);
+            // This should only occur for custom columns
-          columns.push({
+            columns[i] = { visible: true, searchable: true };
-            data: String(i),
+          }
-            defaultContent: '',
+          columns[i].data = String(i);
-            visible: visible,
+          columns[i].defaultContent = '';
-            searchable: searchable
-          });
         }
         return columns;
       })(),
@@ -133,7 +131,6 @@ var RepositoryDatatable = (function(global) {
                                  rowsSelected.length +
                                  ' entries selected)');
         initRowSelection();
-        initHeaderTooltip();
       },
       preDrawCallback: function() {
         animateSpinner(this);
@@ -152,6 +149,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;
@@ -170,16 +173,16 @@ var RepositoryDatatable = (function(global) {
           type: 'POST'
         });
         loadFirstTime = false;
-        initHeaderTooltip();
       },
       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[i]) {
+          if (myData.columns && myData.columns[i]) {
             visibility = myData.columns[i].visible;
           }
           if (typeof (visibility) === 'string') {
@@ -188,6 +191,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
@@ -196,10 +206,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 +390,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 +1028,6 @@ var RepositoryDatatable = (function(global) {
     currentMode = 'editMode';
     // Table specific stuff
     TABLE.button(0).enable(false);
-    initHeaderTooltip();
   }
 
   /*
@@ -1146,7 +1113,6 @@ var RepositoryDatatable = (function(global) {
         li.removeClass('col-invisible');
         column.visible(true);
         TABLE.setColumnSearchable(column.index(), true);
-        initHeaderTooltip();
       }
 
       // Re-filter/search if neccesary

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',
+            partial: 'repository_columns/delete_column_modal_body.html.erb'
-            locals: { column_index: params[:column_index] }
           )
         }
       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
-          RepositoryTableState.update_state(
-            @del_repository_column,
-            params[:repository_column][:column_index],
-            current_user
-          )
           render json: {
             message: t('libraries.repository_columns.destroy.success_flash',
-                       name: @del_repository_column.name),
+                       name: column_name),
             id: column_id,
             status: :ok
           }

app/controllers/user_repositories_controller.rb

@@ -2,15 +2,8 @@ class UserRepositoriesController < ApplicationController
   before_action :load_vars
 
   def save_table_state
-    table_state = RepositoryTableState.where(user: current_user,
+    service = RepositoryTableStateService.new(current_user, @repository)
-                                             repository: @repository).first
+    service.update_state(params[:state])
-    if table_state
-      table_state.update(state: params[:state])
-    else
-      RepositoryTableState.create(user: current_user,
-                                  repository: @repository,
-                                  state: params[:state])
-    end
     respond_to do |format|
       format.json do
         render json: {
@@ -21,13 +14,13 @@ class UserRepositoriesController < ApplicationController
   end
 
   def load_table_state
-    table_state = RepositoryTableState.load_state(current_user,
+    service = RepositoryTableStateService.new(current_user, @repository)
-                                                  @repository).first
+    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

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

app/models/repository_column.rb

@@ -19,12 +19,35 @@ class RepositoryColumn < ApplicationRecord
   validates :repository, presence: true
   validates :data_type, presence: true
 
-  after_create :update_repository_table_state
+  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_state
+  def update_repository_table_states_with_new_column
-    RepositoryTableState.update_state(self, nil, created_by)
+    service = RepositoryTableStateColumnUpdateService.new
+    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?

app/models/repository_table_state.rb

@@ -3,66 +3,4 @@ class RepositoryTableState < ApplicationRecord
   belongs_to :repository, inverse_of: :repository_table_states, optional: true
 
   validates :user, :repository, presence: true
-
-  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)
-    end
-    table_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
-  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
-    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)
-    end
-    RepositoryTableState.create(user: user,
-                                repository: repository,
-                                state: repository_state)
-  end
 end

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
+    service = RepositoryTableStateService.new(@user, @repository)
-                           .find_by_user_id(@user.id)
+    col_order = service.load_state.state['ColReorder']
-                           .state['ColReorder']
     column_id = col_order[column_index].to_i
 
     if sortable_columns[column_id - 1] == 'assigned'

app/services/repository_table_state_column_update_service.rb

@@ -0,0 +1,71 @@
+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(repository)
+    raise ArgumentError, 'repository is empty' if repository.blank?
+
+    RepositoryTableState.where(
+      repository: 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)
+    raise ArgumentError, 'repository is empty' if repository.blank?
+    raise ArgumentError, 'old_column_index is empty' if old_column_index.blank?
+
+    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['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
+    end
+  end
+end

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

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

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 %>
   <p><%= t("repositories.modal_delete_column.message", column: @repository_column.name) %></p>
   <div class="alert alert-danger" role="alert">
     <span class="glyphicon glyphicon-exclamation-sign"></span>

config/initializers/constants.rb

@@ -843,28 +843,34 @@ class Constants
 
   # Repository default table state
   REPOSITORY_TABLE_DEFAULT_STATE = {
-    'time' => 0,
+    time: 0,
-    'start' => 0,
+    start: 0,
-    'length' => 6,
+    length: 6,
-    'order' => [[3, 'desc']],
+    order: { 0 => [2, 'asc'] }, # Default sorting by 'ID' column
-    'search' => { 'search' => '',
+    search: { search: '',
-                  'smart' => true,
+              smart: true,
-                  'regex' => false,
+              regex: false,
-                  'caseInsensitive' => true },
+              caseInsensitive: true },
-    'columns' => [],
+    columns: {},
-    'assigned' => 'assigned',
+    assigned: 'assigned',
-    'ColReorder' => [*0..5]
+    ColReorder: [*0..5]
   }
-  6.times do
+  6.times do |i|
-    REPOSITORY_TABLE_DEFAULT_STATE['columns'] << {
+    REPOSITORY_TABLE_DEFAULT_STATE[:columns][i] = {
-      'visible' => true,
+      visible: true,
-      'search' => { 'search' => '',
+      searchable: i >= 1, # Checkboxes column is not searchable
-                    'smart' => true,
+      search: { search: '',
-                    'regex' => false,
+                smart: true,
-                    'caseInsensitive' => true }
+                regex: false,
+                caseInsensitive: true }
     }
   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
 
   EXPORTABLE_ZIP_EXPIRATION_DAYS = 7
 

config/initializers/helpers_in_assets.rb

@@ -0,0 +1,7 @@
+# This code will include the listed helpers in all the assets
+Rails.application.config.assets.configure do |env|
+  env.context_class.class_eval do
+    # This is required for repository_datatable.js.erb
+    include RepositoryDatatableHelper
+  end
+end