From b4a3aaab19576378c2121918d0b7aee2ac58cf3b Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 11 Jul 2023 16:16:27 +1000 Subject: [PATCH 1/5] Add select variant stimulus controller This is similar to variantAutocomplete directive used on the new order page. It doesn't have all the same feature, but it's a start --- .../controllers/select_variant_controller.js | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 app/webpacker/controllers/select_variant_controller.js diff --git a/app/webpacker/controllers/select_variant_controller.js b/app/webpacker/controllers/select_variant_controller.js new file mode 100644 index 0000000000..778f1717c8 --- /dev/null +++ b/app/webpacker/controllers/select_variant_controller.js @@ -0,0 +1,66 @@ +import TomSelectController from "./tom_select_controller"; + +// This is simalar to the "variantAutocomplete" directive that uses "select2", but it doesn't +// have all the same feature +// +export default class extends TomSelectController { + static values = { options: Object, distributor: Number }; + + connect() { + const options = { + valueField: 'id', + searchField: ['name', 'sku'], + load: this.#load.bind(this), + shouldLoad: (query) => query.length > 2, + render: { + option: this.#renderOption.bind(this), + item: this.#renderItem.bind(this), + }, + }; + super.connect(options); + } + + // private + + #load(query, callback) { + const url = '/admin/variants/search.json?q=' + encodeURIComponent(query); + fetch(url) + .then(response => response.json()) + .then(json => { + callback(json); + }).catch((error) => { + console.log(error); + callback(); + }); + } + + #renderOption(variant, escape) { + return `
+
+ ${ variant.image ? `` : "" } +
+
+
${ escape(variant.name)}
+
    +
  • ${ I18n.t('spree.admin.variants.autocomplete.producer_name') }: ${ escape(variant.producer_name) }
    • +
+
    +
  • ${ I18n.t('admin.sku') }: ${ escape(variant.sku)}
    • + ${ + variant.on_demand + ? `
  • ${I18n.t("on_demand")}
    • ` + : `
  • + ${I18n.t("on_hand")}: ${escape(variant.on_hand)} +
    • ` + } +
  • ${ I18n.t('spree.admin.variants.autocomplete.unit') }: ${ escape(variant.options_text) }
    • +
+
+
`; + } + + #renderItem(variant, escape) { + return `${ escape(variant.name) }` + } +} + From ec78de7cea5b79cdf1929da5ca6e197869f35255 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Mon, 17 Jul 2023 16:38:45 +1000 Subject: [PATCH 2/5] Add product dropdown on Order and Fulfillment reports It includes loading the selected product to populate the dropdown when needed. --- app/controllers/admin/reports_controller.rb | 9 +++ .../filters/_orders_and_fulfillment.html.haml | 6 ++ .../controllers/select_variant_controller.js | 58 +++++++++++++------ 3 files changed, 56 insertions(+), 17 deletions(-) diff --git a/app/controllers/admin/reports_controller.rb b/app/controllers/admin/reports_controller.rb index 3af1c0714a..e43d4e4145 100644 --- a/app/controllers/admin/reports_controller.rb +++ b/app/controllers/admin/reports_controller.rb @@ -55,6 +55,15 @@ module Admin @report_title = report_title @rendering_options = rendering_options @data = Reporting::FrontendData.new(spree_current_user) + + variant_id_in = params[:variant_id_in].reject(&:blank?) + load_selected_variant if variant_id_in.present? + end + + # Orders and Fulfillment Reports include a per product filter, load any selected product + def load_selected_variant + variant = Spree::Variant.find(params[:variant_id_in][0]) + @variant_serialized = Api::Admin::VariantSerializer.new(variant) end def render_data? diff --git a/app/views/admin/reports/filters/_orders_and_fulfillment.html.haml b/app/views/admin/reports/filters/_orders_and_fulfillment.html.haml index 0a9963544f..aa6097c6ba 100755 --- a/app/views/admin/reports/filters/_orders_and_fulfillment.html.haml +++ b/app/views/admin/reports/filters/_orders_and_fulfillment.html.haml @@ -12,3 +12,9 @@ .alpha.two.columns= label_tag nil, t(:report_customers_cycle) .omega.fourteen.columns = f.select(:order_cycle_id_in, report_order_cycle_options(@data.order_cycles), {selected: params.dig(:q, :order_cycle_id_in)}, {class: "select2 fullwidth", multiple: true}) +.row + .alpha.two.columns= label_tag :add_variant_id, Spree.t(:name_or_sku) + .omega.fourteen.columns + - variant_json = @variant_serialized.present? ? @variant_serialized.to_json() : {} + = select_tag(:variant_id_in, params[:variant_id_in], { class: "fullwidth", multiple: true , data: { controller: "select-variant", "select-variant-selected-value": "#{variant_json}" } }) + diff --git a/app/webpacker/controllers/select_variant_controller.js b/app/webpacker/controllers/select_variant_controller.js index 778f1717c8..75d9a9ebcf 100644 --- a/app/webpacker/controllers/select_variant_controller.js +++ b/app/webpacker/controllers/select_variant_controller.js @@ -1,15 +1,15 @@ import TomSelectController from "./tom_select_controller"; -// This is simalar to the "variantAutocomplete" directive that uses "select2", but it doesn't +// This is simalar to the "variantAutocomplete" directive that uses "select2", but it doesn't // have all the same feature // export default class extends TomSelectController { - static values = { options: Object, distributor: Number }; + static values = { options: Object, distributor: Number, selected: Object }; connect() { const options = { - valueField: 'id', - searchField: ['name', 'sku'], + valueField: "id", + searchField: ["name", "sku"], load: this.#load.bind(this), shouldLoad: (query) => query.length > 2, render: { @@ -18,17 +18,33 @@ export default class extends TomSelectController { }, }; super.connect(options); + // Add the selected value if any and select it. + // It will need to include data used in the templates below: + // - id + // - image + // - name + // - producer_name + // - sku + // - on_demand + // - on_hand + // - options_text + // + if (this.hasSelectedValue && Object.keys(this.selectedValue).length > 0) { + this.control.addOption(this.selectedValue); + this.control.addItem(this.selectedValue.id); + } } // private #load(query, callback) { - const url = '/admin/variants/search.json?q=' + encodeURIComponent(query); + const url = "/admin/variants/search.json?q=" + encodeURIComponent(query); fetch(url) - .then(response => response.json()) - .then(json => { + .then((response) => response.json()) + .then((json) => { callback(json); - }).catch((error) => { + }) + .catch((error) => { console.log(error); callback(); }); @@ -37,30 +53,38 @@ export default class extends TomSelectController { #renderOption(variant, escape) { return `
- ${ variant.image ? `` : "" } + ${variant.image ? `` : ""}
-
${ escape(variant.name)}
+
${escape(variant.name)}
    -
  • ${ I18n.t('spree.admin.variants.autocomplete.producer_name') }: ${ escape(variant.producer_name) }
    • +
  • + ${I18n.t("spree.admin.variants.autocomplete.producer_name")}: + ${escape(variant.producer_name)} +
    -
  • ${ I18n.t('admin.sku') }: ${ escape(variant.sku)}
    • - ${ +
  • + ${I18n.t("admin.sku")}: + ${escape(variant.sku)} +
    • + ${ variant.on_demand ? `
  • ${I18n.t("on_demand")}
    • ` : `
  • ${I18n.t("on_hand")}: ${escape(variant.on_hand)}
    • ` } -
  • ${ I18n.t('spree.admin.variants.autocomplete.unit') }: ${ escape(variant.options_text) }
    • +
  • + ${I18n.t("spree.admin.variants.autocomplete.unit")}: + ${escape(variant.options_text)} +
`; - } + } #renderItem(variant, escape) { - return `${ escape(variant.name) }` + return `${escape(variant.name)}`; } } - From 2fff77b2eebd38f995125ce0170ab438ed87366b Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 18 Jul 2023 16:17:53 +1000 Subject: [PATCH 3/5] Report, add filtering by product for line items --- app/controllers/admin/reports_controller.rb | 2 +- lib/reporting/line_items.rb | 6 ++++ spec/lib/reports/line_items_spec.rb | 36 +++++++++++++++++---- 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/app/controllers/admin/reports_controller.rb b/app/controllers/admin/reports_controller.rb index e43d4e4145..cd66e3d8c2 100644 --- a/app/controllers/admin/reports_controller.rb +++ b/app/controllers/admin/reports_controller.rb @@ -56,7 +56,7 @@ module Admin @rendering_options = rendering_options @data = Reporting::FrontendData.new(spree_current_user) - variant_id_in = params[:variant_id_in].reject(&:blank?) + variant_id_in = params[:variant_id_in]&.reject(&:blank?) load_selected_variant if variant_id_in.present? end diff --git a/lib/reporting/line_items.rb b/lib/reporting/line_items.rb index 714d071405..87afa58a64 100644 --- a/lib/reporting/line_items.rb +++ b/lib/reporting/line_items.rb @@ -23,6 +23,12 @@ module Reporting line_items = line_items.supplied_by_any(@params[:supplier_id_in]) end + # Filter by product + variant_id_in = @params[:variant_id_in].reject(&:blank?) + if variant_id_in.present? + line_items = line_items.where("spree_line_items.variant_id": variant_id_in) + end + if line_item_includes.present? line_items = line_items.includes(*line_item_includes).references(:line_items) end diff --git a/spec/lib/reports/line_items_spec.rb b/spec/lib/reports/line_items_spec.rb index 3a0bd9beb8..0814778837 100644 --- a/spec/lib/reports/line_items_spec.rb +++ b/spec/lib/reports/line_items_spec.rb @@ -9,13 +9,13 @@ describe Reporting::LineItems do # under test and the various objects it depends on. Other more common moking strategies where very # hard. class FakeOrderPermissions - def initialize(line_item, orders_relation) - @relation = Spree::LineItem.where(id: line_item.id) + def initialize(line_items, orders_relation) + @relations = Spree::LineItem.where(id: line_items.map(&:id)) @orders_relation = orders_relation end def visible_line_items - relation + relations end def editable_line_items @@ -29,7 +29,7 @@ describe Reporting::LineItems do private - attr_reader :relation, :orders_relation + attr_reader :relations, :orders_relation end describe '#list' do @@ -41,15 +41,39 @@ describe Reporting::LineItems do shipments: [build(:shipment)] ) end - let!(:line_item) { create(:line_item, order: order) } + let!(:line_item1) { create(:line_item, order: order) } let(:orders_relation) { Spree::Order.where(id: order.id) } - let(:order_permissions) { FakeOrderPermissions.new(line_item, orders_relation) } + let(:order_permissions) { FakeOrderPermissions.new([line_item1], orders_relation) } let(:params) { {} } it 'returns masked data' do line_items = reports_line_items.list expect(line_items.first.order.email).to eq('HIDDEN') end + + context "when filtering by product" do + subject(:line_items) { reports_line_items.list } + + let!(:line_item2) { create(:line_item, order: order) } + let!(:line_item3) { create(:line_item, order: order) } + let(:order_permissions) do + FakeOrderPermissions.new([line_item1, line_item2, line_item3], orders_relation) + end + let(:params) { { variant_id_in: [line_item3.variant.id, line_item1.variant.id] } } + + context "with an empty array" do + let(:params) { { variant_id_in: [""] } } + + it "does not filter" do + expect(line_items).to include(line_item1, line_item2, line_item3) + end + end + + it "includes selected products" do + expect(line_items).to include(line_item1, line_item3) + expect(line_items).not_to include(line_item2) + end + end end end From 196b674bf13ec0e95d9e461273e81baa9d3f7cbd Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Fri, 21 Jul 2023 13:52:05 +1000 Subject: [PATCH 4/5] Add system spec to test filtering by product --- .../reports/orders_and_fulfillment_spec.rb | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/spec/system/admin/reports/orders_and_fulfillment_spec.rb b/spec/system/admin/reports/orders_and_fulfillment_spec.rb index 9a6642405a..05d44ca27a 100644 --- a/spec/system/admin/reports/orders_and_fulfillment_spec.rb +++ b/spec/system/admin/reports/orders_and_fulfillment_spec.rb @@ -176,6 +176,36 @@ describe "Orders And Fulfillment" do expect(rows[3]).to have_content "Ave Zebu" end end + + context "When filtering by product" do + let(:variant1) { create(:variant, product: product, unit_description: "Big") } + let(:variant3) { create(:variant)} + + before do + create(:line_item_with_shipment, variant: variant1, quantity: 1, order: order1) + create(:line_item_with_shipment, variant: variant2, quantity: 3, order: order1) + create(:line_item_with_shipment, variant: variant1, quantity: 2, order: order2) + create(:line_item_with_shipment, variant: variant3, quantity: 1, order: order2) + end + + it "includes only selected product" do + tomselect_search_and_select(variant3.sku, from: "variant_id_in[]") + click_button 'Go' + + rows = find("table.report__table").all("tbody tr") + table = rows.map { |r| r.all("td").map { |c| c.text.strip } } + expect(table).to have_content(variant3.product.name) + expect(table).not_to have_content(product.name) + + # Check the product dropdown still show the selected product + selected_product = page + .find("[name='variant_id_in[]']") + .sibling(".ts-wrapper") + .first(".ts-control") + .first(".item") + expect(selected_product.text).to have_content(variant3.product.name) + end + end end describe "Order Cycle Supplier" do @@ -508,6 +538,7 @@ describe "Orders And Fulfillment" do expect(page).to have_checked_field('Summary Row') end end + context "Columns to show" do it "should store columns to show for every report separately" do # Step 1: Update report rendering options on two reports @@ -542,6 +573,7 @@ describe "Orders And Fulfillment" do end end end + context "Revisiting a report after logout" do context "Display options" do it "should store display options" do From f9afc0ba96856cd28d2c1720f4ad19781c363206 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Fri, 21 Jul 2023 14:21:57 +1000 Subject: [PATCH 5/5] Fix rubocop warning --- app/controllers/admin/reports_controller.rb | 2 +- lib/reporting/line_items.rb | 4 ++-- spec/system/admin/reports/orders_and_fulfillment_spec.rb | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/admin/reports_controller.rb b/app/controllers/admin/reports_controller.rb index cd66e3d8c2..ac3adec7f8 100644 --- a/app/controllers/admin/reports_controller.rb +++ b/app/controllers/admin/reports_controller.rb @@ -56,7 +56,7 @@ module Admin @rendering_options = rendering_options @data = Reporting::FrontendData.new(spree_current_user) - variant_id_in = params[:variant_id_in]&.reject(&:blank?) + variant_id_in = params[:variant_id_in]&.compact_blank load_selected_variant if variant_id_in.present? end diff --git a/lib/reporting/line_items.rb b/lib/reporting/line_items.rb index 87afa58a64..a949e02dce 100644 --- a/lib/reporting/line_items.rb +++ b/lib/reporting/line_items.rb @@ -24,9 +24,9 @@ module Reporting end # Filter by product - variant_id_in = @params[:variant_id_in].reject(&:blank?) + variant_id_in = @params[:variant_id_in]&.compact_blank if variant_id_in.present? - line_items = line_items.where("spree_line_items.variant_id": variant_id_in) + line_items = line_items.where('spree_line_items.variant_id': variant_id_in) end if line_item_includes.present? diff --git a/spec/system/admin/reports/orders_and_fulfillment_spec.rb b/spec/system/admin/reports/orders_and_fulfillment_spec.rb index 05d44ca27a..97dcc5cc11 100644 --- a/spec/system/admin/reports/orders_and_fulfillment_spec.rb +++ b/spec/system/admin/reports/orders_and_fulfillment_spec.rb @@ -179,7 +179,7 @@ describe "Orders And Fulfillment" do context "When filtering by product" do let(:variant1) { create(:variant, product: product, unit_description: "Big") } - let(:variant3) { create(:variant)} + let(:variant3) { create(:variant) } before do create(:line_item_with_shipment, variant: variant1, quantity: 1, order: order1)