diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 08bd761dc4..04ee675c17 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -226,11 +226,13 @@ Metrics/ClassLength: - 'app/models/spree/user.rb' - 'app/models/spree/variant.rb' - 'app/models/spree/zone.rb' + - 'app/reflexes/products_reflex.rb' - 'app/serializers/api/cached_enterprise_serializer.rb' - 'app/serializers/api/enterprise_shopfront_serializer.rb' - 'app/services/cart_service.rb' - 'app/services/order_cycle_form.rb' - 'app/services/order_syncer.rb' + - 'app/services/sets/product_set.rb' - 'engines/order_management/app/services/order_management/order/updater.rb' - 'lib/open_food_network/enterprise_fee_calculator.rb' - 'lib/open_food_network/order_cycle_form_applicator.rb' diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index bc8bcd9081..8e18198a40 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -31,6 +31,23 @@ class ProductsReflex < ApplicationReflex fetch_and_render_products end + def bulk_update + product_set = product_set_from_params + + product_set.collection.each { |p| authorize! :update, p } + @products = product_set.collection # use instance variable mainly for testing + + if product_set.save + # flash[:success] = with_locale { I18n.t('.success') } + # morph_admin_flashes # ERROR: selector morph type has already been set + elsif product_set.errors.present? + # @error_msg = with_locale{ I18n.t('.products_have_error', count: product_set.invalid.count) } + @error_msg = "#{product_set.invalid.count} products have errors." + end + + render_products_form + end + private def init_filters_params @@ -69,6 +86,19 @@ class ProductsReflex < ApplicationReflex morph :nothing end + def render_products_form + cable_ready.replace( + selector: "#products-form", + html: render(partial: "admin/products_v3/table", + locals: { products: @products, error_msg: @error_msg }) + ).broadcast + morph :nothing + + # dunno why this doesn't work. + # morph "#products-form", render(partial: "admin/products_v3/table", + # locals: { products: products }) + end + def producers producers = OpenFoodNetwork::Permissions.new(current_user) .managed_product_enterprises.is_primary_producer.by_name @@ -130,4 +160,30 @@ class ProductsReflex < ApplicationReflex url.query += "&_category_id=#{@category_id}" if @category_id.present? url.to_s end + + # Similar to spree/admin/products_controller + def product_set_from_params + # Form field names: + # '[products][0][id]' (hidden field) + # '[products][0][name]' + # + # Resulting in params: + # "products" => { + # "" => { + # "id" => "123" + # "name" => "Pommes", + # } + # } + + collection_hash = products_bulk_params[:products].each_with_index + .to_h { |p, i| + [i, p] + }.with_indifferent_access + Sets::ProductSet.new(collection_attributes: collection_hash) + end + + def products_bulk_params + params.permit(products: ::PermittedAttributes::Product.attributes) + .to_h.with_indifferent_access + end end diff --git a/app/services/sets/model_set.rb b/app/services/sets/model_set.rb index d5ff4e2ccb..7347e2d9c9 100644 --- a/app/services/sets/model_set.rb +++ b/app/services/sets/model_set.rb @@ -54,6 +54,10 @@ module Sets errors end + def invalid + @collection.select { |model| model.errors.any? } + end + def save collection_to_delete.each(&:destroy) collection_to_keep.all?(&:save) diff --git a/app/services/sets/product_set.rb b/app/services/sets/product_set.rb index 101104ad38..7eb73e3bee 100644 --- a/app/services/sets/product_set.rb +++ b/app/services/sets/product_set.rb @@ -1,20 +1,28 @@ # frozen_string_literal: true module Sets + # Accepts a collection_hash in format: + # { + # 0=> {id:"7449", name:"Pommes"}, + # 1=> {...} + # } + # class ProductSet < ModelSet def initialize(attributes = {}) super(Spree::Product, [], attributes) end def save - @collection_hash.each_value.all? do |product_attributes| + # Attempt to save all records, collecting model errors. + @collection_hash.each_value.map do |product_attributes| update_product_attributes(product_attributes) - end + end.all? end def collection_attributes=(attributes) - @collection = Spree::Product - .where(id: attributes.each_value.map { |product| product[:id] }) + ids = attributes.values.pluck(:id).compact + # Find and load existing products in the order they are provided + @collection = Spree::Product.find(ids) @collection_hash = attributes end diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml index 779f9c5275..ff9c2e19d9 100644 --- a/app/views/admin/products_v3/_table.html.haml +++ b/app/views/admin/products_v3/_table.html.haml @@ -1,66 +1,80 @@ -%table.products - %col{ width:"15%" } - %col{ width:"5%", style: "max-width:5em" } - %col{ width:"8%" } - %col{ width:"5%", style: "max-width:5em"} - %col{ width:"5%", style: "max-width:5em"} - %col{ width:"10%" }= # producer - %col{ width:"10%" } - %col{ width:"5%" } - %col{ width:"5%", style: "max-width:5em" } - %thead - %tr - %th.align-left= t('admin.products_page.columns.name') - %th.align-right= t('admin.products_page.columns.sku') - %th.align-right= t('admin.products_page.columns.unit') - %th.align-right= t('admin.products_page.columns.price') - %th.align-right= t('admin.products_page.columns.on_hand') - %th.align-left= t('admin.products_page.columns.producer') - %th.align-left= t('admin.products_page.columns.category') - %th.align-left= t('admin.products_page.columns.tax_category') - %th.align-left= t('admin.products_page.columns.inherits_properties') - - products.each do |product| - %tbody.relaxed - %tr - %td.align-left.header - .line-clamp-1= product.name - %td.align-right - .line-clamp-1= product.sku - %td.align-right - .line-clamp-1 - = product.variant_unit.upcase_first - / TODO: properly handle custom unit names - = WeightsAndMeasures::UNITS[product.variant_unit] && "(" + WeightsAndMeasures::UNITS[product.variant_unit][product.variant_unit_scale]["name"] + ")" - %td.align-right - -# empty - %td.align-right - -# TODO: new requirement "DISPLAY ON DEMAND IF ALL VARIANTS ARE ON DEMAND". And translate value - .line-clamp-1= if product.variants.all?(&:on_demand) then "On demand" else product.on_hand || 0 end - %td.align-left - .line-clamp-1= product.supplier.name - %td.align-left - .line-clamp-1= product.primary_taxon.name - %td.align-left - %td.align-left - .line-clamp-1= product.inherits_properties ? 'YES' : 'NO' #TODO: consider using https://github.com/RST-J/human_attribute_values, else use I18n.t (also below) - - product.variants.each do |variant| - %tr.condensed - %td.align-left - .line-clamp-1= variant.display_name - %td.align-right - .line-clamp-1= variant.sku - %td.align-right - .line-clamp-1= variant.unit_to_display - %td.align-right - .line-clamp-1= number_to_currency(variant.price) - %td.align-right - .line-clamp-1= variant.on_hand || 0 #TODO: spec for this according to requirements. - %td.align-left - .line-clamp-1= variant.product.supplier.name # same as product - %td.align-left - -# empty - %td.align-left - .line-clamp-1= variant.tax_category&.name || "None" # TODO: convert to dropdown, else translate hardcoded string. - %td.align-left - -# empty += form_with url: bulk_update_admin_products_v3_index_path, method: :patch, id: "products-form", + html: {'data-reflex-serialize-form': true, 'data-reflex': 'submit->products#bulk_update'} do |form| + %fieldset.form-actions + .container + .status.ten.columns + / = t('.products_modified', count: 'X') + - if defined?(error_msg) && error_msg.present? + .error + = error_msg + .form-buttons.six.columns + = form.submit t('.reset'), type: :reset, class: "medium" + = form.submit t('.save'), class: "medium" + %table.products + %col{ width:"15%" } + %col{ width:"5%", style: "max-width:5em" } + %col{ width:"8%" } + %col{ width:"5%", style: "max-width:5em"} + %col{ width:"5%", style: "max-width:5em"} + %col{ width:"10%" }= # producer + %col{ width:"10%" } + %col{ width:"5%" } + %col{ width:"5%", style: "max-width:5em" } + %thead + %tr + %th.align-left.with-input= t('admin.products_page.columns.name') + %th.align-right= t('admin.products_page.columns.sku') + %th.align-right= t('admin.products_page.columns.unit') + %th.align-right= t('admin.products_page.columns.price') + %th.align-right= t('admin.products_page.columns.on_hand') + %th.align-left= t('admin.products_page.columns.producer') + %th.align-left= t('admin.products_page.columns.category') + %th.align-left= t('admin.products_page.columns.tax_category') + %th.align-left= t('admin.products_page.columns.inherits_properties') + - products.each do |product, i| + = form.fields_for(product) do |product_form| + %tbody.relaxed + %tr + %td.align-left.header + = product_form.hidden_field :id, name: "[products][#{i}][id]" #todo: can we remove #{i} and implicitly pop? + .line-clamp-1= product_form.text_field :name, name: "[products][#{i}][name]", id: "_product_name_#{product.id}", 'aria-label': t('admin.products_page.columns.name') + %td.align-right + .line-clamp-1= product.sku + %td.align-right + .line-clamp-1 + = product.variant_unit.upcase_first + / TODO: properly handle custom unit names + = WeightsAndMeasures::UNITS[product.variant_unit] && "(" + WeightsAndMeasures::UNITS[product.variant_unit][product.variant_unit_scale]["name"] + ")" + %td.align-right + -# empty + %td.align-right + -# TODO: new requirement "DISPLAY ON DEMAND IF ALL VARIANTS ARE ON DEMAND". And translate value + .line-clamp-1= if product.variants.all?(&:on_demand) then "On demand" else product.on_hand || 0 end + %td.align-left + .line-clamp-1= product.supplier.name + %td.align-left + .line-clamp-1= product.primary_taxon.name + %td.align-left + %td.align-left + .line-clamp-1= product.inherits_properties ? 'YES' : 'NO' #TODO: consider using https://github.com/RST-J/human_attribute_values, else use I18n.t (also below) + - product.variants.each do |variant| + %tr.condensed + %td.align-left + .line-clamp-1= variant.display_name + %td.align-right + .line-clamp-1= variant.sku + %td.align-right + .line-clamp-1= variant.unit_to_display + %td.align-right + .line-clamp-1= number_to_currency(variant.price) + %td.align-right + .line-clamp-1= variant.on_hand || 0 #TODO: spec for this according to requirements. + %td.align-left + .line-clamp-1= variant.product.supplier.name # same as product + %td.align-left + -# empty + %td.align-left + .line-clamp-1= variant.tax_category&.name || "None" # TODO: convert to dropdown, else translate hardcoded string. + %td.align-left + -# empty diff --git a/app/webpacker/css/admin/globals/variables.scss b/app/webpacker/css/admin/globals/variables.scss index 86fb56b7af..7ea08bbc7d 100644 --- a/app/webpacker/css/admin/globals/variables.scss +++ b/app/webpacker/css/admin/globals/variables.scss @@ -67,6 +67,8 @@ $color-sel-hover-text: $color-1 !default; $color-txt-brd: $color-border !default; $color-txt-text: $color-3 !default; $color-txt-hover-brd: $color-2 !default; +$vpadding-txt: 7px; +$hpadding-txt: 10px; // Modal colors $color-modal-close-btn: $color-5 !default; diff --git a/app/webpacker/css/admin/products_v3.scss b/app/webpacker/css/admin/products_v3.scss index ba34c089d4..5b5cb2fe9c 100644 --- a/app/webpacker/css/admin/products_v3.scss +++ b/app/webpacker/css/admin/products_v3.scss @@ -23,10 +23,27 @@ padding: 4px; border-collapse: separate; // This is needed for the outer padding. Also should be helpful to give more flexibility of borders between rows. + // Additional horizontal padding to align with input contents + thead th.with-input { + padding-left: $padding-tbl-cell + $hpadding-txt; + padding-right: $padding-tbl-cell + $hpadding-txt; + } + + // Row hover tr:hover { - th, td { background-color: $light-grey; + position: relative; + + // Left border + &:first-child:before { + content: ""; + position: absolute; + top: 0; + left: -4px; + border-left: 4px solid $teal; + height: 100%; + } } } @@ -75,6 +92,21 @@ padding: $padding-tbl-cell-condensed; } } + + // "Naked" inputs. Row hover helps reveal them. + input { + border-color: transparent; + + &:focus { + border-color: $color-txt-hover-brd; + } + } + + .field_with_errors { + input { + border-color: $color-error; + } + } } #no-products { diff --git a/app/webpacker/css/admin/shared/forms.scss b/app/webpacker/css/admin/shared/forms.scss index 6e956df095..da593e3300 100644 --- a/app/webpacker/css/admin/shared/forms.scss +++ b/app/webpacker/css/admin/shared/forms.scss @@ -8,7 +8,7 @@ input[type="number"], textarea, fieldset { @include border-radius($border-radius); - padding: 7px 10px; + padding: $vpadding-txt $hpadding-txt; border: 1px solid $color-txt-brd; color: $color-txt-text; font-size: 90%; @@ -236,9 +236,6 @@ fieldset { } } -.form-actions { - margin-top: 18px; -} .form-buttons { text-align: center; } diff --git a/app/webpacker/css/admin_v3/all.scss b/app/webpacker/css/admin_v3/all.scss index 7f93f602ad..0f48247044 100644 --- a/app/webpacker/css/admin_v3/all.scss +++ b/app/webpacker/css/admin_v3/all.scss @@ -31,7 +31,7 @@ @import "shared/typography"; // admin_v3 @import "shared/tables"; // admin_v3 @import "shared/icons"; // admin_v3 -@import "../admin/shared/forms"; +@import "shared/forms"; // admin_v3 @import "shared/layout"; // admin_v3 @import "../admin/shared/scroll_bar"; diff --git a/app/webpacker/css/admin_v3/components/buttons.scss b/app/webpacker/css/admin_v3/components/buttons.scss index f233e23a48..9b77e318d2 100644 --- a/app/webpacker/css/admin_v3/components/buttons.scss +++ b/app/webpacker/css/admin_v3/components/buttons.scss @@ -1,4 +1,5 @@ input[type="submit"], +input[type="reset"], input[type="button"]:not(.trix-button), button:not(.plain):not(.trix-button), .button { @@ -11,7 +12,6 @@ button:not(.plain):not(.trix-button), background-color: $color-btn-bg; border: 1px solid $color-btn-bg; color: $color-btn-text; - text-transform: uppercase; line-height: $btn-regular-height - 2px; // remove 2px to compensate for border height: $btn-regular-height; font-weight: bold; @@ -112,6 +112,25 @@ button:not(.plain):not(.trix-button), } } +input[type="reset"] { + // Reset button looks like a link, but has a border the same as buttons when active. + background: none; + border: 1px solid transparent; + color: $color-link; + + &:active { + color: $color-link-active; + } + &:focus { + color: $color-link-focus; + } + &:hover { + color: $color-link-hover; + background: none; + border-color: transparent; + } +} + a.button { text-decoration: none; } diff --git a/app/webpacker/css/admin_v3/globals/variables.scss b/app/webpacker/css/admin_v3/globals/variables.scss index cf8fa70498..7706a4073d 100644 --- a/app/webpacker/css/admin_v3/globals/variables.scss +++ b/app/webpacker/css/admin_v3/globals/variables.scss @@ -14,9 +14,9 @@ $color-body-bg: $white !default; $color-body-text: $near-black !default; $color-headers: $dark-blue !default; $color-link: $teal !default; -$color-link-hover: lighten($color-link, 2) !default; -$color-link-active: $green !default; -$color-link-focus: $green !default; +$color-link-hover: $orient !default; +$color-link-active: $dark-blue !default; +$color-link-focus: $orient !default; $color-link-visited: $teal !default; $color-border: $light-grey !default; @@ -33,9 +33,9 @@ $color-tbl-cell-shadow: rgb(0, 0, 0, 0.15) !default; $color-tbl-thead-txt: $color-headers !default; $color-tbl-thead-bg: $light-grey !default; $color-tbl-border: $pale-blue !default; -$padding-tbl-cell: 12px 12px; +$padding-tbl-cell: 12px; $padding-tbl-cell-condensed: 10px 12px; -$padding-tbl-cell-relaxed: 16px 12px; +$padding-tbl-cell-relaxed: 12px 12px; // Button colors $color-btn-bg: $teal !default; @@ -70,10 +70,12 @@ $color-sel-text: $white !default; $color-sel-hover-bg: lighten($color-sel-bg, 2) !default; $color-sel-hover-text: $white !default; -// Text inputs colors +// Text inputs styles $color-txt-brd: $color-border !default; $color-txt-text: $near-black !default; $color-txt-hover-brd: $teal !default; +$vpadding-txt: 5px; +$hpadding-txt: 8px; // Modal colors $color-modal-close-btn: $color-5 !default; diff --git a/app/webpacker/css/admin_v3/shared/forms.scss b/app/webpacker/css/admin_v3/shared/forms.scss new file mode 100644 index 0000000000..9f5fe2ac58 --- /dev/null +++ b/app/webpacker/css/admin_v3/shared/forms.scss @@ -0,0 +1,282 @@ +$text-inputs: "input[type=text], input[type=password], input[type=email], input[type=url], input[type=tel]"; + +#{$text-inputs}, +input[type="date"], +input[type="datetime"], +input[type="time"], +input[type="number"], +textarea, +fieldset { + @include border-radius($border-radius); + padding: $vpadding-txt $hpadding-txt; + border: 1px solid $color-txt-brd; + color: $color-txt-text; + font-size: 90%; + + &:focus { + outline: none; + border-color: $color-txt-hover-brd; + } + + &[disabled] { + opacity: 0.7; + } +} + +textarea { + line-height: 19px; +} + +.fullwidth { + width: 100%; +} + +label { + font-weight: 600; + text-transform: uppercase; + font-size: 85%; + display: inline; + margin-bottom: 5px; + color: $color-4; + + &.inline { + display: inline-block !important; + } + + &.block { + display: block !important; + } +} + +.label-block label { + display: block; +} + +span.info { + font-style: italic; + font-size: 85%; + color: lighten($color-body-text, 15); + display: block; + line-height: 20px; + margin: 5px 0; +} + +.field { + padding: 10px 0; + + &.checkbox { + min-height: 70px; + + input[type="checkbox"] { + display: inline-block; + width: auto; + } + + label { + cursor: pointer; + display: block; + } + } + + ul { + border-top: 1px solid $color-border; + list-style: none; + padding-top: 5px; + + li { + display: inline-block; + padding-right: 10px; + + label { + font-weight: normal; + text-transform: none; + } + &.white-space-nowrap { + white-space: nowrap; + } + } + } + + &.withError { + .field_with_errors { + label { + color: $color-error; + } + + input { + border-color: $color-error; + } + } + .formError { + color: $color-error; + font-style: italic; + font-size: 85%; + } + } +} + +fieldset { + box-shadow: none; + box-sizing: border-box; + border-color: $color-border; + -webkit-box-sizing: border-box; + -moz-box-sizing: border-box; + margin-left: 0; + margin-right: 0; + position: relative; + margin-bottom: 35px; + padding: 10px 0 15px 0; + background-color: transparent; + border-left: none; + border-right: none; + border-radius: 0; + + &.no-border-bottom { + border-bottom: none; + margin-bottom: 0; + } + + &.no-border-top { + border-top: none; + padding-top: 0; + } + + legend { + background-color: $color-1; + color: $color-2; + font-size: 14px; + font-weight: 600; + text-transform: uppercase; + text-align: center; + padding: 8px 15px; + margin: 0 auto; + + -webkit-font-smoothing: antialiased; + -moz-osx-font-smoothing: grayscale; + + i { + color: $color-link; + } + } + + label { + color: lighten($color-body-text, 8); + } + + .filter-actions { + margin-bottom: -32px; + margin-top: 15px; + text-align: center; + + form { + display: inline-block; + } + + button, + .button, + input[type="submit"], + input[type="button"], + span.or { + @include border-radius($border-radius); + + -webkit-box-shadow: 0 0 0 15px $color-1; + -moz-box-shadow: 0 0 0 15px $color-1; + -ms-box-shadow: 0 0 0 15px $color-1; + -o-box-shadow: 0 0 0 15px $color-1; + box-shadow: 0 0 0 15px $color-1; + + &:hover { + border-color: $color-1; + } + + &:first-of-type { + margin-right: 1.25em; + } + } + + span.or { + background-color: $color-1; + border-width: 5px; + margin-left: 5px; + margin-right: 5px; + position: relative; + + -webkit-box-shadow: 0 0 0 5px $color-1; + -moz-box-shadow: 0 0 0 5px $color-1; + -ms-box-shadow: 0 0 0 5px $color-1; + -o-box-shadow: 0 0 0 5px $color-1; + box-shadow: 0 0 0 5px $color-1; + } + } + + &.labels-inline { + .field { + margin-bottom: 0; + display: table; + width: 100%; + + label, + input { + display: table-cell !important; + } + input { + width: 100%; + } + + &.checkbox { + input { + width: auto !important; + } + } + } + .actions { + padding: 0; + text-align: right; + } + } +} + +.form-actions { + @include defaultBoxShadow; + background-color: $fair-pink; + border: none; + border-left: 4px solid $red; + border-radius: 4px; + margin: 0.5em 0; + padding: 0; + + .status { + font-size: 1rem; + font-weight: bold; + padding: 0.75em 1em; + } + + .form-buttons { + padding: 0.5em 1em; + text-align: right; + } +} + +.form-buttons { + text-align: center; +} + +select { + @extend input, [type="text"]; + background-color: white; +} + +.inline-checkbox { + display: inline-flex; + align-items: center; + margin-top: 3px; + + input, + label { + cursor: pointer; + } + label { + margin: 0; + padding-left: 0.4rem; + } +} diff --git a/config/locales/en.yml b/config/locales/en.yml index c9f57070de..9020961d32 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -818,6 +818,14 @@ en: no_products_found: No products found import_products: Import multiple products no_products_found_for_search: No products found for your search criteria + table: + save: Save changes + reset: Discard changes + bulk_update: # TODO: fix these + success: "Products successfully updated" #TODO: add count + products_have_error: + one: "%{count} product has an error." + other: "%{count} products have an error." product_import: title: Product Import file_not_found: File not found or could not be opened diff --git a/config/routes/admin.rb b/config/routes/admin.rb index dea9207fb2..95da893872 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -70,7 +70,9 @@ Openfoodnetwork::Application.routes.draw do post '/product_import/reset_absent', to: 'product_import#reset_absent_products', as: 'product_import_reset_async' constraints FeatureToggleConstraint.new(:admin_style_v3) do - resources :products_v3, only: :index + resources :products_v3, as: :products_v3, only: :index do + patch :bulk_update, on: :collection + end end resources :variant_overrides do diff --git a/spec/reflexes/products_reflex_spec.rb b/spec/reflexes/products_reflex_spec.rb index 4b1b6a6b90..c927930efc 100644 --- a/spec/reflexes/products_reflex_spec.rb +++ b/spec/reflexes/products_reflex_spec.rb @@ -32,4 +32,88 @@ describe ProductsReflex, type: :reflex do end end end + + describe '#bulk_update' do + let!(:product_b) { create(:simple_product, name: "Bananas") } + let!(:product_a) { create(:simple_product, name: "Apples") } + + it "saves valid changes" do + params = { + # '[products][][name]' + "products" => [ + { + "id" => product_a.id.to_s, + "name" => "Pommes", + } + ] + } + + expect{ + run_reflex(:bulk_update, params:) + product_a.reload + }.to change(product_a, :name).to("Pommes") + end + + describe "sorting" do + let(:params) { + { + "products" => [ + { + "id" => product_a.id.to_s, + "name" => "Pommes", + }, + { + "id" => product_b.id.to_s, + }, + ] + } + } + subject{ run_reflex(:bulk_update, params:) } + + it "Should retain sort order, even when names change" do + expect(subject.get(:products).map(&:id)).to eq [ + product_a.id, + product_b.id, + ] + end + end + + describe "error messages" do + it "summarises error messages" do + params = { + "products" => [ + { + "id" => product_a.id.to_s, + "name" => "", + }, + { + "id" => product_b.id.to_s, + "name" => "", + }, + ] + } + + reflex = run_reflex(:bulk_update, params:) + expect(reflex.get(:error_msg)).to include "2 products have errors" + + # # WTF + # expect{ reflex(:bulk_update, params:) }.to broadcast( + # replace: { + # selector: "#products-form", + # html: /2 products have errors/, + # }, + # broadcast: nil + # ) + end + end + end +end + +# Build and run a reflex using the context +# Parameters can be added with params: option +# For more options see https://github.com/podia/stimulus_reflex_testing#usage +def run_reflex(method_name, opts = {}) + build_reflex(method_name:, **context.merge(opts)).tap{ |reflex| + reflex.run(method_name) + } end diff --git a/spec/services/sets/model_set_spec.rb b/spec/services/sets/model_set_spec.rb index 416aa63fb4..8008771343 100644 --- a/spec/services/sets/model_set_spec.rb +++ b/spec/services/sets/model_set_spec.rb @@ -55,5 +55,32 @@ describe Sets::ModelSet do expect { ms.save }.to change(Enterprise, :count).by(0) end + + describe "errors" do + let(:product_b) { create(:simple_product, name: "Bananas") } + let(:product_a) { create(:simple_product, name: "Apples") } + let(:collection_attributes) do + { + 0 => { + id: product_a.id, + name: "", # Product Name can't be blank + }, + 1 => { + id: product_b.id, + name: "Bananes", + }, + } + end + subject{ Sets::ModelSet.new(Spree::Product, [product_a, product_b], collection_attributes:) } + + it 'errors are aggregated' do + subject.save + + expect(subject.errors.full_messages).to eq ["Product Name can't be blank"] + + expect(subject.invalid).to include product_a + expect(subject.invalid).to_not include product_b + end + end end end diff --git a/spec/services/sets/product_set_spec.rb b/spec/services/sets/product_set_spec.rb index 07c17d2995..e68601f887 100644 --- a/spec/services/sets/product_set_spec.rb +++ b/spec/services/sets/product_set_spec.rb @@ -198,6 +198,65 @@ describe Sets::ProductSet do end end end + + context 'when there are multiple products' do + let!(:product_b) { create(:simple_product, name: "Bananas") } + let!(:product_a) { create(:simple_product, name: "Apples") } + + let(:collection_hash) do + { + 0 => { + id: product_a.id, + name: "Pommes", + }, + 1 => { + id: product_b.id, + name: "Bananes", + }, + } + end + + it 'updates the products' do + product_set.save + + expect(product_a.reload.name).to eq "Pommes" + expect(product_b.reload.name).to eq "Bananes" + end + + it 'retains the order of products' do + product_set.save + + expect(product_set.collection[0]).to eq product_a.reload + expect(product_set.collection[1]).to eq product_b.reload + end + + context 'first product has an error' do + let(:collection_hash) do + { + 0 => { + id: product_a.id, + name: "", # Product Name can't be blank + }, + 1 => { + id: product_b.id, + name: "Bananes", + }, + } + end + + it 'continues to update subsequent products' do + product_set.save + + # Errors are logged on the model + first_item = product_set.collection[0] + expect(first_item.errors.full_messages.to_sentence).to eq "Product Name can't be blank" + expect(first_item.name).to eq "" + + # Subsequent product was updated + expect(product_b.reload.name).to eq "Bananes" + end + end + end end end end diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb index bddcfab8c1..b0e98484c4 100644 --- a/spec/system/admin/products_v3/products_spec.rb +++ b/spec/system/admin/products_v3/products_spec.rb @@ -24,7 +24,7 @@ describe 'As an admin, I can see the new product page' do end describe "sorting" do - let!(:product_z) { create(:simple_product, name: "Bananas") } + let!(:product_b) { create(:simple_product, name: "Bananas") } let!(:product_a) { create(:simple_product, name: "Apples") } before do @@ -32,7 +32,13 @@ describe 'As an admin, I can see the new product page' do end it "Should sort products alphabetically by default" do - expect(page).to have_content /Apples.*Bananas/ + within "table.products" do + # Gather input values, because page.content doesn't include them. + input_content = page.find_all('input[type=text]').map(&:value).join + + # Products are in correct order. + expect(input_content).to match /Apples.*Bananas/ + end end end @@ -91,7 +97,7 @@ describe 'As an admin, I can see the new product page' do search_for "searchable product" expect(page).to have_field "search_term", with: "searchable product" expect_products_count_to_be 1 - expect(page).to have_selector "table.products tbody tr td", text: product_by_name.name + expect(page).to have_field "Name", with: product_by_name.name click_link "Clear search" expect(page).to have_field "search_term", with: "" @@ -130,11 +136,32 @@ describe 'As an admin, I can see the new product page' do expect(page).to have_select "category_id", selected: "Category 1" expect_products_count_to_be 1 - expect(page).to have_selector "table.products tbody tr td", text: product_by_category.name + expect(page).to have_field "Name", with: product_by_category.name end end end + describe "updating" do + before do + visit admin_products_v3_index_url + end + + it "can update product fields" do + fill_in id: "_product_name_#{product_1.id}", with: "An updated name" + + expect { + click_button "Save changes" + product_1.reload + }.to( + change { product_1.name }.to("An updated name") + ) + + expect(page).to have_field id: "_product_name_#{product_1.id}", with: "An updated name" + pending + expect(page).to have_content "Changes saved" + end + end + def expect_page_to_be(page_number) expect(page).to have_selector ".pagination span.page.current", text: page_number.to_s end diff --git a/spec/system/support/capybara_setup.rb b/spec/system/support/capybara_setup.rb index e253f192fe..6e46d77420 100644 --- a/spec/system/support/capybara_setup.rb +++ b/spec/system/support/capybara_setup.rb @@ -1,5 +1,9 @@ # frozen_string_literal: true +# Whether fields, links, and buttons will match against aria-label attribute. +# This allows us to find with `expect(page).to have_field "Name"` +Capybara.enable_aria_label = true + # Usually, especially when using Selenium, developers tend to increase the max wait time. # With Cuprite, there is no need for that. # We use a Capybara default value here explicitly.