From b12f6f1b92c36b145366dc6aff43d4c9a535406c Mon Sep 17 00:00:00 2001 From: zmagod Date: Mon, 9 Apr 2018 12:50:25 +0200 Subject: [PATCH 01/34] set limit on maximum number of dropdown list items [fixes SCI-2259] --- app/models/repository_list_item.rb | 2 ++ app/validators/repository_list_item_validator.rb | 9 +++++++++ app/views/repositories/_parse_records_modal.html.erb | 4 ++-- config/initializers/constants.rb | 2 ++ config/locales/en.yml | 2 +- spec/models/repository_list_item_spec.rb | 4 ++-- 6 files changed, 18 insertions(+), 5 deletions(-) create mode 100644 app/validators/repository_list_item_validator.rb diff --git a/app/models/repository_list_item.rb b/app/models/repository_list_item.rb index e3dbeeb5f..99f66c597 100644 --- a/app/models/repository_list_item.rb +++ b/app/models/repository_list_item.rb @@ -1,4 +1,5 @@ class RepositoryListItem < ApplicationRecord + include ActiveModel::Validations has_many :repository_list_values, inverse_of: :repository_list_item belongs_to :repository, inverse_of: :repository_list_items belongs_to :repository_column, inverse_of: :repository_list_items @@ -12,4 +13,5 @@ class RepositoryListItem < ApplicationRecord presence: true, uniqueness: { scope: :repository_column, case_sensitive: false }, length: { maximum: Constants::TEXT_MAX_LENGTH } + validates_with RepositoryListItemValidator end diff --git a/app/validators/repository_list_item_validator.rb b/app/validators/repository_list_item_validator.rb new file mode 100644 index 000000000..740bb9afc --- /dev/null +++ b/app/validators/repository_list_item_validator.rb @@ -0,0 +1,9 @@ +class RepositoryListItemValidator < ActiveModel::Validator + def validate(record) + return unless record.repository_column + items_length = record.repository_column.repository_list_items.size + if items_length >= Constants::REPOSITORY_LIST_ITEMS_PER_COLUMN + record.errors[:items_limit] << 'Maximum number of list items reached!' + end + end +end diff --git a/app/views/repositories/_parse_records_modal.html.erb b/app/views/repositories/_parse_records_modal.html.erb index 44c398a16..14536ce18 100644 --- a/app/views/repositories/_parse_records_modal.html.erb +++ b/app/views/repositories/_parse_records_modal.html.erb @@ -64,8 +64,8 @@ diff --git a/config/initializers/constants.rb b/config/initializers/constants.rb index 54a793cc1..bface4e3e 100644 --- a/config/initializers/constants.rb +++ b/config/initializers/constants.rb @@ -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 diff --git a/config/locales/en.yml b/config/locales/en.yml index 31847fa5c..e5410189b 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -966,7 +966,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' diff --git a/spec/models/repository_list_item_spec.rb b/spec/models/repository_list_item_spec.rb index dde1ae856..40f9937e8 100644 --- a/spec/models/repository_list_item_spec.rb +++ b/spec/models/repository_list_item_spec.rb @@ -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 From e4b49e62a338d2a430039abb93963898258fb08c Mon Sep 17 00:00:00 2001 From: zmagod Date: Tue, 10 Apr 2018 15:45:51 +0200 Subject: [PATCH 02/34] fixes import limit --- .../repository_columns_controller.rb | 6 ++++ app/models/repository_list_item.rb | 2 -- .../repository_import_parser/importer.rb | 33 +++++++++++++++++-- .../list_items_column.rb | 10 ++++++ .../repository_cell_value_resolver.rb | 24 +++++++++----- .../repository_list_item_validator.rb | 9 ----- .../repository_cell_value_resolver_spec.rb | 4 +-- 7 files changed, 65 insertions(+), 23 deletions(-) create mode 100644 app/utilities/repository_import_parser/list_items_column.rb delete mode 100644 app/validators/repository_list_item_validator.rb diff --git a/app/controllers/repository_columns_controller.rb b/app/controllers/repository_columns_controller.rb index 6c02dc3cf..216d7312f 100644 --- a/app/controllers/repository_columns_controller.rb +++ b/app/controllers/repository_columns_controller.rb @@ -162,7 +162,9 @@ 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 item_names.split(',').uniq.each do |name| + next if column_items >= Constants::REPOSITORY_LIST_ITEMS_PER_COLUMN RepositoryListItem.create( repository: @repository, repository_column: @repository_column, @@ -170,11 +172,13 @@ class RepositoryColumnsController < ApplicationController created_by: current_user, last_modified_by: current_user ) + column_items += 1 end 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| @@ -191,6 +195,7 @@ class RepositoryColumnsController < ApplicationController end items_list.each do |name| next if @repository_column.repository_list_items.find_by_data(name) + next if column_items >= Constants::REPOSITORY_LIST_ITEMS_PER_COLUMN RepositoryListItem.create( repository: @repository, repository_column: @repository_column, @@ -198,6 +203,7 @@ class RepositoryColumnsController < ApplicationController created_by: current_user, last_modified_by: current_user ) + column_items += 1 end end end diff --git a/app/models/repository_list_item.rb b/app/models/repository_list_item.rb index 99f66c597..e3dbeeb5f 100644 --- a/app/models/repository_list_item.rb +++ b/app/models/repository_list_item.rb @@ -1,5 +1,4 @@ class RepositoryListItem < ApplicationRecord - include ActiveModel::Validations has_many :repository_list_values, inverse_of: :repository_list_item belongs_to :repository, inverse_of: :repository_list_items belongs_to :repository_column, inverse_of: :repository_list_items @@ -13,5 +12,4 @@ class RepositoryListItem < ApplicationRecord presence: true, uniqueness: { scope: :repository_column, case_sensitive: false }, length: { maximum: Constants::TEXT_MAX_LENGTH } - validates_with RepositoryListItemValidator end diff --git a/app/utilities/repository_import_parser/importer.rb b/app/utilities/repository_import_parser/importer.rb index c3123f9a5..e244fc80f 100644 --- a/app/utilities/repository_import_parser/importer.rb +++ b/app/utilities/repository_import_parser/importer.rb @@ -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.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 diff --git a/app/utilities/repository_import_parser/list_items_column.rb b/app/utilities/repository_import_parser/list_items_column.rb new file mode 100644 index 000000000..dc9a0a76b --- /dev/null +++ b/app/utilities/repository_import_parser/list_items_column.rb @@ -0,0 +1,10 @@ +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 + end +end diff --git a/app/utilities/repository_import_parser/repository_cell_value_resolver.rb b/app/utilities/repository_import_parser/repository_cell_value_resolver.rb index 2ae63e988..6cb6c9530 100644 --- a/app/utilities/repository_import_parser/repository_cell_value_resolver.rb +++ b/app/utilities/repository_import_parser/repository_cell_value_resolver.rb @@ -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 diff --git a/app/validators/repository_list_item_validator.rb b/app/validators/repository_list_item_validator.rb deleted file mode 100644 index 740bb9afc..000000000 --- a/app/validators/repository_list_item_validator.rb +++ /dev/null @@ -1,9 +0,0 @@ -class RepositoryListItemValidator < ActiveModel::Validator - def validate(record) - return unless record.repository_column - items_length = record.repository_column.repository_list_items.size - if items_length >= Constants::REPOSITORY_LIST_ITEMS_PER_COLUMN - record.errors[:items_limit] << 'Maximum number of list items reached!' - end - end -end diff --git a/spec/utilities/repository_import_parser/repository_cell_value_resolver_spec.rb b/spec/utilities/repository_import_parser/repository_cell_value_resolver_spec.rb index e213002f2..85abfa928 100644 --- a/spec/utilities/repository_import_parser/repository_cell_value_resolver_spec.rb +++ b/spec/utilities/repository_import_parser/repository_cell_value_resolver_spec.rb @@ -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 From 23653d1d95f5152a15a0fa5812ebe4d3a6bfae99 Mon Sep 17 00:00:00 2001 From: zmagod Date: Wed, 11 Apr 2018 09:29:08 +0200 Subject: [PATCH 03/34] refactor --- app/utilities/repository_import_parser/importer.rb | 2 +- app/utilities/repository_import_parser/list_items_column.rb | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/utilities/repository_import_parser/importer.rb b/app/utilities/repository_import_parser/importer.rb index e244fc80f..0274ec295 100644 --- a/app/utilities/repository_import_parser/importer.rb +++ b/app/utilities/repository_import_parser/importer.rb @@ -142,7 +142,7 @@ module RepositoryImportParser def get_items_column(list, column) current_column = nil list.each do |element| - current_column = element if element.column == column + current_column = element if element.has_column? column end unless current_column new_column = RepositoryImportParser::ListItemsColumn.new( diff --git a/app/utilities/repository_import_parser/list_items_column.rb b/app/utilities/repository_import_parser/list_items_column.rb index dc9a0a76b..b7bd1167b 100644 --- a/app/utilities/repository_import_parser/list_items_column.rb +++ b/app/utilities/repository_import_parser/list_items_column.rb @@ -6,5 +6,9 @@ module RepositoryImportParser @column = column @list_items_number = list_items_number end + + def has_column?(column) + @column == column + end end end From 6291857bbf1f16fb0d472d3b4d34f94cee4b5dcc Mon Sep 17 00:00:00 2001 From: zmagod Date: Wed, 11 Apr 2018 17:17:19 +0200 Subject: [PATCH 04/34] enables copying of repository items [fixes SCI-2207] --- .../repositories/repository_datatable.js.erb | 27 +++++ app/controllers/repository_rows_controller.rb | 17 ++- app/services/repository_actions/duplicate.rb | 57 +++++++++ .../repository_cell_resolver.rb | 113 ++++++++++++++++++ .../repositories/_repository_table.html.erb | 1 + app/views/repositories/show.html.erb | 13 +- config/locales/en.yml | 2 + config/routes.rb | 3 + .../repository_rows_controller_spec.rb | 8 ++ 9 files changed, 234 insertions(+), 7 deletions(-) create mode 100644 app/services/repository_actions/duplicate.rb create mode 100644 app/services/repository_actions/repository_cell_resolver.rb diff --git a/app/assets/javascripts/repositories/repository_datatable.js.erb b/app/assets/javascripts/repositories/repository_datatable.js.erb index 78a571825..727dd12eb 100644 --- a/app/assets/javascripts/repositories/repository_datatable.js.erb +++ b/app/assets/javascripts/repositories/repository_datatable.js.erb @@ -565,6 +565,29 @@ var RepositoryDatatable = (function(global) { }); } + + global.onClickCopyRepositoryRecords = function() { + animateSpinner(); + $.ajax({ + url: $('table' + TABLE_ID).data('copy-records'), + type: 'POST', + dataType: 'json', + data: { selected_rows: rowsSelected }, + success: function(data) { + HelperModule.flashAlertMsg(data.flash, 'success'); + rowsSelected = []; + onClickCancel(); + }, + error: function(e) { + if (e.status === 403) { + HelperModule.flashAlertMsg( + I18n.t('repositories.js.permission_error'), e.responseJSON.style + ); + } + } + }); + } + // Edit record global.onClickEdit = function() { if (rowsSelected.length !== 1) { @@ -768,6 +791,8 @@ var RepositoryDatatable = (function(global) { $('.repository-row-selector').removeClass('disabled'); $('.repository-row-selector').prop('disabled', false); if (rowsSelected.length === 0) { + $('#copyRepositoryRecords').prop('disabled', true); + $('#copyRepositoryRecords').addClass('disabled'); $('#editRepositoryRecord').prop('disabled', true); $('#editRepositoryRecord').addClass('disabled'); $('#deleteRepositoryRecordsButton').prop('disabled', true); @@ -805,6 +830,8 @@ var RepositoryDatatable = (function(global) { } $('#deleteRepositoryRecordsButton').prop('disabled', false); $('#deleteRepositoryRecordsButton').removeClass('disabled'); + $('#copyRepositoryRecords').prop('disabled', false); + $('#copyRepositoryRecords').removeClass('disabled'); $('#assignRepositoryRecords').removeClass('disabled'); $('#assignRepositoryRecords').prop('disabled', false); $('#unassignRepositoryRecords').removeClass('disabled'); diff --git a/app/controllers/repository_rows_controller.rb b/app/controllers/repository_rows_controller.rb index e8c191253..b02d96545 100644 --- a/app/controllers/repository_rows_controller.rb +++ b/app/controllers/repository_rows_controller.rb @@ -5,9 +5,11 @@ class RepositoryRowsController < ApplicationController before_action :load_info_modal_vars, only: :show before_action :load_vars, only: %i(edit update) - before_action :load_repository, only: %i(create delete_records index) + before_action :load_repository, + only: %i(create delete_records index copy_records) before_action :check_create_permissions, only: :create - before_action :check_manage_permissions, only: %i(edit update delete_records) + before_action :check_manage_permissions, + only: %i(edit update delete_records copy_records) def index @draw = params[:draw].to_i @@ -276,6 +278,17 @@ class RepositoryRowsController < ApplicationController end end + def copy_records + duplicate_service = RepositoryActions::Duplicate.new(current_user, + @repository, + params[:selected_rows]) + duplicate_service.call + render json: { + flash: t('repositories.copy_records_report', + number: duplicate_service.number_of_duplicated_items) + }, status: :ok + end + private def load_info_modal_vars diff --git a/app/services/repository_actions/duplicate.rb b/app/services/repository_actions/duplicate.rb new file mode 100644 index 000000000..c4f2ac102 --- /dev/null +++ b/app/services/repository_actions/duplicate.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'repository_actions/repository_cell_resolver' + +module RepositoryActions + class Duplicate + attr_reader :number_of_duplicated_items + def initialize(user, repository, rows_to_copy = []) + @user = user + @repository = repository + @rows_to_copy = rows_to_copy.map(&:to_i).uniq + @number_of_duplicated_items = 0 + end + + def call + sanitize_rows_to_copy + @rows_to_copy.each do |row_id| + copy_row(row_id) + end + end + + private + + def sanitize_rows_to_copy + ids = @repository.repository_rows.pluck(:id) + @rows_to_copy.map! { |el| ids.include?(el) ? el : nil }.compact! + end + + def copy_row(id) + row = RepositoryRow.find_by_id(id) + new_row = RepositoryRow.new( + row.attributes.merge(new_row_attributes(row.name)) + ) + + if new_row.save + @number_of_duplicated_items += 1 + row.repository_cells.each do |cell| + copy_repository_cell(cell, new_row) + end + end + end + + def new_row_attributes(name) + timestamp = DateTime.now + { id: nil, + name: "#{name} (1)", + created_at: timestamp, + updated_at: timestamp } + end + + def copy_repository_cell(cell, new_row) + RepositoryActions::RepositoryCellResolver.new(cell, + new_row, + @user.current_team).call + end + end +end diff --git a/app/services/repository_actions/repository_cell_resolver.rb b/app/services/repository_actions/repository_cell_resolver.rb new file mode 100644 index 000000000..334648b1a --- /dev/null +++ b/app/services/repository_actions/repository_cell_resolver.rb @@ -0,0 +1,113 @@ +# frozen_string_literal: true + +module RepositoryActions + class RepositoryCellResolver + def initialize(cell, new_row, team) + @cell = cell + @new_row = new_row + @team = team + end + + def call + case @cell.value_type + when 'RepositoryListValue' + clone_repository_list_value + when 'RepositoryTextValue' + clone_repository_text_value + when 'RepositoryAssetValue' + clone_repository_asset_value + when 'RepositoryDateValue' + clone_repository_date_value + end + end + + private + + def clone_repository_list_value + old_value = @cell.value + RepositoryListValue.create( + old_value.attributes.merge(id: nil, + repository_cell_attributes: { + repository_row: @new_row, + repository_column: @cell.repository_column + }) + ) + end + + def clone_repository_text_value + old_value = @cell.value + RepositoryTextValue.create( + old_value.attributes.merge(id: nil, + repository_cell_attributes: { + repository_row: @new_row, + repository_column: @cell.repository_column + }) + ) + end + + def clone_repository_asset_value + old_value = @cell.value + new_asset = create_new_asset(old_value.asset) + RepositoryAssetValue.create( + old_value.attributes.merge( + id: nil, + asset: new_asset, + repository_cell_attributes: { + repository_row: @new_row, + repository_column: @cell.repository_column + } + ) + ) + end + + def clone_repository_date_value + old_value = @cell.value + RepositoryDateValue.create( + old_value.attributes.merge(id: nil, + repository_cell_attributes: { + repository_row: @new_row, + repository_column: @cell.repository_column + }) + ) + end + + # reuses the same code we have in copy protocols action + def create_new_asset(old_asset) + new_asset = Asset.new_empty( + old_asset.file_file_name, + old_asset.file_file_size + ) + new_asset.created_by = old_asset.created_by + new_asset.team = old_asset.team + new_asset.last_modified_by = old_asset.last_modified_by + new_asset.file_processing = true if old_asset.is_image? + new_asset.file = old_asset.file + new_asset.save + + return unless new_asset.valid? + + if new_asset.is_image? + new_asset.file.reprocess!(:large) + new_asset.file.reprocess!(:medium) + end + + # Clone extracted text data if it exists + if old_asset.asset_text_datum + AssetTextDatum.create(data: new_asset.data, asset: new_asset) + end + + # Update estimated size of cloned asset + # (& file_present flag) + new_asset.update( + estimated_size: old_asset.estimated_size, + file_present: true + ) + + # Update team's space taken + @team.reload + @team.take_space(new_asset.estimated_size) + @team.save! + new_asset + end + end +end diff --git a/app/views/repositories/_repository_table.html.erb b/app/views/repositories/_repository_table.html.erb index aad7bcc3c..3505fdde6 100644 --- a/app/views/repositories/_repository_table.html.erb +++ b/app/views/repositories/_repository_table.html.erb @@ -6,6 +6,7 @@ data-num-columns="<%= 6 + repository.repository_columns.count %>" data-create-record="<%= repository_repository_rows_path(repository) %>" data-delete-record="<%= repository_delete_records_path(repository) %>" + data-copy-records="<%= repository_copy_records_path(repository) %>" data-max-dropdown-length="<%= Constants::NAME_TRUNCATION_LENGTH_DROPDOWN %>" data-save-text="<%= I18n.t('general.save') %>" data-edit-text="<%= I18n.t('general.edit') %>" diff --git a/app/views/repositories/show.html.erb b/app/views/repositories/show.html.erb index 944be2e31..1a8589c07 100644 --- a/app/views/repositories/show.html.erb +++ b/app/views/repositories/show.html.erb @@ -105,12 +105,11 @@ diff --git a/config/locales/en.yml b/config/locales/en.yml index 1210b2f88..71dc89901 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -941,6 +941,7 @@ en: errors_list_title: "Items were not imported because one or more errors were found:" no_repository_name: "Item name is required!" edit_record: "Edit" + copy_record: "Copy" delete_record: "Delete" save_record: "Save" cancel_save: "Cancel" @@ -993,6 +994,7 @@ en: no_records_assigned_flash: "No items were assigned to task" no_records_unassigned_flash: "No items were unassigned from task" default_column: 'Name' + copy_records_report: "%{number} item(s) successfully copied." libraries: manange_modal_column: diff --git a/config/routes.rb b/config/routes.rb index 89089aed8..65334a66c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -467,6 +467,9 @@ Rails.application.routes.draw do to: 'repository_rows#delete_records', as: 'delete_records', defaults: { format: 'json' } + post 'copy_records', + to: 'repository_rows#copy_records', + defaults: { format: 'json' } get 'repository_columns/:id/destroy_html', to: 'repository_columns#destroy_html', as: 'columns_destroy_html' diff --git a/spec/controllers/repository_rows_controller_spec.rb b/spec/controllers/repository_rows_controller_spec.rb index feb487cb0..72dc1d42d 100644 --- a/spec/controllers/repository_rows_controller_spec.rb +++ b/spec/controllers/repository_rows_controller_spec.rb @@ -115,4 +115,12 @@ describe RepositoryRowsController, type: :controller do end end end + + describe 'POST #copy_records' do + it 'returns a success response' do + post :copy_records, params: { repository_id: repository.id, + selected_rows: [repository_row.id] } + expect(response).to have_http_status(:success) + end + end end From 7f93cf6220bf09fbdbc526eefbd2aeb553ccdb82 Mon Sep 17 00:00:00 2001 From: zmagod Date: Thu, 12 Apr 2018 13:00:21 +0200 Subject: [PATCH 05/34] adds specs for duplicate class --- .../repository_actions/duplicate_spec.rb | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 spec/services/repository_actions/duplicate_spec.rb diff --git a/spec/services/repository_actions/duplicate_spec.rb b/spec/services/repository_actions/duplicate_spec.rb new file mode 100644 index 000000000..daf4287db --- /dev/null +++ b/spec/services/repository_actions/duplicate_spec.rb @@ -0,0 +1,83 @@ +require 'rails_helper' + +describe RepositoryActions::Duplicate do + let!(:user) { create :user } + let!(:repository) { create :repository } + let!(:list_column) do + create(:repository_column, name: 'list', + repository: repository, + created_by: user, + data_type: 'RepositoryListValue') + end + let!(:text_column) do + create(:repository_column, name: 'text', + repository: repository, + created_by: user, + data_type: 'RepositoryTextValue') + end + + describe '#call' do + before do + @rows_ids = [] + + 3.times do |index| + row = create :repository_row, name: "row (#{index})", + repository: repository + create :repository_text_value, data: "text (#{index})", + repository_cell_attributes: { + repository_row: row, + repository_column: text_column + } + create :repository_list_value, + repository_list_item: create(:repository_list_item, + repository: repository, + repository_column: list_column, + data: "list item (#{index})"), + repository_cell_attributes: { + repository_row: row, + repository_column: list_column + } + @rows_ids << row.id.to_s + end + end + + it 'generates a copy of selected items' do + expect(repository.repository_rows.reload.size).to eq 3 + described_class.new(user, repository, @rows_ids).call + expect(repository.repository_rows.reload.size).to eq 6 + end + + it 'generates an exact copy of the row with custom column values' do + described_class.new(user, repository, [@rows_ids.first]).call + duplicated_row = repository.repository_rows.reload.last + expect(duplicated_row.name).to eq 'row (0) (1)' + duplicated_row.repository_cells.each do |cell| + if cell.value_type == 'RepositoryListValue' + expect(cell.value.data).to eq 'list item (0)' + else + expect(cell.value.data).to eq 'text (0)' + end + end + end + + it 'prevents to copy items that do not already belong to repository' do + new_repository = create :repository, name: 'new repo' + new_row = create :repository_row, name: 'other row', + repository: new_repository + described_class.new(user, repository, [new_row.id]).call + expect(repository.repository_rows.reload.size).to eq 3 + end + + it 'returns the number of copied items' do + service_obj = described_class.new(user, repository, @rows_ids) + service_obj.call + expect(service_obj.number_of_duplicated_items).to eq 3 + end + + it 'returns the number of copied items' do + service_obj = described_class.new(user, repository, []) + service_obj.call + expect(service_obj.number_of_duplicated_items).to eq 0 + end + end +end From cebb06269b93474ece68d6d9517751b9d77912bd Mon Sep 17 00:00:00 2001 From: zmagod Date: Thu, 12 Apr 2018 13:34:34 +0200 Subject: [PATCH 06/34] refactor specs --- spec/services/repository_actions/duplicate_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/services/repository_actions/duplicate_spec.rb b/spec/services/repository_actions/duplicate_spec.rb index daf4287db..e323310be 100644 --- a/spec/services/repository_actions/duplicate_spec.rb +++ b/spec/services/repository_actions/duplicate_spec.rb @@ -49,7 +49,7 @@ describe RepositoryActions::Duplicate do it 'generates an exact copy of the row with custom column values' do described_class.new(user, repository, [@rows_ids.first]).call - duplicated_row = repository.repository_rows.reload.last + duplicated_row = repository.repository_rows.order('created_at ASC').last expect(duplicated_row.name).to eq 'row (0) (1)' duplicated_row.repository_cells.each do |cell| if cell.value_type == 'RepositoryListValue' From 9812a9f16091da3e4da47ff4646f909156c74fab Mon Sep 17 00:00:00 2001 From: zmagod Date: Fri, 13 Apr 2018 09:43:14 +0200 Subject: [PATCH 07/34] adds correct preview images to files [fixes SCI-2254] --- app/helpers/file_icons_helper.rb | 6 +++++- app/views/my_modules/archive.html.erb | 5 ++--- app/views/my_modules/archive/_result.html.erb | 5 ++++- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/app/helpers/file_icons_helper.rb b/app/helpers/file_icons_helper.rb index 8c12947e2..1bed35a35 100644 --- a/app/helpers/file_icons_helper.rb +++ b/app/helpers/file_icons_helper.rb @@ -9,10 +9,14 @@ module FileIconsHelper file_ext = asset.file_file_name.split('.').last if %w(doc docm docx dot dotm dotx odt rtf).include?(file_ext) fa_class = 'fa-file-word' - elsif %w(csv ods xls xlsb xlsm xlsx).include?(file_ext) + elsif %w(ods xls xlsb xlsm xlsx).include?(file_ext) fa_class = 'fa-file-excel' elsif %w(odp pot potm potx pps ppsm ppsx ppt pptm pptx).include?(file_ext) fa_class = 'fa-file-powerpoint' + elsif %w(pdf).include?(file_ext) + fa_class = 'fa-file-pdf' + elsif %w(txt csv tab tex).include?(file_ext) + fa_class = 'fa-file-alt' end # Now check for custom mappings or possible overrides diff --git a/app/views/my_modules/archive.html.erb b/app/views/my_modules/archive.html.erb index 6e9871e84..cd698b95f 100644 --- a/app/views/my_modules/archive.html.erb +++ b/app/views/my_modules/archive.html.erb @@ -6,14 +6,13 @@
<% i = 0 %> <% @archived_results.each do |result| %> - <%= render partial: "my_modules/archive/result.html.erb", locals: { result: result } %> - <% i = i + 1 %> <% end %> - <% if i == 0 %> <%=t "my_modules.module_archive.no_archived_results" %> <% end %>
+ + diff --git a/app/views/my_modules/archive/_result.html.erb b/app/views/my_modules/archive/_result.html.erb index f0f0c25c1..64cd5a73b 100644 --- a/app/views/my_modules/archive/_result.html.erb +++ b/app/views/my_modules/archive/_result.html.erb @@ -9,7 +9,10 @@
  • <% option_text = t("my_modules.module_archive.option_download") %> <% if result.is_asset %> - <%= link_to option_text, download_asset_path(result.asset), data: {no_turbolink: true} %> + <%= link_to 'View', download_asset_path(result.asset), + class: 'file-preview-link', + id: "modal_link#{result.asset.id}", + data: { no_turbolink: true, id: true, status: 'asset-present', 'preview-url': asset_file_preview_path(result.asset) } %> <% elsif result.is_text %> <%= link_to option_text, result_text_download_path(result.result_text_id), data: {no_turbolink: true} %> <% elsif result.is_table %> From 4f218333732c10ddfad12da8f7f37c2e95148e4d Mon Sep 17 00:00:00 2001 From: zmagod Date: Fri, 13 Apr 2018 11:47:33 +0200 Subject: [PATCH 08/34] changes per @duco 's request --- app/controllers/repository_rows_controller.rb | 6 ++-- ...ory_cell_resolver.rb => duplicate_cell.rb} | 21 ++++--------- .../{duplicate.rb => duplicate_rows.rb} | 31 +++++++++---------- ...plicate_spec.rb => duplicate_rows_spec.rb} | 12 +++---- 4 files changed, 30 insertions(+), 40 deletions(-) rename app/services/repository_actions/{repository_cell_resolver.rb => duplicate_cell.rb} (83%) rename app/services/repository_actions/{duplicate.rb => duplicate_rows.rb} (50%) rename spec/services/repository_actions/{duplicate_spec.rb => duplicate_rows_spec.rb} (89%) diff --git a/app/controllers/repository_rows_controller.rb b/app/controllers/repository_rows_controller.rb index b02d96545..4a0827638 100644 --- a/app/controllers/repository_rows_controller.rb +++ b/app/controllers/repository_rows_controller.rb @@ -279,9 +279,9 @@ class RepositoryRowsController < ApplicationController end def copy_records - duplicate_service = RepositoryActions::Duplicate.new(current_user, - @repository, - params[:selected_rows]) + duplicate_service = RepositoryActions::DuplicateRows.new( + current_user, @repository, params[:selected_rows] + ) duplicate_service.call render json: { flash: t('repositories.copy_records_report', diff --git a/app/services/repository_actions/repository_cell_resolver.rb b/app/services/repository_actions/duplicate_cell.rb similarity index 83% rename from app/services/repository_actions/repository_cell_resolver.rb rename to app/services/repository_actions/duplicate_cell.rb index 334648b1a..ff3c2937d 100644 --- a/app/services/repository_actions/repository_cell_resolver.rb +++ b/app/services/repository_actions/duplicate_cell.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module RepositoryActions - class RepositoryCellResolver + class DuplicateCell def initialize(cell, new_row, team) @cell = cell @new_row = new_row @@ -9,21 +9,12 @@ module RepositoryActions end def call - case @cell.value_type - when 'RepositoryListValue' - clone_repository_list_value - when 'RepositoryTextValue' - clone_repository_text_value - when 'RepositoryAssetValue' - clone_repository_asset_value - when 'RepositoryDateValue' - clone_repository_date_value - end + self.send("duplicate_#{@cell.value_type.underscore}") end private - def clone_repository_list_value + def duplicate_repository_list_value old_value = @cell.value RepositoryListValue.create( old_value.attributes.merge(id: nil, @@ -34,7 +25,7 @@ module RepositoryActions ) end - def clone_repository_text_value + def duplicate_repository_text_value old_value = @cell.value RepositoryTextValue.create( old_value.attributes.merge(id: nil, @@ -45,7 +36,7 @@ module RepositoryActions ) end - def clone_repository_asset_value + def duplicate_repository_asset_value old_value = @cell.value new_asset = create_new_asset(old_value.asset) RepositoryAssetValue.create( @@ -60,7 +51,7 @@ module RepositoryActions ) end - def clone_repository_date_value + def duplicate_repository_date_value old_value = @cell.value RepositoryDateValue.create( old_value.attributes.merge(id: nil, diff --git a/app/services/repository_actions/duplicate.rb b/app/services/repository_actions/duplicate_rows.rb similarity index 50% rename from app/services/repository_actions/duplicate.rb rename to app/services/repository_actions/duplicate_rows.rb index c4f2ac102..d0af22bb4 100644 --- a/app/services/repository_actions/duplicate.rb +++ b/app/services/repository_actions/duplicate_rows.rb @@ -1,32 +1,31 @@ # frozen_string_literal: true -require 'repository_actions/repository_cell_resolver' +require 'repository_actions/duplicate_cell' module RepositoryActions - class Duplicate + class DuplicateRows attr_reader :number_of_duplicated_items - def initialize(user, repository, rows_to_copy = []) + def initialize(user, repository, rows_ids = []) @user = user @repository = repository - @rows_to_copy = rows_to_copy.map(&:to_i).uniq + @rows_to_duplicate = sanitize_rows_to_duplicate(rows_ids) @number_of_duplicated_items = 0 end def call - sanitize_rows_to_copy - @rows_to_copy.each do |row_id| - copy_row(row_id) + @rows_to_duplicate.each do |row_id| + duplicate_row(row_id) end end private - def sanitize_rows_to_copy - ids = @repository.repository_rows.pluck(:id) - @rows_to_copy.map! { |el| ids.include?(el) ? el : nil }.compact! + def sanitize_rows_to_duplicate(rows_ids) + process_ids = rows_ids.map(&:to_i).uniq + @repository.repository_rows.where(id: process_ids).pluck(:id) end - def copy_row(id) + def duplicate_row(id) row = RepositoryRow.find_by_id(id) new_row = RepositoryRow.new( row.attributes.merge(new_row_attributes(row.name)) @@ -35,7 +34,7 @@ module RepositoryActions if new_row.save @number_of_duplicated_items += 1 row.repository_cells.each do |cell| - copy_repository_cell(cell, new_row) + duplicate_repository_cell(cell, new_row) end end end @@ -48,10 +47,10 @@ module RepositoryActions updated_at: timestamp } end - def copy_repository_cell(cell, new_row) - RepositoryActions::RepositoryCellResolver.new(cell, - new_row, - @user.current_team).call + def duplicate_repository_cell(cell, new_row) + RepositoryActions::DuplicateCell.new( + cell, new_row, @user.current_team + ).call end end end diff --git a/spec/services/repository_actions/duplicate_spec.rb b/spec/services/repository_actions/duplicate_rows_spec.rb similarity index 89% rename from spec/services/repository_actions/duplicate_spec.rb rename to spec/services/repository_actions/duplicate_rows_spec.rb index e323310be..2dc3b2cd0 100644 --- a/spec/services/repository_actions/duplicate_spec.rb +++ b/spec/services/repository_actions/duplicate_rows_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -describe RepositoryActions::Duplicate do +describe RepositoryActions::DuplicateRows do let!(:user) { create :user } let!(:repository) { create :repository } let!(:list_column) do @@ -41,13 +41,13 @@ describe RepositoryActions::Duplicate do end end - it 'generates a copy of selected items' do + it 'generates a duplicate of selected items' do expect(repository.repository_rows.reload.size).to eq 3 described_class.new(user, repository, @rows_ids).call expect(repository.repository_rows.reload.size).to eq 6 end - it 'generates an exact copy of the row with custom column values' do + it 'generates an exact duplicate of the row with custom column values' do described_class.new(user, repository, [@rows_ids.first]).call duplicated_row = repository.repository_rows.order('created_at ASC').last expect(duplicated_row.name).to eq 'row (0) (1)' @@ -60,7 +60,7 @@ describe RepositoryActions::Duplicate do end end - it 'prevents to copy items that do not already belong to repository' do + it 'prevents to duplicate items that do not already belong to repository' do new_repository = create :repository, name: 'new repo' new_row = create :repository_row, name: 'other row', repository: new_repository @@ -68,13 +68,13 @@ describe RepositoryActions::Duplicate do expect(repository.repository_rows.reload.size).to eq 3 end - it 'returns the number of copied items' do + it 'returns the number of duplicated items' do service_obj = described_class.new(user, repository, @rows_ids) service_obj.call expect(service_obj.number_of_duplicated_items).to eq 3 end - it 'returns the number of copied items' do + it 'returns the number of duplicated items' do service_obj = described_class.new(user, repository, []) service_obj.call expect(service_obj.number_of_duplicated_items).to eq 0 From 0adc06b1a1b62775d8785e4d41ec39a82642a176 Mon Sep 17 00:00:00 2001 From: zmagod Date: Fri, 13 Apr 2018 13:11:29 +0200 Subject: [PATCH 09/34] adds edit repository item permission to normal user [fixes SCI-2297] --- app/helpers/repository_datatable_helper.rb | 7 +++++++ app/views/repositories/show.html.erb | 5 +++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/app/helpers/repository_datatable_helper.rb b/app/helpers/repository_datatable_helper.rb index dee34f23a..d5e858d6c 100644 --- a/app/helpers/repository_datatable_helper.rb +++ b/app/helpers/repository_datatable_helper.rb @@ -58,4 +58,11 @@ module RepositoryDatatableHelper " " end end + + def can_perform_repository_actions(repository) + team = repository.team + can_manage_repository?(repository) || + can_create_repositories?(team) || + can_manage_repository_rows?(team) + end end diff --git a/app/views/repositories/show.html.erb b/app/views/repositories/show.html.erb index 944be2e31..3d224cff4 100644 --- a/app/views/repositories/show.html.erb +++ b/app/views/repositories/show.html.erb @@ -20,11 +20,12 @@ data-toggle="dropdown" aria-haspopup="true" aria-expanded="true" - <%= "disabled" unless can_manage_repository?(@repository) || can_create_repositories?(@repository.team) %>> + <%= "disabled" unless can_perform_repository_actions(@repository) %>> - <% if can_manage_repository?(@repository) || can_create_repositories?(@repository.team) %> + + <% if can_perform_repository_actions(@repository) %> - - + + <% if project_page? %> + <% title_element = @project %> + <% elsif experiment_page? %> + <% title_element = @experiment %> + <% elsif module_page? %> + <% title_element = @my_module %> + <% end %> + + + diff --git a/config/locales/en.yml b/config/locales/en.yml index efb2a7dde..2a61368bc 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -917,9 +917,6 @@ en: name_placeholder: "My inventory" submit: "Create inventory" success_flash: "Inventory %{name} successfully created." - nav: - breadcrumbs: - repositories: "Inventories" table: id: 'ID' assigned: "Assigned" @@ -1449,10 +1446,6 @@ en: leave_flash: "Successfuly left team %{team}." protocols: - nav: - breadcrumbs: - manager: "Protocol management" - edit: "Edit protocol" protocols_io_import: title_too_long: "... Text is too long so we had to cut it off." too_long: "... Text is too long so we had to cut it off." From cc34376253bdcb127954006bcae8dbc069319454 Mon Sep 17 00:00:00 2001 From: mlorb Date: Wed, 18 Apr 2018 14:40:53 +0200 Subject: [PATCH 19/34] update reports show page to work with new navigation --- app/assets/javascripts/reports/new.js.erb | 228 +----------------- app/assets/javascripts/sidebar.js.erb | 24 -- app/assets/stylesheets/reports.scss | 68 ++---- app/controllers/reports_controller.rb | 2 - app/views/reports/new.html.erb | 2 - .../reports/new/_report_navigation.html.erb | 98 +++----- config/locales/en.yml | 6 +- 7 files changed, 60 insertions(+), 368 deletions(-) diff --git a/app/assets/javascripts/reports/new.js.erb b/app/assets/javascripts/reports/new.js.erb index 50d3ba047..656f595db 100644 --- a/app/assets/javascripts/reports/new.js.erb +++ b/app/assets/javascripts/reports/new.js.erb @@ -5,7 +5,6 @@ $.fn.findWithSelf = function(selector) { }; var REPORT_CONTENT = "#report-content"; -var SIDEBAR_PARENT_TREE = "#report-sidebar-tree"; var ADD_CONTENTS_FORM_ID = "#add-contents-form"; var SAVE_REPORT_FORM_ID = "#save-report-form"; @@ -31,12 +30,14 @@ var ignoreUnsavedWorkAlert; initializeSaveToPdf(); initializeSaveReport(); initializeAddContentsModal(); - initializeSidebarNavigation(); initializeUnsavedWorkDialog(); $('.report-nav-link').each(function() { truncateLongString($(this), <%= Constants::NAME_TRUNCATION_LENGTH %>); }); + + // Automatically display the "Add content" modal + $('.new-element.initial').click(); } /** @@ -238,9 +239,6 @@ function initializeNewElement(newEl) { } else if (data.status == 200) { // Add elements addElements(el, data.responseJSON.elements); - - // Update sidebar - initializeSidebarNavigation(); } }) .on("ajax:error", function(e, xhr, settings, error) { @@ -486,140 +484,6 @@ function initializeUnsavedWorkDialog() { $(document).on('page:before-change', beforeUnload); } -/** - * SIDEBAR CODE - */ - - /** - * Get the sidebar
  • element for the specified report element. - * @param reportEl - The .report-element in the report. - * @return The corresponding sidebar
  • . - */ -function getSidebarEl(reportEl) { - var type = reportEl.data("type"); - var scrollId = reportEl.data("scroll-id"); - return $(SIDEBAR_PARENT_TREE).find( - "li" + - "[data-type='" + type + "']" + - "[data-scroll-id='" + scrollId + "']" - ); -} - -/** - * Get the report element for the specified - * sidebar element. - * @param sidebarEl - The
  • sidebar element. - * @return The corresponding report element. - */ -function getReportEl(sidebarEl) { - var type = sidebarEl.data("type"); - var scrollId = sidebarEl.data("scroll-id"); - return $(REPORT_CONTENT).find( - "div.report-element" + - "[data-type='" + type + "']" + - "[data-scroll-id='" + scrollId + "']" - ); -} - -/** - * Initialize the sidebar navigation pane. - */ -function initializeSidebarNavigation() { - var reportContent = $(REPORT_CONTENT); - var treeParent = $(SIDEBAR_PARENT_TREE); - - // Remove existing contents (also remove click listeners) - treeParent.find(".report-nav-link").off("click"); - treeParent.children().remove(); - - // Re-populate the sidebar - _.each(reportContent.children(".report-element"), function(child) { - var li = initSidebarElement($(child)); - li.appendTo(treeParent); - }); - - // Add click listener on all links - treeParent.find(".report-nav-link").click(function(e) { - var el = $(this).closest("li"); - scrollToElement(el); - - e.preventDefault(); - e.stopPropagation(); - return false; - }); - - // Call to sidebar function to re-initialize tree functionality - setupSidebarTree(); -} - -/** - * Recursive call to initialize sidebar elements. - * @param reportEl - The report element for which to - * generate the sidebar. - * @return A
  • jQuery element containing sidebar entry. - */ -function initSidebarElement(reportEl) { - var elChildrenContainer = reportEl.children(".report-element-children"); - var type = reportEl.data("type"); - var name = reportEl.data("name"); - var scrollId = reportEl.data("scroll-id"); - var iconClass = reportEl.data("icon-class"); - - // Generate list element - var newLi = $(document.createElement("li")); - newLi - .attr("data-type", type) - .attr("data-scroll-id", scrollId); - - var newSpan = $(document.createElement("span")); - newSpan.appendTo(newLi); - var newI = $(document.createElement("i")); - newI.appendTo(newSpan); - var newHref = $(document.createElement("a")); - newHref - .attr("href", "") - .addClass("report-nav-link") - .text(name) - .appendTo(newSpan); - var newIcon = $(document.createElement("span")); - newIcon.addClass(iconClass).prependTo(newHref); - - if (elChildrenContainer.length && elChildrenContainer.length > 0) { - var elChildren = elChildrenContainer.children(".report-element"); - if (elChildren.length && elChildren.length > 0) { - var newUl = $(document.createElement("ul")); - newUl.appendTo(newLi); - - _.each(elChildren, function(child) { - var li = initSidebarElement($(child)); - li.appendTo(newUl); - }); - } - } - - return newLi; -} - -/** - * Scroll to the specified element in the report. - * @param sidebarEl - The sidebar element. - */ -function scrollToElement(sidebarEl) { - var el = getReportEl(sidebarEl); - - if (el.length && el.length == 1) { - var content = $("body"); - content.scrollTo( - el, - { - axis: 'y', - duration: 500, - offset: -150 - } - ); - } -} - /** * INDIVIDUAL ELEMENTS SORTING/MODIFYING FUNCTIONS */ @@ -683,8 +547,6 @@ function sortWholeReport(asc) { sortElementChildren($(el), asc, true); }); - // Reinitialize sidebar - initializeSidebarNavigation(); animateLoading(false); } @@ -734,22 +596,6 @@ function sortElementChildren(el, asc, recursive) { sortElementChildren($(child), asc, true); } }); - - // Update sidebar - var prevEl = null; - _.each(children, function(child) { - var sidebarEl = getSidebarEl($(child)); - if (sidebarEl.length && sidebarEl.length == 1) { - var sidebarParent = sidebarEl.closest("ul"); - sidebarEl.detach(); - if (prevEl === null) { - sidebarParent.prepend(sidebarEl); - } else { - prevEl.after(sidebarEl); - } - prevEl = sidebarEl; - } - }); } /** @@ -843,46 +689,6 @@ function moveElement(el, up) { !nextNewEl.hasClass("new-element")) { return; } - - var sidebarEl; - if (up) { - var prevEl = prevNewEl.prev(); - if (!prevEl.length || !prevEl.hasClass("report-element")) { - return; - } - - // Move sidebar element up - sidebarEl = getSidebarEl(el); - var sidebarPrev = sidebarEl.prev(); - sidebarEl.detach(); - sidebarPrev.before(sidebarEl); - - el.detach(); - nextNewEl.detach(); - prevEl.before(el); - prevEl.before(nextNewEl); - updateElementControls(prevEl); - - - } else { - var nextEl = nextNewEl.next(); - if (!nextEl.length || !nextEl.hasClass("report-element")) { - return; - } - - // Move sidebar element up - sidebarEl = getSidebarEl(el); - var sidebarNext = sidebarEl.next(); - sidebarEl.detach(); - sidebarNext.after(sidebarEl); - - prevNewEl.detach(); - el.detach(); - nextEl.after(el); - nextEl.after(prevNewEl); - updateElementControls(nextEl); - } - updateElementControls(el); } @@ -905,10 +711,6 @@ function removeElement(el) { // TODO Remove event listeners - // Remove sidebar entry - var sidebarEl = getSidebarEl(el); - sidebarEl.remove(); - prevNewEl.remove(); el.remove(); @@ -933,10 +735,6 @@ function removeResultCommentsElement(el) { // TODO Remove event listeners - // Remove sidebar entry - var sidebarEl = getSidebarEl(el); - sidebarEl.remove(); - // Remove element, show the new element container el.remove(); parent.children(".new-element").removeClass("hidden"); @@ -1125,26 +923,6 @@ function constructElementContentsJson(el) { return jsonEl; } -/** - * Binds listeners to sidebar - * that truncate long strings - */ -function initializeReportSidebartruncation() { - var target = document.getElementById("report-sidebar-tree"); - var observer = new MutationObserver( - function() { - $.each($("a.report-nav-link"), - function(){ - truncateLongString($(this), - <%= Constants::NAME_TRUNCATION_LENGTH %>); - }); - } - ); - var config = { childList: true }; - - observer.observe(target, config); -} - $(document).ready(function() { // Check if we are actually at new report page if ($(REPORT_CONTENT).length) { diff --git a/app/assets/javascripts/sidebar.js.erb b/app/assets/javascripts/sidebar.js.erb index 2d2558fbf..121639c68 100644 --- a/app/assets/javascripts/sidebar.js.erb +++ b/app/assets/javascripts/sidebar.js.erb @@ -252,30 +252,6 @@ function setupSidebarTree() { $(".tree li span.tree-link ").after("
    "); } -/** - * Initialize the show/hide toggling of sidebar. - */ -function initializeSidebarToggle() { - var wrapper = $("#wrapper"); - var toggled = sessionIsSidebarToggled(); - - if (toggled || toggled === null && $(window).width() < - <%= Constants::SCREEN_WIDTH_LARGE %>) { - wrapper.addClass("no-animation"); - wrapper.addClass("toggled"); - // Cause reflow of the wrapper element - wrapper[0].offsetHeight; - wrapper.removeClass("no-animation"); - $(".navbar-secondary").addClass("navbar-without-sidebar"); - } - - $(".toggle-sidebar-link").on("click", function() { - $("#wrapper").toggleClass("toggled"); - sessionToggleSidebar(); - $(".navbar-secondary").toggleClass("navbar-without-sidebar", sessionIsSidebarToggled()); - return false; - }); -} // Resize the sidebar to accomodate to the page size function resizeSidebarContents() { diff --git a/app/assets/stylesheets/reports.scss b/app/assets/stylesheets/reports.scss index 180cb7c48..bf2ae8481 100644 --- a/app/assets/stylesheets/reports.scss +++ b/app/assets/stylesheets/reports.scss @@ -8,27 +8,33 @@ /* New page navbar */ .navbar-report { - border-left: none; - border-top: none; - border-right: none; + background: $color-concrete; border-bottom: 4px solid $color-silver; - background: $color-concrete !important; + border-left: 0; + border-right: 0; + border-top: 0; margin-bottom: 0; min-width: 320px; padding: 0 15px; - z-index: 500; + padding-right: 100px; position: fixed; width: 100%; + z-index: 500; div.row { margin-right: 0; } #report-menu { + margin: 15px 0; form { display: inline-block; } + + .form-group { + margin-bottom: 0; + } } & > div.row { @@ -54,43 +60,6 @@ label { } } -/* New page sidebar */ -.report-sidebar-wrapper { - background-color: $color-white !important; -} - -// Some additional styling on the treeview -.report-tree { - li { - padding: 0 0 0 15px; - - a.report-nav-link:visited { - text-decoration: none; - } - a.report-nav-link:hover { - text-decoration: none; - } - [data-type='step']:not(.parent_li) { - padding-left: 27px; - } - } -} -.report-sidebar-panel-description { - margin: 10px 10px 0 10px; -} - -.report-item-elements { - margin-top: 10px !important; - margin-left: 15px !important; - - li { - margin: 5px 5px 5px 15px; - } - - ul { - padding-left: 15px !important; - } -} /** * Global fix for handsontable @@ -115,19 +84,20 @@ label { .report-container { overflow-x: auto; overflow-y: auto; - padding-top: 30px; - padding-bottom: 30px; + padding-left: 0; + width: auto; } #report-content { - color: $color-black; + @include box-shadow(0 0 58px -10px $color-black); background: $color-white; - @include box-shadow(0px 0px 58px -10px $color-black); - max-width: 800px; - min-width: 230px; - min-height: 1200px; + color: $color-black; margin-left: auto; margin-right: auto; + margin-top: 50px; + max-width: 800px; + min-height: 1200px; + min-width: 230px; padding: 45px; } diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index 83cd7850e..b4daa5dd8 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -49,8 +49,6 @@ class ReportsController < ApplicationController result_contents ) - layout 'fluid' - # Index showing all reports of a single project def index end diff --git a/app/views/reports/new.html.erb b/app/views/reports/new.html.erb index 41c43a58f..21916d75f 100644 --- a/app/views/reports/new.html.erb +++ b/app/views/reports/new.html.erb @@ -1,9 +1,7 @@ <% provide(:head_title, t("projects.reports.new.head_title", project: h(@project.name)).html_safe) %> <% provide(:body_class, "report-body") %> -<% provide(:sidebar_wrapper_class, "report-sidebar-wrapper") %> <% provide(:container_class, "report-container") %> -<%= render partial: "reports/new/report_sidebar" %> <%= render partial: "reports/new/report_navigation" %>
    - -
  • <% option_text = t("my_modules.module_archive.option_download") %> <% if result.is_asset %> - <%= link_to 'View', download_asset_path(result.asset), + <%= link_to t('protocols.index.archive_results.preview'), download_asset_path(result.asset), class: 'file-preview-link', id: "modal_link#{result.asset.id}", data: { no_turbolink: true, id: true, status: 'asset-present', 'preview-url': asset_file_preview_path(result.asset) } %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 2621e0a95..479b7b89c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1654,6 +1654,7 @@ en: row_success: "Archived" row_renamed: "Archived & renamed" row_failed: "Failed" + preview: "View" restore_results: title: "Restore results" message_failed: "Failed to restore %{nr} protocols." From f3af8f4524bccb978a4ab1f417879f8b907be427 Mon Sep 17 00:00:00 2001 From: zmagod Date: Thu, 19 Apr 2018 13:04:16 +0200 Subject: [PATCH 23/34] switch current_team to repository.team --- app/services/repository_actions/duplicate_cell.rb | 4 ++-- app/services/repository_actions/duplicate_rows.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/services/repository_actions/duplicate_cell.rb b/app/services/repository_actions/duplicate_cell.rb index 78ce26d2e..c70a5d0ad 100644 --- a/app/services/repository_actions/duplicate_cell.rb +++ b/app/services/repository_actions/duplicate_cell.rb @@ -2,11 +2,11 @@ module RepositoryActions class DuplicateCell - def initialize(cell, new_row, user) + def initialize(cell, new_row, user, team) @cell = cell @new_row = new_row @user = user - @team = user.current_team + @team = team end def call diff --git a/app/services/repository_actions/duplicate_rows.rb b/app/services/repository_actions/duplicate_rows.rb index 4a23936e9..91a412ed3 100644 --- a/app/services/repository_actions/duplicate_rows.rb +++ b/app/services/repository_actions/duplicate_rows.rb @@ -49,7 +49,7 @@ module RepositoryActions def duplicate_repository_cell(cell, new_row) RepositoryActions::DuplicateCell.new( - cell, new_row, @user + cell, new_row, @user, @repository.team ).call end end From c1832969b4be09c25897690e429fb2170b9b8a59 Mon Sep 17 00:00:00 2001 From: mlorb Date: Thu, 19 Apr 2018 13:26:10 +0200 Subject: [PATCH 24/34] fix top navigation style --- app/assets/stylesheets/themes/main_navigation.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/assets/stylesheets/themes/main_navigation.scss b/app/assets/stylesheets/themes/main_navigation.scss index e4a25938b..26e144dd7 100644 --- a/app/assets/stylesheets/themes/main_navigation.scss +++ b/app/assets/stylesheets/themes/main_navigation.scss @@ -270,6 +270,10 @@ padding-top: 5px; } + li:last-child { + border-bottom: 0; + } + .btn-default { text-align: right; width: 300px; From ce52eb851b2e01d0302c27a221516ab360264f42 Mon Sep 17 00:00:00 2001 From: zmagod Date: Thu, 19 Apr 2018 14:02:42 +0200 Subject: [PATCH 25/34] add validation to dropdown items --- .../repository_columns/index.js.erb | 12 ++- .../repository_columns_controller.rb | 88 ++++++++++++------- config/locales/en.yml | 1 + 3 files changed, 68 insertions(+), 33 deletions(-) diff --git a/app/assets/javascripts/repository_columns/index.js.erb b/app/assets/javascripts/repository_columns/index.js.erb index 474c4141b..89c8fdcf8 100644 --- a/app/assets/javascripts/repository_columns/index.js.erb +++ b/app/assets/javascripts/repository_columns/index.js.erb @@ -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(); + $('.bootstrap-tagsinput').after( + "" + message + "" + ); + } else { + var field = { "name": xhr.responseJSON.message } + $(form).renderFormErrors('repository_column', field, true, e); + } } else { HelperModule.flashAlertMsg(xhr.responseJSON.message, 'danger'); } diff --git a/app/controllers/repository_columns_controller.rb b/app/controllers/repository_columns_controller.rb index 216d7312f..d4d5cb241 100644 --- a/app/controllers/repository_columns_controller.rb +++ b/app/controllers/repository_columns_controller.rb @@ -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 @@ -163,8 +179,12 @@ 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| - next if column_items >= Constants::REPOSITORY_LIST_ITEMS_PER_COLUMN + if column_items >= Constants::REPOSITORY_LIST_ITEMS_PER_COLUMN + success = false + next + end RepositoryListItem.create( repository: @repository, repository_column: @repository_column, @@ -174,6 +194,7 @@ class RepositoryColumnsController < ApplicationController ) column_items += 1 end + success end def update_repository_list_items(item_names) @@ -193,9 +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) - next if column_items >= Constants::REPOSITORY_LIST_ITEMS_PER_COLUMN + if column_items >= Constants::REPOSITORY_LIST_ITEMS_PER_COLUMN + success = false + next + end RepositoryListItem.create( repository: @repository, repository_column: @repository_column, @@ -205,5 +230,6 @@ class RepositoryColumnsController < ApplicationController ) column_items += 1 end + success end end diff --git a/config/locales/en.yml b/config/locales/en.yml index e5410189b..bb5b31c05 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -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: From 56e00740c15a1515997e43018120ac1932605fbb Mon Sep 17 00:00:00 2001 From: mlorb Date: Thu, 19 Apr 2018 14:40:46 +0200 Subject: [PATCH 26/34] fix blocked buttons edit and eye icon --- app/views/shared/_left_menu_bar.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/shared/_left_menu_bar.html.erb b/app/views/shared/_left_menu_bar.html.erb index a8218df95..1ce887cbf 100644 --- a/app/views/shared/_left_menu_bar.html.erb +++ b/app/views/shared/_left_menu_bar.html.erb @@ -20,7 +20,7 @@ <% end %>
  • "> - <%= link_to repositories_path, id: "repositories-link", title: t('left_menu_bar.repositories') do %> + <%= link_to repositories_path, data: { no_turbolink: false }, id: "repositories-link", title: t('left_menu_bar.repositories') do %> <%= t('left_menu_bar.repositories') %> <% end %> From c149b3cc2ae82e2c8ddafc922e51b4b0a73d8053 Mon Sep 17 00:00:00 2001 From: mlorb Date: Thu, 19 Apr 2018 15:12:26 +0200 Subject: [PATCH 27/34] refactoring --- app/helpers/secondary_navigation_helper.rb | 10 ++++++++++ app/views/shared/_secondary_navigation.html.erb | 8 -------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/app/helpers/secondary_navigation_helper.rb b/app/helpers/secondary_navigation_helper.rb index e867d3c4a..de5477453 100644 --- a/app/helpers/secondary_navigation_helper.rb +++ b/app/helpers/secondary_navigation_helper.rb @@ -55,4 +55,14 @@ module SecondaryNavigationHelper def is_module_archive? action_name == 'archive' end + + def title_element + if project_page? + @project + elsif experiment_page? + @experiment + elsif module_page? + @my_module + end + end end diff --git a/app/views/shared/_secondary_navigation.html.erb b/app/views/shared/_secondary_navigation.html.erb index dc7c4527c..912521baa 100644 --- a/app/views/shared/_secondary_navigation.html.erb +++ b/app/views/shared/_secondary_navigation.html.erb @@ -161,14 +161,6 @@ - <% if project_page? %> - <% title_element = @project %> - <% elsif experiment_page? %> - <% title_element = @experiment %> - <% elsif module_page? %> - <% title_element = @my_module %> - <% end %> -
  • + <% end %> + <% if can_create_repository_columns?(@repository.team) %>
  • <%= link_to t('repositories.index.options_dropdown.manage_columns'), repository_repository_columns_path(@repository) %>