diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb
index 8e18198a40..01736eedab 100644
--- a/app/reflexes/products_reflex.rb
+++ b/app/reflexes/products_reflex.rb
@@ -41,8 +41,7 @@ class ProductsReflex < ApplicationReflex
# 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."
+ @error_counts = { saved: product_set.saved_count, invalid: product_set.invalid.count }
end
render_products_form
@@ -87,10 +86,12 @@ class ProductsReflex < ApplicationReflex
end
def render_products_form
+ locals = { products: @products }
+ locals[:error_counts] = @error_counts if @error_counts.present?
+
cable_ready.replace(
selector: "#products-form",
- html: render(partial: "admin/products_v3/table",
- locals: { products: @products, error_msg: @error_msg })
+ html: render(partial: "admin/products_v3/table", locals:)
).broadcast
morph :nothing
diff --git a/app/services/sets/product_set.rb b/app/services/sets/product_set.rb
index 23be044239..362acae3f1 100644
--- a/app/services/sets/product_set.rb
+++ b/app/services/sets/product_set.rb
@@ -8,11 +8,15 @@ module Sets
# }
#
class ProductSet < ModelSet
+ attr_reader :saved_count
+
def initialize(attributes = {})
super(Spree::Product, [], attributes)
end
def save
+ @saved_count = 0
+
# Attempt to save all records, collecting model errors.
@collection_hash.each_value.map do |product_attributes|
update_product_attributes(product_attributes)
@@ -71,7 +75,10 @@ module Sets
validate_presence_of_unit_value_in_product(product)
- product.errors.empty? && product.save
+ changed = product.changed?
+ success = product.errors.empty? && product.save
+ count_result(success && changed)
+ success
end
def validate_presence_of_unit_value_in_product(product)
@@ -105,8 +112,15 @@ module Sets
if variant.present?
variant.update(variant_attributes.except(:id))
else
- create_variant(product, variant_attributes)
+ variant = create_variant(product, variant_attributes)
end
+
+ # Copy any variant errors to product
+ variant&.errors&.each do |error|
+ # The name is namespaced to avoid confusion with product attrs of same name.
+ product.errors.add("variant_#{error.attribute}".to_sym, error.message)
+ end
+ variant&.errors.blank?
end
def create_variant(product, variant_attributes)
@@ -117,11 +131,7 @@ module Sets
on_demand = variant_attributes.delete(:on_demand)
variant = product.variants.create(variant_attributes)
-
- if variant.errors.present?
- product.errors.merge!(variant.errors)
- return false
- end
+ return variant if variant.errors.present?
begin
variant.on_demand = on_demand if on_demand.present?
@@ -130,6 +140,12 @@ module Sets
notify_bugsnag(e, product, variant, variant_attributes)
raise e
end
+
+ variant
+ end
+
+ def count_result(saved)
+ @saved_count += 1 if saved
end
def notify_bugsnag(error, product, variant, variant_attributes)
diff --git a/app/views/admin/products_v3/_sort.html.haml b/app/views/admin/products_v3/_sort.html.haml
index 20c67e083b..d3b4829f84 100644
--- a/app/views/admin/products_v3/_sort.html.haml
+++ b/app/views/admin/products_v3/_sort.html.haml
@@ -4,6 +4,6 @@
- if search_term.present? || producer_id.present? || category_id.present?
%a{ href: "#", class: "button disruptive", data: { reflex: "click->products#clear_search" } }
= t(".pagination.clear_search")
- %div.with-dropdown
+ %form.with-dropdown
= t(".pagination.per_page.show")
= select_tag :per_page, options_for_select([15, 25, 50, 100].collect{|i| [t('.pagination.per_page.per_page', num: i), i]}, pagy.items), data: { reflex: "change->products#change_per_page" }
diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml
index 33806fc195..3c6d40fa74 100644
--- a/app/views/admin/products_v3/_table.html.haml
+++ b/app/views/admin/products_v3/_table.html.haml
@@ -1,15 +1,18 @@
= form_with url: bulk_update_admin_products_path, method: :patch, id: "products-form",
builder: BulkFormBuilder,
html: {'data-reflex-serialize-form': true, 'data-reflex': 'submit->products#bulk_update',
- 'data-controller': "bulk-form", 'data-bulk-form-disable-selector-value': "#sort,#filters"} do |form|
- %fieldset.form-actions.hidden{ 'data-bulk-form-target': "actions" }
+ 'data-controller': "bulk-form", 'data-bulk-form-disable-selector-value': "#sort,#filters",
+ 'data-bulk-form-error-value': defined?(error_counts),
+ } do |form|
+ %fieldset.form-actions{ class: ("hidden" unless defined?(error_counts)), 'data-bulk-form-target': "actions" }
.container
- .status.ten.columns
- .changed_summary{ 'data-bulk-form-target': "changedSummary", 'data-translation-key': 'admin.products_v3.table.changed_summary'}
- - if defined?(error_msg) && error_msg.present?
- .error
- = error_msg
- .form-buttons.six.columns
+ .status.eleven.columns
+ .modified_summary{ 'data-bulk-form-target': "changedSummary", 'data-translation-key': 'admin.products_v3.table.changed_summary'}
+ - if defined?(error_counts)
+ .error_summary
+ -# X products were saved correctly, but Y products could not be saved correctly. Please review errors and try again
+ = t('.error_summary.saved', count: error_counts[:saved]) + t('.error_summary.invalid', count: error_counts[:invalid])
+ .form-buttons.five.columns
= form.submit t('.reset'), type: :reset, class: "medium", 'data-reflex': 'click->products#fetch'
= form.submit t('.save'), class: "medium"
%table.products
diff --git a/app/webpacker/controllers/bulk_form_controller.js b/app/webpacker/controllers/bulk_form_controller.js
index 842a1aaa89..ab93978584 100644
--- a/app/webpacker/controllers/bulk_form_controller.js
+++ b/app/webpacker/controllers/bulk_form_controller.js
@@ -5,6 +5,7 @@ export default class BulkFormController extends Controller {
static targets = ["actions", "changedSummary"];
static values = {
disableSelector: String,
+ error: Boolean,
};
recordElements = {};
@@ -25,6 +26,8 @@ export default class BulkFormController extends Controller {
this.recordElements[recordId].push(element);
}
}
+
+ this.toggleFormChanged();
}
disconnect() {
@@ -45,15 +48,16 @@ export default class BulkFormController extends Controller {
const changedRecordCount = Object.values(this.recordElements).filter((elements) =>
elements.some(this.#isChanged)
).length;
- const formChanged = changedRecordCount > 0;
+ const formChanged = changedRecordCount > 0 || this.errorValue;
// Show actions
this.actionsTarget.classList.toggle("hidden", !formChanged);
this.#disableOtherElements(formChanged); // like filters and sorting
// Display number of records changed
- const key = this.changedSummaryTarget && this.changedSummaryTarget.dataset.translationKey;
+ const key = this.hasChangedSummaryTarget && this.changedSummaryTarget.dataset.translationKey;
if (key) {
+ // TODO: save processing and only run if changedRecordCount has changed.
this.changedSummaryTarget.textContent = I18n.t(key, { count: changedRecordCount });
}
@@ -76,13 +80,23 @@ export default class BulkFormController extends Controller {
// private
#disableOtherElements(disable) {
+ if (!this.hasDisableSelectorValue) return;
+
this.disableElements ||= document.querySelectorAll(this.disableSelectorValue);
- if (this.disableElements) {
- this.disableElements.forEach((element) => {
- element.classList.toggle("disabled-section", disable);
- });
- }
+ if (!this.disableElements) return;
+
+ this.disableElements.forEach((element) => {
+ element.classList.toggle("disabled-section", disable);
+
+ // Also disable any form elements
+ let forms = element.tagName == "FORM" ? [element] : element.querySelectorAll("form");
+
+ forms &&
+ forms.forEach((form) =>
+ Array.from(form.elements).forEach((formElement) => (formElement.disabled = disable))
+ );
+ });
}
#isChanged(element) {
diff --git a/app/webpacker/css/admin_v3/shared/forms.scss b/app/webpacker/css/admin_v3/shared/forms.scss
index 96cef47615..35f8697e04 100644
--- a/app/webpacker/css/admin_v3/shared/forms.scss
+++ b/app/webpacker/css/admin_v3/shared/forms.scss
@@ -282,6 +282,20 @@ fieldset {
text-align: center;
}
+.modified_summary {
+ font-size: inherit;
+}
+
+.error_summary {
+ font-size: inherit;
+
+ @extend .icon-remove-sign;
+ &:before {
+ font-family: FontAwesome;
+ color: $color-error;
+ }
+}
+
select {
@extend input, [type="text"];
background-color: white;
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 68cd6ef091..edbc5b677f 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -837,13 +837,18 @@ en:
zero: ""
one: "%{count} product modified."
other: "%{count} products modified."
+ error_summary:
+ saved:
+ zero: ""
+ one: "%{count} product was saved correctly, but "
+ other: "%{count} products were saved correctly, but "
+ invalid:
+ one: "%{count} product could not be saved. Please review the errors and try again."
+ other: "%{count} products could not be saved. Please review the errors and try again."
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/spec/javascripts/stimulus/bulk_form_controller_test.js b/spec/javascripts/stimulus/bulk_form_controller_test.js
index db0f990eef..d558e4b37a 100644
--- a/spec/javascripts/stimulus/bulk_form_controller_test.js
+++ b/spec/javascripts/stimulus/bulk_form_controller_test.js
@@ -11,44 +11,43 @@ describe("BulkFormController", () => {
application.register("bulk-form", bulk_form_controller);
});
- // Mock I18n. TODO: moved to a shared helper
- beforeAll(() => {
- const mockedT = jest.fn();
- mockedT.mockImplementation((string, opts) => (string + ', ' + JSON.stringify(opts)));
-
- global.I18n = {
- t: mockedT
- };
- })
-
- // (jest still doesn't have aroundEach https://github.com/jestjs/jest/issues/4543 )
- afterAll(() => {
- delete global.I18n;
- })
-
- beforeEach(() => {
- document.body.innerHTML = `
-
-
-
- `;
- });
-
describe("Modifying input values", () => {
+ // Mock I18n. TODO: moved to a shared helper
+ beforeAll(() => {
+ const mockedT = jest.fn();
+ mockedT.mockImplementation((string, opts) => (string + ', ' + JSON.stringify(opts)));
+
+ global.I18n = {
+ t: mockedT
+ };
+ })
+ // (jest still doesn't have aroundEach https://github.com/jestjs/jest/issues/4543 )
+ afterAll(() => {
+ delete global.I18n;
+ })
+
beforeEach(() => {
+ document.body.innerHTML = `
+
+
+
+ `;
+
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");
@@ -115,7 +114,9 @@ describe("BulkFormController", () => {
expect(actions.classList).not.toContain('hidden');
expect(changed_summary.textContent).toBe('changed_summary, {"count":1}');
expect(disable1.classList).toContain('disabled-section');
+ expect(disable1_element.disabled).toBe(true);
expect(disable2.classList).toContain('disabled-section');
+ expect(disable2_element.disabled).toBe(true);
// Record 1: Second field changed
input1b.value = 'updated1b';
@@ -148,7 +149,9 @@ describe("BulkFormController", () => {
expect(actions.classList).not.toContain('hidden');
expect(changed_summary.textContent).toBe('changed_summary, {"count":1}');
expect(disable1.classList).toContain('disabled-section');
+ expect(disable1_element.disabled).toBe(true);
expect(disable2.classList).toContain('disabled-section');
+ expect(disable2_element.disabled).toBe(true);
// Record 2: Change back to original value
input2.value = 'initial2';
@@ -157,11 +160,50 @@ describe("BulkFormController", () => {
expect(actions.classList).toContain('hidden');
expect(changed_summary.textContent).toBe('changed_summary, {"count":0}');
expect(disable1.classList).not.toContain('disabled-section');
+ expect(disable1_element.disabled).toBe(false);
expect(disable2.classList).not.toContain('disabled-section');
+ expect(disable2_element.disabled).toBe(false);
});
});
});
+ describe("When there are errors", () => {
+ beforeEach(() => {
+ document.body.innerHTML = `
+
+ `;
+
+ const actions = document.getElementById("actions");
+ const changed_summary = document.getElementById("changed_summary");
+ const input1a = document.getElementById("input1a");
+ });
+
+ it("form actions section remains visible", () => {
+ // Expect actions to remain visible
+ expect(actions.classList).not.toContain('hidden');
+
+ // Record 1: First field changed
+ input1a.value = 'updated1a';
+ input1a.dispatchEvent(new Event("change"));
+ // Expect actions to remain visible
+ expect(actions.classList).not.toContain('hidden');
+
+ // Change back to original value
+ input1a.value = 'initial1a';
+ input1a.dispatchEvent(new Event("change"));
+ // Expect actions to remain visible
+ expect(actions.classList).not.toContain('hidden');
+ });
+ });
+
// unable to test disconnect at this stage
// describe("disconnect()", () => {
// it("resets other elements", () => {
diff --git a/spec/reflexes/products_reflex_spec.rb b/spec/reflexes/products_reflex_spec.rb
index ae95f16b8c..0672c46863 100644
--- a/spec/reflexes/products_reflex_spec.rb
+++ b/spec/reflexes/products_reflex_spec.rb
@@ -13,7 +13,7 @@ describe ProductsReflex, type: :reflex do
Flipper.enable(:admin_style_v3)
end
- describe 'fetch' do
+ describe '#fetch' do
subject{ build_reflex(method_name: :fetch, **context) }
describe "sorting" do
@@ -41,6 +41,7 @@ describe ProductsReflex, type: :reflex do
sku: "APL-01",
price: 5.25)
}
+ let!(:product_c) { create(:simple_product, name: "Carrots", sku: "CAR-00") }
let!(:product_b) { create(:simple_product, name: "Bananas", sku: "BAN-00") }
let!(:product_a) { create(:simple_product, name: "Apples", sku: "APL-00") }
@@ -123,17 +124,21 @@ describe ProductsReflex, type: :reflex do
"products" => [
{
"id" => product_a.id.to_s,
- "name" => "",
+ "name" => "Pommes",
},
{
"id" => product_b.id.to_s,
- "name" => "",
+ "name" => "", # Name can't be blank
+ },
+ {
+ "id" => product_c.id.to_s,
+ "name" => "", # Name can't be blank
},
]
}
reflex = run_reflex(:bulk_update, params:)
- expect(reflex.get(:error_msg)).to include "2 products have errors"
+ expect(reflex.get(:error_counts)).to eq({ saved: 1, invalid: 2 })
# # WTF
# expect{ reflex(:bulk_update, params:) }.to broadcast(
diff --git a/spec/services/sets/product_set_spec.rb b/spec/services/sets/product_set_spec.rb
index 18964d7f21..4dd6b94315 100644
--- a/spec/services/sets/product_set_spec.rb
+++ b/spec/services/sets/product_set_spec.rb
@@ -7,6 +7,7 @@ describe Sets::ProductSet do
let(:product_set) do
described_class.new(collection_attributes: collection_hash)
end
+ subject{ product_set.save }
context 'when the product does not exist yet' do
let!(:stock_location) { create(:stock_location, backorderable_default: false) }
@@ -33,6 +34,30 @@ describe Sets::ProductSet do
end
context 'when the product does exist' do
+ let(:product) { create(:simple_product, name: "product name") }
+
+ context "with valid name" do
+ let(:collection_hash) {
+ { 0 => { id: product.id, name: "New season product" } }
+ }
+
+ it "returns true and counts results" do
+ is_expected.to eq true
+ expect(product_set.saved_count).to eq 1
+ end
+ end
+
+ context "with invalid name" do
+ let(:collection_hash) {
+ { 0 => { id: product.id, name: "" } } # Product Name can't be blank
+ }
+
+ it "returns false and counts results" do
+ is_expected.to eq false
+ expect(product_set.saved_count).to eq 0
+ end
+ end
+
context 'when a different variant_unit is passed' do
let!(:product) do
create(
@@ -57,6 +82,7 @@ describe Sets::ProductSet do
it 'updates the product without error' do
expect(product_set.save).to eq true
+ expect(product_set.saved_count).to eq 1
expect(product.reload.attributes).to include(
'variant_unit' => 'weight'
@@ -68,7 +94,6 @@ describe Sets::ProductSet do
context "when the product is in an order cycle" do
let(:producer) { create(:enterprise) }
- let(:product) { create(:simple_product) }
let(:distributor) { create(:distributor_enterprise) }
let!(:order_cycle) {
@@ -111,36 +136,117 @@ describe Sets::ProductSet do
end
end
- describe "updating variants" do
- let!(:product) { create(:simple_product) }
- let(:collection_hash) { { 0 => { id: product.id } } }
+ describe "updating a product's variants" do
+ let(:product) { create(:simple_product) }
+ let(:variant) { product.variants.first }
+ let(:product_attributes) { {} }
+ let(:variant_attributes) { { sku: "var_sku" } }
+ let(:variants_attributes) { [{ **variant_attributes, id: product.variants.first.id.to_s }] }
+ let(:collection_hash) {
+ {
+ 0 => { id: product.id, **product_attributes, variants_attributes: }
+ }
+ }
- context 'when :variants_attributes are passed' do
- let(:variants_attributes) { [{ sku: '123', id: product.variants.first.id.to_s }] }
-
- before { collection_hash[0][:variants_attributes] = variants_attributes }
-
- it 'updates the attributes of the variant' do
+ it "updates the variant" do
+ expect {
product_set.save
+ variant.reload
+ }.to change { variant.sku }.to("var_sku")
- expect(product.reload.variants.first[:sku]).to eq variants_attributes.first[:sku]
+ pending
+ expect(product_set.saved_count).to eq 1
+ end
+
+ shared_examples "nothing saved" do
+ it "doesn't update product" do
+ expect {
+ product_set.save
+ product.reload
+ }.to_not change { product.sku }
+
+ expect(product_set.saved_count).to be_zero
+ expect(product_set.invalid.count).to be_positive
end
- context 'and when product attributes are also passed' do
- it 'updates product and variant attributes' do
- collection_hash[0][:sku] = "test_sku"
+ it "doesn't update variant" do
+ expect {
+ product_set.save
+ variant.reload
+ }.to_not change { variant.sku }
+ end
- expect {
- product_set.save
- product.reload
- }.to change { product.sku }.to("test_sku")
- .and change { product.variants.first.sku }.to("123")
+ it 'assigns the in-memory attributes of the variant' do
+ pending
+ expect {
+ product_set.save
+ }.to change { variant.sku }.to("123")
+ end
+ end
+
+ context "variant has error" do
+ let(:variant_attributes) { { sku: "var_sku", display_name: "A" * 256 } } # maximum length
+
+ include_examples "nothing saved"
+ end
+
+ context "when products attributes are also updated" do
+ let(:product_attributes) {
+ { sku: "prod_sku" }
+ }
+
+ it "updates product and variant" do
+ expect {
+ product_set.save
+ product.reload
+ }.to change { product.sku }.to("prod_sku")
+ .and change { product.variants.first.sku }.to("var_sku")
+
+ expect(product_set.saved_count).to eq 1
+ end
+
+ xcontext "variant has error" do
+ let(:variant_attributes) { { sku: "var_sku", display_name: "A" * 256 } } # maximum length
+
+ include_examples "nothing saved"
+ end
+
+ context "product has error" do
+ before { collection_hash[0][:name] = "" } # product.name can't be blank
+
+ include_examples "nothing saved"
+ end
+ end
+
+ context "when multiple variants are updated" do
+ let(:variant2) { create(:variant, product:) }
+ let(:variants_attributes) {
+ [
+ { **variant_attributes, id: product.variants.first.id.to_s },
+ { sku: "var_sku2", id: variant2.id.to_s },
+ ]
+ }
+
+ it "updates each variant" do
+ expect {
+ product_set.save
+ variant2.reload
+ }.to change { product.variants.first.sku }.to("var_sku")
+ .and change { variant2.sku }.to("var_sku2")
+ end
+
+ xcontext "variant has error" do
+ let(:variant_attributes) { { sku: "var_sku", display_name: "A" * 256 } } # maximum length
+
+ include_examples "nothing saved" do
+ after { expect(variant2.reload.sku).to_not eq "var_sku2" }
end
end
end
end
context 'when there are multiple products' do
+ let(:product_c) { create(:simple_product, name: "Carrots") }
let!(:product_b) { create(:simple_product, name: "Bananas") }
let!(:product_a) { create(:simple_product, name: "Apples") }
@@ -154,39 +260,37 @@ describe Sets::ProductSet do
id: product_b.id,
name: "Bananes",
},
+ 2 => {
+ id: product_c.id,
+ name: "Carrots",
+ },
}
end
it 'updates the products' do
- product_set.save
+ expect(product_set.save).to eq true
+ expect(product_set.saved_count).to eq 2 # only two were changed
expect(product_a.reload.name).to eq "Pommes"
expect(product_b.reload.name).to eq "Bananes"
+ expect(product_c.reload.name).to eq "Carrots" # no change
end
it 'retains the order of products' do
product_set.save
+ # even though the first product is now alphabetically last
expect(product_set.collection[0]).to eq product_a.reload
expect(product_set.collection[1]).to eq product_b.reload
+ expect(product_set.collection[2]).to eq product_c.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
+ before { collection_hash[0][:name] = "" } # product.name can't be blank
it 'continues to update subsequent products' do
product_set.save
+ expect(product_set.saved_count).to eq 1
# Errors are logged on the model
first_item = product_set.collection[0]
diff --git a/spec/system/admin/bulk_product_update_spec.rb b/spec/system/admin/bulk_product_update_spec.rb
index 9444cc87ab..199c8e218f 100644
--- a/spec/system/admin/bulk_product_update_spec.rb
+++ b/spec/system/admin/bulk_product_update_spec.rb
@@ -307,7 +307,7 @@ describe '
click_button 'Save Changes', match: :first
expect(page.find("#status-message"))
- .to have_content "Unit value can't be blank Unit value is not a number"
+ .to have_content "Variant unit value can't be blank Variant unit value is not a number"
end
it "creating a variant with unit value is: '120g' and 'on_demand' checked" do
@@ -323,7 +323,7 @@ describe '
click_button 'Save Changes', match: :first
expect(page.find("#status-message"))
- .to have_content "Unit value can't be blank Unit value is not a number"
+ .to have_content "Variant unit value can't be blank Variant unit value is not a number"
end
end
end
diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb
index 8ea6f57c48..5316a3118f 100644
--- a/spec/system/admin/products_v3/products_spec.rb
+++ b/spec/system/admin/products_v3/products_spec.rb
@@ -203,7 +203,6 @@ describe 'As an admin, I can see the new product page' do
price: 5.25)
}
let!(:product_a) { create(:simple_product, name: "Apples", sku: "APL-00") }
-
before do
visit admin_products_url
end
@@ -270,37 +269,71 @@ describe 'As an admin, I can see the new product page' do
end
end
- it "shows errors for both product and variant fields" do
- within row_containing_name("Apples") do
- fill_in "Name", with: ""
- fill_in "SKU", with: "A" * 256
- end
- within row_containing_name("Medium box") do
- fill_in "Name", with: "L" * 256
- fill_in "SKU", with: "1" * 256
+ context "with invalid data" do
+ let!(:product_b) { create(:simple_product, name: "Bananas") }
+
+ before do
+ within row_containing_name("Apples") do
+ fill_in "Name", with: ""
+ fill_in "SKU", with: "A" * 256
+ end
+ within row_containing_name("Medium box") do
+ fill_in "Name", with: "L" * 256
+ fill_in "SKU", with: "1" * 256
+ fill_in "Price", with: "10.25"
+ end
end
- expect {
- click_button "Save changes"
- product_a.reload
- }.to_not change { product_a.name }
+ it "shows errors for both product and variant fields" do
+ # Also update another product with valid data
+ within row_containing_name("Bananas") do
+ fill_in "Name", with: "Bananes"
+ end
- # (there's no identifier displayed, so the user must remember which product it is..)
- within row_containing_name("") do
- expect(page).to have_field "Name", with: ""
- expect(page).to have_content "can't be blank"
- expect(page).to have_field "SKU", with: "A" * 256
- expect(page).to have_content "is too long"
- end
- pending
- within row_containing_name("L" * 256) do
- expect(page).to have_field "Name", with: "L" * 256
- expect(page).to have_field "SKU", with: "1" * 256
- expect(page).to have_content "is too long"
- expect(page).to have_field "Price", with: "10.25"
+ expect {
+ click_button "Save changes"
+ product_a.reload
+ }.to_not change { product_a.name }
+
+ # pending("unchanged rows are being saved") # TODO: don't report unchanged rows
+ # expect(page).to_not have_content("rows were saved correctly")
+ # Both the product and variant couldn't be saved.
+ expect(page).to have_content("1 product could not be saved")
+ expect(page).to have_content "Please review the errors and try again"
+
+ # (there's no identifier displayed, so the user must remember which product it is..)
+ within row_containing_name("") do
+ expect(page).to have_field "Name", with: ""
+ expect(page).to have_content "can't be blank"
+ expect(page).to have_field "SKU", with: "A" * 256
+ expect(page).to have_content "is too long"
+ end
+
+ pending "bug #11748"
+ within row_containing_name("L" * 256) do
+ expect(page).to have_field "Name", with: "L" * 256
+ expect(page).to have_field "SKU", with: "1" * 256
+ expect(page).to have_content "is too long"
+ expect(page).to have_field "Price", with: "10.25" # other updated value is retained
+ end
end
- expect(page).to have_content "Please review the errors and try again"
+ it "saves changes after fixing errors" do
+ within row_containing_name("Apples") do
+ fill_in "Name", with: "Pommes"
+ fill_in "SKU", with: "POM-00"
+ end
+
+ expect {
+ click_button "Save changes"
+ product_a.reload
+ variant_a1.reload
+ }.to change { product_a.name }.to("Pommes")
+ .and change{ product_a.sku }.to("POM-00")
+
+ pending
+ expect(page).to have_content "Changes saved"
+ end
end
end