raggedstaff/openfoodnetwork
1
0
Fork 0
mirror of https://github.com/openfoodfoundation/openfoodnetwork synced 2026-04-03 06:59:14 +00:00
Code Issues Packages Projects Releases Wiki Activity

Move index and bulk_update actions to good ol' HTTP requests

Browse Source 
We've found that we just can't rely in StimulusReflex (and the underlying WebSockets stack) to guarantee a response to a request.
Because of this, there was intermittent issues when the server was overloaded with large requests, and the response never arrived, leaving an infinite loader, and a poor user wondering if anything was still happening.
This commit is contained in:
David Cook
2024-04-02 17:08:44 +11:00
committed by Filipe
parent dadabcf8ad
commit 8696e05e66
9 changed files with 164 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

130
app/controllers/admin/products_v3_controller.rb
View File

@@ -2,6 +2,134 @@
module Admin
class ProductsV3Controller < Spree::Admin::BaseController
def index; end
before_action :init_filters_params
before_action :init_pagination_params
def index
fetch_products
render "index", locals: { producers:, categories:, flash: }
end
def bulk_update
product_set = product_set_from_params
product_set.collection.each { |p| authorize! :update, p }
@products = product_set.collection # use instance variable mainly for testing
if product_set.save
flash[:success] = I18n.t('admin.products_v3.bulk_update.success')
redirect_to [:index, { page: @page, per_page: @per_page }]
elsif product_set.errors.present?
@error_counts = { saved: product_set.saved_count, invalid: product_set.invalid.count }
render "index", locals: { producers:, categories:, flash: }
end
end
def index_url
"/admin/products" # todo: fix routing so this can be automatically generated
end
private
def init_filters_params
# params comes from the form
# _params comes from the url
# priority is given to params from the form (if present) over url params
@search_term = params[:search_term] || params[:_search_term]
@producer_id = params[:producer_id] || params[:_producer_id]
@category_id = params[:category_id] || params[:_category_id]
end
def init_pagination_params
# prority is given to element dataset (if present) over url params
@page = params[:_page] || 1
@per_page = params[:_per_page] || 15
end
def producers
producers = OpenFoodNetwork::Permissions.new(spree_current_user)
.managed_product_enterprises.is_primary_producer.by_name
producers.map { |p| [p.name, p.id] }
end
def categories
Spree::Taxon.order(:name).map { |c| [c.name, c.id] }
end
def fetch_products
product_query = OpenFoodNetwork::Permissions.new(spree_current_user)
.editable_products.merge(product_scope).ransack(ransack_query).result
@pagy, @products = pagy(product_query.order(:name), items: @per_page, page: @page,
size: [1, 2, 2, 1])
end
def product_scope
user = spree_current_user
scope = if user.has_spree_role?("admin") || user.enterprises.present?
Spree::Product
else
Spree::Product.active
end
scope.includes(product_query_includes)
end
def ransack_query
query = {}
query.merge!(supplier_id_in: @producer_id) if @producer_id.present?
if @search_term.present?
query.merge!(Spree::Variant::SEARCH_KEY => @search_term)
end
query.merge!(primary_taxon_id_in: @category_id) if @category_id.present?
query
end
# Optimise by pre-loading required columns
def product_query_includes
# TODO: add other fields used in columns? (eg supplier: [:name])
[
# variants: [
# :default_price,
# :stock_locations,
# :stock_items,
# :variant_overrides
# ]
]
end
# Similar to spree/admin/products_controller
def product_set_from_params
# Form field names:
# '[products][0][id]' (hidden field)
# '[products][0][name]'
# '[products][0][variants_attributes][0][id]' (hidden field)
# '[products][0][variants_attributes][0][display_name]'
#
# Resulting in params:
# "products" => {
# "0" => {
# "id" => "123"
# "name" => "Pommes",
# "variants_attributes" => {
# "0" => {
# "id" => "1234",
# "display_name" => "Large box",
# }
# }
# }
collection_hash = products_bulk_params[:products]
.transform_values { |product|
# Convert variants_attributes form hash to an array if present
product[:variants_attributes] &&= product[:variants_attributes].values
product
}.with_indifferent_access
Sets::ProductSet.new(collection_attributes: collection_hash)
end
def products_bulk_params
params.permit(products: ::PermittedAttributes::Product.attributes)
.to_h.with_indifferent_access
end
end
end

21
app/reflexes/products_reflex.rb
View File

@@ -16,12 +16,6 @@ class ProductsReflex < ApplicationReflex
fetch_and_render_products_with_flash
end
def filter
@page = 1
fetch_and_render_products_with_flash
end
def clear_search
@search_term = nil
@producer_id = nil
@@ -31,21 +25,6 @@ class ProductsReflex < ApplicationReflex
fetch_and_render_products_with_flash
end
def bulk_update
product_set = product_set_from_params
product_set.collection.each { |p| authorize! :update, p }
@products = product_set.collection # use instance variable mainly for testing
if product_set.save
flash[:success] = I18n.t('admin.products_v3.bulk_update.success')
elsif product_set.errors.present?
@error_counts = { saved: product_set.saved_count, invalid: product_set.invalid.count }
end
render_products_form_with_flash
end
def delete_product
id = current_id_from_element(element)
product = product_finder(id).find_product

4
app/views/admin/products_v3/_filters.html.haml
View File

@@ -1,4 +1,4 @@
%form{ id: "filters", 'data-reflex-serialize-form': true, 'data-reflex': 'submit->products#filter' }
%form{ id: "filters" }
.query
.search-input
= text_field_tag :search_term, search_term, placeholder: t('.search_products')
@@ -15,4 +15,4 @@
data: { "controller": "tom-select", 'tom-select-placeholder-value': t('.search_for_categories')}
.submit
.search-button
= button_tag t(".search"), class: "secondary icon-search relaxed"
= button_tag t(".search"), class: "secondary icon-search relaxed", name: nil

2
app/views/admin/products_v3/_sort.html.haml
View File

@@ -2,7 +2,7 @@
%div
= t(".pagination.total_html", total: pagy.count, from: pagy.from, to: pagy.to)
- if search_term.present? || producer_id.present? || category_id.present?
%a{ href: "#", class: "button disruptive", data: { reflex: "click->products#clear_search" } }
%a{ href: "#", class: "button disruptive" }
= t(".pagination.clear_search")
%form.with-dropdown
= t(".pagination.per_page.show")

19
app/views/admin/products_v3/_table.html.haml
View File

@@ -1,8 +1,7 @@
= form_with url: bulk_update_admin_products_path, method: :patch, id: "products-form",
= form_with url: bulk_update_admin_products_path, method: :post, id: "products-form",
builder: BulkFormBuilder,
html: { data: { reflex: 'submit->products#bulk_update', 'reflex-serialize-form': true,
controller: "bulk-form", 'bulk-form-disable-selector-value': "#sort,#filters",
'bulk-form-error-value': defined?(error_counts),
html: { data: { controller: "bulk-form", 'bulk-form-disable-selector-value': "#sort,#filters",
'bulk-form-error-value': defined?(@error_counts),
} } do |form|
= render(partial: "admin/shared/flashes", locals: { flashes: }) if defined? flashes
%table.products
@@ -23,20 +22,20 @@
%tr
%td.form-actions-wrapper{ colspan: 12 }
.form-actions-wrapper2
%fieldset.form-actions{ class: ("hidden" unless defined?(error_counts)), 'data-bulk-form-target': "actions" }
%fieldset.form-actions{ class: ("hidden" unless defined?(@error_counts)), 'data-bulk-form-target': "actions" }
.container
.status
.modified_summary{ 'data-bulk-form-target': "changedSummary", 'data-translation-key': 'admin.products_v3.table.changed_summary'}
- if defined?(error_counts)
- if defined?(@error_counts)
.error_summary
- if error_counts[:saved] > 0
- if @error_counts[:saved] > 0
-# X products were saved correctly, but Y products could not be saved correctly. Please review errors and try again
= t('.error_summary.saved', count: error_counts[:saved]) + t('.error_summary.invalid', count: error_counts[:invalid])
= t('.error_summary.saved', count: @error_counts[:saved]) + t('.error_summary.invalid', count: @error_counts[:invalid])
- else
-# Y products could not be saved correctly. Please review errors and try again
= t('.error_summary.invalid', count: error_counts[:invalid])
= t('.error_summary.invalid', count: @error_counts[:invalid])
.form-buttons
= form.submit t('.reset'), type: :reset, class: "medium", 'data-reflex': 'click->products#fetch'
= form.submit t('.reset'), type: :reset, class: "medium"
= form.submit t('.save'), class: "medium"
%tr
%th.align-left= # image

9
app/views/admin/products_v3/index.html.haml
View File

@@ -14,12 +14,15 @@
= render partial: 'spree/admin/shared/product_sub_menu'
#products_v3_page{ "data-controller": "products" }
.spinner-overlay{ "data-controller": "loading", "data-products-target": "loading" }
.spinner-overlay{ "data-controller": "loading", "data-products-target": "loading", class: "hidden" }
.spinner-container
.spinner
= t('.loading')
#products-content
= 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,
flashes: flash }
- %w[product variant].each do |object_type|
= render partial: 'delete_modal', locals: { object_type: }
#modal-component

2
app/webpacker/controllers/products_controller.js
View File

@@ -6,8 +6,6 @@ export default class extends ApplicationController {
connect() {
super.connect();
// Fetch the products on page load
this.stimulate("Products#fetch");
}
beforeReflex() {

6
config/routes/admin.rb
View File

@@ -70,9 +70,9 @@ Openfoodnetwork::Application.routes.draw do
resources :dfc_product_imports, only: [:index]
constraints FeatureToggleConstraint.new(:admin_style_v3) do
resources :products, to: 'products_v3#index', only: :index do
patch :bulk_update, on: :collection
end
# This might be easier to arrange once we rename the controller to plain old "products"
post '/products/bulk_update', to: 'products_v3#bulk_update'
get '/products', to: 'products_v3#index'
end
resources :variant_overrides do

19
spec/system/admin/products_v3/products_spec.rb
View File

@@ -383,6 +383,8 @@ describe 'As an admin, I can manage products', feature: :admin_style_v3 do
let!(:product_b) { create(:simple_product, name: "Bananas") }
before do
visit admin_products_url
within row_containing_name("Apples") do
fill_in "Name", with: ""
fill_in "SKU", with: "A" * 256
@@ -580,6 +582,8 @@ describe 'As an admin, I can manage products', feature: :admin_style_v3 do
let!(:product_b) { create(:simple_product, name: "Bananas") }
before do
visit admin_products_url
within row_containing_name("Apples") do
fill_in "Name", with: ""
fill_in "SKU", with: "A" * 256
@@ -741,11 +745,11 @@ describe 'As an admin, I can manage products', feature: :admin_style_v3 do
"tr:has(input[aria-label=Price][value='#{product_a.price}'])"
}
before do
visit admin_products_url
end
describe "Actions columns (delete)" do
before do
visit admin_products_url
end
it "shows an actions menu with a delete link when clicking on icon for product. " \
"doesn't show delete link for the single variant" do
within product_selector do
@@ -761,13 +765,14 @@ describe 'As an admin, I can manage products', feature: :admin_style_v3 do
end
end
it "shows an actions menu with a delete link when clicking on icon for variant" \
it "shows an actions menu with a delete link when clicking on icon for variant " \
"if have multiple" do
create(:variant,
product: product_a,
display_name: "Medium box",
sku: "APL-01",
price: 5.25)
visit admin_products_url
# to select the default variant
within default_variant_selector do
@@ -796,6 +801,8 @@ describe 'As an admin, I can manage products', feature: :admin_style_v3 do
context "when 'keep product/variant' is selected" do
it 'should not delete the product/variant' do
visit admin_products_url
# Keep Product
within product_selector do
page.find(".vertical-ellipsis-menu").click
@@ -828,6 +835,7 @@ describe 'As an admin, I can manage products', feature: :admin_style_v3 do
let(:success_flash_message_selector) { "div.flash.success" }
let(:error_flash_message_selector) { "div.flash.error" }
it 'should successfully delete the product/variant' do
visit admin_products_url
# Delete Variant
within variant_selector do
page.find(".vertical-ellipsis-menu").click
@@ -867,6 +875,7 @@ describe 'As an admin, I can manage products', feature: :admin_style_v3 do
end
it 'should be failed to delete the product/variant' do
visit admin_products_url
allow_any_instance_of(Spree::Product).to receive(:destroy).and_return(false)
allow_any_instance_of(Spree::Variant).to receive(:destroy).and_return(false)