Improve reports AJAX search safety, permissions, and TomSelect UX

- Sanitize AJAX search queries to safely support wildcard searches (ILIKE)
- Centralize reports search actions in Spree::Ability and reuse across roles
- Fix TomSelect remote loading to fetch on dropdown open and typing
- Surface HTTP errors in TomSelect via showHttpError and improve error handling
- Update dropdown behavior to show proper “no results” feedback
- Move reports AJAX specs to request specs and expand pagination coverage
- Simplify searchable dropdown component attribute passing

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

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
 

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

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";

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

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);
     });
   });
 });

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