Merge pull request #2277 from urbanrotnik/ur-sci-4183-refactor-statuses-for-new-dropdown

Refactor statuses for new dropdown [SCI-4183]
2024-09-21 15:36:22 +08:00 · 2019-12-24 08:14:17 +01:00 · 2019-12-24 08:14:17 +01:00 · 4e39547ae8
parent 8929b9f51e 356ba6572d
commit 4e39547ae8
12 changed files with 134 additions and 120 deletions
--- a/.rubocop.yml
+++ b/.rubocop.yml
@ -83,7 +83,7 @@ Naming/FileName:
  Enabled: false
  Exclude: []

-Layout/IndentFirstArgument:
+Layout/FirstArgumentIndentation:
  EnforcedStyle: consistent

 Style/For:
@ -364,14 +364,6 @@ Metrics/ModuleLength:
 Metrics/CyclomaticComplexity:
  Enabled: false

-Metrics/LineLength:
-  Max: 120
-  AllowHeredoc: true
-  AllowURI: true
-  URISchemes:
-    - http
-    - https
-
 Metrics/MethodLength:
  Enabled: false

@ -392,6 +384,14 @@ Layout/EndAlignment:
 Layout/DefEndAlignment:
  EnforcedStyleAlignWith: start_of_line

+Layout/LineLength:
+  Max: 120
+  AllowHeredoc: true
+  AllowURI: true
+  URISchemes:
+    - http
+    - https
+
 ##################### Lint #####################################

 Lint/AssignmentInCondition:
--- a/2
+++ b/2
@ -122,7 +122,7 @@ group :development, :test do
  gem 'pry-rails'
  gem 'rails-controller-testing'
  gem 'rspec-rails', '>= 4.0.0.beta2'
-  gem 'rubocop', '>= 0.59.0', require: false
+  gem 'rubocop', '>= 0.75.0', require: false
  gem 'rubocop-performance'
  gem 'rubocop-rails'
  gem 'timecop'
--- a/Gemfile.lock
+++ b/Gemfile.lock
@ -279,7 +279,7 @@ GEM
      mini_magick (>= 4.9.5, < 5)
      ruby-vips (>= 2.0.13, < 3)
    iniparse (1.4.4)
-    jaro_winkler (1.5.3)
+    jaro_winkler (1.5.4)
    jbuilder (2.9.1)
      activesupport (>= 4.2.0)
    jmespath (1.4.0)
@ -382,8 +382,8 @@ GEM
      mime-types
      mimemagic (~> 0.3.0)
      terrapin (~> 0.6.0)
-    parallel (1.17.0)
-    parser (2.6.4.0)
+    parallel (1.19.1)
+    parser (2.6.5.0)
      ast (~> 2.4.0)
    pg (1.1.4)
    pg_search (2.3.0)
@ -481,16 +481,16 @@ GEM
      rspec-mocks (~> 3.8)
      rspec-support (~> 3.8)
    rspec-support (3.8.2)
-    rubocop (0.74.0)
+    rubocop (0.78.0)
      jaro_winkler (~> 1.5.1)
      parallel (~> 1.10)
      parser (>= 2.6)
      rainbow (>= 2.2.2, < 4.0)
      ruby-progressbar (~> 1.7)
      unicode-display_width (>= 1.4.0, < 1.7)
-    rubocop-performance (1.4.1)
+    rubocop-performance (1.5.1)
      rubocop (>= 0.71.0)
-    rubocop-rails (2.3.2)
+    rubocop-rails (2.4.0)
      rack (>= 1.1)
      rubocop (>= 0.72.0)
    ruby-graphviz (1.2.4)
@ -667,7 +667,7 @@ DEPENDENCIES
  rgl
  roo (~> 2.8.2)
  rspec-rails (>= 4.0.0.beta2)
-  rubocop (>= 0.59.0)
+  rubocop (>= 0.75.0)
  rubocop-performance
  rubocop-rails
  ruby-graphviz (~> 1.2)
--- a/app/assets/javascripts/repositories/renderers/columns/status.js
+++ b/app/assets/javascripts/repositories/renderers/columns/status.js
@ -1,52 +1,46 @@
 /* global dropdownSelector */
 /* eslint-disable no-unused-vars */

-var Status = (function() {
-  function statusItemDropdown(options, currentValueId, columnId, formId) {
-    var html = `<select class="form-control selectpicker repository-dropdown"
-                data-abs-min-length="2" data-live-search="true" from="${formId}"
-                data-container="body" column_id="${columnId}">
-                <option value="-1"></option>`;
-    $.each(options, function(index, value) {
-      var selected = (parseInt(currentValueId, 10) === value[0]) ? 'selected' : '';
-      html += `<option value="${value[0]}" ${selected}>${value[2]} ${value[1]}</option>`;
-    });
-    html += '</select>';
-    return html;
+var StatusColumnHelper = (function() {
+  function statusSelect(select, url, value) {
+    var selectedOption = '';
+    if (value && value.value) {
+      selectedOption = `<option value="${value.value}">${value.label}</option>`;
+    }
+
+    return $(`<select 
+              id="${select}"
+              data-placeholder = "Select option..."
+              data-ajax-url = "${url}"
+            >${selectedOption}</select>`);
  }

-  function initialStatusItemsRequest(columnId, currentValue, formId, url) {
-    var massageResponse = [];
-    $.ajax({
-      url: url,
-      type: 'GET',
-      dataType: 'json',
-      async: false,
-      data: {
-        column_id: columnId
-      }
-    }).done(function(data) {
-      $.each(data.status_items, function(index, el) {
-        massageResponse.push([el.id, el.status, el.icon]);
-      });
-    });
-    return statusItemDropdown(massageResponse, currentValue, columnId, formId);
+  function statusHiddenField(formId, columnId, value) {
+    var originalValue = value ? value.value : '';
+    return $(`<input form="${formId}"
+             type="hidden"
+             name="repository_cells[${columnId}]"
+             value="${originalValue}"
+             data-type="RepositoryStatusValue">`);
  }

-  function initStatusSelectPicker($select, $hiddenField) {
-    dropdownSelector.init($select, {
-      noEmptyOption: true,
+  function initialStatusEditMode(formId, columnId, cell, value = null) {
+    var select = 'status-list-' + columnId;
+    var listUrl = $('.repository-column#' + columnId).data('items-url');
+    var $select = statusSelect(select, listUrl, value);
+    var $hiddenField = statusHiddenField(formId, columnId, value);
+    cell.html($select).append($hiddenField);
+    dropdownSelector.init('#' + select, {
      singleSelect: true,
-      closeOnSelect: true,
      selectAppearance: 'simple',
      onChange: function() {
-        $hiddenField.val(dropdownSelector.getValues($select));
+        var values = dropdownSelector.getValues('#' + select);
+        $hiddenField.val(values);
      }
    });
  }

  return {
-    initialStatusItemsRequest: initialStatusItemsRequest,
-    initStatusSelectPicker: initStatusSelectPicker
+    initialStatusEditMode: initialStatusEditMode
  };
 }());
--- a/app/assets/javascripts/repositories/renderers/edit_renderers.js
+++ b/app/assets/javascripts/repositories/renderers/edit_renderers.js
@ -1,5 +1,5 @@
 /*
-global ListColumnHelper ChecklistColumnHelper Status SmartAnnotation I18n
+global ListColumnHelper ChecklistColumnHelper StatusColumnHelper SmartAnnotation I18n
 GLOBAL_CONSTANTS DateTimeHelper
 */

@ -76,19 +76,17 @@ $.fn.dataTable.render.editRepositoryListValue = function(formId, columnId, cell)

 $.fn.dataTable.render.editRepositoryStatusValue = function(formId, columnId, cell) {
  let $cell = $(cell.node());
-  let currentValueId = $cell.find('.status-label').attr('data-value-id');
+  var currentElement = $cell.find('.status-label');
+  var iconElement = $cell.find('.repository-status-value-icon');
+  var currentValue = null;
+  if (currentElement.length) {
+    currentValue = {
+      value: currentElement.attr('data-value-id'),
+      label: iconElement.text() + ' ' + currentElement.text()
+    };
+  }

-  let url = $cell.closest('table').data('status-items-path');
-  let hiddenField = `
-    <input form="${formId}"
-           type="hidden"
-           name="repository_cells[${columnId}]"
-           value=""
-           data-type="RepositoryStatusValue">`;
-
-  $cell.html(hiddenField + Status.initialStatusItemsRequest(columnId, currentValueId, formId, url));
-
-  Status.initStatusSelectPicker($cell.find('select'), $cell.find(`[name='repository_cells[${columnId}]']`));
+  StatusColumnHelper.initialStatusEditMode(formId, columnId, $cell, currentValue);
 };

 $.fn.dataTable.render.editRepositoryDateTimeValue = function(formId, columnId, cell) {
--- a/app/assets/javascripts/repositories/renderers/new_renderers.js
+++ b/app/assets/javascripts/repositories/renderers/new_renderers.js
@ -1,5 +1,5 @@
 /*
-global ListColumnHelper ChecklistColumnHelper Status SmartAnnotation I18n
+global ListColumnHelper ChecklistColumnHelper StatusColumnHelper SmartAnnotation I18n
 GLOBAL_CONSTANTS DateTimeHelper
 */

@ -55,17 +55,7 @@ $.fn.dataTable.render.newRepositoryListValue = function(formId, columnId, $cell)
 };

 $.fn.dataTable.render.newRepositoryStatusValue = function(formId, columnId, $cell) {
-  let url = $cell.closest('table').data('status-items-path');
-  let hiddenField = `
-    <input form="${formId}"
-           type="hidden"
-           name="repository_cells[${columnId}]"
-           value=""
-           data-type="RepositoryStatusValue">`;
-
-  $cell.html(hiddenField + Status.initialStatusItemsRequest(columnId, '', formId, url));
-
-  Status.initStatusSelectPicker($cell.find('select'), $cell.find(`[name='repository_cells[${columnId}]']`));
+  StatusColumnHelper.initialStatusEditMode(formId, columnId, $cell);
 };

 $.fn.dataTable.render.newRepositoryChecklistValue = function(formId, columnId, $cell) {
--- a/app/controllers/repository_columns/status_columns_controller.rb
+++ b/app/controllers/repository_columns/status_columns_controller.rb
@ -3,9 +3,9 @@
 module RepositoryColumns
  class StatusColumnsController < BaseColumnsController
    include InputSanitizeHelper
-    before_action :load_column, only: %i(update destroy)
+    before_action :load_column, only: %i(update destroy items)
    before_action :check_create_permissions, only: :create
-    before_action :check_manage_permissions, only: %i(update destroy)
+    before_action :check_manage_permissions, only: %i(update destroy items)

    def create
      service = RepositoryColumns::CreateColumnService
@ -45,8 +45,22 @@ module RepositoryColumns
      end
    end

+    def items
+      column_status_items = @repository_column.repository_status_items
+                                              .where('status ILIKE ?',
+                                                     "%#{search_params[:query]}%")
+                                              .select(:id, :icon, :status)
+
+      render json: column_status_items
+        .map { |i| { value: i.id, label: "#{i.icon} #{escape_input(i.status)}" } }, status: :ok
+    end
+
    private

+    def search_params
+      params.permit(:query, :column_id)
+    end
+
    def repository_column_params
      params.require(:repository_column).permit(:name, repository_status_items_attributes: %i(status icon))
    end
--- a/app/controllers/repository_status_items_controller.rb
+++ b/app/controllers/repository_status_items_controller.rb
@ -1,27 +0,0 @@
-# frozen_string_literal: true
-
-class RepositoryStatusItemsController < ApplicationController
-  before_action :load_vars
-
-  def search
-    status_items = @repository_column.repository_status_items
-                                     .where('status ILIKE ?', "%#{search_params[:q]}%")
-                                     .limit(Constants::SEARCH_LIMIT)
-                                     .select(:id, :icon, :status)
-
-    render json: { status_items: status_items }, status: :ok
-  end
-
-  private
-
-  def search_params
-    params.permit(:q, :column_id)
-  end
-
-  def load_vars
-    @repository_column = RepositoryColumn.find_by(id: search_params[:column_id])
-    repository = @repository_column.repository if @repository_column
-    render_404 and return unless @repository_column&.data_type == 'RepositoryStatusValue'
-    render_403 unless can_manage_repository_rows?(repository)
-  end
-end
--- a/app/services/repository_rows/update_repository_row_service.rb
+++ b/app/services/repository_rows/update_repository_row_service.rb
@ -29,10 +29,12 @@ module RepositoryRows
            cell.destroy!
            @record_updated = true
            next
-          elsif cell.blank?
+          elsif cell.blank? && value.present?
            RepositoryCell.create_with_value!(@repository_row, column, value, @user)
            @record_updated = true
            next
+          elsif cell.blank? && value.blank?
+            next
          end

          if cell.value.data_changed?(value)
--- a/app/views/repositories/_repository_table.html.erb
+++ b/app/views/repositories/_repository_table.html.erb
@ -19,8 +19,6 @@
    data-columns-changed="<%= I18n.t('repositories.columns_changed') %>"
    data-default-order="<%= default_table_order_as_js_array %>"
    data-default-table-columns="<%= default_table_columns %>"
-    data-checklist-items-path="<%= Rails.application.routes.url_helpers.repository_list_items_path %>"
-    data-status-items-path="<%= Rails.application.routes.url_helpers.repository_status_items_path %>"
    data-editable="<%= can_manage_repository_rows?(repository) %>">
    <thead>
      <tr>
@ -38,8 +36,9 @@
            <% column.metadata.each do |k, v| %>
              <%= "data-metadata-#{k}=#{v}" %>
            <% end %>
-            <%= "data-items-url=#{items_repository_repository_columns_checklist_column_path(repository, column)}" if column.data_type == 'RepositoryChecklistValue' %>
-            <%= "data-items-url=#{items_repository_repository_columns_list_column_path(repository, column)}" if column.data_type == 'RepositoryListValue' %>
+            <%= "data-items-url=#{items_repository_repository_columns_checklist_column_path(repository, column)}" if column.repository_checklist_value? %>
+            <%= "data-items-url=#{items_repository_repository_columns_list_column_path(repository, column)}" if column.repository_list_value? %>
+            <%= "data-items-url=#{items_repository_repository_columns_status_column_path(repository, column)}" if column.repository_status_value? %>
          >
            <%= display_tooltip(column.name) %>
          </th>
--- a/config/routes.rb
+++ b/config/routes.rb
@ -581,31 +581,30 @@ Rails.application.routes.draw do
      namespace :repository_columns do
        resources :text_columns, only: %i(create update destroy)
        resources :number_columns, only: %i(create update destroy)
+        resources :asset_columns, only: %i(create update destroy)
+        resources :date_columns, only: %i(create update destroy)
+        resources :date_time_columns, only: %i(create update destroy)
        resources :list_columns, only: %i(create update destroy) do
          member do
            get 'items'
          end
        end
-        resources :asset_columns, only: %i(create update destroy)
-        resources :date_columns, only: %i(create update destroy)
-        resources :status_columns, only: %i(create update destroy)
-        resources :date_time_columns, only: %i(create update destroy)
        resources :checklist_columns, only: %i(create update destroy) do
          member do
            get 'items'
          end
        end
+        resources :status_columns, only: %i(create update destroy) do
+          member do
+            get 'items'
+          end
+        end
      end
    end

    post 'available_rows', to: 'repository_rows#available_rows',
                           defaults: { format: 'json' }

-    post 'repository_list_items', to: 'repository_list_items#search',
-                                  defaults: { format: 'json' }
-
-    get 'repository_status_items', to: 'repository_status_items#search'
-
    get 'repository_rows/:id', to: 'repository_rows#show',
                               as: :repository_row,
                               defaults: { format: 'json' }
--- a/spec/controllers/repository_columns/status_columns_controller_spec.rb
+++ b/spec/controllers/repository_columns/status_columns_controller_spec.rb
@ -231,4 +231,49 @@ RSpec.describe RepositoryColumns::StatusColumnsController, type: :controller do
      end
    end
  end
+
+  describe 'GET repository_status_column, #items' do
+    let(:action) { get :items, params: params }
+
+    let(:params) do
+      {
+        repository_id: repository.id,
+        id: repository_column.id
+      }
+    end
+
+    it 'respons with status 200' do
+      action
+
+      expect(response).to(have_http_status(200))
+    end
+
+    context 'when column is not found' do
+      let(:params) do
+        {
+          repository_id: repository.id,
+          id: -1
+        }
+      end
+
+      it 'respons with status 404' do
+        action
+
+        expect(response).to(have_http_status(404))
+      end
+    end
+
+    context 'when user does not have permissions' do
+      before do
+        user_team.role = :guest
+        user_team.save
+      end
+
+      it 'respons with status 403' do
+        action
+
+        expect(response).to(have_http_status(403))
+      end
+    end
+  end
 end