diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index 01736eedab..c75eddce24 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -167,18 +167,26 @@ class ProductsReflex < ApplicationReflex # Form field names: # '[products][0][id]' (hidden field) # '[products][0][name]' + # '[products][0][variants_attributes][0][id]' (hidden field) + # '[products][0][variants_attributes][0][display_name]' # # Resulting in params: # "products" => { - # "" => { + # "0" => { # "id" => "123" # "name" => "Pommes", + # "variants_attributes" => { + # "0" => { + # "id" => "1234", + # "display_name" => "Large box", + # } # } # } - - collection_hash = products_bulk_params[:products].each_with_index - .to_h { |p, i| - [i, p] + collection_hash = products_bulk_params[:products] + .transform_values { |product| + # Convert variants_attributes form hash to an array if present + product[:variants_attributes] &&= product[:variants_attributes].values + product }.with_indifferent_access Sets::ProductSet.new(collection_attributes: collection_hash) end diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml index f7a4b69df6..519738169c 100644 --- a/app/views/admin/products_v3/_table.html.haml +++ b/app/views/admin/products_v3/_table.html.haml @@ -38,8 +38,8 @@ %th.align-left= t('admin.products_page.columns.tax_category') %th.align-left= t('admin.products_page.columns.inherits_properties') %th.align-right= t('admin.products_page.columns.actions') - - products.each do |product| - = form.fields_for("products", product, index: nil) do |product_form| + - products.each_with_index do |product, product_index| + = form.fields_for("products", product, index: product_index) do |product_form| %tbody.relaxed{ 'data-record-id': product_form.object.id } %tr %td.field.align-left.header @@ -57,8 +57,7 @@ %td.align-right -# empty %td.align-right - -# TODO: new requirement "DISPLAY ON DEMAND IF ALL VARIANTS ARE ON DEMAND". And translate value - .content= if product.variants.all?(&:on_demand) then "On demand" else product.on_hand || 0 end + -# empty %td.align-left .content= product.supplier&.name %td.align-left @@ -71,8 +70,8 @@ = link_to t('admin.products_page.actions.edit'), edit_admin_product_path(product) = link_to t('admin.products_page.actions.clone'), clone_admin_product_path(product) - - product.variants.each do |variant| - = form.fields_for("products][][variants_attributes][", variant, index: nil) do |variant_form| + - product.variants.each_with_index do |variant, variant_index| + = form.fields_for("products][#{product_index}][variants_attributes][", variant, variant_index:) do |variant_form| %tr.condensed %td.field = variant_form.hidden_field :id @@ -86,8 +85,17 @@ %td.field = variant_form.text_field :price, 'aria-label': t('admin.products_page.columns.price'), value: number_to_currency(variant.price, unit: '')&.strip # TODO: add a spec to prove that this formatting is necessary. If so, it should be in a shared form helper for currency inputs = error_message_on variant, :price - %td.align-right - .content= variant.on_hand || 0 #TODO: spec for this according to requirements. + %td.field.on-hand__wrapper{'data-controller': "popout"} + %button.on-hand__button{'data-popout-target': "button", 'aria-label': t('admin.products_page.columns.on_hand')} + = variant.on_demand ? t(:on_demand) : variant.on_hand + %div.on-hand__popout{ style: 'display: none;', 'data-controller': 'toggle-control', 'data-popout-target': "dialog" } + .field + = variant_form.number_field :on_hand, 'aria-label': t('admin.products_page.columns.on_hand'), 'data-toggle-control-target': 'control', disabled: variant_form.object.on_demand + = error_message_on variant, :on_hand + .field.checkbox + = variant_form.label :on_demand do + = variant_form.check_box :on_demand, 'data-action': 'change->toggle-control#disableIfPresent change->popout#closeIfChecked' + = t(:on_demand) %td.align-left .content= variant.product.supplier&.name # same as product %td.align-left diff --git a/app/webpacker/controllers/bulk_form_controller.js b/app/webpacker/controllers/bulk_form_controller.js index ab93978584..91af33e9e0 100644 --- a/app/webpacker/controllers/bulk_form_controller.js +++ b/app/webpacker/controllers/bulk_form_controller.js @@ -15,8 +15,7 @@ export default class BulkFormController extends Controller { // Start listening for any changes within the form // this.element.addEventListener('change', this.toggleChanged.bind(this)); // dunno why this doesn't work for (const element of this.form.elements) { - element.addEventListener("keyup", this.toggleChanged.bind(this)); // instant response - element.addEventListener("change", this.toggleChanged.bind(this)); // just in case (eg right-click paste) + element.addEventListener("input", this.toggleChanged.bind(this)); // immediately respond to any change // Set up a tree of fields according to their associated record const recordContainer = element.closest("[data-record-id]"); // The JS could be more efficient if this data was added to each element. But I didn't want to pollute the HTML too much. @@ -100,6 +99,10 @@ export default class BulkFormController extends Controller { } #isChanged(element) { - return element.value != element.defaultValue; + if (element.type == "checkbox") { + return element.defaultChecked !== undefined && element.checked != element.defaultChecked; + } else { + return element.defaultValue !== undefined && element.value != element.defaultValue; + } } } diff --git a/app/webpacker/controllers/popout_controller.js b/app/webpacker/controllers/popout_controller.js new file mode 100644 index 0000000000..0e430ce1a9 --- /dev/null +++ b/app/webpacker/controllers/popout_controller.js @@ -0,0 +1,91 @@ +import { Controller } from "stimulus"; + +// Allows a form section to "pop out" and show additional options +export default class PopoutController extends Controller { + static targets = ["button", "dialog"]; + + connect() { + this.first_input = this.dialogTarget.querySelector("input"); + this.displayElements = Array.from(this.element.querySelectorAll('input:not([type="hidden"]')); + + // Show when click or down-arrow on button + this.buttonTarget.addEventListener("click", this.show.bind(this)); + this.buttonTarget.addEventListener("keydown", this.showIfDownArrow.bind(this)); + + // Close when click or tab outside of dialog. Run async (don't block primary event handlers). + this.closeIfOutsideBound = this.closeIfOutside.bind(this); // Store reference for removing listeners later. + document.addEventListener("click", this.closeIfOutsideBound, { passive: true }); + document.addEventListener("focusin", this.closeIfOutsideBound, { passive: true }); + } + + disconnect() { + // Clean up handlers registered outside the controller element. + // (jest cleans up document too early) + if (document) { + document.removeEventListener("click", this.closeIfOutsideBound); + document.removeEventListener("focusin", this.closeIfOutsideBound); + } + } + + show(e) { + this.dialogTarget.style.display = "block"; + this.first_input.focus(); + e.preventDefault(); + } + + showIfDownArrow(e) { + if (e.keyCode == 40) { + this.show(e); + } + } + + close() { + // Close if not already closed + if (this.dialogTarget.style.display != "none") { + // Update button to represent any changes + this.buttonTarget.innerText = this.#displayValue(); + this.buttonTarget.classList.toggle("changed", this.#isChanged()); + + this.dialogTarget.style.display = "none"; + } + } + + closeIfOutside(e) { + if (!this.dialogTarget.contains(e.target)) { + this.close(); + } + } + + // Close if checked + closeIfChecked(e) { + if (e.target.checked) { + this.close(); + this.buttonTarget.focus(); + } + } + + // private + + // Summarise the active field(s) + #displayValue() { + let values = this.#enabledDisplayElements().map((element) => { + if (element.type == "checkbox") { + if (element.checked && element.labels[0]) { + return element.labels[0].innerText; + } + } else { + return element.value; + } + }); + // Filter empty values and convert to string + return values.filter(Boolean).join(); + } + + #isChanged() { + return this.#enabledDisplayElements().some((element) => element.classList.contains("changed")); + } + + #enabledDisplayElements() { + return this.displayElements.filter((element) => !element.disabled); + } +} diff --git a/app/webpacker/controllers/toggle_control_controller.js b/app/webpacker/controllers/toggle_control_controller.js new file mode 100644 index 0000000000..c9a64d9b29 --- /dev/null +++ b/app/webpacker/controllers/toggle_control_controller.js @@ -0,0 +1,33 @@ +import { Controller } from "stimulus"; + +export default class extends Controller { + static targets = ["control"]; + + disableIfPresent(event) { + const input = event.currentTarget; + const disable = !!this.#inputValue(input); // Coerce value to boolean + + this.controlTargets.forEach((target) => { + target.disabled = disable; + }); + + // Focus when enabled + if (!disable) { + this.controlTargets[0].focus(); + } + } + + //todo: can a new method disableIfBlank replace ButtonDisabledController? + //todo: can a new method toggleDisplay replace ToggleController? + //todo: can toggleDisplay with optional chevron-target replace RemoteToggleController? + + // private + + // Return input's value, but only if it would be submitted by a form + // Radio buttons not supported (yet) + #inputValue(input) { + if (input.type != "checkbox" || input.checked) { + return input.value; + } + } +} diff --git a/app/webpacker/css/admin/orders.scss b/app/webpacker/css/admin/orders.scss index 8738ce51c7..74531609f8 100644 --- a/app/webpacker/css/admin/orders.scss +++ b/app/webpacker/css/admin/orders.scss @@ -1,3 +1,10 @@ +.admin-orders-index-search { + .inline-checkbox { + // Ensure it lines up with other fields + min-height: 5.4em; + } +} + input, div { &.update-pending { diff --git a/app/webpacker/css/admin/products_v3.scss b/app/webpacker/css/admin/products_v3.scss index 2764f4b954..f8d9a8272f 100644 --- a/app/webpacker/css/admin/products_v3.scss +++ b/app/webpacker/css/admin/products_v3.scss @@ -114,20 +114,24 @@ } } + .field { + padding: 0; + margin-bottom: 0.75em; + } + + label { + margin: 0; + } + // "Naked" inputs. Row hover helps reveal them. - input { - border-color: transparent; + input:not([type="checkbox"]) { background-color: $color-tbl-cell-bg; height: auto; font-size: inherit; font-weight: inherit; - &:focus { - border-color: $color-txt-hover-brd; - } - - &.changed { - border-color: $color-txt-changed-brd; + &:not(:focus):not(.changed):not([disabled]) { + border-color: transparent; } } @@ -265,4 +269,82 @@ z-index: 1; // Ensure tom-select is covered } } + + // Stock popout widget + .on-hand { + &__wrapper { + position: relative; + } + + &__button { + // override button styles + &.on-hand__button { + background: $color-tbl-cell-bg; + color: $color-txt-text; + white-space: nowrap; + border-color: transparent; + font-weight: normal; + padding-left: $border-radius; // Super compact + padding-right: 1rem; // Retain space for arrow + height: auto; + + &:hover, + &:active, + &:focus { + background: $color-tbl-cell-bg; + color: $color-txt-text; + position: relative; + } + + &.changed { + border-color: $color-txt-changed-brd; + } + } + + &:hover:not(:active):not(:focus):not(.changed) { + border-color: transparent; + } + + &:hover, + &:active, + &:focus { + // for some reason, sass ignores &:active, &:focus here. we could make this a mixin and include it in multiple rules instead + &:before { + // for some reason, sass seems to extends the selector to include every other :before selector in the app! probably causing the above, and potentially breaking other styles. + // extending .icon-chevron-down causes infinite loop in compilation. does @include work for classes? + font-family: FontAwesome; + text-decoration: inherit; + display: inline-block; + speak: none; + content: "\f078"; + + position: absolute; + right: $border-radius; + font-size: 0.67em; + } + } + } + + &__popout { + position: absolute; + top: -1em; + left: -1em; + z-index: 1; // Cover below row when hover + width: 9em; + + padding: $padding-tbl-cell; + + background: $color-tbl-cell-bg; + border-radius: $border-radius; + box-shadow: 0px 0px 8px 0px rgba($near-black, 0.25); + + .field:last-child { + margin-bottom: 0; + } + + input[disabled] { + color: transparent; // hide value completely + } + } + } } diff --git a/app/webpacker/css/admin_v3/globals/palette.scss b/app/webpacker/css/admin_v3/globals/palette.scss index 66e557b63a..59b3c88c3a 100644 --- a/app/webpacker/css/admin_v3/globals/palette.scss +++ b/app/webpacker/css/admin_v3/globals/palette.scss @@ -13,7 +13,7 @@ $lighter-grey: #f8f9fa !default; // Lighter grey $light-grey: #eff1f2 !default; // Light grey (Porcelain) $medium-grey: #919191 !default; // Medium grey $dark-grey: #2e3132 !default; // Dark Grey -$near-black: #191c1d !default; // Near-black +$near-black: #191c1d !default; // Near-black (Shark) $fair-pink: #ffefeb !default; // Fair Pink $roof-terracotta: #b83b1f !default; // Roof Terracotta diff --git a/app/webpacker/css/admin_v3/globals/variables.scss b/app/webpacker/css/admin_v3/globals/variables.scss index a85da4fcc6..7eecf26ea8 100644 --- a/app/webpacker/css/admin_v3/globals/variables.scss +++ b/app/webpacker/css/admin_v3/globals/variables.scss @@ -75,10 +75,16 @@ $color-sel-hover-bg: $lighter-grey !default; $color-txt-brd: $color-border !default; $color-txt-text: $near-black !default; $color-txt-hover-brd: $teal !default; -$color-txt-changed-brd: $bright-orange !default; +$color-txt-disabled-text: $medium-grey !default; +$color-txt-disabled-brd: $light-grey !default; +$color-txt-changed-brd: $bright-orange !default; $vpadding-txt: 5px; $hpadding-txt: 8px; +// Checkboxes +$color-checkbox-brd: $near-black !default; +$color-checkbox-fill: $teal !default; + // Modal colors $color-modal-close-btn: $color-5 !default; $color-modal-close-btn-hover: darken($color-5, 5%) !default; diff --git a/app/webpacker/css/admin_v3/shared/forms.scss b/app/webpacker/css/admin_v3/shared/forms.scss index 35f8697e04..8a37545a29 100644 --- a/app/webpacker/css/admin_v3/shared/forms.scss +++ b/app/webpacker/css/admin_v3/shared/forms.scss @@ -28,7 +28,8 @@ fieldset { } &[disabled] { - opacity: 0.7; + color: $color-txt-disabled-text; + border-color: $color-txt-disabled-brd; } &.changed { @@ -36,6 +37,18 @@ fieldset { } } +input[type="checkbox"] { + width: 1em; + height: 1em; + margin-right: 3px; +} + +input[type="checkbox"], +input[type="radio"], +input[type="range"] { + accent-color: $color-checkbox-fill; +} + textarea { line-height: 19px; } @@ -76,16 +89,11 @@ span.info { padding: 10px 0; &.checkbox { - min-height: 70px; - - input[type="checkbox"] { - display: inline-block; - width: auto; - } - label { cursor: pointer; display: block; + font-size: inherit; + font-weight: normal; } } diff --git a/spec/javascripts/stimulus/bulk_form_controller_test.js b/spec/javascripts/stimulus/bulk_form_controller_test.js index d558e4b37a..b0a3386ce9 100644 --- a/spec/javascripts/stimulus/bulk_form_controller_test.js +++ b/spec/javascripts/stimulus/bulk_form_controller_test.js @@ -36,6 +36,7 @@ describe("BulkFormController", () => {
+
@@ -43,22 +44,12 @@ describe("BulkFormController", () => { `; - - const disable1 = document.getElementById("disable1"); - const disable1_element = document.getElementById("disable1_element"); - const disable2 = document.getElementById("disable2"); - const disable2_element = document.getElementById("disable2_element"); - const actions = document.getElementById("actions"); - const changed_summary = document.getElementById("changed_summary"); - const input1a = document.getElementById("input1a"); - const input1b = document.getElementById("input1b"); - const input2 = document.getElementById("input2"); }); describe("marking changed fields", () => { - it("onChange", () => { + it("onInput", () => { input1a.value = 'updated1a'; - input1a.dispatchEvent(new Event("change")); + input1a.dispatchEvent(new Event("input")); // Expect only first field to show changed expect(input1a.classList).toContain('changed'); expect(input1b.classList).not.toContain('changed'); @@ -66,30 +57,16 @@ describe("BulkFormController", () => { // Change back to original value input1a.value = 'initial1a'; - input1a.dispatchEvent(new Event("change")); + input1a.dispatchEvent(new Event("input")); expect(input1a.classList).not.toContain('changed'); }); - it("onKeyup", () => { - input1a.value = 'u1a'; - input1a.dispatchEvent(new Event("keyup")); - // Expect only first field to show changed - expect(input1a.classList).toContain('changed'); - expect(input1b.classList).not.toContain('changed'); - expect(input2.classList).not.toContain('changed'); - - // Change back to original value - input1a.value = 'initial1a'; - input1a.dispatchEvent(new Event("keyup")); - expect(input1a.classList).not.toContain('changed'); - }); - it("multiple fields", () => { input1a.value = 'updated1a'; - input1a.dispatchEvent(new Event("change")); + input1a.dispatchEvent(new Event("input")); input2.value = 'updated2'; - input2.dispatchEvent(new Event("change")); + input2.dispatchEvent(new Event("input")); // Expect only first field to show changed expect(input1a.classList).toContain('changed'); expect(input1b.classList).not.toContain('changed'); @@ -97,7 +74,7 @@ describe("BulkFormController", () => { // Change only one back to original value input1a.value = 'initial1a'; - input1a.dispatchEvent(new Event("change")); + input1a.dispatchEvent(new Event("input")); expect(input1a.classList).not.toContain('changed'); expect(input1b.classList).not.toContain('changed'); expect(input2.classList).toContain('changed'); @@ -109,7 +86,7 @@ describe("BulkFormController", () => { it("counts changed records ", () => { // Record 1: First field changed input1a.value = 'updated1a'; - input1a.dispatchEvent(new Event("change")); + input1a.dispatchEvent(new Event("input")); // Actions and changed summary are shown, with other sections disabled expect(actions.classList).not.toContain('hidden'); expect(changed_summary.textContent).toBe('changed_summary, {"count":1}'); @@ -120,21 +97,21 @@ describe("BulkFormController", () => { // Record 1: Second field changed input1b.value = 'updated1b'; - input1b.dispatchEvent(new Event("change")); + input1b.dispatchEvent(new Event("input")); // Expect to show same summary translation expect(actions.classList).not.toContain('hidden'); expect(changed_summary.textContent).toBe('changed_summary, {"count":1}'); // Record 2: has been changed input2.value = 'updated2'; - input2.dispatchEvent(new Event("change")); + input2.dispatchEvent(new Event("input")); // Expect summary to count both records expect(actions.classList).not.toContain('hidden'); expect(changed_summary.textContent).toBe('changed_summary, {"count":2}'); // Record 1: Change first field back to original value input1a.value = 'initial1a'; - input1a.dispatchEvent(new Event("change")); + input1a.dispatchEvent(new Event("input")); // Both records are still changed. expect(input1a.classList).not.toContain('changed'); expect(input1b.classList).toContain('changed'); @@ -144,7 +121,7 @@ describe("BulkFormController", () => { // Record 1: Change second field back to original value input1b.value = 'initial1b'; - input1b.dispatchEvent(new Event("change")); + input1b.dispatchEvent(new Event("input")); // Both fields for record 1 show unchanged, but second record is still changed expect(actions.classList).not.toContain('hidden'); expect(changed_summary.textContent).toBe('changed_summary, {"count":1}'); @@ -155,7 +132,7 @@ describe("BulkFormController", () => { // Record 2: Change back to original value input2.value = 'initial2'; - input2.dispatchEvent(new Event("change")); + input2.dispatchEvent(new Event("input")); // Actions are hidden and other sections are now re-enabled expect(actions.classList).toContain('hidden'); expect(changed_summary.textContent).toBe('changed_summary, {"count":0}'); @@ -192,13 +169,13 @@ describe("BulkFormController", () => { // Record 1: First field changed input1a.value = 'updated1a'; - input1a.dispatchEvent(new Event("change")); + input1a.dispatchEvent(new Event("input")); // Expect actions to remain visible expect(actions.classList).not.toContain('hidden'); // Change back to original value input1a.value = 'initial1a'; - input1a.dispatchEvent(new Event("change")); + input1a.dispatchEvent(new Event("input")); // Expect actions to remain visible expect(actions.classList).not.toContain('hidden'); }); diff --git a/spec/javascripts/stimulus/popout_controller_test.js b/spec/javascripts/stimulus/popout_controller_test.js new file mode 100644 index 0000000000..60b6f8ce89 --- /dev/null +++ b/spec/javascripts/stimulus/popout_controller_test.js @@ -0,0 +1,104 @@ +/** + * @jest-environment jsdom + */ + +import { Application } from "stimulus"; +import popout_controller from "../../../app/webpacker/controllers/popout_controller"; + +describe("PopoutController", () => { + beforeAll(() => { + const application = Application.start(); + application.register("popout", popout_controller); + }); + + beforeEach(() => { + document.body.innerHTML = ` +
+ + +
+ + `; + }); + + describe("Show", () => { + it("shows the dialog on click", () => { + // button.click(); // For some reason this fails due to passive: true, but works in real life. + button.dispatchEvent(new Event("click")); + + expectToBeShown(dialog); + }); + + it("shows the dialog on keyboard down arrow", () => { + button.dispatchEvent(new KeyboardEvent("keydown", { keyCode: 40 })); + + expectToBeShown(dialog); + }); + + it("doesn't show the dialog on other key press (tab)", () => { + button.dispatchEvent(new KeyboardEvent("keydown", { keyCode: 9 })); + + expectToBeClosed(dialog); + }); + }); + + describe("Close", () => { + beforeEach(() => { + button.dispatchEvent(new Event("click")); // Dialog is open + }) + + it("closes the dialog when click outside", () => { + input4.click(); + + expectToBeClosed(dialog); + expect(button.innerText).toBe("value1"); + }); + + it("closes the dialog when focusing another field (eg with tab)", () => { + input4.focus(); + + expectToBeClosed(dialog); + expect(button.innerText).toBe("value1"); + }); + + it("doesn't close the dialog when focusing internal field", () => { + input2.focus(); + + expectToBeShown(dialog); + }); + + it("closes the dialog when checkbox is checked", () => { + input2.click(); + + expectToBeClosed(dialog); + expect(button.innerText).toBe("value1");// The checkbox label should be here, but I just couldn't get the test to work with labels. Don't worry, it works in the browser. + }); + + it("doesn't close the dialog when checkbox is unchecked", () => { + input2.click(); + button.dispatchEvent(new Event("click")); // Dialog is opened again + input2.click(); + + expect(input2.checked).toBe(false); + expectToBeShown(dialog); + }); + }); + + describe("Cleaning up", () => { + // unable to test disconnect + }); +}); + +function expectToBeShown(element) { + expect(element.style.display).toBe("block"); +} +function expectToBeClosed(element) { + expect(element.style.display).toBe("none"); +} diff --git a/spec/javascripts/stimulus/toggle_control_controller_test.js b/spec/javascripts/stimulus/toggle_control_controller_test.js new file mode 100644 index 0000000000..f56686fe9d --- /dev/null +++ b/spec/javascripts/stimulus/toggle_control_controller_test.js @@ -0,0 +1,64 @@ +/** + * @jest-environment jsdom + */ + +import { Application } from "stimulus"; +import toggle_controller from "../../../app/webpacker/controllers/toggle_control_controller"; + +describe("ToggleControlController", () => { + beforeAll(() => { + const application = Application.start(); + application.register("toggle-control", toggle_controller); + }); + + describe("#disableIfPresent", () => { + describe("with checkbox", () => { + beforeEach(() => { + document.body.innerHTML = `
+ + +
`; + }); + + it("Disables when checkbox is checked", () => { + checkbox.click(); + expect(checkbox.checked).toBe(true); + + expect(control.disabled).toBe(true); + }); + + it("Enables when checkbox is un-checked", () => { + checkbox.click(); + checkbox.click(); + expect(checkbox.checked).toBe(false); + + expect(control.disabled).toBe(false); + }); + }); + describe("with input", () => { + beforeEach(() => { + document.body.innerHTML = `
+ + +
`; + }); + + it("Disables when input is filled", () => { + input.value = "test" + input.dispatchEvent(new Event("input")); + + expect(control.disabled).toBe(true); + }); + + it("Enables when input is emptied", () => { + input.value = "test" + input.dispatchEvent(new Event("input")); + + input.value = "" + input.dispatchEvent(new Event("input")); + + expect(control.disabled).toBe(false); + }); + }); + }); +}); diff --git a/spec/reflexes/products_reflex_spec.rb b/spec/reflexes/products_reflex_spec.rb index ba22d41b84..debfcd24c8 100644 --- a/spec/reflexes/products_reflex_spec.rb +++ b/spec/reflexes/products_reflex_spec.rb @@ -30,11 +30,8 @@ describe ProductsReflex, type: :reflex, feature: :admin_style_v3 do describe '#bulk_update' do let!(:variant_a1) { - create(:variant, - product: product_a, - display_name: "Medium box", - sku: "APL-01", - price: 5.25) + create(:variant, product: product_a, display_name: "Medium box", sku: "APL-01", price: 5.25, + on_hand: 5, on_demand: false) } let!(:product_c) { create(:simple_product, name: "Carrots", sku: "CAR-00") } let!(:product_b) { create(:simple_product, name: "Bananas", sku: "BAN-00") } @@ -42,14 +39,14 @@ describe ProductsReflex, type: :reflex, feature: :admin_style_v3 do it "saves valid changes" do params = { - # '[products][][name]' - "products" => [ - { + # '[products][0][name]' + "products" => { + "0" => { "id" => product_a.id.to_s, "name" => "Pommes", "sku" => "POM-00", - } - ] + }, + }, } expect{ @@ -60,23 +57,27 @@ describe ProductsReflex, type: :reflex, feature: :admin_style_v3 do end it "saves valid changes to products and nested variants" do + # Form field names: + # '[products][0][id]' (hidden field) + # '[products][0][name]' + # '[products][0][variants_attributes][0][id]' (hidden field) + # '[products][0][variants_attributes][0][display_name]' params = { - # '[products][][name]' - # '[products][][variants_attributes][][display_name]' - "products" => [ - { + "products" => { + "0" => { "id" => product_a.id.to_s, "name" => "Pommes", - "variants_attributes" => [ - { + "variants_attributes" => { + "0" => { "id" => variant_a1.id.to_s, "display_name" => "Large box", "sku" => "POM-01", "price" => "10.25", - } - ], - } - ] + "on_hand" => "6", + }, + }, + }, + }, } expect{ @@ -87,20 +88,21 @@ describe ProductsReflex, type: :reflex, feature: :admin_style_v3 do .and change{ variant_a1.display_name }.to("Large box") .and change{ variant_a1.sku }.to("POM-01") .and change{ variant_a1.price }.to(10.25) + .and change{ variant_a1.on_hand }.to(6) end describe "sorting" do let(:params) { { - "products" => [ - { + "products" => { + "0" => { "id" => product_a.id.to_s, "name" => "Pommes", }, - { + "1" => { "id" => product_b.id.to_s, }, - ] + }, } } subject{ run_reflex(:bulk_update, params:) } @@ -116,20 +118,20 @@ describe ProductsReflex, type: :reflex, feature: :admin_style_v3 do describe "error messages" do it "summarises error messages" do params = { - "products" => [ - { + "products" => { + "0" => { "id" => product_a.id.to_s, "name" => "Pommes", }, - { + "1" => { "id" => product_b.id.to_s, "name" => "", # Name can't be blank }, - { + "2" => { "id" => product_c.id.to_s, "name" => "", # Name can't be blank }, - ] + }, } reflex = run_reflex(:bulk_update, params:) diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb index c9f35240d2..b4294bdc4d 100644 --- a/spec/system/admin/products_v3/products_spec.rb +++ b/spec/system/admin/products_v3/products_spec.rb @@ -194,11 +194,8 @@ describe 'As an admin, I can see the new product page', feature: :admin_style_v3 describe "updating" do let!(:variant_a1) { - create(:variant, - product: product_a, - display_name: "Medium box", - sku: "APL-01", - price: 5.25) + create(:variant, product: product_a, display_name: "Medium box", sku: "APL-01", price: 5.25, + on_hand: 5, on_demand: false) } let!(:product_a) { create(:simple_product, name: "Apples", sku: "APL-00") } before do @@ -214,6 +211,8 @@ describe 'As an admin, I can see the new product page', feature: :admin_style_v3 fill_in "Name", with: "Large box" fill_in "SKU", with: "POM-01" fill_in "Price", with: "10.25" + click_on "On Hand" # activate stock popout + fill_in "On Hand", with: "6" end expect { @@ -225,6 +224,7 @@ describe 'As an admin, I can see the new product page', feature: :admin_style_v3 .and change{ variant_a1.display_name }.to("Large box") .and change{ variant_a1.sku }.to("POM-01") .and change{ variant_a1.price }.to(10.25) + .and change{ variant_a1.on_hand }.to(6) within row_containing_name("Pommes") do expect(page).to have_field "Name", with: "Pommes" @@ -234,6 +234,29 @@ describe 'As an admin, I can see the new product page', feature: :admin_style_v3 expect(page).to have_field "Name", with: "Large box" expect(page).to have_field "SKU", with: "POM-01" expect(page).to have_field "Price", with: "10.25" + expect(page).to have_css "button[aria-label='On Hand']", text: "6" + end + + pending + expect(page).to have_content "Changes saved" + end + + it "switches stock to on-demand" do + within row_containing_name("Medium box") do + click_on "On Hand" # activate stock popout + check "On demand" + + expect(page).to have_css "button[aria-label='On Hand']", text: "On demand" + end + + expect { + click_button "Save changes" + product_a.reload + variant_a1.reload + }.to change{ variant_a1.on_demand }.to(true) + + within row_containing_name("Medium box") do + expect(page).to have_css "button[aria-label='On Hand']", text: "On demand" end pending