Merge pull request #1083 from ZmagoD/zd_SCI_2259

set limit on maximum number of dropdown list items [fixes SCI-2259]
This commit is contained in:
Zmago Devetak 2018-04-19 17:17:54 +02:00 committed by GitHub
commit 93fc1af12d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 142 additions and 48 deletions

View file

@ -189,8 +189,16 @@
}).on('ajax:error', function(e, xhr) {
animateSpinner(null, false);
if (modalID) {
var field = { "name": xhr.responseJSON.message }
$(form).renderFormErrors('repository_column', field, true, e);
if(xhr.responseJSON.message.hasOwnProperty('repository_list_items')) {
var message = xhr.responseJSON.message['repository_list_items'];
$('.dnd-error').remove();
$('#manageRepositoryColumn ').find('.bootstrap-tagsinput').after(
"<i class='dnd-error'>" + message + "</i>"
);
} else {
var field = { "name": xhr.responseJSON.message }
$(form).renderFormErrors('repository_column', field, true, e);
}
} else {
HelperModule.flashAlertMsg(xhr.responseJSON.message, 'danger');
}

View file

@ -28,28 +28,35 @@ class RepositoryColumnsController < ApplicationController
@repository_column.created_by = current_user
respond_to do |format|
if @repository_column.save
generate_repository_list_items(params[:list_items])
format.json do
render json: {
id: @repository_column.id,
name: escape_input(@repository_column.name),
message: t('libraries.repository_columns.create.success_flash',
name: @repository_column.name),
edit_url:
edit_repository_repository_column_path(@repository,
@repository_column),
update_url:
repository_repository_column_path(@repository,
@repository_column),
destroy_html_url:
repository_columns_destroy_html_path(@repository,
@repository_column)
},
status: :ok
end
else
format.json do
format.json do
if @repository_column.save
if generate_repository_list_items(params[:list_items])
render json: {
id: @repository_column.id,
name: escape_input(@repository_column.name),
message: t('libraries.repository_columns.create.success_flash',
name: @repository_column.name),
edit_url:
edit_repository_repository_column_path(@repository,
@repository_column),
update_url:
repository_repository_column_path(@repository,
@repository_column),
destroy_html_url:
repository_columns_destroy_html_path(@repository,
@repository_column)
},
status: :ok
else
render json: {
message: {
repository_list_items:
t('libraries.repository_columns.repository_list_items_limit',
limit: Constants::REPOSITORY_LIST_ITEMS_PER_COLUMN)
}
}, status: :unprocessable_entity
end
else
render json: { message: @repository_column.errors.full_messages },
status: :unprocessable_entity
end
@ -74,13 +81,22 @@ class RepositoryColumnsController < ApplicationController
format.json do
@repository_column.update_attributes(repository_column_params)
if @repository_column.save
update_repository_list_items(params[:list_items])
render json: {
id: @repository_column.id,
name: escape_input(@repository_column.name),
message: t('libraries.repository_columns.update.success_flash',
name: @repository_column.name)
}, status: :ok
if update_repository_list_items(params[:list_items])
render json: {
id: @repository_column.id,
name: escape_input(@repository_column.name),
message: t('libraries.repository_columns.update.success_flash',
name: @repository_column.name)
}, status: :ok
else
render json: {
message: {
repository_list_items:
t('libraries.repository_columns.repository_list_items_limit',
limit: Constants::REPOSITORY_LIST_ITEMS_PER_COLUMN)
}
}, status: :unprocessable_entity
end
else
render json: { message: @repository_column.errors.full_messages },
status: :unprocessable_entity
@ -162,7 +178,13 @@ class RepositoryColumnsController < ApplicationController
def generate_repository_list_items(item_names)
return unless @repository_column.data_type == 'RepositoryListValue'
column_items = @repository_column.repository_list_items.size
success = true
item_names.split(',').uniq.each do |name|
if column_items >= Constants::REPOSITORY_LIST_ITEMS_PER_COLUMN
success = false
next
end
RepositoryListItem.create(
repository: @repository,
repository_column: @repository_column,
@ -170,11 +192,14 @@ class RepositoryColumnsController < ApplicationController
created_by: current_user,
last_modified_by: current_user
)
column_items += 1
end
success
end
def update_repository_list_items(item_names)
return unless @repository_column.data_type == 'RepositoryListValue'
column_items = @repository_column.repository_list_items.size
items_list = item_names.split(',').uniq
existing = @repository_column.repository_list_items.pluck(:data)
existing.each do |name|
@ -189,8 +214,13 @@ class RepositoryColumnsController < ApplicationController
list_item_id
).destroy_all
end
success = true
items_list.each do |name|
next if @repository_column.repository_list_items.find_by_data(name)
if column_items >= Constants::REPOSITORY_LIST_ITEMS_PER_COLUMN
success = false
next
end
RepositoryListItem.create(
repository: @repository,
repository_column: @repository_column,
@ -198,6 +228,8 @@ class RepositoryColumnsController < ApplicationController
created_by: current_user,
last_modified_by: current_user
)
column_items += 1
end
success
end
end

View file

@ -53,6 +53,7 @@ module RepositoryImportParser
def import_rows!
errors = false
column_items = []
@rows.each do |row|
# Skip empty rows
next if row.empty?
@ -71,16 +72,28 @@ module RepositoryImportParser
end
row_cell_values = []
row.each.with_index do |value, index|
column = @columns[index]
size = 0
if column && value.present?
if column.data_type == 'RepositoryListValue'
current_items_column = get_items_column(column_items, column)
size = current_items_column.list_items_number
end
# uses RepositoryCellValueResolver to retrieve the correct value
cell_value_resolver =
RepositoryImportParser::RepositoryCellValueResolver.new(
column, @user, @repository
column,
@user,
@repository,
size
)
cell_value = cell_value_resolver.get_value(value, record_row)
if column.data_type == 'RepositoryListValue'
current_items_column.list_items_number =
cell_value_resolver.column_list_items_size
end
next if cell_value.nil? # checks the case if we reach items limit
unless cell_value.valid?
errors = true
raise ActiveRecord::Rollback
@ -125,5 +138,21 @@ module RepositoryImportParser
).failed_instances.any?
true
end
def get_items_column(list, column)
current_column = nil
list.each do |element|
current_column = element if element.has_column? column
end
unless current_column
new_column = RepositoryImportParser::ListItemsColumn.new(
column,
column.repository_list_items.size
)
list << new_column
return new_column
end
current_column
end
end
end

View file

@ -0,0 +1,14 @@
module RepositoryImportParser
class ListItemsColumn
attr_accessor :column, :list_items_number
def initialize(column, list_items_number)
@column = column
@list_items_number = list_items_number
end
def has_column?(column)
@column == column
end
end
end

View file

@ -4,10 +4,12 @@
# it to the repository_row
module RepositoryImportParser
class RepositoryCellValueResolver
def initialize(column, user, repository)
attr_reader :column_list_items_size
def initialize(column, user, repository, size)
@column = column
@user = user
@repository = repository
@column_list_items_size = size
end
def get_value(value, record_row)
@ -30,6 +32,7 @@ module RepositoryImportParser
def new_repository_list_value(value, record_row)
list_item = @column.repository_list_items.find_by_data(value)
list_item ||= create_repository_list_item(value)
return unless list_item
RepositoryListValue.new(
created_by: @user,
last_modified_by: @user,
@ -42,13 +45,18 @@ module RepositoryImportParser
end
def create_repository_list_item(value)
RepositoryListItem.create(
data: value,
created_by: @user,
last_modified_by: @user,
repository_column: @column,
repository: @repository
)
if @column_list_items_size >= Constants::REPOSITORY_LIST_ITEMS_PER_COLUMN
return
end
item = RepositoryListItem.new(data: value,
created_by: @user,
last_modified_by: @user,
repository_column: @column,
repository: @repository)
if item.save
@column_list_items_size += 1
return item
end
end
end
end

View file

@ -64,8 +64,8 @@
</div>
<div class="alert alert-info" role="alert">
<ul>
<li><%= t('repositories.modal_parse.warning_1') %></li>
<li><%= t('repositories.modal_parse.warning_2') %></li>
<li><%=t 'repositories.modal_parse.warning_1', limit: Constants::REPOSITORY_LIST_ITEMS_PER_COLUMN %></li>
<li><%=t 'repositories.modal_parse.warning_2' %></li>
</ul>
</div>
</div>

View file

@ -868,6 +868,8 @@ class Constants
EXPORTABLE_ZIP_EXPIRATION_DAYS = 7
REPOSITORY_LIST_ITEMS_PER_COLUMN = 500
# Very basic regex to check for validity of emails
BASIC_EMAIL_REGEX = URI::MailTo::EMAIL_REGEXP

View file

@ -965,7 +965,7 @@ en:
delete: "Delete column"
modal_parse:
title: 'Import items'
warning_1: 'Be careful when importing into Dropdown column/s! Each new unique cell value from the file will create a new Dropdown option. This could result in large amounts of Dropdown options.'
warning_1: 'Be careful when importing into Dropdown column/s! Each new unique cell value from the file will create a new Dropdown option. Maximum nr. of Dropdown options is %{limit}.'
warning_2: 'Importing into file columns is not supported.'
modal_import:
title: 'Import items'
@ -1007,6 +1007,7 @@ en:
repository_columns:
head_title: '%{repository} | Manage Columns'
repository_list_items_limit: "Dropdown items limit reached max. %{limit}"
update:
success_flash: "Column %{name} was successfully updated."
create:

View file

@ -22,14 +22,14 @@ RSpec.describe RepositoryListItem, type: :model do
end
describe 'Validations' do
let!(:user) { create :user }
let!(:repository_one) { create :repository }
it { should validate_presence_of(:data) }
it do
should validate_length_of(:data).is_at_most(Constants::TEXT_MAX_LENGTH)
end
context 'has a uniq data scoped on repository column' do
let!(:user) { create :user }
let!(:repository_one) { create :repository }
let!(:repository_column) do
create :repository_column, name: 'My column', repository: repository_one
end

View file

@ -28,7 +28,7 @@ describe RepositoryImportParser::RepositoryCellValueResolver do
context 'RepositoryListValue' do
let(:subject) do
RepositoryImportParser::RepositoryCellValueResolver.new(
sample_group_column, user, repository
sample_group_column, user, repository, 0
)
end
@ -51,7 +51,7 @@ describe RepositoryImportParser::RepositoryCellValueResolver do
context 'RepositoryTextValue' do
let(:subject) do
RepositoryImportParser::RepositoryCellValueResolver.new(
custom_column, user, repository
custom_column, user, repository, 0
)
it 'returns a valid RepositoryTextValue object' do