Fix Bulk products edit page , part 1

app/views/admin/products_v3/_product_row.html.haml

@@ -7,18 +7,8 @@
 %td.col-sku.field.naked_inputs
   = f.text_field :sku, 'aria-label': t('admin.products_page.columns.sku')
   = error_message_on product, :sku
-%td.col-unit_scale.field.naked_inputs{ 'data-controller': 'toggle-control', 'data-toggle-control-match-value': 'items' }
-  = f.hidden_field :variant_unit
-  = f.hidden_field :variant_unit_scale
-  = f.select :variant_unit_with_scale,
-      options_for_select(WeightsAndMeasures.variant_unit_options, product.variant_unit_with_scale),
-      {},
-      class: "fullwidth no-input",
-      'aria-label': t('admin.products_page.columns.unit_scale'),
-      data: { "controller": "tom-select", "tom-select-options-value": '{ "plugins": [] }', action: "change->toggle-control#displayIfMatch"}
-  .field
-    = f.text_field :variant_unit_name, 'aria-label': t('items'), 'data-toggle-control-target': 'control', style: (product.variant_unit == "items" ? "" : "display: none")
-    = error_message_on product, :variant_unit_name, 'data-toggle-control-target': 'control'
+%td.col-unit_scale.align-right
+  -# empty
 %td.col-unit.align-right
   -# empty
 %td.col-price.align-right

app/views/admin/products_v3/_variant_row.html.haml

@@ -7,8 +7,18 @@
 %td.col-sku.field.naked_inputs
   = f.text_field :sku, 'aria-label': t('admin.products_page.columns.sku')
   = error_message_on variant, :sku
-%td.col-unit_scale
-  -# empty
+%td.col-unir_scale.field.naked_inputs{ 'data-controller': 'toggle-control', 'data-toggle-control-match-value': 'items' }
+  = f.hidden_field :variant_unit
+  = f.hidden_field :variant_unit_scale
+  = f.select :variant_unit_with_scale,
+      options_for_select(WeightsAndMeasures.variant_unit_options, variant.variant_unit_with_scale),
+      {},
+      class: "fullwidth no-input",
+      'aria-label': t('admin.products_page.columns.unit_scale'),
+      data: { "controller": "tom-select", "tom-select-options-value": '{ "plugins": [] }', action: "change->toggle-control#displayIfMatch"}
+  .field
+    = f.text_field :variant_unit_name, 'aria-label': t('items'), 'data-toggle-control-target': 'control', style: (variant.variant_unit == "items" ? "" : "display: none")
+    = error_message_on variant, :variant_unit_name, 'data-toggle-control-target': 'control'
 %td.col-unit.field.popout{'data-controller': "popout", 'data-popout-update-display-value': "false"}
   = f.button :unit_to_display, class: "popout__button", 'aria-label': t('admin.products_page.columns.unit'), 'data-popout-target': "button" do
     = variant.unit_to_display # Show the generated summary of unit values

app/views/admin/products_v3/index.html.haml

@@ -11,7 +11,7 @@
 
 = render partial: 'spree/admin/shared/product_sub_menu'
 
-#products_v3_page{ "data-controller": "products", 'data-turbo': true }
+#products_v3_page{ 'data-turbo': true }
   = render partial: "content", locals: { products: @products, pagy: @pagy, search_term: @search_term,
                                producer_options: producers, producer_id: @producer_id,
                                category_options: categories, category_id: @category_id,

app/webpacker/controllers/product_controller.js

@@ -1,38 +0,0 @@
-import { Controller } from "stimulus";
-
-// Dynamically update related Product unit fields (expected to move to Variant due to Product Refactor)
-//
-export default class ProductController extends Controller {
-  connect() {
-    // idea: create a helper that includes a nice getter/setter for Rails model attr values, just pass it the attribute name.
-    // It could automatically find (and cache a ref to) each dom element and get/set the values.
-    this.variantUnit = this.element.querySelector('[name$="[variant_unit]"]');
-    this.variantUnitScale = this.element.querySelector('[name$="[variant_unit_scale]"]');
-    this.variantUnitWithScale = this.element.querySelector('[name$="[variant_unit_with_scale]"]');
-
-    // on variant_unit_with_scale changed; update variant_unit and variant_unit_scale
-    this.variantUnitWithScale.addEventListener("change", this.#updateUnitAndScale.bind(this), {
-      passive: true,
-    });
-  }
-
-  // private
-
-  // Extract variant_unit and variant_unit_scale from dropdown variant_unit_with_scale,
-  // and update hidden product fields
-  #updateUnitAndScale(event) {
-    const variant_unit_with_scale = this.variantUnitWithScale.value;
-    const match = variant_unit_with_scale.match(/^([^_]+)_([\d\.]+)$/); // eg "weight_1000"
-
-    if (match) {
-      this.variantUnit.value = match[1];
-      this.variantUnitScale.value = parseFloat(match[2]);
-    } else {
-      // "items"
-      this.variantUnit.value = variant_unit_with_scale;
-      this.variantUnitScale.value = "";
-    }
-    this.variantUnit.dispatchEvent(new Event("change"));
-    this.variantUnitScale.dispatchEvent(new Event("change"));
-  }
-}

app/webpacker/controllers/variant_controller.js

@@ -5,11 +5,17 @@ import OptionValueNamer from "js/services/option_value_namer";
 //
 export default class VariantController extends Controller {
   connect() {
-    // Assuming these will be available on the variant soon, just a quick hack to find the product fields:
-    const product = this.element.closest("[data-record-id]");
-    this.variantUnit = product.querySelector('[name$="[variant_unit]"]');
-    this.variantUnitScale = product.querySelector('[name$="[variant_unit_scale]"]');
-    this.variantUnitName = product.querySelector('[name$="[variant_unit_name]"]');
+    // idea: create a helper that includes a nice getter/setter for Rails model attr values, just pass it the attribute name.
+    // It could automatically find (and cache a ref to) each dom element and get/set the values.
+    this.variantUnit = this.element.querySelector('[name$="[variant_unit]"]');
+    this.variantUnitScale = this.element.querySelector('[name$="[variant_unit_scale]"]');
+    this.variantUnitName = this.element.querySelector('[name$="[variant_unit_name]"]');
+    this.variantUnitWithScale = this.element.querySelector('[name$="[variant_unit_with_scale]"]');
+    
+    // on variant_unit_with_scale changed; update variant_unit and variant_unit_scale
+    this.variantUnitWithScale.addEventListener("change", this.#updateUnitAndScale.bind(this), {
+      passive: true,
+    });
 
     this.unitValue = this.element.querySelector('[name$="[unit_value]"]');
     this.unitDescription = this.element.querySelector('[name$="[unit_description]"]');
@@ -76,11 +82,27 @@ export default class VariantController extends Controller {
     return {
       unit_value: parseFloat(this.unitValue.value),
       unit_description: this.unitDescription.value,
-      product: {
-        variant_unit: this.variantUnit.value,
-        variant_unit_scale: parseFloat(this.variantUnitScale.value),
-        variant_unit_name: this.variantUnitName.value,
-      },
+      variant_unit: this.variantUnit.value,
+      variant_unit_scale: parseFloat(this.variantUnitScale.value),
+      variant_unit_name: this.variantUnitName.value,
     };
   }
+  
+  // Extract variant_unit and variant_unit_scale from dropdown variant_unit_with_scale,
+  // and update hidden product fields
+  #updateUnitAndScale(event) {
+    const variant_unit_with_scale = this.variantUnitWithScale.value;
+    const match = variant_unit_with_scale.match(/^([^_]+)_([\d\.]+)$/); // eg "weight_1000"
+
+    if (match) {
+      this.variantUnit.value = match[1];
+      this.variantUnitScale.value = parseFloat(match[2]);
+    } else {
+      // "items"
+      this.variantUnit.value = variant_unit_with_scale;
+      this.variantUnitScale.value = "";
+    }
+    this.variantUnit.dispatchEvent(new Event("change"));
+    this.variantUnitScale.dispatchEvent(new Event("change"));
+  }
 }

app/webpacker/js/services/option_value_namer.js

@@ -1,4 +1,4 @@
-import VariantUnitManager from "../../js/services/variant_unit_manager";
+import VariantUnitManager from "js/services/variant_unit_manager";
 
 // Javascript clone of VariantUnits::OptionValueNamer, for bulk product editing.
 export default class OptionValueNamer {
@@ -24,17 +24,17 @@ export default class OptionValueNamer {
   }
 
   value_scaled() {
-    return !!this.variant.product.variant_unit_scale;
+    return !!this.variant.variant_unit_scale;
   }
 
   option_value_value_unit() {
     let value, unit_name;
     if (this.variant.unit_value) {
-      if (this.variant.product.variant_unit === "weight" || this.variant.product.variant_unit === "volume") {
+      if (this.variant.variant_unit === "weight" || this.variant.variant_unit === "volume") {
         [value, unit_name] = this.option_value_value_unit_scaled();
       } else {
         value = this.variant.unit_value;
-        unit_name = this.pluralize(this.variant.product.variant_unit_name, value);
+        unit_name = this.pluralize(this.variant.variant_unit_name, value);
       }
       if (value == parseInt(value, 10)) {
         value = parseInt(value, 10);
@@ -83,16 +83,17 @@ export default class OptionValueNamer {
     // to >= 1 when expressed in it.
     // If there is none available where this is true, use the smallest
     // available unit.
-    const product = this.variant.product;
-    const scales = this.variantUnitManager.compatibleUnitScales(product.variant_unit_scale, product.variant_unit);
+    const scales = this.variantUnitManager.compatibleUnitScales(
+      this.variant.variant_unit_scale, this.variant.variant_unit
+    );
     const variantUnitValue = this.variant.unit_value;
 
     // sets largestScale = last element in filtered scales array
     const largestScale = scales.filter(s => variantUnitValue / s >= 1).slice(-1)[0];
     if (largestScale) {
-      return [largestScale, this.variantUnitManager.getUnitName(largestScale, product.variant_unit)];
+      return [largestScale, this.variantUnitManager.getUnitName(largestScale, this.variant.variant_unit)];
     } else {
-      return [scales[0], this.variantUnitManager.getUnitName(scales[0], product.variant_unit)];
+      return [scales[0], this.variantUnitManager.getUnitName(scales[0], this.variant.variant_unit)];
     }
   }
 }

spec/javascripts/services/option_value_namer_test.js

@@ -2,7 +2,7 @@
  * @jest-environment jsdom
  */
 
-import OptionValueNamer from "../../../app/webpacker/js/services/option_value_namer";
+import OptionValueNamer from "js/services/option_value_namer";
 
 describe("OptionValueNamer", () => {
   beforeAll(() => {
@@ -53,14 +53,12 @@ describe("OptionValueNamer", () => {
     });
 
     describe("determining if a variant's value is scaled", function() {
-      var p;
       beforeEach(function() {
-        p = {};
-        v = { product: p };
+        v = {};
         namer = new OptionValueNamer(v);
       });
       it("returns true when the product has a scale", function() {
-        p.variant_unit_scale = 1000;
+        v.variant_unit_scale = 1000;
         expect(namer.value_scaled()).toBe(true);
       });
       it("returns false otherwise", function() {
@@ -69,7 +67,7 @@ describe("OptionValueNamer", () => {
     });
 
     describe("generating option value's value and unit", function() {
-      var v, p, namer;
+      var v, namer;
 
       // Mock I18n. TODO: moved to a shared helper
       beforeAll(() => {
@@ -84,40 +82,39 @@ describe("OptionValueNamer", () => {
       })
 
       beforeEach(function() {
-        p = {};
-        v = { product: p };
+        v = {};
         namer = new OptionValueNamer(v);
       });
       it("generates simple values", function() {
-        p.variant_unit = 'weight';
-        p.variant_unit_scale = 1.0;
+        v.variant_unit = 'weight';
+        v.variant_unit_scale = 1.0;
         v.unit_value = 100;
         expect(namer.option_value_value_unit()).toEqual([100, 'g']);
       });
       it("generates values when unit value is non-integer", function() {
-        p.variant_unit = 'weight';
-        p.variant_unit_scale = 1.0;
+        v.variant_unit = 'weight';
+        v.variant_unit_scale = 1.0;
         v.unit_value = 123.45;
         expect(namer.option_value_value_unit()).toEqual([123.45, 'g']);
       });
       it("returns a value of 1 when unit value equals the scale", function() {
-        p.variant_unit = 'weight';
-        p.variant_unit_scale = 1000.0;
+        v.variant_unit = 'weight';
+        v.variant_unit_scale = 1000.0;
         v.unit_value = 1000.0;
         expect(namer.option_value_value_unit()).toEqual([1, 'kg']);
       });
       it("generates values for all weight scales", function() {
         [[1.0, 'g'], [1000.0, 'kg'], [1000000.0, 'T']].forEach(([scale, unit]) => {
-          p.variant_unit = 'weight';
-          p.variant_unit_scale = scale;
+          v.variant_unit = 'weight';
+          v.variant_unit_scale = scale;
           v.unit_value = 100 * scale;
           expect(namer.option_value_value_unit()).toEqual([100, unit]);
         });
       });
       it("generates values for all volume scales", function() {
         [[0.001, 'mL'], [1.0, 'L'], [1000.0, 'kL']].forEach(([scale, unit]) => {
-          p.variant_unit = 'volume';
-          p.variant_unit_scale = scale;
+          v.variant_unit = 'volume';
+          v.variant_unit_scale = scale;
           v.unit_value = 100 * scale;
           expect(namer.option_value_value_unit()).toEqual([100, unit]);
         });
@@ -125,14 +122,14 @@ describe("OptionValueNamer", () => {
       it("generates right values for volume with rounded values", function() {
         var unit;
         unit = 'L';
-        p.variant_unit = 'volume';
-        p.variant_unit_scale = 1.0;
+        v.variant_unit = 'volume';
+        v.variant_unit_scale = 1.0;
         v.unit_value = 0.7;
         expect(namer.option_value_value_unit()).toEqual([700, 'mL']);
       });
       it("chooses the correct scale when value is very small", function() {
-        p.variant_unit = 'volume';
-        p.variant_unit_scale = 0.001;
+        v.variant_unit = 'volume';
+        v.variant_unit_scale = 0.001;
         v.unit_value = 0.0001;
         expect(namer.option_value_value_unit()).toEqual([0.1, 'mL']);
       });
@@ -145,16 +142,16 @@ describe("OptionValueNamer", () => {
         //   subject.option_value_value_unit.should == [100, unit.pluralize]
       });
       it("generates singular values for item units when value is 1", function() {
-        p.variant_unit = 'items';
-        p.variant_unit_scale = null;
-        p.variant_unit_name = 'packet';
+        v.variant_unit = 'items';
+        v.variant_unit_scale = null;
+        v.variant_unit_name = 'packet';
         v.unit_value = 1;
         expect(namer.option_value_value_unit()).toEqual([1, 'packet']);
       });
       it("returns [null, null] when unit value is not set", function() {
-        p.variant_unit = 'items';
-        p.variant_unit_scale = null;
-        p.variant_unit_name = 'foo';
+        v.variant_unit = 'items';
+        v.variant_unit_scale = null;
+        v.variant_unit_name = 'foo';
         v.unit_value = null;
         expect(namer.option_value_value_unit()).toEqual([null, null]);
       });

spec/javascripts/stimulus/product_controller_test.js

@@ -1,56 +0,0 @@
-/**
- * @jest-environment jsdom
- */
-
-import { Application } from "stimulus";
-import product_controller from "../../../app/webpacker/controllers/product_controller";
-
-describe("ProductController", () => {
-  beforeAll(() => {
-    const application = Application.start();
-    application.register("product", product_controller);
-  });
-
-  describe("variant_unit_with_scale", () => {
-    beforeEach(() => {
-      document.body.innerHTML = `
-        <div data-controller="product">
-          <input id="variant_unit" name="[products][0][variant_unit]" value="weight">
-          <input id="variant_unit_scale" name="[products][0][variant_unit_scale]" value="1.0">
-          <select id="variant_unit_with_scale" name="[products][0][variant_unit_with_scale]">
-            <option selected="selected" value="weight_1">Weight (g)</option>
-            <option value="weight_1000">Weight (kg)</option>
-            <option value="volume_4.54609">Volume (gal)</option>
-            <option value="items">Items</option>
-          </select>
-        </div>
-      `;
-    });
-
-    describe("change", () => {
-      it("weight_1000", () => {
-        variant_unit_with_scale.selectedIndex = 1;
-        variant_unit_with_scale.dispatchEvent(new Event("change"));
-
-        expect(variant_unit.value).toBe("weight");
-        expect(variant_unit_scale.value).toBe("1000");
-      });
-
-      it("volume_4.54609", () => {
-        variant_unit_with_scale.selectedIndex = 2;
-        variant_unit_with_scale.dispatchEvent(new Event("change"));
-
-        expect(variant_unit.value).toBe("volume");
-        expect(variant_unit_scale.value).toBe("4.54609");
-      });
-
-      it("items", () => {
-        variant_unit_with_scale.selectedIndex = 3;
-        variant_unit_with_scale.dispatchEvent(new Event("change"));
-
-        expect(variant_unit.value).toBe("items");
-        expect(variant_unit_scale.value).toBe("");
-      });
-    })
-  });
-});

spec/javascripts/stimulus/variant_controller_test.js

@@ -0,0 +1,87 @@
+/**
+ * @jest-environment jsdom
+ */
+
+import { Application } from "stimulus";
+import variant_controller from "controllers/variant_controller";
+
+
+describe("VariantController", () => {
+  beforeAll(() => {
+    // Requires global var from page
+    global.ofn_available_units_sorted = {
+      "weight": {
+       "1.0":{"name":"g","system":"metric"},
+       "1000.0":{"name":"kg","system":"metric"},
+       "1000000.0":{"name":"T","system":"metric"}
+      },
+      "volume":{
+        "0.001":{"name":"mL","system":"metric"},
+        "1.0":{"name":"L","system":"metric"},
+        "4.54609":{"name":"gal","system":"imperial"},
+        "1000.0":{"name":"kL","system":"metric"}
+       }
+    };
+
+    const mockedT = jest.fn();
+    mockedT.mockImplementation((string, opts) => (string + ', ' + JSON.stringify(opts)));
+
+    global.I18n =  { t: mockedT };
+
+    const application = Application.start();
+    application.register("variant", variant_controller);
+  });
+
+  afterAll(() => {
+    delete global.I18n;
+  })
+
+  describe("variant_unit_with_scale", () => {
+    beforeEach(() => {
+      document.body.innerHTML = `
+        <div data-controller="variant">
+          <input id="variant_unit" name="[products][0][variants_attributes][0][variant_unit]" value="weight">
+          <input id="variant_unit_scale" name="[products][0][variants_attributes][0][variant_unit_scale]" value="1.0">
+          <select id="variant_unit_with_scale" name="[products][0][variants_attributes][0][variant_unit_with_scale]">
+            <option selected="selected" value="weight_1">Weight (g)</option>
+            <option value="weight_1000">Weight (kg)</option>
+            <option value="volume_4.54609">Volume (gal)</option>
+            <option value="items">Items</option>
+          </select>
+          <input id="variant_unit_name" name="[products][0][variants_attributes][0][variant_unit_name]" type="text" >
+          <button id="unit_to_display" name="[products][0][variants_attributes][][0][unit_to_display]" type="submit" >2kg</button>
+          <input id="unit_value" name="[products][0][variants_attributes][0][unit_value]" value="2000.0"  >
+          <input id="unit_description" name="[products][0][variants_attributes][0][unit_description]" >
+          <input id="unit_value_with_description" name="[products][0][variants_attributes][0][unit_value_with_description]" value="2" type="text" >
+          <input id="display_as" name="[products][0][variants_attributes][0][display_as]" placeholder="2kg" type="text" >
+        </div>
+      `;
+    });
+
+    describe("change", () => {
+      it("weight_1000", () => {
+        variant_unit_with_scale.selectedIndex = 1;
+        variant_unit_with_scale.dispatchEvent(new Event("change"));
+
+        expect(variant_unit.value).toBe("weight");
+        expect(variant_unit_scale.value).toBe("1000");
+      });
+
+      it("volume_4.54609", () => {
+        variant_unit_with_scale.selectedIndex = 2;
+        variant_unit_with_scale.dispatchEvent(new Event("change"));
+
+        expect(variant_unit.value).toBe("volume");
+        expect(variant_unit_scale.value).toBe("4.54609");
+      });
+
+      it("items", () => {
+        variant_unit_with_scale.selectedIndex = 3;
+        variant_unit_with_scale.dispatchEvent(new Event("change"));
+
+        expect(variant_unit.value).toBe("items");
+        expect(variant_unit_scale.value).toBe("");
+      });
+    })
+  });
+});