diff --git a/app/controllers/admin/products_v3_controller.rb b/app/controllers/admin/products_v3_controller.rb index ede851965d..7a00d77144 100644 --- a/app/controllers/admin/products_v3_controller.rb +++ b/app/controllers/admin/products_v3_controller.rb @@ -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 diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index e8d7634b7c..66a5ca17e7 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -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 diff --git a/app/views/admin/products_v3/_filters.html.haml b/app/views/admin/products_v3/_filters.html.haml index e413f481d3..c8b173aea7 100644 --- a/app/views/admin/products_v3/_filters.html.haml +++ b/app/views/admin/products_v3/_filters.html.haml @@ -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 diff --git a/app/views/admin/products_v3/_sort.html.haml b/app/views/admin/products_v3/_sort.html.haml index b89d926cef..f0de4b76d4 100644 --- a/app/views/admin/products_v3/_sort.html.haml +++ b/app/views/admin/products_v3/_sort.html.haml @@ -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") diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml index 750746ddc3..6b85adaf05 100644 --- a/app/views/admin/products_v3/_table.html.haml +++ b/app/views/admin/products_v3/_table.html.haml @@ -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 diff --git a/app/views/admin/products_v3/index.html.haml b/app/views/admin/products_v3/index.html.haml index f6d3be9f81..2ac344c476 100644 --- a/app/views/admin/products_v3/index.html.haml +++ b/app/views/admin/products_v3/index.html.haml @@ -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 diff --git a/app/webpacker/controllers/products_controller.js b/app/webpacker/controllers/products_controller.js index 6fe8662f50..9ec3ac1a86 100644 --- a/app/webpacker/controllers/products_controller.js +++ b/app/webpacker/controllers/products_controller.js @@ -6,8 +6,6 @@ export default class extends ApplicationController { connect() { super.connect(); - // Fetch the products on page load - this.stimulate("Products#fetch"); } beforeReflex() { diff --git a/config/routes/admin.rb b/config/routes/admin.rb index 8d0a9f6705..1a7ca8c3cc 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -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 diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb index df24a79483..14e1765036 100644 --- a/spec/system/admin/products_v3/products_spec.rb +++ b/spec/system/admin/products_v3/products_spec.rb @@ -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)