diff --git a/app/components/searchable_dropdown_component/searchable_dropdown_component.html.haml b/app/components/searchable_dropdown_component/searchable_dropdown_component.html.haml index 85529d4c89..50d574d360 100644 --- a/app/components/searchable_dropdown_component/searchable_dropdown_component.html.haml +++ b/app/components/searchable_dropdown_component/searchable_dropdown_component.html.haml @@ -1,4 +1,4 @@ - if uses_form_builder? = f.select name, options, { selected: selected_option, include_blank:, multiple: }, class: classes, data:, 'aria-label': aria_label, **other_attrs - else - = select_tag name, options_for_select(options, selected_option), include_blank: include_blank, multiple: multiple, class: classes, data: data, 'aria-label': aria_label, **other_attrs + = select_tag name, options_for_select(options, selected_option), include_blank:, multiple:, class: classes, data:, 'aria-label': aria_label, **other_attrs diff --git a/app/controllers/concerns/reports/ajax_search.rb b/app/controllers/concerns/reports/ajax_search.rb index 7194dc77a7..03df572a48 100644 --- a/app/controllers/concerns/reports/ajax_search.rb +++ b/app/controllers/concerns/reports/ajax_search.rb @@ -62,13 +62,16 @@ module Reports search_term = params[:q] return query if search_term.blank? + escaped_search_term = ActiveRecord::Base.sanitize_sql_like(search_term) + pattern = "%#{escaped_search_term}%" + # Handle different model types if query.model == OrderCycle - query.where("order_cycles.name ILIKE ?", "%#{search_term}%") + query.where("order_cycles.name ILIKE ?", pattern) elsif query.model == Customer - query.where("customers.email ILIKE ?", "%#{search_term}%") + query.where("customers.email ILIKE ?", pattern) else - query.where("name ILIKE ?", "%#{search_term}%") + query.where("name ILIKE ?", pattern) end end diff --git a/app/models/spree/ability.rb b/app/models/spree/ability.rb index 654b76a781..4695559945 100644 --- a/app/models/spree/ability.rb +++ b/app/models/spree/ability.rb @@ -6,6 +6,11 @@ module Spree class Ability include CanCan::Ability + REPORTS_SEARCH_ACTIONS = [ + :search_enterprise_fees, :search_enterprise_fee_owners, :search_distributors, + :search_suppliers, :search_order_cycles, :search_order_customers + ].freeze + def initialize(user) clear_aliased_actions @@ -260,8 +265,7 @@ module Spree can [:admin, :index, :import], ::Admin::DfcProductImportsController # Reports page - can [:admin, :index, :show, :create, :search_enterprise_fees, :search_enterprise_fee_owners, - :search_distributors, :search_suppliers, :search_order_cycles, :search_order_customers], + can [:admin, :index, :show, :create, *REPORTS_SEARCH_ACTIONS], ::Admin::ReportsController can [:admin, :show, :create, :customers, :orders_and_distributors, :group_buys, :payments, :orders_and_fulfillment, :products_and_inventory, :order_cycle_management, @@ -394,7 +398,7 @@ module Spree end # Reports page - can [:admin, :index, :show, :create], ::Admin::ReportsController + can [:admin, :index, :show, :create, *REPORTS_SEARCH_ACTIONS], ::Admin::ReportsController can [:admin, :customers, :group_buys, :sales_tax, :payments, :orders_and_distributors, :orders_and_fulfillment, :products_and_inventory, :order_cycle_management, :xero_invoices, :enterprise_fee_summary, :bulk_coop], :report diff --git a/app/webpacker/controllers/tom_select_controller.js b/app/webpacker/controllers/tom_select_controller.js index e64bb60939..fcc6b72e02 100644 --- a/app/webpacker/controllers/tom_select_controller.js +++ b/app/webpacker/controllers/tom_select_controller.js @@ -1,5 +1,6 @@ import { Controller } from "stimulus"; import TomSelect from "tom-select/dist/esm/tom-select.complete"; +import showHttpError from "../../webpacker/js/services/show_http_error"; export default class extends Controller { static values = { @@ -51,7 +52,13 @@ export default class extends Controller { const url = this.control.getUrl(query); fetch(url) - .then((response) => response.json()) + .then((response) => { + if (!response.ok) { + showHttpError(response.status); + throw response; + } + return response.json(); + }) .then((json) => { /** * Expected API shape: @@ -69,12 +76,15 @@ export default class extends Controller { callback(json.results || []); }) - .catch(() => { + .catch((error) => { callback(); + console.error(error); }); } #addRemoteOptions(options) { + this.openedByClick = false; + options.firstUrl = (query) => { return this.#buildUrl(query, 1); }; @@ -85,6 +95,19 @@ export default class extends Controller { this.control.load("", () => {}); }.bind(this); + options.onDropdownOpen = function () { + this.openedByClick = true; + }.bind(this); + + options.onType = function () { + this.openedByClick = false; + }.bind(this); + + // As per TomSelect source code, no result feedback after API call is shown when this callback returns true. + options.shouldLoad = function (query) { + return this.openedByClick || query.length > 0; + }.bind(this); + options.valueField = "value"; options.labelField = "label"; options.searchField = "label"; diff --git a/spec/controllers/admin/reports_controller_spec.rb b/spec/controllers/admin/reports_controller_spec.rb index f38f6b837e..6fbf75f98e 100644 --- a/spec/controllers/admin/reports_controller_spec.rb +++ b/spec/controllers/admin/reports_controller_spec.rb @@ -368,117 +368,4 @@ RSpec.describe Admin::ReportsController do end end end - - context "AJAX Search" do - let(:enterprise_fee1) { - create(:enterprise_fee, name: "Delivery Fee", enterprise: distributor1) - } - let(:enterprise_fee2) { create(:enterprise_fee, name: "Admin Fee", enterprise: distributor2) } - - before do - controller_login_as_admin - orderA1.adjustments.create!( - originator: enterprise_fee1, - label: "Delivery Fee", - amount: 5.0, - state: "finalized", - order: orderA1 - ) - orderB1.adjustments.create!( - originator: enterprise_fee2, - label: "Admin Fee", - amount: 3.0, - state: "finalized", - order: orderB1 - ) - end - - describe "#search_enterprise_fees" do - it "returns paginated JSON with enterprise fees ordered by name" do - spree_get( - :search_enterprise_fees, - report_type: :enterprise_fee_summary, - report_subtype: :enterprise_fees_with_tax_report_by_order - ) - - expect(response).to have_http_status(:ok) - json_response = response.parsed_body - - names = json_response["results"].pluck("label") - expect(names).to eq(['Admin Fee', 'Delivery Fee']) - end - end - - describe "#search_enterprise_fee_owners" do - it "returns paginated JSON with unique enterprise owners ordered by name" do - distributor1.update!(name: "Zebra Farm") - distributor2.update!(name: "Alpha Market") - - spree_get( - :search_enterprise_fee_owners, - report_type: :enterprise_fee_summary, - report_subtype: :enterprise_fees_with_tax_report_by_order - ) - - expect(response).to have_http_status(:ok) - json_response = response.parsed_body - - names = json_response["results"].pluck("label") - expect(names).to eq(['Alpha Market', 'Zebra Farm']) - end - end - - describe "#search_order_customers" do - it "filters customers by email and returns paginated results" do - customer1 = create(:customer, email: "alice@example.com", enterprise: distributor1) - customer2 = create(:customer, email: "bob@example.com", enterprise: distributor1) - orderA1.update!(customer: customer1) - orderA2.update!(customer: customer2) - - spree_get( - :search_order_customers, - report_type: :enterprise_fee_summary, - report_subtype: :enterprise_fees_with_tax_report_by_order, - q: "alice" - ) - - json_response = response.parsed_body - expect(json_response["results"].pluck("label")).to eq(["alice@example.com"]) - end - end - - describe "#search_order_cycles" do - it "filters order cycles by name and orders by close date" do - ocA.update!(name: "Winter Market") - ocB.update!(name: "Summer Market") - - spree_get( - :search_order_cycles, - report_type: :enterprise_fee_summary, - report_subtype: :enterprise_fees_with_tax_report_by_order, - q: "Winter" - ) - - json_response = response.parsed_body - expect(json_response["results"].pluck("label")).to eq(["Winter Market"]) - end - end - - describe "#search_distributors" do - it "filters distributors by name" do - distributor1.update!(name: "Alpha Farm") - distributor2.update!(name: "Beta Market") - - spree_get( - :search_distributors, - report_type: :enterprise_fee_summary, - report_subtype: :enterprise_fees_with_tax_report_by_order, - q: "Alpha" - ) - - json_response = response.parsed_body - expect(json_response["results"].pluck("label")).to eq(["Alpha Farm"]) - end - end - end end diff --git a/spec/javascripts/stimulus/tom_select_controller_test.js b/spec/javascripts/stimulus/tom_select_controller_test.js index cba45dfede..f853e6dc94 100644 --- a/spec/javascripts/stimulus/tom_select_controller_test.js +++ b/spec/javascripts/stimulus/tom_select_controller_test.js @@ -5,6 +5,12 @@ import { Application } from "stimulus"; import { fireEvent, waitFor } from "@testing-library/dom"; import tom_select_controller from "controllers/tom_select_controller"; +import showHttpError from "js/services/show_http_error"; + +jest.mock("js/services/show_http_error", () => ({ + __esModule: true, + default: jest.fn(), +})); /* ------------------------------------------------------------------ * Helpers @@ -28,6 +34,7 @@ const openDropdown = () => fireEvent.click(document.getElementById("select-ts-co const mockRemoteFetch = (...responses) => { responses.forEach((response) => { fetch.mockResolvedValueOnce({ + ok: true, json: async () => response, }); }); @@ -68,10 +75,11 @@ const expectDropdownToContain = (text) => { expect(document.querySelector(".ts-dropdown-content")?.textContent).toContain(text); }; -const expectEmptyDropdown = () => { - expect(document.querySelector(".ts-dropdown-content")?.textContent).toBe(""); +const expectDropdownWithNoResults = () => { + expect(document.querySelector(".ts-dropdown-content")?.textContent).toBe("No results found"); }; + /* ------------------------------------------------------------------ * Specs * ------------------------------------------------------------------ */ @@ -86,6 +94,7 @@ describe("TomSelectController", () => { beforeEach(() => { global.fetch = jest.fn(); + global.I18n = { t: (key) => key }; }); afterEach(() => { @@ -240,13 +249,49 @@ describe("TomSelectController", () => { }); it("handles fetch errors gracefully", async () => { - fetch.mockRejectedValueOnce(new Error("Network error")); + fetch.mockRejectedValueOnce(new Error("Fetch error")); openDropdown(); await waitFor(() => { - expectEmptyDropdown(); + expectDropdownWithNoResults(); }); + + expect(showHttpError).not.toHaveBeenCalled(); + }); + + it("displays HTTP error on failure", async () => { + fetch.mockResolvedValueOnce({ + ok: false, + status: 500, + json: async () => ({}), + }); + + openDropdown(); + + await waitFor(() => { + expect(showHttpError).toHaveBeenCalledWith(500); + }); + + expectDropdownWithNoResults(); + }); + + it("controls loading behavior based on user interaction", () => { + const settings = getTomSelect().settings; + + // Initial state: openedByClick is false, query is empty + expect(settings.shouldLoad("")).toBe(false); + + // Simulating opening the dropdown + settings.onDropdownOpen(); + expect(settings.shouldLoad("")).toBe(true); + + // Simulating typing + settings.onType(); + expect(settings.shouldLoad("")).toBe(false); + + // Query present + expect(settings.shouldLoad("a")).toBe(true); }); }); }); diff --git a/spec/requests/admin/reports_ajax_api_spec.rb b/spec/requests/admin/reports_ajax_api_spec.rb new file mode 100644 index 0000000000..37654fb18e --- /dev/null +++ b/spec/requests/admin/reports_ajax_api_spec.rb @@ -0,0 +1,271 @@ +# frozen_string_literal: true + +RSpec.describe "Admin Reports AJAX Search API" do + let(:bill_address) { create(:address) } + let(:ship_address) { create(:address) } + let(:instructions) { "pick up on thursday please" } + let(:coordinator1) { create(:distributor_enterprise) } + let(:supplier1) { create(:supplier_enterprise) } + let(:supplier2) { create(:supplier_enterprise) } + let(:supplier3) { create(:supplier_enterprise) } + let(:distributor1) { create(:distributor_enterprise) } + let(:distributor2) { create(:distributor_enterprise) } + let(:product1) { create(:product, price: 12.34, supplier_id: supplier1.id) } + let(:product2) { create(:product, price: 23.45, supplier_id: supplier2.id) } + let(:product3) { create(:product, price: 34.56, supplier_id: supplier3.id) } + + let(:ocA) { + create(:simple_order_cycle, coordinator: coordinator1, + distributors: [distributor1, distributor2], + suppliers: [supplier1, supplier2, supplier3], + variants: [product1.variants.first, product3.variants.first]) + } + let(:ocB) { + create(:simple_order_cycle, coordinator: coordinator1, + distributors: [distributor1, distributor2], + suppliers: [supplier1, supplier2, supplier3], + variants: [product2.variants.first]) + } + + let(:orderA1) do + order = create(:order, distributor: distributor1, bill_address:, + ship_address:, special_instructions: instructions, + order_cycle: ocA) + order.line_items << create(:line_item, variant: product1.variants.first) + order.line_items << create(:line_item, variant: product3.variants.first) + order.finalize! + order.save + order + end + + let(:orderA2) do + order = create(:order, distributor: distributor2, bill_address:, + ship_address:, special_instructions: instructions, + order_cycle: ocA) + order.line_items << create(:line_item, variant: product2.variants.first) + order.finalize! + order.save + order + end + + let(:orderB1) do + order = create(:order, distributor: distributor1, bill_address:, + ship_address:, special_instructions: instructions, + order_cycle: ocB) + order.line_items << create(:line_item, variant: product1.variants.first) + order.line_items << create(:line_item, variant: product3.variants.first) + order.finalize! + order.save + order + end + + context "AJAX Search" do + let(:enterprise_fee1) { + create(:enterprise_fee, name: "Delivery Fee", enterprise: distributor1) + } + let(:enterprise_fee2) { create(:enterprise_fee, name: "Admin Fee", enterprise: distributor2) } + + before do + login_as create(:admin_user) + orderA1.adjustments.create!( + originator: enterprise_fee1, + label: "Delivery Fee", + amount: 5.0, + state: "finalized", + order: orderA1 + ) + orderB1.adjustments.create!( + originator: enterprise_fee2, + label: "Admin Fee", + amount: 3.0, + state: "finalized", + order: orderB1 + ) + end + + describe "GET /admin/reports/search_enterprise_fees" do + it "returns paginated JSON with enterprise fees ordered by name" do + get "/admin/reports/search_enterprise_fees", params: { + report_type: :enterprise_fee_summary, + report_subtype: :enterprise_fees_with_tax_report_by_order + } + + expect(response).to have_http_status(:ok) + json_response = response.parsed_body + + names = json_response["results"].pluck("label") + expect(names).to eq(['Admin Fee', 'Delivery Fee']) + expect(json_response["pagination"]["more"]).to be false + end + + it "paginates results and sets more flag correctly with more than 30 records" do + create_list(:enterprise_fee, 35, enterprise: distributor1) do |fee, i| + index = (i + 1).to_s.rjust(2, "0") + fee.update!(name: "Fee #{index}") + orderA1.adjustments.create!( + originator: fee, + label: "Fee #{index}", + amount: 1.0, + state: "finalized", + order: orderA1 + ) + end + + get "/admin/reports/search_enterprise_fees", params: { + report_type: :enterprise_fee_summary, + report_subtype: :enterprise_fees_with_tax_report_by_order, + page: 1 + } + + json_response = response.parsed_body + expect(json_response["results"].length).to eq(30) + expect(json_response["pagination"]["more"]).to be true + + get "/admin/reports/search_enterprise_fees", params: { + report_type: :enterprise_fee_summary, + report_subtype: :enterprise_fees_with_tax_report_by_order, + page: 2 + } + + json_response = response.parsed_body + expect(json_response["results"].length).to eq(7) + expect(json_response["pagination"]["more"]).to be false + end + end + + describe "GET /admin/reports/search_enterprise_fee_owners" do + it "returns paginated JSON with unique enterprise owners ordered by name" do + distributor1.update!(name: "Zebra Farm") + distributor2.update!(name: "Alpha Market") + + get "/admin/reports/search_enterprise_fee_owners", params: { + report_type: :enterprise_fee_summary, + report_subtype: :enterprise_fees_with_tax_report_by_order + } + + expect(response).to have_http_status(:ok) + json_response = response.parsed_body + + names = json_response["results"].pluck("label") + expect(names).to eq(['Alpha Market', 'Zebra Farm']) + expect(json_response["pagination"]["more"]).to be false + end + end + + describe "GET /admin/reports/search_order_customers" do + it "filters customers by email and returns paginated results" do + customer1 = create(:customer, email: "alice@example.com", enterprise: distributor1) + customer2 = create(:customer, email: "bob@example.com", enterprise: distributor1) + orderA1.update!(customer: customer1) + orderA2.update!(customer: customer2) + + get "/admin/reports/search_order_customers", params: { + report_type: :enterprise_fee_summary, + report_subtype: :enterprise_fees_with_tax_report_by_order, + q: "alice" + } + + json_response = response.parsed_body + expect(json_response["results"].pluck("label")).to eq(["alice@example.com"]) + expect(json_response["pagination"]["more"]).to be false + end + + it "paginates customers and sets more flag correctly with more than 30 records" do + create_list(:customer, 35, enterprise: distributor1) do |customer, i| + customer.update!( + email: "customer#{(i + 1).to_s.rjust(2, '0')}@example.com" + ) + order = create( + :order, + distributor: distributor1, + order_cycle: ocA, + customer: customer + ) + order.line_items << create( + :line_item, + variant: product1.variants.first + ) + order.finalize! + end + + get "/admin/reports/search_order_customers", params: { + report_type: :enterprise_fee_summary, + report_subtype: :enterprise_fees_with_tax_report_by_order, + page: 1 + } + + json_response = response.parsed_body + expect(json_response["results"].length).to eq(30) + expect(json_response["pagination"]["more"]).to be true + + get "/admin/reports/search_order_customers", params: { + report_type: :enterprise_fee_summary, + report_subtype: :enterprise_fees_with_tax_report_by_order, + page: 2 + } + + json_response = response.parsed_body + expect(json_response["results"].length).to eq(5) + expect(json_response["pagination"]["more"]).to be false + end + end + + describe "GET /admin/reports/search_order_cycles" do + it "filters order cycles by name and orders by close date" do + ocA.update!(name: "Winter Market") + ocB.update!(name: "Summer Market") + + get "/admin/reports/search_order_cycles", params: { + report_type: :enterprise_fee_summary, + report_subtype: :enterprise_fees_with_tax_report_by_order, + q: "Winter" + } + + json_response = response.parsed_body + expect(json_response["results"].pluck("label")).to eq(["Winter Market"]) + expect(json_response["pagination"]["more"]).to be false + end + end + + describe "GET /admin/reports/search_distributors" do + it "filters distributors by name" do + distributor1.update!(name: "Alpha Farm") + distributor2.update!(name: "Beta Market") + + get "/admin/reports/search_distributors", params: { + report_type: :enterprise_fee_summary, + report_subtype: :enterprise_fees_with_tax_report_by_order, + q: "Alpha" + } + + json_response = response.parsed_body + expect(json_response["results"].pluck("label")).to eq(["Alpha Farm"]) + expect(json_response["pagination"]["more"]).to be false + end + + it "paginates distributors and sets more flag correctly with more than 30 records" do + create_list(:distributor_enterprise, 35) + + get "/admin/reports/search_distributors", params: { + report_type: :enterprise_fee_summary, + report_subtype: :enterprise_fees_with_tax_report_by_order, + page: 1 + } + + json_response = response.parsed_body + expect(json_response["results"].length).to eq(30) + expect(json_response["pagination"]["more"]).to be true + + get "/admin/reports/search_distributors", params: { + report_type: :enterprise_fee_summary, + report_subtype: :enterprise_fees_with_tax_report_by_order, + page: 2 + } + + json_response = response.parsed_body + expect(json_response["results"].length).to be > 0 + expect(json_response["pagination"]["more"]).to be false + end + end + end +end