From 1580d539dfe3e962034ec66de70b140d30a3f776 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Wed, 11 Sep 2024 00:56:52 +0500 Subject: [PATCH 1/5] 11200: coniditonally hide producer column --- app/helpers/admin/products_helper.rb | 4 ++++ app/models/column_preference.rb | 12 +++++++++++- app/models/spree/user.rb | 1 + app/views/admin/products_v3/_table.html.haml | 2 +- lib/open_food_network/column_preference_defaults.rb | 13 +++++++++++-- 5 files changed, 28 insertions(+), 4 deletions(-) diff --git a/app/helpers/admin/products_helper.rb b/app/helpers/admin/products_helper.rb index 4686e4720a1..d3b06806ba3 100644 --- a/app/helpers/admin/products_helper.rb +++ b/app/helpers/admin/products_helper.rb @@ -37,5 +37,9 @@ def products_return_to_url(url_filters) "#{admin_products_path}#{url_filters.empty? ? '' : "#?#{url_filters.to_query}"}" end + + def hide_producer_column?(producer_options) + spree_current_user.column_preferences.products.empty? && producer_options.one? + end end end diff --git a/app/models/column_preference.rb b/app/models/column_preference.rb index 3e86f364402..093d2d5f30b 100644 --- a/app/models/column_preference.rb +++ b/app/models/column_preference.rb @@ -15,10 +15,11 @@ class ColumnPreference < ApplicationRecord validates :column_name, presence: true, inclusion: { in: proc { |p| valid_columns_for(p.action_name) } } + scope :products, -> { where(action_name: 'products_v3_index') } def self.for(user, action_name) stored_preferences = where(user_id: user.id, action_name:) - default_preferences = __send__("#{action_name}_columns") + default_preferences = get_default_preferences(action_name, user) filter(default_preferences, user, action_name) default_preferences.each_with_object([]) do |(column_name, default_attributes), preferences| stored_preference = stored_preferences.find_by(column_name:) @@ -52,4 +53,13 @@ def self.filter(default_preferences, user, action_name) default_preferences.delete(:schedules) end + + def self.get_default_preferences(action_name, user) + case action_name + when 'products_v3_index' + products_v3_index_columns(user) + else + __send__("#{action_name}_columns") + end + end end diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb index bef76099337..9eae2b50744 100644 --- a/app/models/spree/user.rb +++ b/app/models/spree/user.rb @@ -42,6 +42,7 @@ class User < ApplicationRecord has_many :credit_cards, dependent: :destroy has_many :report_rendering_options, class_name: "::ReportRenderingOptions", dependent: :destroy has_many :webhook_endpoints, dependent: :destroy + has_many :column_preferences, dependent: :destroy has_one :oidc_account, dependent: :destroy accepts_nested_attributes_for :enterprise_roles, allow_destroy: true diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml index b45eabab72a..71e777ba13a 100644 --- a/app/views/admin/products_v3/_table.html.haml +++ b/app/views/admin/products_v3/_table.html.haml @@ -13,7 +13,7 @@ = hidden_field_tag :producer_id, @producer_id = hidden_field_tag :category_id, @category_id - %table.products{ 'data-column-preferences-target': "table" } + %table.products{ 'data-column-preferences-target': "table", class: (hide_producer_column?(producer_options) ? 'hide-producer' : '') } %colgroup -# The `min-width` property works in Chrome but not Firefox so is considered progressive enhancement. %col.col-image{ width:"44px" }= # (image size + padding) diff --git a/lib/open_food_network/column_preference_defaults.rb b/lib/open_food_network/column_preference_defaults.rb index f851efc7948..9da9ef36e9c 100644 --- a/lib/open_food_network/column_preference_defaults.rb +++ b/lib/open_food_network/column_preference_defaults.rb @@ -77,7 +77,9 @@ def products_index_columns } end - def products_v3_index_columns + def products_v3_index_columns(user) + producer_visibility = display_producer_column?(user) + I18n.with_options scope: 'admin.products_page.columns' do { image: { name: t(:image), visible: true }, @@ -87,7 +89,7 @@ def products_v3_index_columns unit_scale: { name: t(:unit_scale), visible: true }, price: { name: t(:price), visible: true }, on_hand: { name: t(:on_hand), visible: true }, - producer: { name: t(:producer), visible: true }, + producer: { name: t(:producer), visible: producer_visibility }, category: { name: t(:category), visible: true }, tax_category: { name: t(:tax_category), visible: true }, inherits_properties: { name: t(:inherits_properties), visible: true }, @@ -134,5 +136,12 @@ def subscriptions_index_columns shipping_method: { name: I18n.t("admin.shipping_method"), visible: false } } end + + def display_producer_column?(user) + producers = OpenFoodNetwork::Permissions.new(user) + .managed_product_enterprises.is_primary_producer + + producers.many? + end end end From f8d3467d46bbe7a148f5122fe7f488c242b96014 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Wed, 11 Sep 2024 01:59:43 +0500 Subject: [PATCH 2/5] 11200: add specs --- app/helpers/admin/products_helper.rb | 7 +- app/models/column_preference.rb | 2 +- .../_product_variant_row.html.haml | 2 +- spec/system/admin/products_v3/actions_spec.rb | 77 +++++++++++-------- spec/system/admin/products_v3/create_spec.rb | 1 + spec/system/admin/products_v3/index_spec.rb | 1 - spec/system/admin/products_v3/update_spec.rb | 3 +- 7 files changed, 57 insertions(+), 36 deletions(-) diff --git a/app/helpers/admin/products_helper.rb b/app/helpers/admin/products_helper.rb index d3b06806ba3..9b404f634b4 100644 --- a/app/helpers/admin/products_helper.rb +++ b/app/helpers/admin/products_helper.rb @@ -10,8 +10,11 @@ def product_image_form_path(product) end end - def prepare_new_variant(product) - product.variants.build + def prepare_new_variant(product, producer_options) + # e.g producer_options = [['producer name', id]] + product.variants.build do |new_variant| + new_variant.supplier_id = producer_options.first.second if producer_options.one? + end end def unit_value_with_description(variant) diff --git a/app/models/column_preference.rb b/app/models/column_preference.rb index 093d2d5f30b..73c59f22a56 100644 --- a/app/models/column_preference.rb +++ b/app/models/column_preference.rb @@ -37,7 +37,7 @@ def self.for(user, action_name) end def self.valid_columns_for(action_name) - __send__("#{action_name}_columns").keys.map(&:to_s) + get_default_preferences(action_name, Spree::User.new).keys.map(&:to_s) end def self.known_actions diff --git a/app/views/admin/products_v3/_product_variant_row.html.haml b/app/views/admin/products_v3/_product_variant_row.html.haml index b41e7c56b5f..12b6702cba3 100644 --- a/app/views/admin/products_v3/_product_variant_row.html.haml +++ b/app/views/admin/products_v3/_product_variant_row.html.haml @@ -11,7 +11,7 @@ %tr.condensed{ id: dom_id(variant), 'data-controller': "variant", 'class': "nested-form-wrapper", 'data-new-record': variant.new_record? ? "true" : false } = render partial: 'variant_row', locals: { variant:, f: variant_form, category_options:, tax_category_options:, producer_options: } - = form.fields_for("products][#{product_index}][variants_attributes][NEW_RECORD", prepare_new_variant(product)) do |new_variant_form| + = form.fields_for("products][#{product_index}][variants_attributes][NEW_RECORD", prepare_new_variant(product, producer_options)) do |new_variant_form| %template{ 'data-nested-form-target': "template" } %tr.condensed{ 'data-controller': "variant", 'class': "nested-form-wrapper", 'data-new-record': "true" } = render partial: 'variant_row', locals: { variant: new_variant_form.object, f: new_variant_form, category_options:, tax_category_options:, producer_options: } diff --git a/spec/system/admin/products_v3/actions_spec.rb b/spec/system/admin/products_v3/actions_spec.rb index e864218c1d7..41a8f0fc6f7 100644 --- a/spec/system/admin/products_v3/actions_spec.rb +++ b/spec/system/admin/products_v3/actions_spec.rb @@ -34,42 +34,59 @@ describe "column selector" do let!(:product) { create(:simple_product) } - before do - visit admin_products_url - end + context "with one producer only" do + before do + visit admin_products_url + end + + it "hides column and remembers saved preference" do + # Name shows by default + expect(page).to have_checked_field "Name" + expect(page).to have_selector "th", text: "Name" + expect_other_columns_visible - it "hides column and remembers saved preference" do - # Name shows by default - expect(page).to have_checked_field "Name" - expect(page).to have_selector "th", text: "Name" - expect_other_columns_visible + # Producer is hidden by if only one producer is present + expect(page).not_to have_checked_field "Producer" + expect(page).not_to have_selector "th", text: "Producer" - # Name is hidden - ofn_drop_down("Columns").click - within ofn_drop_down("Columns") do - uncheck "Name" + # Name is hidden + ofn_drop_down("Columns").click + within ofn_drop_down("Columns") do + uncheck "Name" + end + expect(page).not_to have_selector "th", text: "Name" + expect_other_columns_visible + + # Preference saved + click_on "Save as default" + expect(page).to have_content "Column preferences saved" + refresh + + # Preference remembered + ofn_drop_down("Columns").click + within ofn_drop_down("Columns") do + expect(page).to have_unchecked_field "Name" + end + expect(page).not_to have_selector "th", text: "Name" + expect_other_columns_visible end - expect(page).not_to have_selector "th", text: "Name" - expect_other_columns_visible - - # Preference saved - click_on "Save as default" - expect(page).to have_content "Column preferences saved" - refresh - - # Preference remembered - ofn_drop_down("Columns").click - within ofn_drop_down("Columns") do - expect(page).to have_unchecked_field "Name" + + def expect_other_columns_visible + expect(page).to have_selector "th", text: "Price" + expect(page).to have_selector "th", text: "On Hand" end - expect(page).not_to have_selector "th", text: "Name" - expect_other_columns_visible end - def expect_other_columns_visible - expect(page).to have_selector "th", text: "Producer" - expect(page).to have_selector "th", text: "Price" - expect(page).to have_selector "th", text: "On Hand" + context "with multiple producers" do + let!(:producer2) { create(:supplier_enterprise, owner: user) } + + before { visit admin_products_url } + + it "has selected producer column by default" do + # Producer shows by default + expect(page).to have_checked_field "Producer" + expect(page).to have_selector "th", text: "Producer" + end end end diff --git a/spec/system/admin/products_v3/create_spec.rb b/spec/system/admin/products_v3/create_spec.rb index ce3b1502cff..fbde2272693 100644 --- a/spec/system/admin/products_v3/create_spec.rb +++ b/spec/system/admin/products_v3/create_spec.rb @@ -7,6 +7,7 @@ include WebHelper let!(:supplier) { create(:supplier_enterprise) } + let!(:supplier2) { create(:supplier_enterprise) } let!(:taxon) { create(:taxon) } describe "creating a new product" do diff --git a/spec/system/admin/products_v3/index_spec.rb b/spec/system/admin/products_v3/index_spec.rb index 3d5f30c03f3..15990c34606 100644 --- a/spec/system/admin/products_v3/index_spec.rb +++ b/spec/system/admin/products_v3/index_spec.rb @@ -36,7 +36,6 @@ expect(page).to have_selector "th", text: "Unit" expect(page).to have_selector "th", text: "Price" expect(page).to have_selector "th", text: "On Hand" - expect(page).to have_selector "th", text: "Producer" expect(page).to have_selector "th", text: "Category" expect(page).to have_selector "th", text: "Tax Category" expect(page).to have_selector "th", text: "Inherits Properties?" diff --git a/spec/system/admin/products_v3/update_spec.rb b/spec/system/admin/products_v3/update_spec.rb index fec42361773..57245ecb6b4 100644 --- a/spec/system/admin/products_v3/update_spec.rb +++ b/spec/system/admin/products_v3/update_spec.rb @@ -9,7 +9,8 @@ include FileHelper let(:producer) { create(:supplier_enterprise) } - let(:user) { create(:user, enterprises: [producer]) } + let(:producer2) { create(:supplier_enterprise) } + let(:user) { create(:user, enterprises: [producer, producer2]) } before do login_as user From 76fdf3725a88d83f278b26a8683bcf4e21375cd7 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Wed, 11 Sep 2024 11:41:01 +0500 Subject: [PATCH 3/5] 11200: add explanations --- app/helpers/admin/products_helper.rb | 2 ++ spec/system/admin/products_v3/create_spec.rb | 2 ++ 2 files changed, 4 insertions(+) diff --git a/app/helpers/admin/products_helper.rb b/app/helpers/admin/products_helper.rb index 9b404f634b4..043fed3b5d8 100644 --- a/app/helpers/admin/products_helper.rb +++ b/app/helpers/admin/products_helper.rb @@ -41,6 +41,8 @@ def products_return_to_url(url_filters) "#{admin_products_path}#{url_filters.empty? ? '' : "#?#{url_filters.to_query}"}" end + # if user hasn't saved any preferences on products page and there's only one producer; + # we need to hide producer column def hide_producer_column?(producer_options) spree_current_user.column_preferences.products.empty? && producer_options.one? end diff --git a/spec/system/admin/products_v3/create_spec.rb b/spec/system/admin/products_v3/create_spec.rb index fbde2272693..3f44d09c368 100644 --- a/spec/system/admin/products_v3/create_spec.rb +++ b/spec/system/admin/products_v3/create_spec.rb @@ -7,6 +7,8 @@ include WebHelper let!(:supplier) { create(:supplier_enterprise) } + # Creating another producer such that producer column is visible + # otherwise on one producer, it's hidden by default let!(:supplier2) { create(:supplier_enterprise) } let!(:taxon) { create(:taxon) } From 5be53a40a97fcf156caf920553bf0df6638cde35 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Wed, 11 Sep 2024 11:54:38 +0500 Subject: [PATCH 4/5] 11200: rename products scope --- app/helpers/admin/products_helper.rb | 2 +- app/models/column_preference.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/helpers/admin/products_helper.rb b/app/helpers/admin/products_helper.rb index 043fed3b5d8..fe00c220022 100644 --- a/app/helpers/admin/products_helper.rb +++ b/app/helpers/admin/products_helper.rb @@ -44,7 +44,7 @@ def products_return_to_url(url_filters) # if user hasn't saved any preferences on products page and there's only one producer; # we need to hide producer column def hide_producer_column?(producer_options) - spree_current_user.column_preferences.products.empty? && producer_options.one? + spree_current_user.column_preferences.bulk_edit_product.empty? && producer_options.one? end end end diff --git a/app/models/column_preference.rb b/app/models/column_preference.rb index 73c59f22a56..04a35ebbe86 100644 --- a/app/models/column_preference.rb +++ b/app/models/column_preference.rb @@ -15,7 +15,7 @@ class ColumnPreference < ApplicationRecord validates :column_name, presence: true, inclusion: { in: proc { |p| valid_columns_for(p.action_name) } } - scope :products, -> { where(action_name: 'products_v3_index') } + scope :bulk_edit_product, -> { where(action_name: 'products_v3_index') } def self.for(user, action_name) stored_preferences = where(user_id: user.id, action_name:) From 243a4a55b435a4afef9383d17c4afc9a2e8e1b22 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Wed, 11 Sep 2024 12:03:49 +0500 Subject: [PATCH 5/5] 11200: add spec for display producer column --- spec/system/admin/products_v3/actions_spec.rb | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/spec/system/admin/products_v3/actions_spec.rb b/spec/system/admin/products_v3/actions_spec.rb index 41a8f0fc6f7..bdbc6cd8b6c 100644 --- a/spec/system/admin/products_v3/actions_spec.rb +++ b/spec/system/admin/products_v3/actions_spec.rb @@ -46,9 +46,19 @@ expect_other_columns_visible # Producer is hidden by if only one producer is present - expect(page).not_to have_checked_field "Producer" + expect(page).to have_unchecked_field "Producer" expect(page).not_to have_selector "th", text: "Producer" + # Show Producer column + ofn_drop_down("Columns").click + within ofn_drop_down("Columns") do + check "Producer" + end + + # Preference saved + save_preferences + expect(page).to have_selector "th", text: "Producer" + # Name is hidden ofn_drop_down("Columns").click within ofn_drop_down("Columns") do @@ -58,9 +68,7 @@ expect_other_columns_visible # Preference saved - click_on "Save as default" - expect(page).to have_content "Column preferences saved" - refresh + save_preferences # Preference remembered ofn_drop_down("Columns").click @@ -75,6 +83,13 @@ def expect_other_columns_visible expect(page).to have_selector "th", text: "Price" expect(page).to have_selector "th", text: "On Hand" end + + def save_preferences + # Preference saved + click_on "Save as default" + expect(page).to have_content "Column preferences saved" + refresh + end end context "with multiple producers" do