diff --git a/app/controllers/admin/products_v3_controller.rb b/app/controllers/admin/products_v3_controller.rb index 6d1ae388cb..c503b5e899 100644 --- a/app/controllers/admin/products_v3_controller.rb +++ b/app/controllers/admin/products_v3_controller.rb @@ -11,7 +11,8 @@ module Admin def index fetch_products render "index", - locals: { producers:, categories:, tax_category_options:, available_tags:, flash: } + locals: { producer_options:, categories:, tax_category_options:, available_tags:, + flash:, allowed_producers: } session[:products_return_to_url] = request.url end @@ -32,7 +33,8 @@ module Admin render "index", status: :unprocessable_entity, locals: { - producers:, categories:, tax_category_options:, available_tags:, flash: + producer_options:, categories:, tax_category_options:, available_tags:, + allowed_producers:, flash: } end end @@ -78,27 +80,29 @@ module Admin end def clone - @product = Spree::Product.find(params[:id]) - authorize! :clone, @product + product = Spree::Product.find(params[:id]) + authorize! :clone, product status = :ok begin - @cloned_product = @product.duplicate + cloned_product = product.duplicate flash.now[:success] = t('.success') - @product_index = "-#{@cloned_product.id}" - @producer_options = producers - @category_options = categories - @tax_category_options = tax_category_options + product_index = "-#{cloned_product.id}" rescue ActiveRecord::ActiveRecordError => e flash.now[:error] = clone_error_message(e) status = :unprocessable_entity - @product_index = "-1" # Create a unique enough index + product_index = "-1" # Create a unique enough index end respond_with do |format| - format.turbo_stream { render :clone, status: } + format.turbo_stream { + render :clone, status:, + locals: { product:, cloned_product:, product_index:, producer_options:, + category_options: categories, tax_category_options:, + allowed_producers: } + } end end @@ -132,10 +136,13 @@ module Admin end end - def producers - producers = OpenFoodNetwork::Permissions.new(spree_current_user) + def allowed_producers + OpenFoodNetwork::Permissions.new(spree_current_user) .managed_product_enterprises.is_primary_producer.by_name - producers.map { |p| [p.name, p.id] } + end + + def producer_options + allowed_producers.map { |p| [p.name, p.id] } end def categories diff --git a/app/models/spree/ability.rb b/app/models/spree/ability.rb index 155f611d56..63833e309f 100644 --- a/app/models/spree/ability.rb +++ b/app/models/spree/ability.rb @@ -196,12 +196,15 @@ module Spree def add_product_management_abilities(user) # Enterprise User can only access products that they are a supplier for can [:create], Spree::Product + # An enterperprise user can change a product if they are supplier of at least + # one of the product's associated variants can [:admin, :read, :index, :update, :seo, :group_buy_options, :bulk_update, :clone, :delete, :destroy], Spree::Product do |product| - OpenFoodNetwork::Permissions.new(user).managed_product_enterprises.include?( - product.variants.first.supplier + variant_suppliers = product.variants.map(&:supplier) + OpenFoodNetwork::Permissions.new(user).managed_product_enterprises.intersect?( + variant_suppliers ) end diff --git a/app/views/admin/products_v3/_content.html.haml b/app/views/admin/products_v3/_content.html.haml index 118b470e17..3a84d074c0 100644 --- a/app/views/admin/products_v3/_content.html.haml +++ b/app/views/admin/products_v3/_content.html.haml @@ -1,3 +1,4 @@ +-# locals: (products:, pagy:, search_term:, producer_options:, producer_id:, category_options:, category_id:, tax_category_options:, available_tags:, tags:, flashes:, display_search_filter:, allowed_producers:) %turbo-frame#products-content{ target: "_top", refresh: "morph" } .spinner-overlay{ "data-controller": "loading", "data-products-target": "loading", class: "hidden" } .spinner-container @@ -17,7 +18,7 @@ .container.results .sixteen.columns = render partial: 'sort', locals: { pagy:, search_term:, producer_id:, category_id:, tags: } - = render partial: 'table', locals: { products:, producer_options:, category_options:, tax_category_options: } + = render partial: 'table', locals: { products:, producer_options:, category_options:, tax_category_options: , allowed_producers: } - if pagy.present? && pagy.pages > 1 = render partial: 'admin/shared/stimulus_pagination', locals: { pagy: pagy } - else 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 f49c756078..c2869deb3f 100644 --- a/app/views/admin/products_v3/_product_variant_row.html.haml +++ b/app/views/admin/products_v3/_product_variant_row.html.haml @@ -1,3 +1,4 @@ +-# locals: (form:, product:, product_index:, producer_options:, category_options:, tax_category_options:, allowed_producers:, should_slide_in: false) = form.fields_for("products", product, index: product_index) do |product_form| %tbody.relaxed{ id: dom_id(product), data: { 'record-id': product_form.object.id, controller: "nested-form product", @@ -7,6 +8,9 @@ = render partial: 'product_row', locals: { f: product_form, product:, product_index: } - product.variants.each_with_index do |variant, variant_index| + + -# 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, category_options:, tax_category_options:, producer_options: } diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml index 003194a3fb..7d2750a5a9 100644 --- a/app/views/admin/products_v3/_table.html.haml +++ b/app/views/admin/products_v3/_table.html.haml @@ -1,3 +1,4 @@ +-# locals: (products:, producer_options:, category_options:, tax_category_options:, allowed_producers:) = form_with url: admin_products_bulk_update_path, method: :post, id: "products-form", builder: BulkFormBuilder, html: { data: { 'turbo-frame': "_self", @@ -69,4 +70,4 @@ %th.align-left.col-inherits_properties= t('admin.products_page.columns.inherits_properties') %th.align-right= t('admin.products_page.columns.actions') - products.each_with_index do |product, product_index| - = render partial: 'product_variant_row', locals: { form:, product:, product_index:, producer_options:, category_options:, tax_category_options: } + = render partial: 'product_variant_row', locals: { form:, product:, product_index:, producer_options:, category_options:, tax_category_options: , allowed_producers:} diff --git a/app/views/admin/products_v3/_variant_row.html.haml b/app/views/admin/products_v3/_variant_row.html.haml index 9d8fbeb999..46bdc4ed47 100644 --- a/app/views/admin/products_v3/_variant_row.html.haml +++ b/app/views/admin/products_v3/_variant_row.html.haml @@ -1,3 +1,4 @@ +-# locals: (variant:, f:, category_options:, tax_category_options:, producer_options:) - method_on_demand, method_on_hand = variant.new_record? ? [:on_demand_desired, :on_hand_desired ]: [:on_demand, :on_hand] %td.col-image -# empty diff --git a/app/views/admin/products_v3/clone.turbo_stream.haml b/app/views/admin/products_v3/clone.turbo_stream.haml index 94d39c1acd..a4ba827897 100644 --- a/app/views/admin/products_v3/clone.turbo_stream.haml +++ b/app/views/admin/products_v3/clone.turbo_stream.haml @@ -1,16 +1,18 @@ +-# locals: (product:, cloned_product:, product_index:, producer_options:, category_options: category, tax_category_options:, allowed_producers:) - unless flash[:error] - product_body = nil - form_with do |form| - product_body = render(partial: 'product_variant_row', locals: { form:, - product: @cloned_product, - product_index: @product_index, - producer_options: @producer_options, - category_options: @category_options, - tax_category_options: @tax_category_options, + product: cloned_product, + product_index:, + producer_options:, + category_options:, + tax_category_options:, + allowed_producers:, should_slide_in: true }, formats: :html) - = turbo_stream.after dom_id(@product) do + = turbo_stream.after dom_id(product) do = product_body = turbo_stream.append "flashes" do diff --git a/app/views/admin/products_v3/index.html.haml b/app/views/admin/products_v3/index.html.haml index 38e4ed355b..04d992e3d4 100644 --- a/app/views/admin/products_v3/index.html.haml +++ b/app/views/admin/products_v3/index.html.haml @@ -1,3 +1,4 @@ +-# locals: (producer_options:, categories:, tax_category_options:, available_tags:, flash:, allowed_producers:) - content_for :page_title do = t('.header.title') - content_for :page_actions do @@ -13,11 +14,13 @@ #products_v3_page{ 'data-turbo': true } = render partial: "content", locals: { products: @products, pagy: @pagy, search_term: @search_term, - producer_options: producers, producer_id: @producer_id, + producer_options:, producer_id: @producer_id, category_options: categories, category_id: @category_id, tax_category_options:, available_tags:, tags: @tags, flashes: flash, - display_search_filter: (@products.any? || @search_term.present? || @category_id.present?) } + display_search_filter: (@products.any? || @search_term.present? || @category_id.present?), + allowed_producers:} + - %w[product variant].each do |object_type| = render partial: 'delete_modal', locals: { object_type: } #modal-component diff --git a/spec/models/spree/ability_spec.rb b/spec/models/spree/ability_spec.rb index 42e0300b36..8d0aab1c4c 100644 --- a/spec/models/spree/ability_spec.rb +++ b/spec/models/spree/ability_spec.rb @@ -324,6 +324,29 @@ RSpec.describe Spree::Ability do ) end + context "with mutiple variant with different supplier" do + let(:product1) { create(:product, supplier_id: create(:supplier_enterprise).id) } + let(:product1_other_variant) { create(:variant, product: product1, supplier: s1) } + + it "is able to read/write their enterprises' products and variants" do + product1_other_variant + + is_expected.to have_ability([:admin, :read, :update, :bulk_update, :clone, :destroy], + for: product1) + + is_expected.to have_ability( + [:admin, :index, :read, :edit, :update, :search, :destroy, + :delete], for: product1.variants.last + ) + + # First variant belongs to another supplier + is_expected.not_to have_ability( + [:admin, :index, :read, :edit, :update, :search, :destroy, + :delete], for: product1.variants.first + ) + end + end + it "should be able to read/write related enterprises' products " \ "and variants with manage_products permission" do er_ps