From 60d46efdd79ab1d8b7137d6338e57e64d78c4f85 Mon Sep 17 00:00:00 2001 From: Martin Artnik Date: Fri, 13 Sep 2024 13:43:45 +0200 Subject: [PATCH 1/2] Refactor import and add activities [SCI-11050] --- .../storage_locations_controller.rb | 10 ++ .../storage_locations/import_service.rb | 118 +++++++++--------- 2 files changed, 71 insertions(+), 57 deletions(-) diff --git a/app/controllers/storage_locations_controller.rb b/app/controllers/storage_locations_controller.rb index 1cc42bc50..92d8a33de 100644 --- a/app/controllers/storage_locations_controller.rb +++ b/app/controllers/storage_locations_controller.rb @@ -133,6 +133,16 @@ class StorageLocationsController < ApplicationController def import_container result = StorageLocations::ImportService.new(@storage_location, params[:file], current_user).import_items if result[:status] == :ok + if (result[:assigned_count] + result[:unassigned_count] + result[:updated_count]).positive? + log_activity( + :storage_location_imported, + { + assigned_count: result[:assigned_count], + unassigned_count: result[:unassigned_count] + } + ) + end + render json: result else render json: result, status: :unprocessable_entity diff --git a/app/services/storage_locations/import_service.rb b/app/services/storage_locations/import_service.rb index 7f9830d1b..78216c45b 100644 --- a/app/services/storage_locations/import_service.rb +++ b/app/services/storage_locations/import_service.rb @@ -6,79 +6,94 @@ module StorageLocations class ImportService def initialize(storage_location, file, user) @storage_location = storage_location - @file = file + @assigned_count = 0 + @unassigned_count = 0 + @updated_count = 0 + @sheet = SpreadsheetParser.open_spreadsheet(file) @user = user end def import_items - sheet = SpreadsheetParser.open_spreadsheet(@file) - incoming_items = SpreadsheetParser.spreadsheet_enumerator(sheet).reject { |r| r.all?(&:blank?) } + @rows = SpreadsheetParser.spreadsheet_enumerator(@sheet).reject { |r| r.all?(&:blank?) } # Check if the file has proper headers - header = SpreadsheetParser.parse_row(incoming_items[0], sheet) + header = SpreadsheetParser.parse_row(@rows[0], @sheet) return { status: :error, message: I18n.t('storage_locations.show.import_modal.errors.invalid_structure') } unless header[0] == 'Box position' && header[1] == 'Item ID' - # Remove first row - incoming_items.shift - - incoming_items.map! { |r| SpreadsheetParser.parse_row(r, sheet) } + parse_rows! # Check duplicate positions in the file - if @storage_location.with_grid? && incoming_items.pluck(0).uniq.length != incoming_items.length + if @storage_location.with_grid? && @rows.pluck(:position).uniq.length != @rows.length return { status: :error, message: I18n.t('storage_locations.show.import_modal.errors.invalid_position') } end - existing_items = @storage_location.storage_location_repository_rows.map do |item| - [convert_position_number_to_letter(item), item.repository_row_id, item.id] - end - - items_to_unassign = [] - - existing_items.each do |existing_item| - if incoming_items.any? { |r| r[0] == existing_item[0] && r[1].to_i == existing_item[1] } - incoming_items.reject! { |r| r[0] == existing_item[0] && r[1].to_i == existing_item[1] } - else - items_to_unassign << existing_item[2] - end - end - - error_message = '' - ActiveRecord::Base.transaction do - @storage_location.storage_location_repository_rows.where(id: items_to_unassign).discard_all + unassign_repository_rows! - incoming_items.each do |row| - if @storage_location.with_grid? - position = convert_position_letter_to_number(row[0]) - - unless position[0].to_i <= @storage_location.grid_size[0].to_i && position[1].to_i <= @storage_location.grid_size[1].to_i - error_message = I18n.t('storage_locations.show.import_modal.errors.invalid_position') - raise ActiveRecord::RecordInvalid - end + @rows.each do |row| + if @storage_location.with_grid? && !position_valid?(row[:position]) + @error_message = I18n.t('storage_locations.show.import_modal.errors.invalid_position') + raise ActiveRecord::RecordInvalid end - repository_row = RepositoryRow.find_by(id: row[1]) - - unless repository_row - error_message = I18n.t('storage_locations.show.import_modal.errors.invalid_item', row_id: row[1].to_i) + unless RepositoryRow.exists?(row[:repository_row_id]) + @error_message = I18n.t('storage_locations.show.import_modal.errors.invalid_item', row_id: row[:repository_row_id]) raise ActiveRecord::RecordNotFound end - @storage_location.storage_location_repository_rows.create!( - repository_row: repository_row, - metadata: { position: position }, - created_by: @user - ) + import_row!(row) end - rescue ActiveRecord::RecordNotFound - return { status: :error, message: error_message } end - { status: :ok } + { status: :ok, assigned_count: @assigned_count, unassigned_count: @unassigned_count, updated_count: @updated_count } + rescue ActiveRecord::RecordNotFound, ActiveRecord::RecordInvalid + { status: :error, message: @error_message } end private + def parse_rows! + # Remove first row + @rows.shift + + @rows.map! do |r| + row = SpreadsheetParser.parse_row(r, @sheet) + { + position: convert_position_letter_to_number(row[0]), + repository_row_id: row[1].gsub('IT', '').to_i + } + end + end + + def import_row!(row) + storage_location_repository_row = + @storage_location.storage_location_repository_rows + .find_or_initialize_by( + repository_row_id: row[:repository_row_id], + metadata: { position: row[:position] } + ) + + @assigned_count += 1 if storage_location_repository_row.new_record? + + storage_location_repository_row.update!( + repository_row_id: row[:repository_row_id], + created_by: storage_location_repository_row.created_by || @user + ) + end + + def unassign_repository_rows! + @storage_location.storage_location_repository_rows.each do |s| + if @rows.exclude?({ position: s.metadata['position'], repository_row_id: s.repository_row_id }) + @unassigned_count += 1 + s.discard + end + end + end + + def position_valid?(position) + position[0].to_i <= @storage_location.grid_size[0].to_i && position[1].to_i <= @storage_location.grid_size[1].to_i + end + def convert_position_letter_to_number(position) return unless position @@ -87,16 +102,5 @@ module StorageLocations [column_letter.ord - 64, row_number.to_i] end - - def convert_position_number_to_letter(item) - position = item.metadata['position'] - - return unless position - - column_letter = ('A'..'Z').to_a[position[0] - 1] - row_number = position[1] - - "#{column_letter}#{row_number}" - end end end From 04efd5f84fd9ce04a4700a4c865157bc65cfa835 Mon Sep 17 00:00:00 2001 From: Martin Artnik Date: Fri, 13 Sep 2024 13:58:23 +0200 Subject: [PATCH 2/2] Fix position validation for StorageLocationRepositoryRow [SCI-11050] --- app/models/storage_location_repository_row.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/storage_location_repository_row.rb b/app/models/storage_location_repository_row.rb index 126dfad0b..cf3d9f6ce 100644 --- a/app/models/storage_location_repository_row.rb +++ b/app/models/storage_location_repository_row.rb @@ -9,7 +9,7 @@ class StorageLocationRepositoryRow < ApplicationRecord belongs_to :repository_row, inverse_of: :storage_location_repository_rows belongs_to :created_by, class_name: 'User' - with_options if: -> { storage_location.container && storage_location.metadata['type'] == 'grid' } do + with_options if: -> { storage_location.container && storage_location.metadata['display_type'] == 'grid' } do validate :position_must_be_present validate :ensure_uniq_position end