diff --git a/app/assets/stylesheets/admin/product_import.css.scss b/app/assets/stylesheets/admin/product_import.css.scss index d8ad019af9..20dcc481f1 100644 --- a/app/assets/stylesheets/admin/product_import.css.scss +++ b/app/assets/stylesheets/admin/product_import.css.scss @@ -1,15 +1,22 @@ @import "variables"; -div.panel-section { +$pi-red: $warning-red; +$pi-green: lighten($spree-green, 10%); +$pi-orange: $bright-orange; +$pi-blue: lighten($spree-blue, 10%); +$pi-light-yellow: #faffaf; - .neutral { - color: #bfbfbf; +// scss-lint:disable NestingDepth + +div.panel-section { + .error { + color: $pi-red; } .warning { - color: $warning-red; + color: $bright-orange; } .success { - color: #86d83a; + color: $pi-green; } .info { color: #68b7c0; @@ -85,21 +92,29 @@ div.panel-section { td, th { white-space: nowrap; } - tr.error { - color: #c84C4c; + + tr { + &.error { + color: #c84C4c; + } + + &:hover td.invalid { + background-color: darken(#f05c51, 5%); + } + + i { + display: block; + margin-bottom: -0.2em; + font-size: 1.4em !important; + } } - tr:hover td.invalid { - background-color: #ed5135; - } - tr i { - display: block; - margin-bottom: -0.2em; - font-size: 1.4em !important; - } - td.invalid { - background-color: #f05c51; - box-shadow: inset 0px 0px 1px red; - color: white; + + td { + &.invalid { + background-color: #f05c51; + box-shadow: inset 0px 0px 1px red; + color: white; + } } } @@ -123,52 +138,6 @@ br.panels.clearfix { clear: both; } -table.import-settings { - background-color: transparent !important; - width: auto; - - tbody tr:hover td { - background-color: #f3f3f3; - } - td { - border: 0; - border-bottom: 1px solid #eee; - text-align: left; - - input { - width: 15em; - } - input[type="checkbox"] { - width: auto; - } - - } - td.description { - font-weight: bold; - padding-right: 2.5em; - } - tr:first-child td { - border-top: 0; - } - tr:last-child td { - border-bottom: 0; - } - div.select2-container { - width: 13.5em; - } - div.select2-container.select2-container-disabled { - a.select2-choice, span.select2-arrow { - background-color: #d5d5d5; - } - } - input[disabled], input:disabled { - background-color: #d5d5d5; - opacity: 1; - border-color: transparent; - color: white !important; - } -} - .panel-section.import-settings { .header-description { @@ -177,7 +146,7 @@ table.import-settings { span.header-error { font-size: 0.85em; - color: $warning-red; + color: $pi-red; } .select2-search { @@ -208,10 +177,10 @@ table.import-settings { } i.fa-check-circle { - color: #86d83a; + color: $pi-green; } i.fa-info-circle { - color: #68b7c0; + color: $pi-blue; } } @@ -234,6 +203,10 @@ form.product-import, div.post-save-results, div.import-wrapper { div.import-wrapper { + .alert-box { + margin: 0 0 1.75em; + } + .ng-hide:not(.ng-hide-animate) { // We have to use !important here to override angular's display properties // scss-lint:disable ImportantRule @@ -281,7 +254,7 @@ div.progress-bar { span.progress-track{ display: block; - background: #b7ea53; + background: lighten($pi-green, 10%); height: 100%; border-radius: 0.3em; box-shadow: inset 0 0 3px rgba(0,0,0,0.3); @@ -312,7 +285,7 @@ div.progress-bar { span.category { display: inline-block; - background-color: lighten($spree-blue, 10%); + background-color: $pi-blue; color: white; padding: 0.3em 0.6em; margin: 0 0.4em 0.5em 0; diff --git a/app/assets/stylesheets/admin/variables.css.scss b/app/assets/stylesheets/admin/variables.css.scss index f8c12e0d4d..416daec5a9 100644 --- a/app/assets/stylesheets/admin/variables.css.scss +++ b/app/assets/stylesheets/admin/variables.css.scss @@ -6,6 +6,7 @@ $spree-light-blue: #eff5fc; $warning-red: #da5354; $warning-orange: #da7f52; +$bright-orange: #ffa92e; $medium-grey: #919191; $pale-blue: #cee1f4; diff --git a/app/controllers/admin/product_import_controller.rb b/app/controllers/admin/product_import_controller.rb index 38ea79a97d..eb07ccf9f9 100644 --- a/app/controllers/admin/product_import_controller.rb +++ b/app/controllers/admin/product_import_controller.rb @@ -14,6 +14,7 @@ module Admin @filepath = save_uploaded_file(params[:file]) @importer = ProductImport::ProductImporter.new(File.new(@filepath), spree_current_user, params[:settings]) @original_filename = params[:file].try(:original_filename) + @non_updatable_fields = ProductImport::EntryValidator.non_updatable_fields check_file_errors @importer check_spreadsheet_has_data @importer diff --git a/app/models/product_import/entry_validator.rb b/app/models/product_import/entry_validator.rb index 33a7932f4f..9c20e26415 100644 --- a/app/models/product_import/entry_validator.rb +++ b/app/models/product_import/entry_validator.rb @@ -14,6 +14,16 @@ module ProductImport @import_settings = import_settings end + def self.non_updatable_fields + { + category: :primary_taxon_id, + unit_type: :variant_unit_scale, + variant_unit_name: :variant_unit_name, + tax_category: :tax_category_id, + shipping_category: :shipping_category_id + } + end + def validate_all(entries) entries.each do |entry| supplier_validation(entry) @@ -182,6 +192,8 @@ module ProductImport return end + products.each { |product| product_field_errors(entry, product) } + products.flat_map(&:variants).each do |existing_variant| if entry_matches_existing_variant?(entry, existing_variant) && existing_variant.deleted_at.nil? return mark_as_existing_variant(entry, existing_variant) @@ -215,6 +227,25 @@ module ProductImport end end + def product_field_errors(entry, existing_product) + non_updatable_fields.each do |display_name, attribute| + next if attributes_match?(attribute, existing_product, entry) || attributes_blank?(attribute, existing_product, entry) + mark_as_invalid(entry, attribute: display_name, error: I18n.t('admin.product_import.model.not_updatable')) + end + end + + def non_updatable_fields + EntryValidator.non_updatable_fields + end + + def attributes_match?(attribute, existing_product, entry) + existing_product.send(attribute) == entry.send(attribute) + end + + def attributes_blank?(attribute, existing_product, entry) + existing_product.send(attribute).blank? && entry.send(attribute).blank? + end + def permission_by_name?(supplier_name) @editable_enterprises.key?(supplier_name) end diff --git a/app/models/product_import/product_importer.rb b/app/models/product_import/product_importer.rb index 99362b8c1b..6cabcc9e2f 100644 --- a/app/models/product_import/product_importer.rb +++ b/app/models/product_import/product_importer.rb @@ -57,6 +57,13 @@ module ProductImport @sheet ? @sheet.last_row - 1 : 0 end + def product_field_errors? + @entries.each do |entry| + return true if entry.errors.messages.values.include? [I18n.t('admin.product_import.model.not_updatable')] + end + false + end + def reset_counts # Return indexed data about existing product count, reset count, and updates count per supplier @reset_counts.each do |supplier_id, values| diff --git a/app/views/admin/product_import/_entries_table.html.haml b/app/views/admin/product_import/_entries_table.html.haml index 8ad958b1c2..6b0dcfdf6e 100644 --- a/app/views/admin/product_import/_entries_table.html.haml +++ b/app/views/admin/product_import/_entries_table.html.haml @@ -7,7 +7,7 @@ %th= heading %tr{ng: {repeat: "(line_number, entry) in (entries | entriesFilterValid:'#{entries}')"}} %td - %i{ng: {class: "{'fa fa-warning warning': (count(entry.errors) > 0), 'fa fa-check-circle success': (count(entry.errors) == 0)}"}} + %i{ng: {class: "{'fa fa-warning error': (count(entry.errors) > 0), 'fa fa-check-circle success': (count(entry.errors) == 0)}"}} %td {{line_number}} %td{ng: {repeat: "(attribute, value) in entry.attributes", class: "{'invalid': attribute_invalid(attribute, line_number)}"}} diff --git a/app/views/admin/product_import/_import_review.html.haml b/app/views/admin/product_import/_import_review.html.haml index 9015c31bb8..90d8de9577 100644 --- a/app/views/admin/product_import/_import_review.html.haml +++ b/app/views/admin/product_import/_import_review.html.haml @@ -3,6 +3,12 @@ %div{ng: {controller: 'ImportFeedbackCtrl'}} + - if @importer.product_field_errors? + .alert-box.warning + = t('.not_updatable_tip') + %em= @non_updatable_fields.keys.join(', ') + "." + = t('.fields_ignored') + %div.panel-section{ng: {controller: 'DropdownPanelsCtrl'}} %div.panel-header{ng: {click: 'togglePanel()', class: '{active: active && count((entries | entriesFilterValid:"all"))}'}} %div.header-caret @@ -21,7 +27,7 @@ %div.panel-header{ng: {click: 'togglePanel()', class: '{active: active && count((entries | entriesFilterValid:"invalid"))}'}} %div.header-caret %i{ng: {class: "{'icon-chevron-down': active, 'icon-chevron-right': !active}", hide: 'count((entries | entriesFilterValid:"invalid")) == 0'}} - %div.header-icon.warning + %div.header-icon.error %i.fa.fa-warning %div.header-count %strong.invalid-count diff --git a/config/locales/en.yml b/config/locales/en.yml index b9dabc2129..342e1c80f0 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -514,6 +514,7 @@ en: conditional_blank: can't be blank if unit_type is blank no_product: did not match any products in the database not_found: not found in database + not_updatable: cannot be updated on existing products via product import blank: can't be blank products_no_permission: you do not have permission to manage products for this enterprise inventory_no_permission: you do not have permission to create inventory for this producer @@ -572,6 +573,11 @@ en: inventory_to_reset: Existing inventory items will have their stock reset to zero line: Line item_line: Item line + import_review: + not_updatable_tip: "The following fields cannot be updated via bulk import for existing products:" + fields_ignored: These fields will be ignored when the imported products are saved. + entries_table: + not_updatable: This field is not updatable via bulk import on existing products save_results: final_results: Import final results products_created: Products created diff --git a/spec/models/product_importer_spec.rb b/spec/models/product_importer_spec.rb index b587ad8d76..a13e951076 100644 --- a/spec/models/product_importer_spec.rb +++ b/spec/models/product_importer_spec.rb @@ -16,22 +16,23 @@ describe ProductImport::ProductImporter do let!(:category) { create(:taxon, name: 'Vegetables') } let!(:category2) { create(:taxon, name: 'Cake') } - let!(:category3) { create(:taxon, name: 'Cereal') } + let!(:category3) { create(:taxon, name: 'Meat') } + let!(:category4) { create(:taxon, name: 'Cereal') } let!(:tax_category) { create(:tax_category) } let!(:tax_category2) { create(:tax_category) } let!(:shipping_category) { create(:shipping_category) } - let!(:product) { create(:simple_product, supplier: enterprise2, name: 'Hypothetical Cake') } + let!(:product) { create(:simple_product, supplier: enterprise2, name: 'Hypothetical Cake', primary_taxon_id: category2.id) } let!(:variant) { create(:variant, product_id: product.id, price: '8.50', on_hand: '100', unit_value: '500', display_name: 'Preexisting Banana') } - let!(:product2) { create(:simple_product, supplier: enterprise, on_hand: '100', name: 'Beans', unit_value: '500') } - let!(:product3) { create(:simple_product, supplier: enterprise, on_hand: '100', name: 'Sprouts', unit_value: '500') } - let!(:product4) { create(:simple_product, supplier: enterprise, on_hand: '100', name: 'Cabbage', unit_value: '500') } - let!(:product5) { create(:simple_product, supplier: enterprise2, on_hand: '100', name: 'Lettuce', unit_value: '500') } - let!(:product6) { create(:simple_product, supplier: enterprise3, on_hand: '100', name: 'Beetroot', unit_value: '500', on_demand: true, variant_unit_scale: 1, variant_unit: 'weight') } - let!(:product7) { create(:simple_product, supplier: enterprise3, on_hand: '100', name: 'Tomato', unit_value: '500', variant_unit_scale: 1, variant_unit: 'weight') } + let!(:product2) { create(:simple_product, supplier: enterprise, on_hand: '100', name: 'Beans', unit_value: '500', primary_taxon_id: category.id) } + let!(:product3) { create(:simple_product, supplier: enterprise, on_hand: '100', name: 'Sprouts', unit_value: '500', primary_taxon_id: category.id) } + let!(:product4) { create(:simple_product, supplier: enterprise, on_hand: '100', name: 'Cabbage', unit_value: '500', primary_taxon_id: category.id) } + let!(:product5) { create(:simple_product, supplier: enterprise2, on_hand: '100', name: 'Lettuce', unit_value: '500', primary_taxon_id: category.id) } + let!(:product6) { create(:simple_product, supplier: enterprise3, on_hand: '100', name: 'Beetroot', unit_value: '500', on_demand: true, variant_unit_scale: 1, variant_unit: 'weight', primary_taxon_id: category.id) } + let!(:product7) { create(:simple_product, supplier: enterprise3, on_hand: '100', name: 'Tomato', unit_value: '500', variant_unit_scale: 1, variant_unit: 'weight', primary_taxon_id: category.id) } - let!(:product8) { create(:simple_product, supplier: enterprise, on_hand: '100', name: 'Oats', unit_value: '500', variant_unit_scale: 1, variant_unit: 'weight', primary_taxon_id: category3.id) } - let!(:product9) { create(:simple_product, supplier: enterprise, on_hand: '100', name: 'Oats', unit_value: '500', variant_unit_scale: 1, variant_unit: 'weight', primary_taxon_id: category3.id) } + let!(:product8) { create(:simple_product, supplier: enterprise, on_hand: '100', name: 'Oats', unit_value: '500', variant_unit_scale: 1, variant_unit: 'weight', primary_taxon_id: category4.id) } + let!(:product9) { create(:simple_product, supplier: enterprise, on_hand: '100', name: 'Oats', unit_value: '500', variant_unit_scale: 1, variant_unit: 'weight', primary_taxon_id: category4.id) } let!(:variant2) { create(:variant, product_id: product8.id, price: '4.50', on_hand: '100', unit_value: '500', display_name: 'Porridge Oats') } let!(:variant3) { create(:variant, product_id: product8.id, price: '5.50', on_hand: '100', unit_value: '500', display_name: 'Rolled Oats') } let!(:variant4) { create(:variant, product_id: product9.id, price: '6.50', on_hand: '100', unit_value: '500', display_name: 'Flaked Oats') } @@ -327,6 +328,33 @@ describe ProductImport::ProductImporter do end end + describe "updating non-updatable fields on existing products" do + before do + csv_data = CSV.generate do |csv| + csv << ["name", "supplier", "category", "on_hand", "price", "units", "unit_type"] + csv << ["Beetroot", "And Another Enterprise", "Meat", "5", "3.50", "500", "g"] + csv << ["Tomato", "And Another Enterprise", "Vegetables", "6", "5.50", "500", "Kg"] + end + File.write('/tmp/test-m.csv', csv_data) + file = File.new('/tmp/test-m.csv') + settings = {'import_into' => 'product_list'} + @importer = ProductImport::ProductImporter.new(file, admin, start: 1, end: 100, settings: settings) + end + after { File.delete('/tmp/test-m.csv') } + + it "does not allow updating" do + @importer.validate_entries + entries = JSON.parse(@importer.entries_json) + + expect(filter('valid', entries)).to eq 0 + expect(filter('invalid', entries)).to eq 2 + + @importer.all_entries.each do |entry| + expect(entry.errors.messages.values).to include [I18n.t('admin.product_import.model.not_updatable')] + end + end + end + describe "when more than one product of the same name already exists with multiple variants each" do before do csv_data = CSV.generate do |csv|