mirror of
https://github.com/openfoodfoundation/openfoodnetwork
synced 2026-02-27 01:43:22 +00:00
Merge pull request #11702 from dacook/buu-editing-part5b-11059
[BUU] Mark changed fields, when error
This commit is contained in:
@@ -248,7 +248,6 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout
|
||||
else
|
||||
product.variant_unit = product.variant_unit_scale = null
|
||||
|
||||
$scope.packVariant product, product.master if product.master
|
||||
|
||||
if product.variants
|
||||
for id, variant of product.variants
|
||||
@@ -299,7 +298,6 @@ filterSubmitProducts = (productsToFilter) ->
|
||||
if product.hasOwnProperty("id")
|
||||
filteredProduct = {id: product.id}
|
||||
filteredVariants = []
|
||||
filteredMaster = null
|
||||
hasUpdatableProperty = false
|
||||
|
||||
if product.hasOwnProperty("variants")
|
||||
@@ -309,16 +307,6 @@ filterSubmitProducts = (productsToFilter) ->
|
||||
variantHasUpdatableProperty = result.hasUpdatableProperty
|
||||
filteredVariants.push filteredVariant if variantHasUpdatableProperty
|
||||
|
||||
if product.master?.hasOwnProperty("unit_value")
|
||||
filteredMaster ?= { id: product.master.id }
|
||||
filteredMaster.unit_value = product.master.unit_value
|
||||
if product.master?.hasOwnProperty("unit_description")
|
||||
filteredMaster ?= { id: product.master.id }
|
||||
filteredMaster.unit_description = product.master.unit_description
|
||||
if product.master?.hasOwnProperty("display_as")
|
||||
filteredMaster ?= { id: product.master.id }
|
||||
filteredMaster.display_as = product.master.display_as
|
||||
|
||||
if product.hasOwnProperty("sku")
|
||||
filteredProduct.sku = product.sku
|
||||
hasUpdatableProperty = true
|
||||
@@ -350,9 +338,6 @@ filterSubmitProducts = (productsToFilter) ->
|
||||
if product.hasOwnProperty("inherits_properties")
|
||||
filteredProduct.inherits_properties = product.inherits_properties
|
||||
hasUpdatableProperty = true
|
||||
if filteredMaster?
|
||||
filteredProduct.master_attributes = filteredMaster
|
||||
hasUpdatableProperty = true
|
||||
if filteredVariants.length > 0 # Note that the name of the property changes to enable mass assignment of variants attributes with rails
|
||||
filteredProduct.variants_attributes = filteredVariants
|
||||
hasUpdatableProperty = true
|
||||
|
||||
13
app/helpers/bulk_form_builder.rb
Normal file
13
app/helpers/bulk_form_builder.rb
Normal file
@@ -0,0 +1,13 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
class BulkFormBuilder < ActionView::Helpers::FormBuilder
|
||||
def text_field(field, **opts)
|
||||
# Mark field if it is changed (unsaved)
|
||||
changed_method = "#{field}_changed?"
|
||||
if object.respond_to?(changed_method) && object.public_send(changed_method)
|
||||
opts[:class] = "#{opts[:class]} changed".strip
|
||||
end
|
||||
|
||||
super(field, **opts)
|
||||
end
|
||||
end
|
||||
@@ -24,6 +24,10 @@ module Spree
|
||||
amount
|
||||
end
|
||||
|
||||
def price_changed?
|
||||
amount_changed?
|
||||
end
|
||||
|
||||
def price=(price)
|
||||
self[:amount] = parse_price(price)
|
||||
end
|
||||
|
||||
@@ -50,7 +50,8 @@ module Spree
|
||||
has_many :prices,
|
||||
class_name: 'Spree::Price',
|
||||
dependent: :destroy
|
||||
delegate :display_price, :display_amount, :price, :price=, :currency, :currency=,
|
||||
delegate :display_price, :display_amount, :price, :price_changed?, :price=,
|
||||
:currency, :currency=,
|
||||
to: :find_or_build_default_price
|
||||
|
||||
has_many :exchange_variants
|
||||
|
||||
@@ -79,5 +79,11 @@ module Sets
|
||||
def persisted?
|
||||
false
|
||||
end
|
||||
|
||||
def find_model(collection, model_id)
|
||||
collection.find do |model|
|
||||
model.id.to_s == model_id.to_s && model.persisted?
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -59,12 +59,11 @@ module Sets
|
||||
|
||||
ExchangeVariantDeleter.new.delete(product) if product.saved_change_to_supplier_id?
|
||||
|
||||
update_product_variants(product, attributes) &&
|
||||
update_product_master(product, attributes)
|
||||
update_product_variants(product, attributes)
|
||||
end
|
||||
|
||||
def update_product_only_attributes(product, attributes)
|
||||
variant_related_attrs = [:id, :variants_attributes, :master_attributes]
|
||||
variant_related_attrs = [:id, :variants_attributes]
|
||||
product_related_attrs = attributes.except(*variant_related_attrs)
|
||||
return true if product_related_attrs.blank?
|
||||
|
||||
@@ -94,12 +93,6 @@ module Sets
|
||||
update_variants_attributes(product, attributes[:variants_attributes])
|
||||
end
|
||||
|
||||
def update_product_master(product, attributes)
|
||||
return true unless attributes[:master_attributes]
|
||||
|
||||
create_or_update_variant(product, attributes[:master_attributes])
|
||||
end
|
||||
|
||||
def update_variants_attributes(product, variants_attributes)
|
||||
variants_attributes.each do |attributes|
|
||||
create_or_update_variant(product, attributes)
|
||||
@@ -119,6 +112,7 @@ module Sets
|
||||
def create_variant(product, variant_attributes)
|
||||
return if variant_attributes.blank?
|
||||
|
||||
# 'You need to save the variant to create a stock item before you can set stock levels.'
|
||||
on_hand = variant_attributes.delete(:on_hand)
|
||||
on_demand = variant_attributes.delete(:on_demand)
|
||||
|
||||
@@ -147,11 +141,5 @@ module Sets
|
||||
report.add_metadata(:variant_error, variant.errors.first) unless variant.valid?
|
||||
end
|
||||
end
|
||||
|
||||
def find_model(collection, model_id)
|
||||
collection.find do |model|
|
||||
model.id.to_s == model_id.to_s && model.persisted?
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -1,10 +1,11 @@
|
||||
= 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" }
|
||||
.container
|
||||
.status.ten.columns
|
||||
.modified_summary{ 'data-bulk-form-target': "modifiedSummary", 'data-translation-key': 'admin.products_v3.table.modified_summary'}
|
||||
.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
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
import { Controller } from "stimulus";
|
||||
|
||||
// Manages "modified" state for a form with multiple records
|
||||
// Manages "changed" state for a form with multiple records
|
||||
export default class BulkFormController extends Controller {
|
||||
static targets = ["actions", "modifiedSummary"];
|
||||
static targets = ["actions", "changedSummary"];
|
||||
static values = {
|
||||
disableSelector: String,
|
||||
};
|
||||
@@ -12,10 +12,10 @@ export default class BulkFormController extends Controller {
|
||||
this.form = this.element;
|
||||
|
||||
// Start listening for any changes within the form
|
||||
// this.element.addEventListener('change', this.toggleModified.bind(this)); // dunno why this doesn't work
|
||||
// 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.toggleModified.bind(this)); // instant response
|
||||
element.addEventListener("change", this.toggleModified.bind(this)); // just in case (eg right-click paste)
|
||||
element.addEventListener("keyup", this.toggleChanged.bind(this)); // instant response
|
||||
element.addEventListener("change", this.toggleChanged.bind(this)); // just in case (eg right-click paste)
|
||||
|
||||
// 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.
|
||||
@@ -33,32 +33,32 @@ export default class BulkFormController extends Controller {
|
||||
window.removeEventListener("beforeunload", this.preventLeavingBulkForm);
|
||||
}
|
||||
|
||||
toggleModified(e) {
|
||||
toggleChanged(e) {
|
||||
const element = e.target;
|
||||
element.classList.toggle("modified", this.#isModified(element));
|
||||
element.classList.toggle("changed", this.#isChanged(element));
|
||||
|
||||
this.toggleFormModified();
|
||||
this.toggleFormChanged();
|
||||
}
|
||||
|
||||
toggleFormModified() {
|
||||
// For each record, check if any fields are modified
|
||||
const modifiedRecordCount = Object.values(this.recordElements).filter((elements) =>
|
||||
elements.some(this.#isModified)
|
||||
toggleFormChanged() {
|
||||
// For each record, check if any fields are changed
|
||||
const changedRecordCount = Object.values(this.recordElements).filter((elements) =>
|
||||
elements.some(this.#isChanged)
|
||||
).length;
|
||||
const formModified = modifiedRecordCount > 0;
|
||||
const formChanged = changedRecordCount > 0;
|
||||
|
||||
// Show actions
|
||||
this.actionsTarget.classList.toggle("hidden", !formModified);
|
||||
this.#disableOtherElements(formModified); // like filters and sorting
|
||||
this.actionsTarget.classList.toggle("hidden", !formChanged);
|
||||
this.#disableOtherElements(formChanged); // like filters and sorting
|
||||
|
||||
// Display number of records modified
|
||||
const key = this.modifiedSummaryTarget && this.modifiedSummaryTarget.dataset.translationKey;
|
||||
// Display number of records changed
|
||||
const key = this.changedSummaryTarget && this.changedSummaryTarget.dataset.translationKey;
|
||||
if (key) {
|
||||
this.modifiedSummaryTarget.textContent = I18n.t(key, { count: modifiedRecordCount });
|
||||
this.changedSummaryTarget.textContent = I18n.t(key, { count: changedRecordCount });
|
||||
}
|
||||
|
||||
// Prevent accidental data loss
|
||||
if (formModified) {
|
||||
if (formChanged) {
|
||||
window.addEventListener("beforeunload", this.preventLeavingBulkForm);
|
||||
} else {
|
||||
window.removeEventListener("beforeunload", this.preventLeavingBulkForm);
|
||||
@@ -85,7 +85,7 @@ export default class BulkFormController extends Controller {
|
||||
}
|
||||
}
|
||||
|
||||
#isModified(element) {
|
||||
#isChanged(element) {
|
||||
return element.value != element.defaultValue;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -68,7 +68,7 @@
|
||||
|
||||
.content {
|
||||
// Plain content fields need help to align with text in inputs (due to vertical-align)
|
||||
margin: $vpadding-txt+1px 0;
|
||||
margin: $vpadding-txt + 1px 0;
|
||||
|
||||
@extend .line-clamp-1;
|
||||
}
|
||||
@@ -126,8 +126,8 @@
|
||||
border-color: $color-txt-hover-brd;
|
||||
}
|
||||
|
||||
&.modified {
|
||||
border-color: $color-txt-modified-brd;
|
||||
&.changed {
|
||||
border-color: $color-txt-changed-brd;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -74,7 +74,7 @@ $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-modified-brd: $bright-orange !default;
|
||||
$color-txt-changed-brd: $bright-orange !default;
|
||||
$vpadding-txt: 5px;
|
||||
$hpadding-txt: 8px;
|
||||
|
||||
|
||||
@@ -31,8 +31,8 @@ fieldset {
|
||||
opacity: 0.7;
|
||||
}
|
||||
|
||||
&.modified {
|
||||
border-color: $color-txt-modified-brd;
|
||||
&.changed {
|
||||
border-color: $color-txt-changed-brd;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -833,7 +833,7 @@ en:
|
||||
import_products: Import multiple products
|
||||
no_products_found_for_search: No products found for your search criteria
|
||||
table:
|
||||
modified_summary:
|
||||
changed_summary:
|
||||
zero: ""
|
||||
one: "%{count} product modified."
|
||||
other: "%{count} products modified."
|
||||
|
||||
26
spec/helpers/bulk_form_builder_spec.rb
Normal file
26
spec/helpers/bulk_form_builder_spec.rb
Normal file
@@ -0,0 +1,26 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require 'spec_helper'
|
||||
|
||||
class TestHelper < ActionView::Base; end
|
||||
|
||||
describe BulkFormBuilder do
|
||||
describe '#text_field' do
|
||||
let(:product) { create(:product) }
|
||||
let(:form) { BulkFormBuilder.new(:product, product, self, {}) }
|
||||
|
||||
it { expect(form.text_field(:name)).to_not include "changed" }
|
||||
|
||||
context "attribute has been changed" do
|
||||
before { product.assign_attributes name: "updated name" }
|
||||
|
||||
it { expect(form.text_field(:name)).to include "changed" }
|
||||
|
||||
context "and saved" do
|
||||
before { product.save }
|
||||
|
||||
it { expect(form.text_field(:name)).to_not include "changed" }
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -32,7 +32,7 @@ describe("BulkFormController", () => {
|
||||
<div id="disable2"></div>
|
||||
<form data-controller="bulk-form" data-bulk-form-disable-selector-value="#disable1,#disable2">
|
||||
<div id="actions" data-bulk-form-target="actions" class="hidden"></div>
|
||||
<div id="modified_summary" data-bulk-form-target="modifiedSummary" data-translation-key="modified_summary"></div>
|
||||
<div id="changed_summary" data-bulk-form-target="changedSummary" data-translation-key="changed_summary"></div>
|
||||
<div data-record-id="1">
|
||||
<input id="input1a" type="text" value="initial1a">
|
||||
<input id="input1b" type="text" value="initial1b">
|
||||
@@ -50,7 +50,7 @@ describe("BulkFormController", () => {
|
||||
const disable1 = document.getElementById("disable1");
|
||||
const disable2 = document.getElementById("disable2");
|
||||
const actions = document.getElementById("actions");
|
||||
const modified_summary = document.getElementById("modified_summary");
|
||||
const changed_summary = document.getElementById("changed_summary");
|
||||
const input1a = document.getElementById("input1a");
|
||||
const input1b = document.getElementById("input1b");
|
||||
const input2 = document.getElementById("input2");
|
||||
@@ -60,30 +60,30 @@ describe("BulkFormController", () => {
|
||||
it("onChange", () => {
|
||||
input1a.value = 'updated1a';
|
||||
input1a.dispatchEvent(new Event("change"));
|
||||
// Expect only first field to show modified
|
||||
expect(input1a.classList).toContain('modified');
|
||||
expect(input1b.classList).not.toContain('modified');
|
||||
expect(input2.classList).not.toContain('modified');
|
||||
// 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("change"));
|
||||
expect(input1a.classList).not.toContain('modified');
|
||||
expect(input1a.classList).not.toContain('changed');
|
||||
|
||||
});
|
||||
|
||||
it("onKeyup", () => {
|
||||
input1a.value = 'u1a';
|
||||
input1a.dispatchEvent(new Event("keyup"));
|
||||
// Expect only first field to show modified
|
||||
expect(input1a.classList).toContain('modified');
|
||||
expect(input1b.classList).not.toContain('modified');
|
||||
expect(input2.classList).not.toContain('modified');
|
||||
// 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('modified');
|
||||
expect(input1a.classList).not.toContain('changed');
|
||||
});
|
||||
|
||||
it("multiple fields", () => {
|
||||
@@ -91,29 +91,29 @@ describe("BulkFormController", () => {
|
||||
input1a.dispatchEvent(new Event("change"));
|
||||
input2.value = 'updated2';
|
||||
input2.dispatchEvent(new Event("change"));
|
||||
// Expect only first field to show modified
|
||||
expect(input1a.classList).toContain('modified');
|
||||
expect(input1b.classList).not.toContain('modified');
|
||||
expect(input2.classList).toContain('modified');
|
||||
// Expect only first field to show changed
|
||||
expect(input1a.classList).toContain('changed');
|
||||
expect(input1b.classList).not.toContain('changed');
|
||||
expect(input2.classList).toContain('changed');
|
||||
|
||||
// Change only one back to original value
|
||||
input1a.value = 'initial1a';
|
||||
input1a.dispatchEvent(new Event("change"));
|
||||
expect(input1a.classList).not.toContain('modified');
|
||||
expect(input1b.classList).not.toContain('modified');
|
||||
expect(input2.classList).toContain('modified');
|
||||
expect(input1a.classList).not.toContain('changed');
|
||||
expect(input1b.classList).not.toContain('changed');
|
||||
expect(input2.classList).toContain('changed');
|
||||
});
|
||||
})
|
||||
|
||||
describe("activating sections, and showing a summary", () => {
|
||||
// This scenario should probably be broken up into smaller units.
|
||||
it("counts modified records ", () => {
|
||||
it("counts changed records ", () => {
|
||||
// Record 1: First field changed
|
||||
input1a.value = 'updated1a';
|
||||
input1a.dispatchEvent(new Event("change"));
|
||||
// Actions and modified summary are shown, with other sections disabled
|
||||
// Actions and changed summary are shown, with other sections disabled
|
||||
expect(actions.classList).not.toContain('hidden');
|
||||
expect(modified_summary.textContent).toBe('modified_summary, {"count":1}');
|
||||
expect(changed_summary.textContent).toBe('changed_summary, {"count":1}');
|
||||
expect(disable1.classList).toContain('disabled-section');
|
||||
expect(disable2.classList).toContain('disabled-section');
|
||||
|
||||
@@ -122,31 +122,31 @@ describe("BulkFormController", () => {
|
||||
input1b.dispatchEvent(new Event("change"));
|
||||
// Expect to show same summary translation
|
||||
expect(actions.classList).not.toContain('hidden');
|
||||
expect(modified_summary.textContent).toBe('modified_summary, {"count":1}');
|
||||
expect(changed_summary.textContent).toBe('changed_summary, {"count":1}');
|
||||
|
||||
// Record 2: has been changed
|
||||
input2.value = 'updated2';
|
||||
input2.dispatchEvent(new Event("change"));
|
||||
// Expect summary to count both records
|
||||
expect(actions.classList).not.toContain('hidden');
|
||||
expect(modified_summary.textContent).toBe('modified_summary, {"count":2}');
|
||||
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"));
|
||||
// Both records are still modified.
|
||||
expect(input1a.classList).not.toContain('modified');
|
||||
expect(input1b.classList).toContain('modified');
|
||||
expect(input2.classList).toContain('modified');
|
||||
// Both records are still changed.
|
||||
expect(input1a.classList).not.toContain('changed');
|
||||
expect(input1b.classList).toContain('changed');
|
||||
expect(input2.classList).toContain('changed');
|
||||
expect(actions.classList).not.toContain('hidden');
|
||||
expect(modified_summary.textContent).toBe('modified_summary, {"count":2}');
|
||||
expect(changed_summary.textContent).toBe('changed_summary, {"count":2}');
|
||||
|
||||
// Record 1: Change second field back to original value
|
||||
input1b.value = 'initial1b';
|
||||
input1b.dispatchEvent(new Event("change"));
|
||||
// Both fields for record 1 show unmodified, but second record is still modified
|
||||
// Both fields for record 1 show unchanged, but second record is still changed
|
||||
expect(actions.classList).not.toContain('hidden');
|
||||
expect(modified_summary.textContent).toBe('modified_summary, {"count":1}');
|
||||
expect(changed_summary.textContent).toBe('changed_summary, {"count":1}');
|
||||
expect(disable1.classList).toContain('disabled-section');
|
||||
expect(disable2.classList).toContain('disabled-section');
|
||||
|
||||
@@ -155,7 +155,7 @@ describe("BulkFormController", () => {
|
||||
input2.dispatchEvent(new Event("change"));
|
||||
// Actions are hidden and other sections are now re-enabled
|
||||
expect(actions.classList).toContain('hidden');
|
||||
expect(modified_summary.textContent).toBe('modified_summary, {"count":0}');
|
||||
expect(changed_summary.textContent).toBe('changed_summary, {"count":0}');
|
||||
expect(disable1.classList).not.toContain('disabled-section');
|
||||
expect(disable2.classList).not.toContain('disabled-section');
|
||||
});
|
||||
@@ -170,7 +170,7 @@ describe("BulkFormController", () => {
|
||||
// const controller = document.querySelector('[data-controller="bulk-form"]');
|
||||
// const form = document.querySelector('[data-controller="bulk-form"]');
|
||||
|
||||
// // Form is modified and other sections are disabled
|
||||
// // Form is changed and other sections are disabled
|
||||
// input1a.value = 'updated1a';
|
||||
// input1a.dispatchEvent(new Event("change"));
|
||||
// expect(disable1.classList).toContain('disabled-section');
|
||||
|
||||
@@ -195,10 +195,6 @@ describe "filtering products for submission to database", ->
|
||||
producer_id: 5
|
||||
group_buy: null
|
||||
group_buy_unit_size: null
|
||||
master:
|
||||
id: 2
|
||||
unit_value: 250
|
||||
unit_description: "foo"
|
||||
variants: [
|
||||
id: 1
|
||||
on_hand: 2
|
||||
@@ -221,10 +217,6 @@ describe "filtering products for submission to database", ->
|
||||
variant_unit: 'volume'
|
||||
variant_unit_scale: 1
|
||||
variant_unit_name: 'loaf'
|
||||
master_attributes:
|
||||
id: 2
|
||||
unit_value: 250
|
||||
unit_description: "foo"
|
||||
variants_attributes: [
|
||||
id: 1
|
||||
on_hand: 2
|
||||
@@ -558,17 +550,6 @@ describe "AdminProductEditCtrl", ->
|
||||
variant_unit_scale: null
|
||||
variant_unit_with_scale: 'items'
|
||||
|
||||
it "packs the master variant", ->
|
||||
spyOn $scope, "packVariant"
|
||||
testVariant = {id: 1}
|
||||
testProduct =
|
||||
id: 1
|
||||
master: testVariant
|
||||
|
||||
$scope.packProduct(testProduct)
|
||||
|
||||
expect($scope.packVariant).toHaveBeenCalledWith(testProduct, testVariant)
|
||||
|
||||
it "packs each variant", ->
|
||||
spyOn $scope, "packVariant"
|
||||
testVariant = {id: 1}
|
||||
@@ -986,10 +967,6 @@ describe "AdminProductEditCtrl", ->
|
||||
producer_id: 5
|
||||
group_buy: null
|
||||
group_buy_unit_size: null
|
||||
master:
|
||||
id: 2
|
||||
unit_value: 250
|
||||
unit_description: "foo"
|
||||
|
||||
describe 'product has variant', ->
|
||||
it 'should load the edit product variant page', ->
|
||||
|
||||
@@ -4,210 +4,179 @@ require 'spec_helper'
|
||||
|
||||
describe Sets::ProductSet do
|
||||
describe '#save' do
|
||||
context 'when passing :collection_attributes' do
|
||||
let(:product_set) do
|
||||
described_class.new(collection_attributes: collection_hash)
|
||||
let(:product_set) do
|
||||
described_class.new(collection_attributes: collection_hash)
|
||||
end
|
||||
|
||||
context 'when the product does not exist yet' do
|
||||
let!(:stock_location) { create(:stock_location, backorderable_default: false) }
|
||||
let(:collection_hash) do
|
||||
{
|
||||
0 => {
|
||||
name: 'a product',
|
||||
price: 2.0,
|
||||
supplier_id: create(:enterprise).id,
|
||||
primary_taxon_id: create(:taxon).id,
|
||||
unit_description: 'description',
|
||||
variant_unit: 'items',
|
||||
variant_unit_name: 'bunches',
|
||||
shipping_category_id: create(:shipping_category).id
|
||||
}
|
||||
}
|
||||
end
|
||||
|
||||
context 'when the product does not exist yet' do
|
||||
let!(:stock_location) { create(:stock_location, backorderable_default: false) }
|
||||
it 'does not create a new product' do
|
||||
product_set.save
|
||||
|
||||
expect(Spree::Product.last).to be nil
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the product does exist' do
|
||||
context 'when a different variant_unit is passed' do
|
||||
let!(:product) do
|
||||
create(
|
||||
:simple_product,
|
||||
variant_unit: 'items',
|
||||
variant_unit_scale: nil,
|
||||
variant_unit_name: 'bunches',
|
||||
unit_value: nil,
|
||||
unit_description: 'some description'
|
||||
)
|
||||
end
|
||||
|
||||
let(:collection_hash) do
|
||||
{
|
||||
0 => {
|
||||
name: 'a product',
|
||||
price: 2.0,
|
||||
supplier_id: create(:enterprise).id,
|
||||
primary_taxon_id: create(:taxon).id,
|
||||
unit_description: 'description',
|
||||
variant_unit: 'items',
|
||||
variant_unit_name: 'bunches',
|
||||
shipping_category_id: create(:shipping_category).id
|
||||
id: product.id,
|
||||
variant_unit: 'weight',
|
||||
variant_unit_scale: 1
|
||||
}
|
||||
}
|
||||
end
|
||||
|
||||
it 'does not create a new product' do
|
||||
it 'updates the product without error' do
|
||||
expect(product_set.save).to eq true
|
||||
|
||||
expect(product.reload.attributes).to include(
|
||||
'variant_unit' => 'weight'
|
||||
)
|
||||
|
||||
expect(product_set.errors).to be_empty
|
||||
end
|
||||
end
|
||||
|
||||
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) {
|
||||
create(:simple_order_cycle, variants: [product.variants.first],
|
||||
coordinator: distributor,
|
||||
distributors: [distributor])
|
||||
}
|
||||
|
||||
context 'and only the name changes' do
|
||||
let(:collection_hash) do
|
||||
{ 0 => { id: product.id, name: "New season product" } }
|
||||
end
|
||||
|
||||
it 'updates the product and keeps it in order cycles' do
|
||||
expect {
|
||||
product_set.save
|
||||
product.reload
|
||||
}.to change { product.name }.to("New season product").
|
||||
and change { order_cycle.distributed_variants.count }.by(0)
|
||||
|
||||
expect(order_cycle.distributed_variants).to include product.variants.first
|
||||
end
|
||||
end
|
||||
|
||||
context 'and a different supplier is passed' do
|
||||
let(:collection_hash) do
|
||||
{ 0 => { id: product.id, supplier_id: producer.id } }
|
||||
end
|
||||
|
||||
it 'updates the product and removes the product from order cycles' do
|
||||
expect {
|
||||
product_set.save
|
||||
product.reload
|
||||
}.to change { product.supplier }.to(producer).
|
||||
and change { order_cycle.distributed_variants.count }.by(-1)
|
||||
|
||||
expect(order_cycle.distributed_variants).to_not include product.variants.first
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe "updating variants" do
|
||||
let!(:product) { create(:simple_product) }
|
||||
let(:collection_hash) { { 0 => { id: product.id } } }
|
||||
|
||||
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
|
||||
product_set.save
|
||||
|
||||
expect(Spree::Product.last).to be nil
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the product does exist' do
|
||||
context 'when a different variant_unit is passed' do
|
||||
let!(:product) do
|
||||
create(
|
||||
:simple_product,
|
||||
variant_unit: 'items',
|
||||
variant_unit_scale: nil,
|
||||
variant_unit_name: 'bunches',
|
||||
unit_value: nil,
|
||||
unit_description: 'some description'
|
||||
)
|
||||
end
|
||||
|
||||
let(:collection_hash) do
|
||||
{
|
||||
0 => {
|
||||
id: product.id,
|
||||
variant_unit: 'weight',
|
||||
variant_unit_scale: 1
|
||||
}
|
||||
}
|
||||
end
|
||||
|
||||
it 'updates the product' do
|
||||
product_set.save
|
||||
|
||||
expect(product.reload.attributes).to include(
|
||||
'variant_unit' => 'weight'
|
||||
)
|
||||
end
|
||||
|
||||
it 'does not add an error' do
|
||||
product_set.save
|
||||
expect(product_set.errors)
|
||||
.to be_empty
|
||||
end
|
||||
|
||||
it 'returns true' do
|
||||
expect(product_set.save).to eq(true)
|
||||
end
|
||||
expect(product.reload.variants.first[:sku]).to eq variants_attributes.first[:sku]
|
||||
end
|
||||
|
||||
context "when the product is in an order cycle" do
|
||||
let(:producer) { create(:enterprise) }
|
||||
let(:product) { create(:simple_product) }
|
||||
context 'and when product attributes are also passed' do
|
||||
it 'updates product and variant attributes' do
|
||||
collection_hash[0][:sku] = "test_sku"
|
||||
|
||||
let(:distributor) { create(:distributor_enterprise) }
|
||||
let!(:order_cycle) {
|
||||
create(:simple_order_cycle, variants: [product.variants.first],
|
||||
coordinator: distributor,
|
||||
distributors: [distributor])
|
||||
}
|
||||
|
||||
context 'and only the name changes' do
|
||||
let(:collection_hash) do
|
||||
{ 0 => { id: product.id, name: "New season product" } }
|
||||
end
|
||||
|
||||
it 'updates the product and keeps it in order cycles' do
|
||||
expect {
|
||||
product_set.save
|
||||
product.reload
|
||||
}.to change { product.name }.to("New season product").
|
||||
and change { order_cycle.distributed_variants.count }.by(0)
|
||||
|
||||
expect(order_cycle.distributed_variants).to include product.variants.first
|
||||
end
|
||||
end
|
||||
|
||||
context 'and a different supplier is passed' do
|
||||
let(:collection_hash) do
|
||||
{ 0 => { id: product.id, supplier_id: producer.id } }
|
||||
end
|
||||
|
||||
it 'updates the product and removes the product from order cycles' do
|
||||
expect {
|
||||
product_set.save
|
||||
product.reload
|
||||
}.to change { product.supplier }.to(producer).
|
||||
and change { order_cycle.distributed_variants.count }.by(-1)
|
||||
|
||||
expect(order_cycle.distributed_variants).to_not include product.variants.first
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when attributes of the variants are passed' do
|
||||
let!(:product) { create(:simple_product) }
|
||||
let(:collection_hash) { { 0 => { id: product.id } } }
|
||||
|
||||
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
|
||||
expect {
|
||||
product_set.save
|
||||
|
||||
expect(product.reload.variants.first[:sku]).to eq variants_attributes.first[:sku]
|
||||
end
|
||||
|
||||
context 'and when product attributes are also passed' do
|
||||
it 'updates product and variant attributes' do
|
||||
collection_hash[0][:sku] = "test_sku"
|
||||
|
||||
product_set.save
|
||||
|
||||
expect(product.reload.variants.first[:sku]).to eq variants_attributes.first[:sku]
|
||||
expect(product.reload.attributes).to include(
|
||||
'sku' => "test_sku"
|
||||
)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when :master_attributes is passed' do
|
||||
let(:master_attributes) { { sku: '123' } }
|
||||
|
||||
before do
|
||||
collection_hash[0][:master_attributes] = master_attributes
|
||||
end
|
||||
|
||||
context 'and the variant does exist' do
|
||||
let!(:variant) { create(:variant, product:) }
|
||||
|
||||
before { master_attributes[:id] = variant.id }
|
||||
|
||||
it 'updates the attributes of the master variant' do
|
||||
product_set.save
|
||||
expect(variant.reload.sku).to eq('123')
|
||||
end
|
||||
end
|
||||
|
||||
context 'and the variant does not exist' do
|
||||
context 'and attributes provided are valid' do
|
||||
let(:master_attributes) do
|
||||
attributes_for(:variant).merge(sku: '123')
|
||||
end
|
||||
|
||||
it 'creates it with the specified attributes' do
|
||||
number_of_variants = Spree::Variant.all.size
|
||||
product_set.save
|
||||
expect(Spree::Variant.last.sku).to eq('123')
|
||||
expect(Spree::Variant.all.size).to eq number_of_variants + 1
|
||||
end
|
||||
end
|
||||
|
||||
context 'and attributes provided are not valid' do
|
||||
let(:master_attributes) do
|
||||
# unit_value nil makes the variant invalid
|
||||
# on_hand with a value would make on_hand be updated and fail with exception
|
||||
attributes_for(:variant).merge(unit_value: nil, on_hand: 1, sku: '321')
|
||||
end
|
||||
|
||||
it 'does not create variant and notifies bugsnag still raising the exception' do
|
||||
number_of_variants = Spree::Variant.all.size
|
||||
expect(product_set.save).to eq(false)
|
||||
expect(Spree::Variant.all.size).to eq number_of_variants
|
||||
expect(Spree::Variant.last.sku).not_to eq('321')
|
||||
end
|
||||
end
|
||||
end
|
||||
product.reload
|
||||
}.to change { product.sku }.to("test_sku")
|
||||
.and change { product.variants.first.sku }.to("123")
|
||||
end
|
||||
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") }
|
||||
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: "Pommes",
|
||||
name: "", # Product Name can't be blank
|
||||
},
|
||||
1 => {
|
||||
id: product_b.id,
|
||||
@@ -216,46 +185,17 @@ describe Sets::ProductSet do
|
||||
}
|
||||
end
|
||||
|
||||
it 'updates the products' do
|
||||
it 'continues to update subsequent products' do
|
||||
product_set.save
|
||||
|
||||
expect(product_a.reload.name).to eq "Pommes"
|
||||
# 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
|
||||
|
||||
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
|
||||
|
||||
@@ -7,11 +7,6 @@ describe 'As an admin, I can see the new product page' do
|
||||
include AuthenticationHelper
|
||||
include FileHelper
|
||||
|
||||
# create lot of products
|
||||
70.times do |i|
|
||||
let!("product_#{i}".to_sym) { create(:simple_product, name: "product #{i}") }
|
||||
end
|
||||
|
||||
before do
|
||||
# activate feature toggle admin_style_v3 to use new admin interface
|
||||
Flipper.enable(:admin_style_v3)
|
||||
@@ -43,24 +38,26 @@ describe 'As an admin, I can see the new product page' do
|
||||
end
|
||||
|
||||
describe "pagination" do
|
||||
before do
|
||||
visit admin_products_url
|
||||
end
|
||||
|
||||
it "has a pagination, has 15 products per page by default and can change the page" do
|
||||
create_products 16
|
||||
visit admin_products_url
|
||||
|
||||
expect(page).to have_selector ".pagination"
|
||||
expect_products_count_to_be 15
|
||||
within ".pagination" do
|
||||
click_link "2"
|
||||
end
|
||||
|
||||
expect(page).to have_content "Showing 16 to 30"
|
||||
expect(page).to have_content "Showing 16 to 16" # todo: remove unnecessary duplication
|
||||
expect_page_to_be 2
|
||||
expect_per_page_to_be 15
|
||||
expect_products_count_to_be 15
|
||||
expect_products_count_to_be 1
|
||||
end
|
||||
|
||||
it "can change the number of products per page" do
|
||||
create_products 51
|
||||
visit admin_products_url
|
||||
|
||||
select "50", from: "per_page"
|
||||
|
||||
expect(page).to have_content "Showing 1 to 50"
|
||||
@@ -75,11 +72,10 @@ describe 'As an admin, I can see the new product page' do
|
||||
# create a product with a name that can be searched
|
||||
let!(:product_by_name) { create(:simple_product, name: "searchable product") }
|
||||
|
||||
before do
|
||||
visit admin_products_url
|
||||
end
|
||||
|
||||
it "can search for a product" do
|
||||
create_products 1
|
||||
visit admin_products_url
|
||||
|
||||
search_for "searchable product"
|
||||
|
||||
expect(page).to have_field "search_term", with: "searchable product"
|
||||
@@ -88,20 +84,26 @@ describe 'As an admin, I can see the new product page' do
|
||||
end
|
||||
|
||||
it "reset the page when searching" do
|
||||
create_products 15
|
||||
visit admin_products_url
|
||||
|
||||
within ".pagination" do
|
||||
click_link "2"
|
||||
end
|
||||
|
||||
expect(page).to have_content "Showing 16 to 30"
|
||||
expect(page).to have_content "Showing 16 to 16"
|
||||
expect_page_to_be 2
|
||||
expect_per_page_to_be 15
|
||||
expect_products_count_to_be 15
|
||||
expect_products_count_to_be 1
|
||||
search_for "searchable product"
|
||||
# expect(page).to have_content "1 product found for your search criteria."
|
||||
expect_products_count_to_be 1
|
||||
end
|
||||
|
||||
it "can clear filters" do
|
||||
create_products 1
|
||||
visit admin_products_url
|
||||
|
||||
search_for "searchable product"
|
||||
expect(page).to have_field "search_term", with: "searchable product"
|
||||
# expect(page).to have_content "1 product found for your search criteria."
|
||||
@@ -110,12 +112,13 @@ describe 'As an admin, I can see the new product page' do
|
||||
|
||||
click_link "Clear search"
|
||||
expect(page).to have_field "search_term", with: ""
|
||||
expect(page).to have_content "Showing 1 to 15"
|
||||
expect_page_to_be 1
|
||||
expect_products_count_to_be 15
|
||||
expect(page).to have_content "Showing 1 to 2"
|
||||
expect_products_count_to_be 2
|
||||
end
|
||||
|
||||
it "shows a message when there are no results" do
|
||||
visit admin_products_url
|
||||
|
||||
search_for "no results"
|
||||
expect(page).to have_content "No products found for your search criteria"
|
||||
expect(page).to have_link "Clear search"
|
||||
@@ -123,7 +126,9 @@ describe 'As an admin, I can see the new product page' do
|
||||
end
|
||||
|
||||
context "product has producer" do
|
||||
# create a product with a supplier that can be searched
|
||||
before { create_products 1 }
|
||||
|
||||
# create a product with a different supplier
|
||||
let!(:producer) { create(:supplier_enterprise, name: "Producer 1") }
|
||||
let!(:product_by_supplier) { create(:simple_product, supplier: producer) }
|
||||
|
||||
@@ -139,7 +144,9 @@ describe 'As an admin, I can see the new product page' do
|
||||
end
|
||||
|
||||
context "product has category" do
|
||||
# create a product with a category that can be searched
|
||||
before { create_products 1 }
|
||||
|
||||
# create a product with a different category
|
||||
let!(:product_by_category) {
|
||||
create(:simple_product, primary_taxon: create(:taxon, name: "Category 1"))
|
||||
}
|
||||
@@ -297,6 +304,12 @@ describe 'As an admin, I can see the new product page' do
|
||||
end
|
||||
end
|
||||
|
||||
def create_products(amount)
|
||||
amount.times do |i|
|
||||
create(:simple_product, name: "product #{i}")
|
||||
end
|
||||
end
|
||||
|
||||
def expect_page_to_be(page_number)
|
||||
expect(page).to have_selector ".pagination span.page.current", text: page_number.to_s
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user