From 4ad6971121a22bf4388b02fb7503e41de37ff9a0 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 6 Aug 2024 11:23:36 +1000 Subject: [PATCH] Fix Bulk product edit system spec after rebase --- .../spree/admin/variants_controller.rb | 2 + app/helpers/admin/products_helper.rb | 18 +++--- app/views/admin/products_v3/_table.html.haml | 2 +- .../admin/products_v3/_variant_row.html.haml | 2 +- .../controllers/bulk_form_controller.js | 40 ++++++++---- spec/system/admin/products_v3/actions_spec.rb | 14 ----- spec/system/admin/products_v3/create_spec.rb | 2 +- spec/system/admin/products_v3/update_spec.rb | 61 ++++++++++++++++--- 8 files changed, 94 insertions(+), 47 deletions(-) diff --git a/app/controllers/spree/admin/variants_controller.rb b/app/controllers/spree/admin/variants_controller.rb index 33f29e1b45..765a011dff 100644 --- a/app/controllers/spree/admin/variants_controller.rb +++ b/app/controllers/spree/admin/variants_controller.rb @@ -5,6 +5,8 @@ require 'open_food_network/scope_variants_for_search' module Spree module Admin class VariantsController < ::Admin::ResourceController + helper ::Admin::ProductsHelper + belongs_to 'spree/product' before_action :load_data, only: [:new, :edit] diff --git a/app/helpers/admin/products_helper.rb b/app/helpers/admin/products_helper.rb index fe00c22002..b67aa57540 100644 --- a/app/helpers/admin/products_helper.rb +++ b/app/helpers/admin/products_helper.rb @@ -18,17 +18,15 @@ module Admin end def unit_value_with_description(variant) - precised_unit_value = nil + return "" if variant.unit_value.nil? - if variant.unit_value - scaled_unit_value = variant.unit_value / (variant.product.variant_unit_scale || 1) - precised_unit_value = number_with_precision( - scaled_unit_value, - precision: nil, - strip_insignificant_zeros: true, - significant: false, - ) - end + scaled_unit_value = variant.unit_value / (variant.product.variant_unit_scale || 1) + precised_unit_value = number_with_precision( + scaled_unit_value, + precision: nil, + strip_insignificant_zeros: true, + significant: false, + ) [precised_unit_value, variant.unit_description].compact_blank.join(" ") end diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml index 71e777ba13..5f30291408 100644 --- a/app/views/admin/products_v3/_table.html.haml +++ b/app/views/admin/products_v3/_table.html.haml @@ -47,7 +47,7 @@ .form-buttons %a.button.reset.medium{ href: admin_products_path(page: @page, per_page: @per_page, search_term: @search_term, producer_id: @producer_id, category_id: @category_id), 'data-turbo': "false" } = t('.reset') - = form.submit t('.save'), class: "medium" + = form.submit t('.save'), { class: "medium", data: { action: "click->bulk-form#popoutEmptyVariantUnit" }} %tr %th.col-image.align-left= # image = render partial: 'spree/admin/shared/stimulus_sortable_header', diff --git a/app/views/admin/products_v3/_variant_row.html.haml b/app/views/admin/products_v3/_variant_row.html.haml index 25a387674c..2d3fd67896 100644 --- a/app/views/admin/products_v3/_variant_row.html.haml +++ b/app/views/admin/products_v3/_variant_row.html.haml @@ -13,7 +13,7 @@ = f.select :variant_unit_with_scale, options_for_select(WeightsAndMeasures.variant_unit_options, variant.variant_unit_with_scale), { include_blank: true }, - { class: "fullwidth no-input", 'aria-label': t('admin.products_page.columns.unit_scale'), data: { "controller": "tom-select", "tom-select-options-value": '{ "plugins": [] }', action: "change->toggle-control#displayIfMatch" } } + { class: "fullwidth no-input", 'aria-label': t('admin.products_page.columns.unit_scale'), data: { "controller": "tom-select", "tom-select-options-value": '{ "plugins": [] }', action: "change->toggle-control#displayIfMatch" }, required: true } = error_message_on variant, :variant_unit, 'data-toggle-control-target': 'control' .field = f.text_field :variant_unit_name, 'aria-label': t('items'), 'data-toggle-control-target': 'control', style: (variant.variant_unit == "items" ? "" : "display: none") diff --git a/app/webpacker/controllers/bulk_form_controller.js b/app/webpacker/controllers/bulk_form_controller.js index 5558b6d52a..476bf00391 100644 --- a/app/webpacker/controllers/bulk_form_controller.js +++ b/app/webpacker/controllers/bulk_form_controller.js @@ -93,6 +93,16 @@ export default class BulkFormController extends Controller { } } + // Pop out empty variant unit to allow browser side validation to focus the element + popoutEmptyVariantUnit() { + this.variantUnits = this.element.querySelectorAll("button.popout__button"); + this.variantUnits.forEach((element) => { + if (element.textContent == "") { + element.click(); + } + }); + } + // private #registerSubmit() { @@ -135,7 +145,7 @@ export default class BulkFormController extends Controller { // Check if changed, and mark with class if it is. #checkIsChanged(element) { - if(!element.isConnected) return false; + if (!element.isConnected) return false; const changed = this.#isChanged(element); element.classList.toggle("changed", changed); @@ -143,9 +153,8 @@ export default class BulkFormController extends Controller { } #isChanged(element) { - if (element.type == "checkbox") { + if (element.type == "checkbox") { return element.defaultChecked !== undefined && element.checked != element.defaultChecked; - } else if (element.type == "select-one") { // (weird) Behavior of select element's include_blank option in Rails: // If a select field has include_blank option selected (its value will be ''), @@ -155,42 +164,49 @@ export default class BulkFormController extends Controller { opt.hasAttribute("selected"), ); const selectedOption = element.selectedOptions[0]; - const areBothBlank = selectedOption.value === '' && defaultSelected === undefined + const areBothBlank = selectedOption.value === "" && defaultSelected === undefined; return !areBothBlank && selectedOption !== defaultSelected; - } else { return element.defaultValue !== undefined && element.value != element.defaultValue; } } #removeAnimationClasses(productRowElement) { - productRowElement.classList.remove('slide-in'); - productRowElement.removeEventListener('animationend', this.#removeAnimationClasses.bind(this, productRowElement)); + productRowElement.classList.remove("slide-in"); + productRowElement.removeEventListener( + "animationend", + this.#removeAnimationClasses.bind(this, productRowElement), + ); } #observeProductsTableRows() { this.productsTableObserver = new MutationObserver((mutationList, _observer) => { const mutationRecord = mutationList[0]; - if(mutationRecord) { + if (mutationRecord) { // Right now we are only using it for product clone, so it's always first const productRowElement = mutationRecord.addedNodes[0]; if (productRowElement) { - productRowElement.addEventListener('animationend', this.#removeAnimationClasses.bind(this, productRowElement)); + productRowElement.addEventListener( + "animationend", + this.#removeAnimationClasses.bind(this, productRowElement), + ); // This is equivalent to form.elements. - const productRowFormElements = productRowElement.querySelectorAll('input, select, textarea, button'); + const productRowFormElements = productRowElement.querySelectorAll( + "input, select, textarea, button", + ); this.#registerElements(productRowFormElements); this.toggleFormChanged(); } } }); - const productsTable = document.querySelector('.products'); + const productsTable = document.querySelector(".products"); // Above mutation function will trigger, // whenever +products+ table rows (first level children) are mutated i.e. added or removed - // right now we are using this for product clone + // right now we are using this for product clone this.productsTableObserver.observe(productsTable, { childList: true }); } } diff --git a/spec/system/admin/products_v3/actions_spec.rb b/spec/system/admin/products_v3/actions_spec.rb index 43b776f05f..9abb75cd56 100644 --- a/spec/system/admin/products_v3/actions_spec.rb +++ b/spec/system/admin/products_v3/actions_spec.rb @@ -293,20 +293,6 @@ RSpec.describe 'As an enterprise user, I can manage my products' do expect(input_content).to match /COPY OF Apples/ end end - - it "shows error message when cloning invalid record" do - # Existing product is invalid: - product_a.update_columns(name: nil) - - click_product_clone "Apples" - - expect(page).to have_content "Unit Scale can't be blank" - - within "table.products" do - # Products does not include the cloned product. - expect(all_input_values).not_to match /COPY OF Apples/ - end - end end end diff --git a/spec/system/admin/products_v3/create_spec.rb b/spec/system/admin/products_v3/create_spec.rb index d6db98d60c..e767ab3891 100644 --- a/spec/system/admin/products_v3/create_spec.rb +++ b/spec/system/admin/products_v3/create_spec.rb @@ -114,7 +114,7 @@ RSpec.describe 'As an enterprise user, I can manage my products' do expect(new_variant.display_name).to eq "Small bag" expect(new_variant.variant_unit).to eq "weight" expect(new_variant.variant_unit_scale).to eq 1 # g - expect(new_variant.unit_value).to eq 2.0 + expect(new_variant.unit_value).to eq 0.002 expect(new_variant.display_as).to eq "2 grams" expect(new_variant.unit_presentation).to eq "2 grams" expect(new_variant.price).to eq 11.1 diff --git a/spec/system/admin/products_v3/update_spec.rb b/spec/system/admin/products_v3/update_spec.rb index 73380fbdff..98f73fc15e 100644 --- a/spec/system/admin/products_v3/update_spec.rb +++ b/spec/system/admin/products_v3/update_spec.rb @@ -161,7 +161,7 @@ RSpec.describe 'As an enterprise user, I can update my products' do click_button "Save changes" expect(page).to have_content "Changes saved" - product_a.reload + variant_a1.reload }.to change{ variant_a1.variant_unit }.to("items") .and change{ variant_a1.variant_unit_name }.to("box") @@ -343,8 +343,9 @@ RSpec.describe 'As an enterprise user, I can update my products' do fill_in "Name", with: "Large box" fill_in "SKU", with: "APL-02" + tomselect_select("Weight (kg)", from: "Unit scale") click_on "Unit" # activate popout - fill_in "Unit value", with: "1000" + fill_in "Unit value", with: "1" fill_in "Price", with: 10.25 @@ -366,7 +367,9 @@ RSpec.describe 'As an enterprise user, I can update my products' do expect(new_variant.display_name).to eq "Large box" expect(new_variant.sku).to eq "APL-02" expect(new_variant.price).to eq 10.25 - expect(new_variant.unit_value).to eq 1000 + expect(new_variant.variant_unit).to eq "weight" + expect(new_variant.unit_value).to eq 1 * 1000 + expect(new_variant.variant_unit_scale).to eq 1000 expect(new_variant.on_hand).to eq 3 expect(new_variant.tax_category_id).to be_nil @@ -485,11 +488,12 @@ RSpec.describe 'As an enterprise user, I can update my products' do end context "with invalid data" do + let(:new_variant_row) { find_field("Name", placeholder: "Apples", with: "").ancestor("tr") } + before do click_on "New variant" # find empty row for Apples - new_variant_row = find_field("Name", placeholder: "Apples", with: "").ancestor("tr") expect(new_variant_row).to be_present within new_variant_row do @@ -508,6 +512,30 @@ RSpec.describe 'As an enterprise user, I can update my products' do fill_in "Price", with: "10.25" end + # Client side validation + click_button "Save changes" + within new_variant_row do + expect_browser_validation('select[aria-label="Unit scale"]', + "Please select an item in the list.") + end + + # Fix error + within new_variant_row do + tomselect_select("Weight (kg)", from: "Unit scale") + end + + # Client side validation + click_button "Save changes" + within new_variant_row do + expect_browser_validation('input[aria-label="Unit value"]', + "Please fill in this field.") + end + + # Fix error + within new_variant_row do + fill_in "Unit value", with: "200" + end + expect { click_button "Save changes" @@ -537,6 +565,13 @@ RSpec.describe 'As an enterprise user, I can update my products' do end it "saves changes after fixing errors" do + # Fill value to satisfy client side validation + within new_variant_row do + tomselect_select("Weight (kg)", from: "Unit scale") + click_on "Unit" # activate popout + fill_in "Unit value", with: "200" + end + expect { click_button "Save changes" @@ -547,9 +582,6 @@ RSpec.describe 'As an enterprise user, I can update my products' do fill_in "Name", with: "Nice box" fill_in "SKU", with: "APL-02" - click_on "Unit" # activate popout - fill_in "Unit value", with: "200" - select producer.name, from: 'Producer' select taxon.name, from: 'Category' end @@ -565,10 +597,18 @@ RSpec.describe 'As an enterprise user, I can update my products' do expect(new_variant.display_name).to eq "Nice box" expect(new_variant.sku).to eq "APL-02" expect(new_variant.price).to eq 10.25 - expect(new_variant.unit_value).to eq 200 + expect(new_variant.variant_unit_scale).to eq 1000 + expect(new_variant.unit_value).to eq 200 * 1000 end it "removes unsaved record" do + # Fill value to satisfy client side validation + within new_variant_row do + tomselect_select("Weight (kg)", from: "Unit scale") + click_on "Unit" # activate popout + fill_in "Unit value", with: "200" + end + click_button "Save changes" expect(page).to have_text("1 product could not be saved.") @@ -706,4 +746,9 @@ RSpec.describe 'As an enterprise user, I can update my products' do include_examples "updating image" end end + + def expect_browser_validation(selector, message) + browser_message = page.find(selector)["validationMessage"] + expect(browser_message).to eq message + end end