Merge pull request #14040 from chahmedejaz/task/13797-improve-performance-of-products-page

Fix Admin Bulk Products screen performance issue
This commit is contained in:
Ahmed Ejaz
2026-04-01 00:37:40 +05:00
committed by GitHub
15 changed files with 491 additions and 102 deletions

View File

@@ -0,0 +1,56 @@
# frozen_string_literal: true
module Admin
class AjaxSearchController < Spree::Admin::BaseController
def producers
query = OpenFoodNetwork::Permissions.new(spree_current_user)
.managed_product_enterprises.is_primary_producer.by_name
render json: build_search_response(query)
end
def categories
query = Spree::Taxon.all
render json: build_search_response(query)
end
def tax_categories
query = Spree::TaxCategory.all
render json: build_search_response(query)
end
private
def build_search_response(query)
page = (params[:page] || 1).to_i
per_page = 30
filtered_query = apply_search_filter(query)
total_count = filtered_query.size
items = paginated_items(filtered_query, page, per_page)
results = format_results(items)
{ results: results, pagination: { more: (page * per_page) < total_count } }
end
def apply_search_filter(query)
search_term = params[:q]
return query if search_term.blank?
escaped_search_term = ActiveRecord::Base.sanitize_sql_like(search_term)
pattern = "%#{escaped_search_term}%"
query.where('name ILIKE ?', pattern)
end
def paginated_items(query, page, per_page)
query.order(:name).offset((page - 1) * per_page).limit(per_page).pluck(:name, :id)
end
def format_results(items)
items.map { |label, value| { value:, label: } }
end
end
end

View File

@@ -52,5 +52,15 @@ module Admin
@allowed_source_producers ||= OpenFoodNetwork::Permissions.new(spree_current_user)
.enterprises_granting_linked_variants
end
# Query only name of the model to avoid loading the whole record
def selected_option(id, model)
return [] unless id
name = model.where(id: id).pick(:name)
return [] unless name
[[name, id]]
end
end
end

View File

@@ -225,9 +225,17 @@ module Spree
OpenFoodNetwork::Permissions.new(user).
enterprises_granting_linked_variants.include? variant.supplier
end
can [
:admin,
:index,
:bulk_update,
:destroy,
:destroy_variant,
:clone,
:create_linked_variant
], :products_v3
can [:admin, :index, :bulk_update, :destroy, :destroy_variant, :clone,
:create_linked_variant], :products_v3
can [:admin, :producers, :categories, :tax_categories], :ajax_search
can [:create], Spree::Variant
can [:admin, :index, :read, :edit,

View File

@@ -9,14 +9,22 @@
- if producer_options.many?
.producers
= label_tag :producer_id, t('.producers.label')
= select_tag :producer_id, options_for_select(producer_options, producer_id),
include_blank: t('.all_producers'), class: "fullwidth",
data: { "controller": "tom-select", 'tom-select-placeholder-value': t('.search_for_producers')}
= render(SearchableDropdownComponent.new(name: :producer_id,
aria_label: t('.producers.label'),
options: selected_option(producer_id, Enterprise),
selected_option: producer_id,
remote_url: admin_ajax_search_producers_url,
include_blank: t('.all_producers'),
placeholder_value: t('.search_for_producers')))
.categories
= label_tag :category_id, t('.categories.label')
= select_tag :category_id, options_for_select(category_options, category_id),
include_blank: t('.all_categories'), class: "fullwidth",
data: { "controller": "tom-select", 'tom-select-placeholder-value': t('.search_for_categories')}
= render(SearchableDropdownComponent.new(name: :category_id,
aria_label: t('.categories.label'),
options: selected_option(category_id, Spree::Taxon),
selected_option: category_id,
remote_url: admin_ajax_search_categories_url,
include_blank: t('.all_categories'),
placeholder_value: t('.search_for_categories')))
-if variant_tag_enabled?(spree_current_user)
.tags
= label_tag :tags_name_in, t('.tags.label')

View File

@@ -59,27 +59,28 @@
= render(SearchableDropdownComponent.new(form: f,
name: :supplier_id,
aria_label: t('.producer_field_name'),
options: producer_options,
options: variant.supplier_id ? [[variant.supplier.name, variant.supplier_id]] : [],
selected_option: variant.supplier_id,
include_blank: t('admin.products_v3.filters.select_producer'),
remote_url: admin_ajax_search_producers_url,
placeholder_value: t('admin.products_v3.filters.select_producer')))
= error_message_on variant, :supplier
%td.col-category.field.naked_inputs
= render(SearchableDropdownComponent.new(form: f,
name: :primary_taxon_id,
options: category_options,
options: variant.primary_taxon_id ? [[variant.primary_taxon.name, variant.primary_taxon_id]] : [],
selected_option: variant.primary_taxon_id,
aria_label: t('.category_field_name'),
include_blank: t('admin.products_v3.filters.select_category'),
remote_url: admin_ajax_search_categories_url,
placeholder_value: t('admin.products_v3.filters.select_category')))
= error_message_on variant, :primary_taxon
%td.col-tax_category.field.naked_inputs
= render(SearchableDropdownComponent.new(form: f,
name: :tax_category_id,
options: tax_category_options,
options: variant.tax_category_id ? [[variant.tax_category.name, variant.tax_category_id]] : [],
selected_option: variant.tax_category_id,
include_blank: t('.none_tax_category'),
aria_label: t('.tax_category_field_name'),
include_blank: t('.none_tax_category'),
remote_url: admin_ajax_search_tax_categories_url,
placeholder_value: t('.search_for_tax_categories')))
= error_message_on variant, :tax_category
- if variant_tag_enabled?(spree_current_user)

View File

@@ -83,6 +83,9 @@ export default class extends Controller {
}
#addRemoteOptions(options) {
// by default, for dropdown_input plugin, it's true. Otherwise for multi-select it's false
// it should always be true so to invoke the onDropdownOpen to fetch options
options.shouldOpen = true;
this.openedByClick = false;
options.firstUrl = (query) => {
@@ -91,12 +94,9 @@ export default class extends Controller {
options.load = this.#fetchOptions.bind(this);
options.onFocus = function () {
this.control.load("", () => {});
}.bind(this);
options.onDropdownOpen = function () {
this.openedByClick = true;
this.control.load("", () => {});
}.bind(this);
options.onType = function () {

View File

@@ -83,6 +83,13 @@ Openfoodnetwork::Application.routes.draw do
delete 'products_v3/destroy_variant/:id', to: 'products_v3#destroy_variant', as: 'destroy_variant'
post 'clone/:id', to: 'products_v3#clone', as: 'clone_product'
post 'products/create_linked_variant', to: 'products_v3#create_linked_variant', as: 'create_linked_variant'
scope :ajax_search, as: :ajax_search, controller: :ajax_search do
get :producers
get :categories
get :tax_categories
end
resources :product_preview, only: [:show]
resources :variant_overrides do

View File

@@ -166,7 +166,6 @@ describe("TomSelectController", () => {
expect(settings.searchField).toBe("label");
expect(settings.load).toEqual(expect.any(Function));
expect(settings.firstUrl).toEqual(expect.any(Function));
expect(settings.onFocus).toEqual(expect.any(Function));
});
it("fetches page 1 on focus", async () => {

View File

@@ -0,0 +1,285 @@
# frozen_string_literal: true
RSpec.describe "/admin/ajax_search" do
include AuthenticationHelper
let(:admin_user) { create(:admin_user) }
let(:regular_user) { create(:user) }
describe "GET /admin/ajax_search/producers" do
context "when user is not logged in" do
it "redirects to login" do
get admin_ajax_search_producers_path
expect(response).to redirect_to %r|#/login$|
end
end
context "when user is logged in without permissions" do
before { login_as regular_user }
it "redirects to unauthorized" do
get admin_ajax_search_producers_path
expect(response).to redirect_to('/unauthorized')
end
end
context "when user is an admin" do
before { login_as admin_user }
let!(:producer1) { create(:supplier_enterprise, name: "Apple Farm") }
let!(:producer2) { create(:supplier_enterprise, name: "Berry Farm") }
let!(:producer3) { create(:supplier_enterprise, name: "Cherry Orchard") }
let!(:distributor) { create(:distributor_enterprise, name: "Distributor") }
it "returns producers sorted alphabetically by name" do
get admin_ajax_search_producers_path
expect(response).to have_http_status(:ok)
json_response = response.parsed_body
expect(json_response["results"].pluck("label")).to eq(['Apple Farm', 'Berry Farm',
'Cherry Orchard'])
expect(json_response["pagination"]["more"]).to be false
end
it "filters producers by search query" do
get admin_ajax_search_producers_path, params: { q: "berry" }
json_response = response.parsed_body
expect(json_response["results"].pluck("label")).to eq(['Berry Farm'])
expect(json_response["results"].pluck("value")).to eq([producer2.id])
end
it "filters are case insensitive" do
get admin_ajax_search_producers_path, params: { q: "BERRY" }
json_response = response.parsed_body
expect(json_response["results"].pluck("label")).to eq(['Berry Farm'])
end
it "filters with partial matches" do
get admin_ajax_search_producers_path, params: { q: "Farm" }
json_response = response.parsed_body
expect(json_response["results"].pluck("label")).to eq(['Apple Farm', 'Berry Farm'])
end
it "excludes non-producer enterprises" do
get admin_ajax_search_producers_path
json_response = response.parsed_body
expect(json_response["results"].pluck("label")).not_to include('Distributor')
end
context "with more than 30 producers" do
before do
create_list(:supplier_enterprise, 35) do |enterprise, i|
enterprise.update!(name: "Producer #{(i + 1).to_s.rjust(2, '0')}")
end
end
it "returns first page with 30 results and more flag as true" do
get admin_ajax_search_producers_path, params: { page: 1 }
json_response = response.parsed_body
expect(json_response["results"].length).to eq(30)
expect(json_response["pagination"]["more"]).to be true
end
it "returns remaining results on second page with more flag as false" do
get admin_ajax_search_producers_path, params: { page: 2 }
json_response = response.parsed_body
expect(json_response["results"].length).to eq(8)
expect(json_response["pagination"]["more"]).to be false
end
end
end
context "when user has enterprise permissions" do
let!(:my_producer) { create(:supplier_enterprise, name: "My Producer") }
let!(:other_producer) { create(:supplier_enterprise, name: "Other Producer") }
let(:user_with_producer) { create(:user, enterprises: [my_producer]) }
before { login_as user_with_producer }
it "returns only managed producers" do
get admin_ajax_search_producers_path
json_response = response.parsed_body
expect(json_response["results"].pluck("label")).to eq(['My Producer'])
expect(json_response["results"].pluck("label")).not_to include('Other Producer')
end
end
end
describe "GET /admin/ajax_search/categories" do
context "when user is not logged in" do
it "redirects to login" do
get admin_ajax_search_categories_path
expect(response).to redirect_to %r|#/login$|
end
end
context "when user is logged in without permissions" do
before { login_as regular_user }
it "redirects to unauthorized" do
get admin_ajax_search_categories_path
expect(response).to redirect_to('/unauthorized')
end
end
context "when user is an admin" do
before { login_as admin_user }
let!(:category1) { create(:taxon, name: "Vegetables") }
let!(:category2) { create(:taxon, name: "Fruits") }
let!(:category3) { create(:taxon, name: "Dairy") }
it "returns categories sorted alphabetically by name" do
get admin_ajax_search_categories_path
expect(response).to have_http_status(:ok)
json_response = response.parsed_body
expect(json_response["results"].pluck("label")).to eq(['Dairy', 'Fruits', 'Vegetables'])
expect(json_response["pagination"]["more"]).to be false
end
it "filters categories by search query" do
get admin_ajax_search_categories_path, params: { q: "fruit" }
json_response = response.parsed_body
expect(json_response["results"].pluck("label")).to eq(['Fruits'])
expect(json_response["results"].pluck("value")).to eq([category2.id])
end
it "filters are case insensitive" do
get admin_ajax_search_categories_path, params: { q: "VEGETABLES" }
json_response = response.parsed_body
expect(json_response["results"].pluck("label")).to eq(['Vegetables'])
end
it "filters with partial matches" do
get admin_ajax_search_categories_path, params: { q: "ege" }
json_response = response.parsed_body
expect(json_response["results"].pluck("label")).to eq(['Vegetables'])
end
context "with more than 30 categories" do
before do
create_list(:taxon, 35) do |taxon, i|
taxon.update!(name: "Category #{(i + 1).to_s.rjust(2, '0')}")
end
end
it "returns first page with 30 results and more flag as true" do
get admin_ajax_search_categories_path, params: { page: 1 }
json_response = response.parsed_body
expect(json_response["results"].length).to eq(30)
expect(json_response["pagination"]["more"]).to be true
end
it "returns remaining results on second page with more flag as false" do
get admin_ajax_search_categories_path, params: { page: 2 }
json_response = response.parsed_body
expect(json_response["results"].length).to eq(8)
expect(json_response["pagination"]["more"]).to be false
end
end
end
end
describe "GET /admin/ajax_search/tax_categories" do
context "when user is not logged in" do
it "redirects to login" do
get admin_ajax_search_tax_categories_path
expect(response).to redirect_to %r|#/login$|
end
end
context "when user is logged in without permissions" do
before { login_as regular_user }
it "redirects to unauthorized" do
get admin_ajax_search_tax_categories_path
expect(response).to redirect_to('/unauthorized')
end
end
context "when user is an admin" do
before { login_as admin_user }
let!(:tax_cat1) { create(:tax_category, name: "GST") }
let!(:tax_cat2) { create(:tax_category, name: "VAT") }
let!(:tax_cat3) { create(:tax_category, name: "No Tax") }
it "returns tax categories sorted alphabetically by name" do
get admin_ajax_search_tax_categories_path
expect(response).to have_http_status(:ok)
json_response = response.parsed_body
expect(json_response["results"].pluck("label")).to eq(['GST', 'No Tax', 'VAT'])
expect(json_response["pagination"]["more"]).to be false
end
it "filters tax categories by search query" do
get admin_ajax_search_tax_categories_path, params: { q: "vat" }
json_response = response.parsed_body
expect(json_response["results"].pluck("label")).to eq(['VAT'])
expect(json_response["results"].pluck("value")).to eq([tax_cat2.id])
end
it "filters are case insensitive" do
get admin_ajax_search_tax_categories_path, params: { q: "GST" }
json_response = response.parsed_body
expect(json_response["results"].pluck("label")).to eq(['GST'])
end
it "filters with partial matches" do
get admin_ajax_search_tax_categories_path, params: { q: "tax" }
json_response = response.parsed_body
expect(json_response["results"].pluck("label")).to eq(['No Tax'])
end
context "with more than 30 tax categories" do
before do
create_list(:tax_category, 35) do |tax_cat, i|
tax_cat.update!(name: "Tax Category #{(i + 1).to_s.rjust(2, '0')}")
end
end
it "returns first page with 30 results and more flag as true" do
get admin_ajax_search_tax_categories_path, params: { page: 1 }
json_response = response.parsed_body
expect(json_response["results"].length).to eq(30)
expect(json_response["pagination"]["more"]).to be true
end
it "returns remaining results on second page with more flag as false" do
get admin_ajax_search_tax_categories_path, params: { page: 2 }
json_response = response.parsed_body
expect(json_response["results"].length).to eq(8)
expect(json_response["pagination"]["more"]).to be false
end
end
end
end
end

View File

@@ -19,12 +19,25 @@ module TomSelectHelper
tomselect_wrapper.find(:css, '.ts-dropdown div.create').click
end
# Searches for and selects an option in a TomSelect dropdown with search functionality.
# @param value [String] The text to search for and select from the dropdown
# @param options [Hash] Configuration options
# @option options [String] :from The name/id of the select field
# @option options [Boolean] :remote_search If true, waits for search loading after interactions
#
# @example
# tomselect_search_and_select("Apple", from: "fruit_selector")
# tomselect_search_and_select("California", from: "state", remote_search: true)
def tomselect_search_and_select(value, options)
tomselect_wrapper = page.find_field(options[:from]).sibling(".ts-wrapper")
tomselect_wrapper.find(".ts-control").click
expect_tomselect_loading_completion(tomselect_wrapper, options)
# Use send_keys as setting the value directly doesn't trigger the search
tomselect_wrapper.find(".ts-dropdown input.dropdown-input").send_keys(value)
tomselect_wrapper.find(".ts-dropdown .ts-dropdown-content .option.active", text: value).click
expect_tomselect_loading_completion(tomselect_wrapper, options)
tomselect_wrapper.find(".ts-dropdown .ts-dropdown-content .option", text: value).click
end
def tomselect_select(value, options)
@@ -64,4 +77,52 @@ module TomSelectHelper
end
end
end
# Validates both available options and selected options in a TomSelect dropdown.
# @param from [String] The name/id of the select field
# @param existing_options [Array<String>] List of options that should be available in the dropdown
# @param selected_options [Array<String>] List of options that should currently be selected
#
# @example
# expect_tomselect_existing_with_selected_options(
# from: "category_selector",
# existing_options: ["Fruit", "Vegetables", "Dairy"],
# selected_options: ["Fruit"]
# )
def expect_tomselect_existing_with_selected_options(from:, existing_options:, selected_options:)
tomselect_wrapper = page.find_field(from).sibling(".ts-wrapper")
tomselect_control = tomselect_wrapper.find('.ts-control')
tomselect_control.click # open the dropdown (would work for remote vs non-remote dropdowns)
# validate existing options are present in the dropdown
within(tomselect_wrapper) do
existing_options.each do |option|
expect(page).to have_css(
".ts-dropdown .ts-dropdown-content .option",
text: option
)
end
end
# validate selected options are selected in the dropdown
within(tomselect_wrapper) do
selected_options.each do |option|
expect(page).to have_css(
"div[data-ts-item]",
text: option
)
end
end
# close the dropdown by clicking on the already selected option
tomselect_wrapper.find(".ts-dropdown .ts-dropdown-content .option.active").click
end
def expect_tomselect_loading_completion(tomselect_wrapper, options)
return unless options[:remote_search]
expect(tomselect_wrapper).to have_css(".spinner")
expect(tomselect_wrapper).not_to have_css(".spinner")
end
end

View File

@@ -15,10 +15,6 @@ RSpec.describe 'As an enterprise user, I can perform actions on the products scr
login_as user
end
let(:producer_search_selector) { 'input[placeholder="Select producer"]' }
let(:categories_search_selector) { 'input[placeholder="Select category"]' }
let(:tax_categories_search_selector) { 'input[placeholder="Search for tax categories"]' }
describe "column selector" do
let!(:product) { create(:simple_product) }
@@ -102,54 +98,7 @@ RSpec.describe 'As an enterprise user, I can perform actions on the products scr
}
let!(:product_a) { create(:simple_product, name: "Apples", sku: "APL-00") }
context "when they are under 11" do
before do
create_list(:supplier_enterprise, 9, users: [user])
create_list(:tax_category, 9)
create_list(:taxon, 2)
visit admin_products_url
end
it "should not display search input, change the producers, category and tax category" do
producer_to_select = random_producer(variant_a1)
category_to_select = random_category(variant_a1)
tax_category_to_select = random_tax_category
within row_containing_name(variant_a1.display_name) do
validate_tomselect_without_search!(
page, "Producer",
producer_search_selector
)
tomselect_select(producer_to_select, from: "Producer")
end
within row_containing_name(variant_a1.display_name) do
validate_tomselect_without_search!(
page, "Category",
categories_search_selector
)
tomselect_select(category_to_select, from: "Category")
validate_tomselect_without_search!(
page, "Tax Category",
tax_categories_search_selector
)
tomselect_select(tax_category_to_select, from: "Tax Category")
end
click_button "Save changes"
expect(page).to have_content "Changes saved"
variant_a1.reload
expect(variant_a1.supplier.name).to eq(producer_to_select)
expect(variant_a1.primary_taxon.name).to eq(category_to_select)
expect(variant_a1.tax_category.name).to eq(tax_category_to_select)
end
end
context "when they are over 11" do
context "when there are products" do
before do
create_list(:supplier_enterprise, 11, users: [user])
create_list(:tax_category, 11)
@@ -167,9 +116,13 @@ RSpec.describe 'As an enterprise user, I can perform actions on the products scr
tax_category_to_select = random_tax_category
within row_containing_name(variant_a1.display_name) do
tomselect_search_and_select(producer_to_select, from: "Producer")
tomselect_search_and_select(category_to_select, from: "Category")
tomselect_search_and_select(tax_category_to_select, from: "Tax Category")
tomselect_search_and_select(producer_to_select, from: "Producer", remote_search: true)
tomselect_search_and_select(category_to_select, from: "Category", remote_search: true)
tomselect_search_and_select(
tax_category_to_select,
from: "Tax Category",
remote_search: true
)
end
click_button "Save changes"

View File

@@ -86,14 +86,14 @@ RSpec.describe 'As an enterprise user, I can manage my products' do
find('button[aria-label="On Hand"]').click
find('input[id$="_price"]').fill_in with: "11.1"
select supplier.name, from: 'Producer'
select taxon.name, from: 'Category'
if stock == "on_hand"
find('input[id$="_on_hand_desired"]').fill_in with: "66"
elsif stock == "on_demand"
find('input[id$="_on_demand_desired"]').check
end
tomselect_select supplier.name, from: 'Producer'
tomselect_select taxon.name, from: 'Category'
end
expect(page).to have_content "1 product modified."

View File

@@ -106,13 +106,19 @@ RSpec.describe 'As an enterprise user, I can browse my products' do
visit spree.admin_products_path
within row_containing_name "Variant1" do
expect(page).to have_select "Producer", with_options: ["Producer A", "Producer B"],
selected: "Producer A"
expect_tomselect_existing_with_selected_options(
from: 'Producer',
existing_options: ["Producer A", "Producer B"],
selected_options: ["Producer A"]
)
end
within row_containing_name "Variant2a" do
expect(page).to have_select "Producer", with_options: ["Producer A", "Producer B"],
selected: "Producer B"
expect_tomselect_existing_with_selected_options(
from: 'Producer',
existing_options: ["Producer A", "Producer B"],
selected_options: ["Producer B"]
)
end
end
end
@@ -543,24 +549,21 @@ RSpec.describe 'As an enterprise user, I can browse my products' do
it "shows only suppliers that I manage or have permission to" do
visit spree.admin_products_path
existing_options = [supplier_managed1.name, supplier_managed2.name, supplier_permitted.name]
within row_containing_placeholder(product_supplied.name) do
expect(page).to have_select(
'_products_0_variants_attributes_0_supplier_id',
options: [
'Select producer',
supplier_managed1.name, supplier_managed2.name, supplier_permitted.name
], selected: supplier_managed1.name
expect_tomselect_existing_with_selected_options(
existing_options:,
from: '_products_0_variants_attributes_0_supplier_id',
selected_options: [supplier_managed1.name]
)
end
within row_containing_placeholder(product_supplied_permitted.name) do
expect(page).to have_select(
'_products_1_variants_attributes_0_supplier_id',
options: [
'Select producer',
supplier_managed1.name, supplier_managed2.name, supplier_permitted.name
], selected: supplier_permitted.name
expect_tomselect_existing_with_selected_options(
existing_options:,
from: '_products_1_variants_attributes_0_supplier_id',
selected_options: [supplier_permitted.name]
)
end
end

View File

@@ -350,8 +350,8 @@ RSpec.describe 'As an enterprise user, I can update my products' do
click_on "On Hand" # activate popout
fill_in "On Hand", with: "3"
select producer.name, from: 'Producer'
select taxon.name, from: 'Category'
tomselect_select producer.name, from: 'Producer'
tomselect_select taxon.name, from: 'Category'
end
expect {
@@ -586,8 +586,8 @@ RSpec.describe 'As an enterprise user, I can update my products' do
fill_in "Name", with: "Nice box"
fill_in "SKU", with: "APL-02"
select producer.name, from: 'Producer'
select taxon.name, from: 'Category'
tomselect_select producer.name, from: 'Producer'
tomselect_select taxon.name, from: 'Category'
end
expect {

View File

@@ -17,7 +17,7 @@ RSpec.describe "admin/products_v3/_filters.html.haml" do
end
let(:spree_current_user) { build(:enterprise_user) }
it "shows the producer filter when there are options" do
it "shows the producer filter with the default option initially" do
allow(view).to receive_messages locals.merge(
producer_options: [
["Ada's Apples", 1],
@@ -27,9 +27,7 @@ RSpec.describe "admin/products_v3/_filters.html.haml" do
is_expected.to have_content "Producers"
is_expected.to have_select "producer_id", options: [
"All producers",
"Ada's Apples",
"Ben's Bananas",
"All producers"
], selected: nil
end