From b26152cf0e0c9a54aa7d904c16c0ecacf4ace22b Mon Sep 17 00:00:00 2001 From: David Cook Date: Tue, 24 Feb 2026 17:13:30 +1100 Subject: [PATCH] Only show option when you have permission Preload the allow list once in the controller. This controller was initially set up to avoid instance variables, and pass variables explicitly to the template. That's a good principle, but in practice we have a growing list of variables passed down the chain to multiple partials which is getting cumbersome. I think instance variables have their place after all. --- .../admin/products_v3_controller.rb | 6 +++++ .../_product_variant_row.html.haml | 1 + .../admin/products_v3/_variant_row.html.haml | 6 +++-- spec/system/admin/products_v3/actions_spec.rb | 9 +++---- spec/system/admin/products_v3/index_spec.rb | 26 +++++++++++++++++++ 5 files changed, 41 insertions(+), 7 deletions(-) diff --git a/app/controllers/admin/products_v3_controller.rb b/app/controllers/admin/products_v3_controller.rb index faf4836855..74f108dacc 100644 --- a/app/controllers/admin/products_v3_controller.rb +++ b/app/controllers/admin/products_v3_controller.rb @@ -8,6 +8,7 @@ module Admin before_action :init_filters_params before_action :init_pagination_params before_action :init_none_tag + before_action :fetch_data, include: [:index, :bulk_update] def index fetch_products @@ -209,6 +210,11 @@ module Admin ).distinct.order(:name).pluck(:name) end + def fetch_data + @allowed_source_producers = OpenFoodNetwork::Permissions.new(spree_current_user) + .enterprises_granting_sourced_variants + end + def fetch_products product_query = OpenFoodNetwork::Permissions.new(spree_current_user) .editable_products.merge(product_scope_with_includes).ransack(ransack_query).result 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 deb0c467d0..8c9cfd8895 100644 --- a/app/views/admin/products_v3/_product_variant_row.html.haml +++ b/app/views/admin/products_v3/_product_variant_row.html.haml @@ -11,6 +11,7 @@ -# Filter out variant a user has not permission to update, but keep variant with no supplier - next if variant.supplier.present? && !allowed_producers.include?(variant.supplier) + = form.fields_for("products][#{product_index}][variants_attributes", variant, index: variant_index) do |variant_form| %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, product_index:, category_options:, tax_category_options:, producer_options: } diff --git a/app/views/admin/products_v3/_variant_row.html.haml b/app/views/admin/products_v3/_variant_row.html.haml index e87803d34b..df56c92a2e 100644 --- a/app/views/admin/products_v3/_variant_row.html.haml +++ b/app/views/admin/products_v3/_variant_row.html.haml @@ -91,8 +91,10 @@ = render(VerticalEllipsisMenuComponent.new) do - if variant.persisted? = link_to t('admin.products_page.actions.edit'), edit_admin_product_variant_path(variant.product, variant) - / TODO: only show if have permission. need to load permissions efficiently please. maybe the PErmissions object can cache result for each enterprise. maybe we preload it with the product query. - = link_to t('admin.products_page.actions.create_sourced_variant'), admin_create_sourced_variant_path(variant_id: variant.id, product_index:), 'data-turbo-method': :post + + - if @allowed_source_producers.include?(variant.supplier) + = link_to t('admin.products_page.actions.create_sourced_variant'), admin_create_sourced_variant_path(variant_id: variant.id, product_index:), 'data-turbo-method': :post + - if variant.product.variants.size > 1 %a{ "data-controller": "modal-link", "data-action": "click->modal-link#setModalDataSetOnConfirm click->modal-link#open", "data-modal-link-target-value": "variant-delete-modal", "class": "delete", diff --git a/spec/system/admin/products_v3/actions_spec.rb b/spec/system/admin/products_v3/actions_spec.rb index 5170418283..a2d4f64262 100644 --- a/spec/system/admin/products_v3/actions_spec.rb +++ b/spec/system/admin/products_v3/actions_spec.rb @@ -286,16 +286,14 @@ RSpec.describe 'As an enterprise user, I can perform actions on the products scr permissions_list: [:manage_products]) } - before do - visit admin_products_url - end - context "with create_sourced_variants permission for my, and other's variants" do it "creates a sourced variant" do create(:enterprise_relationship, parent: producer, child: producer, permissions_list: [:create_sourced_variants]) enterprise_relationship.permissions.create! name: :create_sourced_variants + visit admin_products_url + # Check my own variant within row_containing_name("My box") do page.find(".vertical-ellipsis-menu").click @@ -323,7 +321,8 @@ RSpec.describe 'As an enterprise user, I can perform actions on the products scr context "without create_sourced_variants permission" do it "does not show the option in the menu" do - pending "TODO: hide option if you can't use it." + visit admin_products_url + within row_containing_name("My box") do page.find(".vertical-ellipsis-menu").click expect(page).not_to have_link "Create sourced variant" diff --git a/spec/system/admin/products_v3/index_spec.rb b/spec/system/admin/products_v3/index_spec.rb index e74517262f..d835b807c3 100644 --- a/spec/system/admin/products_v3/index_spec.rb +++ b/spec/system/admin/products_v3/index_spec.rb @@ -139,6 +139,32 @@ RSpec.describe 'As an enterprise user, I can browse my products' do expect(page).to have_select "variant_unit_with_scale", selected: "Items" expect(page).to have_field "variant_unit_name", with: "packet" end + + context "with sourced variant" do + let(:source_producer) { create(:supplier_enterprise) } + let(:p3) { create(:product, name: "Product3", supplier_id: source_producer.id) } + + let!(:v3_source) { p3.variants.first } + let!(:v3_sourced) { + create(:variant, display_name: "Variant3-sourced", product: p3, supplier: source_producer) + } + let!(:enterprise_relationship) { + # Other producer grants me access to manage their variant + create(:enterprise_relationship, parent: source_producer, child: producer, + permissions_list: [:manage_products]) + } + + before do + v3_sourced.source_variants << v3_source + visit admin_products_url + end + + it "shows sourced variant with indicator" do + within row_containing_name("Variant3-sourced") do + expect(page).to have_selector 'span[title*="Sourced from"]' + end + end + end end describe "sorting" do