diff --git a/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee b/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee index 9d4668a8cc..52106156da 100644 --- a/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee +++ b/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee @@ -6,6 +6,7 @@ angular.module("ofn.admin").factory 'EnterpriseRelationships', ($http, enterpris 'manage_products' 'edit_profile' 'create_variant_overrides' + 'create_linked_variants' ] constructor: -> @@ -30,3 +31,4 @@ angular.module("ofn.admin").factory 'EnterpriseRelationships', ($http, enterpris when "manage_products" then t('js.services.manage_products') when "edit_profile" then t('js.services.edit_profile') when "create_variant_overrides" then t('js.services.add_products_to_inventory') + when "create_linked_variants" then t('js.services.create_linked_variants') diff --git a/app/controllers/admin/products_v3_controller.rb b/app/controllers/admin/products_v3_controller.rb index d5c3f5383c..80b99aed73 100644 --- a/app/controllers/admin/products_v3_controller.rb +++ b/app/controllers/admin/products_v3_controller.rb @@ -107,6 +107,33 @@ module Admin end end + # Clone a variant, retaining a link to the "source" + def create_linked_variant + linked_variant = Spree::Variant.find(params[:variant_id]) + product_index = params[:product_index] + authorize! :create_linked_variant, linked_variant + status = :ok + + begin + variant = linked_variant.create_linked_variant(spree_current_user) + + flash.now[:success] = t('.success') + variant_index = "-#{variant.id}" + rescue ActiveRecord::RecordInvalid + flash.now[:error] = variant.errors.full_messages.to_sentence + status = :unprocessable_entity + variant_index = "-1" # Create a unique-enough index + end + + respond_with do |format| + format.turbo_stream { + locals = { linked_variant:, variant:, product_index:, variant_index:, + producer_options:, category_options: categories, tax_category_options: } + render :create_linked_variant, status:, locals: + } + end + end + def index_url(params) "/admin/products?#{params.to_query}" # todo: fix routing so this can be automaticly generated end diff --git a/app/helpers/admin/products_helper.rb b/app/helpers/admin/products_helper.rb index 90480e371f..8b07a650d9 100644 --- a/app/helpers/admin/products_helper.rb +++ b/app/helpers/admin/products_helper.rb @@ -47,5 +47,10 @@ module Admin def variant_tag_enabled?(user) feature?(:variant_tag, user) || feature?(:variant_tag, *user.enterprises) end + + def allowed_source_producers + @allowed_source_producers ||= OpenFoodNetwork::Permissions.new(spree_current_user) + .enterprises_granting_linked_variants + end end end diff --git a/app/models/spree/ability.rb b/app/models/spree/ability.rb index e2d64b98b4..c8e4e19831 100644 --- a/app/models/spree/ability.rb +++ b/app/models/spree/ability.rb @@ -202,7 +202,7 @@ 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 + # An enterprise 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, @@ -214,7 +214,16 @@ module Spree ) end - can [:admin, :index, :bulk_update, :destroy, :destroy_variant, :clone], :products_v3 + # An enterprise user can clone if they have been granted permission to the source variant. + # Technically I'd call this permission clone_linked_variant, but it would be less confusing to + # use the same name as everywhere else. + can [:create_linked_variant], Spree::Variant do |variant| + OpenFoodNetwork::Permissions.new(user). + enterprises_granting_linked_variants.include? variant.supplier + end + + can [:admin, :index, :bulk_update, :destroy, :destroy_variant, :clone, + :create_linked_variant], :products_v3 can [:create], Spree::Variant can [:admin, :index, :read, :edit, diff --git a/app/models/spree/variant.rb b/app/models/spree/variant.rb index d6eda45ca8..13d556b377 100644 --- a/app/models/spree/variant.rb +++ b/app/models/spree/variant.rb @@ -40,6 +40,7 @@ module Spree belongs_to :shipping_category, class_name: 'Spree::ShippingCategory', optional: false belongs_to :primary_taxon, class_name: 'Spree::Taxon', touch: true, optional: false belongs_to :supplier, class_name: 'Enterprise', optional: false, touch: true + belongs_to :hub, class_name: 'Enterprise', optional: true delegate :name, :name=, :description, :description=, :meta_keywords, to: :product @@ -72,6 +73,15 @@ module Spree has_many :semantic_links, as: :subject, dependent: :delete_all has_many :supplier_properties, through: :supplier, source: :properties + # Linked variants: I may have one or many sources. + has_many :variant_links_as_target, class_name: 'VariantLink', foreign_key: :target_variant_id, + dependent: :delete_all, inverse_of: :target_variant + has_many :source_variants, through: :variant_links_as_target, source: :source_variant + # I may also have one more many targets. + has_many :variant_links_as_source, class_name: 'VariantLink', foreign_key: :source_variant_id, + dependent: :delete_all, inverse_of: :source_variant + has_many :target_variants, through: :variant_links_as_source, source: :target_variant + localize_number :price, :weight validates_lengths_from_database @@ -263,6 +273,24 @@ module Spree @on_hand_desired = ActiveModel::Type::Integer.new.cast(val) end + # Clone this variant, retaining a 'source' link to it + def create_linked_variant(user) + # Hub owner is my enterprise which has permission to create variant sourced from that supplier + hub_id = EnterpriseRelationship.permitted_by(supplier).permitting(user.enterprises) + .with_permission(:create_linked_variants) + .pick(:child_id) + + dup.tap do |variant| + variant.price = price + variant.source_variants = [self] + variant.stock_items << Spree::StockItem.new(variant:) + variant.hub_id = hub_id + variant.on_demand = on_demand + variant.on_hand = on_hand + variant.save! + end + end + private def check_currency diff --git a/app/models/variant_link.rb b/app/models/variant_link.rb new file mode 100644 index 0000000000..0030b80a5a --- /dev/null +++ b/app/models/variant_link.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +class VariantLink < ApplicationRecord + belongs_to :source_variant, class_name: 'Spree::Variant' + belongs_to :target_variant, class_name: 'Spree::Variant' +end 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 71ab6e6301..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,9 +11,10 @@ -# 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: } + = render partial: 'variant_row', locals: { variant:, f: variant_form, product_index:, category_options:, tax_category_options:, producer_options: } = 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" } diff --git a/app/views/admin/products_v3/_variant_row.html.haml b/app/views/admin/products_v3/_variant_row.html.haml index 54bf2ca861..cd9e8ae6ee 100644 --- a/app/views/admin/products_v3/_variant_row.html.haml +++ b/app/views/admin/products_v3/_variant_row.html.haml @@ -1,7 +1,10 @@ --# locals: (variant:, f:, category_options:, tax_category_options:, producer_options:) +-# haml-lint:disable ViewLength (This file is big, but doesn't make sense to split up at this point) +-# locals: (variant:, f:, product_index: nil, 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 + - variant.source_variants.each do |source_variant| + = content_tag(:span, "🔗", title: t('admin.products_page.variant_row.sourced_from', source_name: source_variant.display_name, source_id: source_variant.id, hub_name: variant.hub&.name)) %td.col-name.field.naked_inputs = f.hidden_field :id = f.text_field :display_name, 'aria-label': t('admin.products_page.columns.name'), placeholder: variant.product.name @@ -88,6 +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) + + - if allowed_source_producers.include?(variant.supplier) + = link_to t('admin.products_page.actions.create_linked_variant'), admin_create_linked_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/app/views/admin/products_v3/create_linked_variant.turbo_stream.haml b/app/views/admin/products_v3/create_linked_variant.turbo_stream.haml new file mode 100644 index 0000000000..88d5931901 --- /dev/null +++ b/app/views/admin/products_v3/create_linked_variant.turbo_stream.haml @@ -0,0 +1,16 @@ +-# locals: (variant:, linked_variant:, product_index:, variant_index:, producer_options:, category_options:, tax_category_options:) +-# Pre-render the form, because you can't do it inside turbo stream block +- variant_row = nil +- fields_for("products][#{product_index}][variants_attributes", variant, index: variant_index) do |f| + - variant_row = render(partial: 'variant_row', formats: :html, + locals: { f:, + variant:, + producer_options:, + category_options:, + tax_category_options:}) += turbo_stream.after dom_id(linked_variant) do + %tr.condensed{ id: dom_id(variant), 'data-controller': "variant", 'class': "nested-form-wrapper slide-in",'data-variant-bulk-form-outlet': "#products-form"} + = variant_row + += turbo_stream.append "flashes" do + = render(partial: 'admin/shared/flashes', locals: { flashes: flash }) diff --git a/app/webpacker/controllers/bulk_form_controller.js b/app/webpacker/controllers/bulk_form_controller.js index 6e82d2ce33..95eb2c1209 100644 --- a/app/webpacker/controllers/bulk_form_controller.js +++ b/app/webpacker/controllers/bulk_form_controller.js @@ -44,12 +44,17 @@ export default class BulkFormController extends Controller { } // Register any new elements (may be called by another controller after dynamically adding fields) - registerElements() { - const registeredElements = Object.values(this.recordElements).flat(); - // Select only elements that haven't been registered yet - const newElements = Array.from(this.form.elements).filter( - (n) => !registeredElements.includes(n), - ); + // May be called with array of elements to register, otherwise finds all un-registered elements. + registerElements(eventOrElements = null) { + let newElements; + + if (Array.isArray(eventOrElements)) { + newElements = eventOrElements; + } else { + const registeredElements = Object.values(this.recordElements).flat(); + // Select only elements that haven't been registered yet + newElements = Array.from(this.form.elements).filter((n) => !registeredElements.includes(n)); + } this.#registerElements(newElements); } diff --git a/app/webpacker/controllers/variant_controller.js b/app/webpacker/controllers/variant_controller.js index 5a529a338f..29f24899a4 100644 --- a/app/webpacker/controllers/variant_controller.js +++ b/app/webpacker/controllers/variant_controller.js @@ -4,6 +4,8 @@ import OptionValueNamer from "js/services/option_value_namer"; // Dynamically update related variant fields // export default class VariantController extends Controller { + static outlets = ["bulk-form"]; + connect() { // idea: create a helper that includes a nice getter/setter for Rails model attr values, just pass it the attribute name. // It could automatically find (and cache a ref to) each dom element and get/set the values. @@ -40,6 +42,12 @@ export default class VariantController extends Controller { // on display_as changed; update unit_to_display // TODO: optimise to avoid unnecessary OptionValueNamer calc this.displayAs.addEventListener("input", this.#updateUnitDisplay.bind(this), { passive: true }); + + // Register with bulk products form to listen for changes. Used when dynamically appending variants. + if (this.hasBulkFormOutlet) { + const formElements = this.element.querySelectorAll("input, select, textarea, button"); + this.bulkFormOutlet.registerElements(formElements); + } } disconnect() { diff --git a/config/locales/en.yml b/config/locales/en.yml index fffe128a73..2b8134bc77 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -717,8 +717,11 @@ en: delete: Delete remove: Remove preview: Preview + create_linked_variant: Create linked variant image: edit: Edit + variant_row: + sourced_from: "Sourced from: %{source_name} (%{source_id}); Hub: %{hub_name}" product_preview: product_preview: Product preview shop_tab: Shop @@ -1098,6 +1101,8 @@ en: clone: success: Successfully cloned the product error: Unable to clone the product + create_linked_variant: + success: "Successfully created linked variant" tag_rules: rules_per_tag: one: "%{tag} has 1 rule" @@ -3901,6 +3906,7 @@ en: manage_products: "manage products" edit_profile: "edit profile" add_products_to_inventory: "add products to inventory" + create_linked_variants: "create linked variants [BETA]" resources: could_not_delete_customer: 'Could not delete customer' product_import: diff --git a/config/routes/admin.rb b/config/routes/admin.rb index e36ae10545..33553f5a51 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -82,6 +82,7 @@ Openfoodnetwork::Application.routes.draw do delete 'products_v3/:id', to: 'products_v3#destroy', as: 'product_destroy' delete 'products_v3/destroy_variant/:id', to: 'products_v3#destroy_variant', as: 'destroy_variant' post 'clone/:id', to: 'products_v3#clone', as: 'clone_product' + post 'products/create_linked_variant', to: 'products_v3#create_linked_variant', as: 'create_linked_variant' resources :product_preview, only: [:show] resources :variant_overrides do diff --git a/db/migrate/20260211055758_create_variant_links.rb b/db/migrate/20260211055758_create_variant_links.rb new file mode 100644 index 0000000000..3109172f8a --- /dev/null +++ b/db/migrate/20260211055758_create_variant_links.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class CreateVariantLinks < ActiveRecord::Migration[7.1] + def change + # Create a join table to join two variants. One is the source of the other. + # Primary key index ensures uniqueness and assists querying. target_variant_id is the most + # likely subject and so is first in the index. + # An additional index for source_variant is also included because it may be helpful + # (https://stackoverflow.com/questions/10790518/best-sql-indexes-for-join-table). + create_table :variant_links, primary_key: [:target_variant_id, :source_variant_id] do |t| + t.integer :source_variant_id, null: false, index: true + t.integer :target_variant_id, null: false + + t.datetime :created_at, null: false + end + add_foreign_key :variant_links, :spree_variants, column: :source_variant_id + add_foreign_key :variant_links, :spree_variants, column: :target_variant_id + end +end diff --git a/db/migrate/20260225022934_add_hub_to_spree_variants.rb b/db/migrate/20260225022934_add_hub_to_spree_variants.rb new file mode 100644 index 0000000000..bcfed767b8 --- /dev/null +++ b/db/migrate/20260225022934_add_hub_to_spree_variants.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddHubToSpreeVariants < ActiveRecord::Migration[7.1] + def change + add_reference :spree_variants, :hub, foreign_key: { to_table: :enterprises } + end +end diff --git a/db/schema.rb b/db/schema.rb index aaef15970f..1c77a7ca8d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1009,6 +1009,8 @@ ActiveRecord::Schema[7.1].define(version: 2026_03_06_015040) do t.bigint "supplier_id" t.float "variant_unit_scale" t.string "variant_unit_name", limit: 255 + t.bigint "hub_id" + t.index ["hub_id"], name: "index_spree_variants_on_hub_id" t.index ["primary_taxon_id"], name: "index_spree_variants_on_primary_taxon_id" t.index ["product_id"], name: "index_variants_on_product_id" t.index ["shipping_category_id"], name: "index_spree_variants_on_shipping_category_id" @@ -1113,6 +1115,13 @@ ActiveRecord::Schema[7.1].define(version: 2026_03_06_015040) do t.datetime "updated_at", precision: nil, null: false end + create_table "variant_links", primary_key: ["target_variant_id", "source_variant_id"], force: :cascade do |t| + t.integer "source_variant_id", null: false + t.integer "target_variant_id", null: false + t.datetime "created_at", null: false + t.index ["source_variant_id"], name: "index_variant_links_on_source_variant_id" + end + create_table "variant_overrides", id: :serial, force: :cascade do |t| t.integer "variant_id", null: false t.integer "hub_id", null: false @@ -1262,6 +1271,7 @@ ActiveRecord::Schema[7.1].define(version: 2026_03_06_015040) do add_foreign_key "spree_tax_rates", "spree_zones", column: "zone_id", name: "spree_tax_rates_zone_id_fk" add_foreign_key "spree_users", "spree_addresses", column: "bill_address_id", name: "spree_users_bill_address_id_fk" add_foreign_key "spree_users", "spree_addresses", column: "ship_address_id", name: "spree_users_ship_address_id_fk" + add_foreign_key "spree_variants", "enterprises", column: "hub_id" add_foreign_key "spree_variants", "enterprises", column: "supplier_id" add_foreign_key "spree_variants", "spree_products", column: "product_id", name: "spree_variants_product_id_fk" add_foreign_key "spree_variants", "spree_shipping_categories", column: "shipping_category_id" @@ -1278,6 +1288,8 @@ ActiveRecord::Schema[7.1].define(version: 2026_03_06_015040) do add_foreign_key "subscriptions", "spree_payment_methods", column: "payment_method_id", name: "subscriptions_payment_method_id_fk" add_foreign_key "subscriptions", "spree_shipping_methods", column: "shipping_method_id", name: "subscriptions_shipping_method_id_fk" add_foreign_key "tag_rules", "enterprises" + add_foreign_key "variant_links", "spree_variants", column: "source_variant_id" + add_foreign_key "variant_links", "spree_variants", column: "target_variant_id" add_foreign_key "variant_overrides", "enterprises", column: "hub_id", name: "variant_overrides_hub_id_fk" add_foreign_key "variant_overrides", "spree_variants", column: "variant_id", name: "variant_overrides_variant_id_fk" add_foreign_key "vouchers", "enterprises" diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb index b3239d1b72..0c9d0778f4 100644 --- a/lib/open_food_network/permissions.rb +++ b/lib/open_food_network/permissions.rb @@ -86,6 +86,10 @@ module OpenFoodNetwork managed_and_related_enterprises_granting :manage_products end + def enterprises_granting_linked_variants + related_enterprises_granting :create_linked_variants + end + def manages_one_enterprise? @user.enterprises.length == 1 end diff --git a/spec/javascripts/unit/admin/services/enterprise_relationships_spec.js.coffee b/spec/javascripts/unit/admin/services/enterprise_relationships_spec.js.coffee index 96ac4a8905..2a5cb5ddf2 100644 --- a/spec/javascripts/unit/admin/services/enterprise_relationships_spec.js.coffee +++ b/spec/javascripts/unit/admin/services/enterprise_relationships_spec.js.coffee @@ -16,3 +16,4 @@ describe "enterprise relationships", -> expect(EnterpriseRelationships.permission_presentation("manage_products")).toEqual "manage products" expect(EnterpriseRelationships.permission_presentation("edit_profile")).toEqual "edit profile" expect(EnterpriseRelationships.permission_presentation("create_variant_overrides")).toEqual "add products to inventory" + expect(EnterpriseRelationships.permission_presentation("create_linked_variants")).toEqual "create linked variants [BETA]" diff --git a/spec/models/spree/ability_spec.rb b/spec/models/spree/ability_spec.rb index 118d84288c..843a77c3e4 100644 --- a/spec/models/spree/ability_spec.rb +++ b/spec/models/spree/ability_spec.rb @@ -364,6 +364,19 @@ RSpec.describe Spree::Ability do for: p2.variants.first) end + describe "create_linked_variant" do + it "should not be able to create linked variant without permission" do + is_expected.not_to have_ability([:create_linked_variant], for: p_related.variants.first) + end + + it "should be able to create linked variant when granted permission" do + create(:enterprise_relationship, parent: s_related, child: s1, + permissions_list: [:create_linked_variants]) + + is_expected.to have_ability([:create_linked_variant], for: p_related.variants.first) + end + end + it "should not be able to access admin actions on orders" do is_expected.not_to have_ability([:admin], for: Spree::Order) end @@ -729,6 +742,19 @@ RSpec.describe Spree::Ability do it "can request permitted enterprise fees for an order cycle" do is_expected.to have_ability([:for_order_cycle], for: EnterpriseFee) end + + describe "create_linked_variant" do + it "should not be able to create linked variant without permission" do + is_expected.not_to have_ability([:create_linked_variant], for: p_related.variants.first) + end + + it "should be able to create linked variant when granted permission" do + create(:enterprise_relationship, parent: s_related, child: d1, + permissions_list: [:create_linked_variants]) + + is_expected.to have_ability([:create_linked_variant], for: p_related.variants.first) + end + end end context 'Order Cycle co-ordinator, distributor enterprise manager' do @@ -804,6 +830,19 @@ RSpec.describe Spree::Ability do it "has the ability to manage vouchers" do is_expected.to have_ability([:admin, :create], for: Voucher) end + + describe "create_linked_variant for own enterprise" do + it "should not be able to create own sourced variant without permission" do + is_expected.not_to have_ability([:create_linked_variant], for: p1.variants.first) + end + + it "should be able to create own sourced variant when granted self permission" do + create(:enterprise_relationship, parent: s1, child: s1, + permissions_list: [:create_linked_variants]) + + is_expected.to have_ability([:create_linked_variant], for: p1.variants.first) + end + end end context 'enterprise owner' do diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb index 86d3ca92ff..085be8a04b 100644 --- a/spec/models/spree/variant_spec.rb +++ b/spec/models/spree/variant_spec.rb @@ -8,6 +8,7 @@ RSpec.describe Spree::Variant do it { is_expected.to have_many :semantic_links } it { is_expected.to belong_to(:product).required } it { is_expected.to belong_to(:supplier).required } + it { is_expected.to belong_to(:hub).optional } it { is_expected.to have_many(:inventory_units) } it { is_expected.to have_many(:line_items) } it { is_expected.to have_many(:stock_items) } @@ -20,6 +21,9 @@ RSpec.describe Spree::Variant do it { is_expected.to have_many(:inventory_items) } it { is_expected.to have_many(:supplier_properties).through(:supplier) } + it { is_expected.to have_many(:source_variants).through(:variant_links_as_target) } + it { is_expected.to have_many(:target_variants).through(:variant_links_as_source) } + describe "shipping category" do it "sets a shipping category if none provided" do variant = build(:variant, shipping_category: nil) @@ -1001,4 +1005,30 @@ RSpec.describe Spree::Variant do expect(variant.unit_presentation).to eq "My display" end end + + describe "#create_linked_variant" do + let(:user) { create(:user, enterprises: [enterprise]) } + let(:supplier) { variant.supplier } + let(:enterprise) { create(:enterprise) } + + context "with create_linked_variants permissions on supplier" do + let!(:enterprise_relationship) { + create(:enterprise_relationship, + parent: supplier, + child: enterprise, + permissions_list: [:create_linked_variants]) + } + let(:variant) { create(:variant, price: 10.95, on_demand: false, on_hand: 5) } + + it "clones the variant, retaining a link to the source" do + linked_variant = variant.create_linked_variant(user) + + expect(linked_variant.source_variants).to eq [variant] + expect(linked_variant.hub).to eq enterprise + expect(linked_variant.price).to eq 10.95 + expect(linked_variant.on_demand).to eq false + expect(linked_variant.on_hand).to eq 5 + end + end + end end diff --git a/spec/requests/admin/products_v3_spec.rb b/spec/requests/admin/products_v3_spec.rb index ff1a6850bb..1f0773753a 100644 --- a/spec/requests/admin/products_v3_spec.rb +++ b/spec/requests/admin/products_v3_spec.rb @@ -59,4 +59,71 @@ RSpec.describe "Admin::ProductsV3" do expect(response).to redirect_to('/unauthorized') end end + + describe "POST /admin/products/create_linked_variant" do + let(:enterprise) { create(:supplier_enterprise) } + let(:user) { create(:user, enterprises: [enterprise]) } + + let(:supplier) { create(:supplier_enterprise) } + let(:variant) { create(:variant, display_name: "Original variant", supplier: supplier) } + + before do + sign_in user + end + + it "checks for permission" do + params = { variant_id: variant.id, product_index: 1 } + + expect { + post(admin_create_linked_variant_path, as: :turbo_stream, params:) + expect(response).to redirect_to('/unauthorized') + }.not_to change { variant.product.variants.count } + end + + context "With create_linked_variants permissions on supplier" do + let!(:enterprise_relationship) { + create(:enterprise_relationship, + parent: supplier, + child: enterprise, + permissions_list: [:create_linked_variants]) + } + + it "clones the variant, retaining link as source" do + params = { variant_id: variant.id, product_index: 1 } + + expect { + post(admin_create_linked_variant_path, as: :turbo_stream, params:) + + expect(response).to have_http_status(:ok) + expect(response.body).to match "Original variant" # cloned variant name + }.to change { variant.product.variants.count }.by(1) + + new_variant = variant.product.variants.order(:id).last + # The new variant is a target of the original. It is a "sourced" variant. + expect(variant.target_variants.first).to eq new_variant + # The new variant's source is the original + expect(new_variant.source_variants.first).to eq variant + end + + context "and I'm also owner of another enterprise" do + let!(:enterprise2) { create(:enterprise) } + let(:user) { create(:user, enterprises: [enterprise, enterprise2]) } + + it "clones the variant, owned by my enterprise that has permission" do + enterprise2.owner = user + params = { variant_id: variant.id, product_index: 1 } + + expect { + post(admin_create_linked_variant_path, as: :turbo_stream, params:) + + expect(response).to have_http_status(:ok) + }.to change { variant.product.variants.count }.by(1) + + # The new variant is owned by my enterprise that has permission, not the other one + new_variant = variant.product.variants.order(:id).last + expect(new_variant.hub).to eq enterprise + end + end + end + end end diff --git a/spec/system/admin/enterprise_relationships_spec.rb b/spec/system/admin/enterprise_relationships_spec.rb index 1e86c9d229..aef6096605 100644 --- a/spec/system/admin/enterprise_relationships_spec.rb +++ b/spec/system/admin/enterprise_relationships_spec.rb @@ -47,18 +47,23 @@ create(:enterprise) uncheck 'to manage products' check 'to edit profile' check 'to add products to inventory' + check 'to create linked variants' select2_select 'Two', from: 'enterprise_relationship_child_id' click_button 'Create' # Wait for row to appear since have_relationship doesn't wait expect(page).to have_selector 'tr', count: 2 + # Permissions appear.. in a different order for some reason. expect_relationship_with_permissions e1, e2, ['to add to order cycle', - 'to add products to inventory', 'to edit profile'] + 'to create linked variants [BETA]', + 'to add products to inventory', + 'to edit profile'] er = EnterpriseRelationship.where(parent_id: e1, child_id: e2).first expect(er).to be_present expect(er.permissions.map(&:name)).to match_array ['add_to_order_cycle', 'edit_profile', - 'create_variant_overrides'] + 'create_variant_overrides', + 'create_linked_variants'] end it "attempting to create a relationship with invalid data" do diff --git a/spec/system/admin/products_v3/actions_spec.rb b/spec/system/admin/products_v3/actions_spec.rb index 383892ba86..16650bd588 100644 --- a/spec/system/admin/products_v3/actions_spec.rb +++ b/spec/system/admin/products_v3/actions_spec.rb @@ -2,7 +2,7 @@ require "system_helper" -RSpec.describe 'As an enterprise user, I can manage my products' do +RSpec.describe 'As an enterprise user, I can perform actions on the products screen' do include AdminHelper include WebHelper include AuthenticationHelper @@ -19,18 +19,6 @@ RSpec.describe 'As an enterprise user, I can manage my products' do let(:categories_search_selector) { 'input[placeholder="Select category"]' } let(:tax_categories_search_selector) { 'input[placeholder="Search for tax categories"]' } - describe "with no products" do - before { visit admin_products_url } - it "can see the new product page" do - expect(page).to have_content "Bulk Edit Products" - expect(page).to have_text "No products found" - # displays buttons to add products with the correct links - expect(page).to have_link(class: "button", text: "New Product", href: "/admin/products/new") - expect(page).to have_link(class: "button", text: "Import multiple products", - href: admin_product_import_path) - end - end - describe "column selector" do let!(:product) { create(:simple_product) } @@ -105,8 +93,6 @@ RSpec.describe 'As an enterprise user, I can manage my products' do end end - describe "columns" - describe "Changing producers, category and tax category" do let!(:variant_a1) { product_a.variants.first.tap{ |v| @@ -260,24 +246,24 @@ RSpec.describe 'As an enterprise user, I can manage my products' do describe "Cloning product" do it "shows the cloned product on page when clicked on the cloned option" do - # TODO, variant supplier missing, needs to be copied from variant and not product - within "table.products" do - # Gather input values, because page.content doesn't include them. - input_content = page.find_all('input[type=text]').map(&:value).join - - # Products does not include the cloned product. - expect(input_content).not_to match /COPY OF Apples/ - end - click_product_clone "Apples" expect(page).to have_content "Successfully cloned the product" within "table.products" do - # Gather input values, because page.content doesn't include them. - input_content = page.find_all('input[type=text]').map(&:value).join + # Product list includes the cloned product. + expect(all_input_values).to match /COPY OF Apples/ - # Products include the cloned product. - expect(input_content).to match /COPY OF Apples/ + # And I can perform actions on the new product + within row_containing_name "COPY OF Apples" do + page.find(".vertical-ellipsis-menu").click + expect(page).to have_link "Edit" + expect(page).to have_link "Clone" + # expect(page).to have_link "Delete" # it's not a proper link :/ + + fill_in "Name", with: "My copy of Apples" + end + + click_button "Save changes" end end end @@ -298,6 +284,88 @@ RSpec.describe 'As an enterprise user, I can manage my products' do end end + describe "Create linked variant" do + let!(:variant) { + create(:variant, display_name: "My box", supplier: producer) + } + let!(:other_producer) { create(:supplier_enterprise) } + let!(:other_variant) { + create(:variant, display_name: "My friends box", supplier: other_producer) + } + let!(:enterprise_relationship) { + # Other producer grants me access to manage their variant + create(:enterprise_relationship, parent: other_producer, child: producer, + permissions_list: [:manage_products]) + } + + context "with create_linked_variants permission for my, and other's variants" do + it "creates a linked variant" do + create(:enterprise_relationship, parent: producer, child: producer, + permissions_list: [:create_linked_variants]) + enterprise_relationship.permissions.create! name: :create_linked_variants + + visit admin_products_url + + # Check my own variant + within row_containing_name("My box") do + page.find(".vertical-ellipsis-menu").click + + expect(page).to have_link "Create linked variant" + end + + # Create linked variant sourced from my friend + within row_containing_name("My friends box") do + page.find(".vertical-ellipsis-menu").click + + click_link "Create linked variant" + end + + expect(page).to have_content "Successfully created linked variant" + + within "table.products" do + # There are now two copies + expect(all_input_values).to match /My friends box.*My friends box/ + # One of them is designated as a linked variant + expect(page).to have_content "🔗" + + last_box = page.all(row_containing_name("My friends box")).last + # Close action menu (shouldn't need this, it should close itself) + last_box.click + + # And I can perform actions on the new product + within last_box do + page.find(".vertical-ellipsis-menu").click + expect(page).to have_link "Edit" + # expect(page).to have_link "Clone" # tofix: menu is partially obscured + # expect(page).to have_link "Delete" # it's not a proper link + + fill_in "Name", with: "My copy of Apples" + end + click_button "Save changes" + + # initially obscured by the previous message, then disappears before capybara sees it. + # expect(page).to have_content "Changes saved" + end + end + end + + context "without create_linked_variants permission" do + it "does not show the option in the menu" do + visit admin_products_url + + within row_containing_name("My box") do + page.find(".vertical-ellipsis-menu").click + expect(page).not_to have_link "Create linked variant" + end + + within row_containing_name("My friends box") do + page.find(".vertical-ellipsis-menu").click + expect(page).not_to have_link "Create linked variant" + end + end + end + end + describe "delete" do let!(:product_a) { create(:simple_product, name: "Apples", sku: "APL-00") } let(:delete_option_selector) { "a[data-controller='modal-link'].delete" } @@ -527,90 +595,6 @@ RSpec.describe 'As an enterprise user, I can manage my products' do end end - context "as an enterprise manager" do - let(:supplier_managed1) { create(:supplier_enterprise, name: 'Supplier Managed 1') } - let(:supplier_managed2) { create(:supplier_enterprise, name: 'Supplier Managed 2') } - let(:supplier_unmanaged) { create(:supplier_enterprise, name: 'Supplier Unmanaged') } - let(:supplier_permitted) { create(:supplier_enterprise, name: 'Supplier Permitted') } - let(:distributor_managed) { create(:distributor_enterprise, name: 'Distributor Managed') } - let(:distributor_unmanaged) { create(:distributor_enterprise, name: 'Distributor Unmanaged') } - let!(:product_supplied) { create(:product, supplier_id: supplier_managed1.id, price: 10.0) } - let!(:product_not_supplied) { create(:product, supplier_id: supplier_unmanaged.id) } - let!(:product_supplied_permitted) { - create(:product, name: 'Product Permitted', supplier_id: supplier_permitted.id, price: 10.0) - } - let(:product_supplied_inactive) { - create(:product, supplier_id: supplier_managed1.id, price: 10.0) - } - - let!(:supplier_permitted_relationship) do - create(:enterprise_relationship, parent: supplier_permitted, child: supplier_managed1, - permissions_list: [:manage_products]) - end - - before do - enterprise_user = create(:user) - enterprise_user.enterprise_roles.build(enterprise: supplier_managed1).save - enterprise_user.enterprise_roles.build(enterprise: supplier_managed2).save - enterprise_user.enterprise_roles.build(enterprise: distributor_managed).save - - login_as enterprise_user - end - - it "shows only products that I supply" do - visit spree.admin_products_path - - # displays permitted product list only - expect(page).to have_selector row_containing_name(product_supplied.name) - expect(page).to have_selector row_containing_name(product_supplied_permitted.name) - expect(page).not_to have_selector row_containing_name(product_not_supplied.name) - end - - it "shows only suppliers that I manage or have permission to" do - visit spree.admin_products_path - - within row_containing_placeholder(product_supplied.name) do - expect(page).to have_select( - '_products_0_variants_attributes_0_supplier_id', - options: [ - 'Select producer', - supplier_managed1.name, supplier_managed2.name, supplier_permitted.name - ], selected: supplier_managed1.name - ) - end - - within row_containing_placeholder(product_supplied_permitted.name) do - expect(page).to have_select( - '_products_1_variants_attributes_0_supplier_id', - options: [ - 'Select producer', - supplier_managed1.name, supplier_managed2.name, supplier_permitted.name - ], selected: supplier_permitted.name - ) - end - end - - it "shows inactive products that I supply" do - product_supplied_inactive - - visit spree.admin_products_path - - expect(page).to have_selector row_containing_name(product_supplied_inactive.name) - end - - it "allows me to update a product" do - visit spree.admin_products_path - - within row_containing_name(product_supplied.name) do - fill_in "Name", with: "Pommes" - end - click_button "Save changes" - - expect(page).to have_content "Changes saved" - expect(page).to have_selector row_containing_name("Pommes") - end - end - def open_action_menu page.find(".vertical-ellipsis-menu").click end diff --git a/spec/system/admin/products_v3/index_spec.rb b/spec/system/admin/products_v3/index_spec.rb index d0f3d92337..d943e5a1d3 100644 --- a/spec/system/admin/products_v3/index_spec.rb +++ b/spec/system/admin/products_v3/index_spec.rb @@ -2,13 +2,13 @@ require "system_helper" -RSpec.describe 'As an enterprise user, I can manage my products' do +RSpec.describe 'As an enterprise user, I can browse my products' do include AdminHelper include WebHelper include AuthenticationHelper include FileHelper - let(:producer) { create(:supplier_enterprise) } + let(:producer) { create(:supplier_enterprise, name: "My Enterprise") } let(:user) { create(:user, enterprises: [producer]) } before do @@ -19,15 +19,25 @@ RSpec.describe 'As an enterprise user, I can manage my products' do let(:categories_search_selector) { 'input[placeholder="Search for categories"]' } let(:tax_categories_search_selector) { 'input[placeholder="Search for tax categories"]' } + describe "with no products" do + before { visit admin_products_url } + it "can see the new product page" do + expect(page).to have_content "Bulk Edit Products" + expect(page).to have_text "No products found" + # displays buttons to add products with the correct links + expect(page).to have_link(class: "button", text: "New Product", href: "/admin/products/new") + expect(page).to have_link(class: "button", text: "Import multiple products", + href: admin_product_import_path) + end + end + describe "listing" do let!(:p1) { create(:product, name: "Product1") } let!(:p2) { create(:product, name: "Product2") } - before do - visit admin_products_url - end - it "displays a list of products" do + visit admin_products_path + within ".products" do # displays table header expect(page).to have_selector "th", text: "Name" @@ -129,6 +139,34 @@ RSpec.describe 'As an enterprise user, I can manage 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, + hub: 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: "]' + expect(page).to have_selector 'span[title*="Hub: My Enterprise"]' + end + end + end end describe "sorting" do @@ -463,4 +501,88 @@ RSpec.describe 'As an enterprise user, I can manage my products' do end end end + + context "as an enterprise manager" do + let(:supplier_managed1) { create(:supplier_enterprise, name: 'Supplier Managed 1') } + let(:supplier_managed2) { create(:supplier_enterprise, name: 'Supplier Managed 2') } + let(:supplier_unmanaged) { create(:supplier_enterprise, name: 'Supplier Unmanaged') } + let(:supplier_permitted) { create(:supplier_enterprise, name: 'Supplier Permitted') } + let(:distributor_managed) { create(:distributor_enterprise, name: 'Distributor Managed') } + let(:distributor_unmanaged) { create(:distributor_enterprise, name: 'Distributor Unmanaged') } + let!(:product_supplied) { create(:product, supplier_id: supplier_managed1.id, price: 10.0) } + let!(:product_not_supplied) { create(:product, supplier_id: supplier_unmanaged.id) } + let!(:product_supplied_permitted) { + create(:product, name: 'Product Permitted', supplier_id: supplier_permitted.id, price: 10.0) + } + let(:product_supplied_inactive) { + create(:product, supplier_id: supplier_managed1.id, price: 10.0) + } + + let!(:supplier_permitted_relationship) do + create(:enterprise_relationship, parent: supplier_permitted, child: supplier_managed1, + permissions_list: [:manage_products]) + end + + before do + enterprise_user = create(:user) + enterprise_user.enterprise_roles.build(enterprise: supplier_managed1).save + enterprise_user.enterprise_roles.build(enterprise: supplier_managed2).save + enterprise_user.enterprise_roles.build(enterprise: distributor_managed).save + + login_as enterprise_user + end + + it "shows only products that I supply" do + visit spree.admin_products_path + + # displays permitted product list only + expect(page).to have_selector row_containing_name(product_supplied.name) + expect(page).to have_selector row_containing_name(product_supplied_permitted.name) + expect(page).not_to have_selector row_containing_name(product_not_supplied.name) + end + + it "shows only suppliers that I manage or have permission to" do + visit spree.admin_products_path + + within row_containing_placeholder(product_supplied.name) do + expect(page).to have_select( + '_products_0_variants_attributes_0_supplier_id', + options: [ + 'Select producer', + supplier_managed1.name, supplier_managed2.name, supplier_permitted.name + ], selected: supplier_managed1.name + ) + end + + within row_containing_placeholder(product_supplied_permitted.name) do + expect(page).to have_select( + '_products_1_variants_attributes_0_supplier_id', + options: [ + 'Select producer', + supplier_managed1.name, supplier_managed2.name, supplier_permitted.name + ], selected: supplier_permitted.name + ) + end + end + + it "shows inactive products that I supply" do + product_supplied_inactive + + visit spree.admin_products_path + + expect(page).to have_selector row_containing_name(product_supplied_inactive.name) + end + + it "allows me to update a product" do + visit spree.admin_products_path + + within row_containing_name(product_supplied.name) do + fill_in "Name", with: "Pommes" + end + click_button "Save changes" + + expect(page).to have_content "Changes saved" + expect(page).to have_selector row_containing_name("Pommes") + end + end end