From 93fb3fd7d981bd1f8d99d115df5f54ccf6a57e5c Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Sun, 16 Feb 2025 01:08:39 +0500 Subject: [PATCH 01/26] add enterprise producers_to_edit_orders setting --- app/services/permitted_attributes/enterprise.rb | 1 + .../enterprises/form/_shop_preferences.html.haml | 14 ++++++++++++++ config/locales/en.yml | 4 ++++ ...able_producers_to_edit_orders_to_enterprises.rb | 7 +++++++ db/schema.rb | 1 - 5 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20250202121858_add_enable_producers_to_edit_orders_to_enterprises.rb diff --git a/app/services/permitted_attributes/enterprise.rb b/app/services/permitted_attributes/enterprise.rb index c25c82a585..5d780bfea3 100644 --- a/app/services/permitted_attributes/enterprise.rb +++ b/app/services/permitted_attributes/enterprise.rb @@ -39,6 +39,7 @@ module PermittedAttributes :preferred_product_low_stock_display, :hide_ofn_navigation, :white_label_logo, :white_label_logo_link, :hide_groups_tab, :external_billing_id, + :enable_producers_to_edit_orders ] end end diff --git a/app/views/admin/enterprises/form/_shop_preferences.html.haml b/app/views/admin/enterprises/form/_shop_preferences.html.haml index 33f4a5f792..096753b879 100644 --- a/app/views/admin/enterprises/form/_shop_preferences.html.haml +++ b/app/views/admin/enterprises/form/_shop_preferences.html.haml @@ -123,3 +123,17 @@ .five.columns.omega = f.radio_button :show_customer_contacts_to_suppliers, false = f.label :show_customer_contacts_to_suppliers, t('.customer_contacts_false'), value: :false + = radio_button :enterprise, :show_customer_names_to_suppliers, false + = label :enterprise_show_customer_names_to_suppliers, t('.customer_names_false'), value: :false + +.row + .three.columns.alpha + %label= t('.producers_to_edit_orders') + %div{'ofn-with-tip' => t('.producers_to_edit_orders_tip')} + %a= t 'admin.whats_this' + .three.columns + = radio_button :enterprise, :enable_producers_to_edit_orders, true + = label :enterprise_enable_producers_to_edit_orders, t('.producers_edit_orders_true'), value: :true + .five.columns.omega + = radio_button :enterprise, :enable_producers_to_edit_orders, false + = label :enterprise_enable_producers_to_edit_orders, t('.producers_edit_orders_false'), value: :false diff --git a/config/locales/en.yml b/config/locales/en.yml index f0d5938f2f..3e49a801f9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1348,12 +1348,16 @@ en: enable_subscriptions_true: "Enabled" customer_names_in_reports: "Customer Names in Reports" customer_names_tip: "Enable your suppliers to see your customers names in reports" + producers_to_edit_orders: "Ability for producers to edit orders" + producers_to_edit_orders_tip: "Enable your suppliers to see orders containing their products, and edit quantity and weight for their own products only." customer_names_false: "Disabled" customer_names_true: "Enabled" customer_contacts_in_reports: "Customer contact details in reports" customer_contacts_tip: "Enable your suppliers to see your customer email and phone numbers in reports" customer_contacts_false: "Disabled" customer_contacts_true: "Enabled" + producers_edit_orders_false: "Disabled" + producers_edit_orders_true: "Enabled" shopfront_message: "Shopfront Message" shopfront_message_placeholder: > An optional message to welcome customers and explain how to shop with you. If text is entered here it will be displayed in a home tab when customers first arrive at your shopfront. diff --git a/db/migrate/20250202121858_add_enable_producers_to_edit_orders_to_enterprises.rb b/db/migrate/20250202121858_add_enable_producers_to_edit_orders_to_enterprises.rb new file mode 100644 index 0000000000..842427aca4 --- /dev/null +++ b/db/migrate/20250202121858_add_enable_producers_to_edit_orders_to_enterprises.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddEnableProducersToEditOrdersToEnterprises < ActiveRecord::Migration[7.0] + def change + add_column :enterprises, :enable_producers_to_edit_orders, :boolean, default: false, null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index fe7789113b..ad2af62ff7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -230,7 +230,6 @@ ActiveRecord::Schema[7.0].define(version: 2025_03_04_234657) do t.text "white_label_logo_link" t.boolean "hide_groups_tab", default: false t.string "external_billing_id", limit: 128 - t.boolean "show_customer_contacts_to_suppliers", default: false, null: false t.index ["address_id"], name: "index_enterprises_on_address_id" t.index ["is_primary_producer", "sells"], name: "index_enterprises_on_is_primary_producer_and_sells" t.index ["name"], name: "index_enterprises_on_name", unique: true From 0a61910cf68e7b46ff1e958ca5db94cf9aab9ac5 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Sun, 16 Feb 2025 01:38:35 +0500 Subject: [PATCH 02/26] add ability to view supplier products containing orders --- app/models/enterprise.rb | 4 ++++ app/models/spree/ability.rb | 23 ++++++++++++++++++-- app/models/spree/order.rb | 8 +++++++ app/models/spree/user.rb | 16 ++++++++++++++ app/services/permissions/order.rb | 20 ++++++++++++++--- app/views/spree/admin/orders/index.html.haml | 7 +++--- 6 files changed, 70 insertions(+), 8 deletions(-) diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index b369712fb1..d0f7c30ced 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -381,6 +381,10 @@ class Enterprise < ApplicationRecord sells == 'any' end + def is_producer + is_primary_producer && sells == 'none' + end + # Simplify enterprise categories for frontend logic and icons, and maybe other things. def category # Make this crazy logic human readable so we can argue about it sanely. diff --git a/app/models/spree/ability.rb b/app/models/spree/ability.rb index 3bce37ff4d..a14bd45d0b 100644 --- a/app/models/spree/ability.rb +++ b/app/models/spree/ability.rb @@ -47,7 +47,11 @@ module Spree add_group_management_abilities user if can_manage_groups? user add_product_management_abilities user if can_manage_products? user add_order_cycle_management_abilities user if can_manage_order_cycles? user - add_order_management_abilities user if can_manage_orders? user + if can_manage_orders? user + add_order_management_abilities user + elsif can_manage_line_items_in_orders? user + add_manage_line_items_abilities user + end add_relationship_management_abilities user if can_manage_relationships? user end @@ -81,7 +85,13 @@ module Spree # Users can manage orders if they have a sells own/any enterprise. def can_manage_orders?(user) - ( user.enterprises.map(&:sells) & %w(own any) ).any? + user.can_manage_orders? + end + + # Users can manage line items in orders if they have producer enterprise and + # any of order distributors allow them to edit their orders. + def can_manage_line_items_in_orders?(user) + user.can_manage_line_items_in_orders? end def can_manage_relationships?(user) @@ -343,6 +353,15 @@ module Spree end end + def add_manage_line_items_abilities(user) + can [:admin, :read, :index], Spree::Order do |order| + if order.distributor&.enable_producers_to_edit_orders + user_enterprises_ids = user.enterprises.ids + order.variants.any? { |variant| user_enterprises_ids.include?(variant.supplier_id) } + end + end + end + def add_relationship_management_abilities(user) can [:admin, :index, :create], EnterpriseRelationship can [:destroy], EnterpriseRelationship do |enterprise_relationship| diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 10dc76c2f0..8b95992855 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -171,6 +171,14 @@ module Spree scope :invoiceable, -> { where(state: [:complete, :resumed]) } scope :by_state, lambda { |state| where(state:) } scope :not_state, lambda { |state| where.not(state:) } + scope :editable_by_producers, ->(enterprises) { + joins( + :distributor, line_items: :supplier + ).where( + supplier: { id: enterprises.ids }, + distributor: { enable_producers_to_edit_orders: true } + ) + } def initialize(*_args) @checkout_processing = nil diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb index 0bbe9d60f2..a4de77ca00 100644 --- a/app/models/spree/user.rb +++ b/app/models/spree/user.rb @@ -147,6 +147,22 @@ module Spree Enterprise.joins(:connected_apps).merge(ConnectedApps::AffiliateSalesData.ready) end + # Users can manage orders if they have a sells own/any enterprise. or is admin + def can_manage_orders? + @can_manage_orders ||= (enterprises.pluck(:sells).intersect?(%w(own any)) or admin?) + end + + # Users can manage line items in orders if they have producer enterprise and + # any of order distributors allow them to edit their orders. + def can_manage_line_items_in_orders? + @can_manage_line_items_in_orders ||= begin + has_any_producer = enterprises.any?(&:is_producer) + has_producer_editable_orders = Spree::Order.editable_by_producers(enterprises).exists? + has_any_producer && has_producer_editable_orders + end + end + + protected def password_required? diff --git a/app/services/permissions/order.rb b/app/services/permissions/order.rb index d4c2f4b0ea..8af71079de 100644 --- a/app/services/permissions/order.rb +++ b/app/services/permissions/order.rb @@ -23,9 +23,16 @@ module Permissions # Any orders that the user can edit def editable_orders - orders = Spree::Order. - where(managed_orders_where_values. - or(coordinated_orders_where_values)) + orders = if @user.can_manage_line_items_in_orders_only? + Spree::Order.joins(:distributor).where( + id: produced_orders.select(:id), + distributor: { enable_producers_to_edit_orders: true } + ) + else + Spree::Order.where( + managed_orders_where_values.or(coordinated_orders_where_values) + ) + end filtered_orders(orders) end @@ -79,6 +86,13 @@ module Permissions reduce(:and) end + def produced_orders + Spree::Order.with_line_items_variants_and_products_outer. + where( + spree_variants: { supplier_id: @permissions.managed_enterprises.select("enterprises.id") } + ) + end + def produced_orders_where_values Spree::Order.with_line_items_variants_and_products_outer. where( diff --git a/app/views/spree/admin/orders/index.html.haml b/app/views/spree/admin/orders/index.html.haml index 26a7965a69..0deea9b1ab 100644 --- a/app/views/spree/admin/orders/index.html.haml +++ b/app/views/spree/admin/orders/index.html.haml @@ -3,9 +3,10 @@ - content_for :minimal_js, true -- content_for :page_actions do - %li - = button_link_to t('.new_order'), spree.new_admin_order_url, icon: 'icon-plus', id: 'admin_new_order' +- if can?(:create, Spree::Order) + - content_for :page_actions do + %li + = button_link_to t('.new_order'), spree.new_admin_order_url, icon: 'icon-plus', id: 'admin_new_order' = render partial: 'spree/admin/shared/order_sub_menu' From 19c5fec9a94942eb4406b99f806ff8f209e92650 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Sun, 16 Feb 2025 01:49:09 +0500 Subject: [PATCH 03/26] add ability update supplier line_items in orders --- app/helpers/spree/admin/orders_helper.rb | 17 ++++++++++++++++ app/models/spree/ability.rb | 20 ++++++++++++++----- app/models/spree/line_item.rb | 7 +++++++ app/models/spree/user.rb | 3 +++ app/services/permissions/order.rb | 8 +++++++- .../admin/orders/_shipment_manifest.html.haml | 2 +- 6 files changed, 50 insertions(+), 7 deletions(-) diff --git a/app/helpers/spree/admin/orders_helper.rb b/app/helpers/spree/admin/orders_helper.rb index fc4f21baa8..65ac3021fc 100644 --- a/app/helpers/spree/admin/orders_helper.rb +++ b/app/helpers/spree/admin/orders_helper.rb @@ -142,6 +142,23 @@ module Spree end number_field_tag :quantity, manifest_item.quantity, html_options end + + def prepare_shipment_manifest(shipment) + manifest = shipment.manifest + + if distributor_allows_order_editing?(shipment.order) + supplier_ids = spree_current_user.enterprises.ids + manifest.select! { |mi| supplier_ids.include?(mi.variant.supplier_id) } + end + + manifest + end + + def distributor_allows_order_editing?(order) + order.distributor&.enable_producers_to_edit_orders && + spree_current_user.can_manage_line_items_in_orders_only? + end + end end end diff --git a/app/models/spree/ability.rb b/app/models/spree/ability.rb index a14bd45d0b..2572697ca9 100644 --- a/app/models/spree/ability.rb +++ b/app/models/spree/ability.rb @@ -354,12 +354,22 @@ module Spree end def add_manage_line_items_abilities(user) - can [:admin, :read, :index], Spree::Order do |order| - if order.distributor&.enable_producers_to_edit_orders - user_enterprises_ids = user.enterprises.ids - order.variants.any? { |variant| user_enterprises_ids.include?(variant.supplier_id) } - end + can_edit_order_lambda = lambda do |order| + return unless order.distributor&.enable_producers_to_edit_orders + + order.variants.any? { |variant| user.enterprises.ids.include?(variant.supplier_id) } end + + can [:admin, :read, :index, :edit, :update], Spree::Order do |order| + can_edit_order_lambda.call(order) + end + can [:admin, :index, :create, :destroy, :update], Spree::LineItem do |item| + can_edit_order_lambda.call(item.order) + end + can [:index, :create, :add, :read, :edit, :update], Spree::Shipment do |shipment| + can_edit_order_lambda.call(shipment.order) + end + end def add_relationship_management_abilities(user) diff --git a/app/models/spree/line_item.rb b/app/models/spree/line_item.rb index b2913e331f..2d9444e18d 100644 --- a/app/models/spree/line_item.rb +++ b/app/models/spree/line_item.rb @@ -108,6 +108,13 @@ module Spree where(spree_adjustments: { id: nil }) } + scope :editable_by_producers, ->(enterprises_ids) { + joins(:variant, order: :distributor).where( + distributor: { enable_producers_to_edit_orders: true }, + spree_variants: { supplier_id: enterprises_ids } + ) + } + def copy_price return unless variant diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb index a4de77ca00..f174722b36 100644 --- a/app/models/spree/user.rb +++ b/app/models/spree/user.rb @@ -162,6 +162,9 @@ module Spree end end + def can_manage_line_items_in_orders_only? + !can_manage_orders? && can_manage_line_items_in_orders? + end protected diff --git a/app/services/permissions/order.rb b/app/services/permissions/order.rb index 8af71079de..29d63b5a2f 100644 --- a/app/services/permissions/order.rb +++ b/app/services/permissions/order.rb @@ -43,7 +43,13 @@ module Permissions # Any line items that I can edit def editable_line_items - Spree::LineItem.where(order_id: editable_orders.select(:id)) + if @user.can_manage_line_items_in_orders_only? + Spree::LineItem.editable_by_producers( + @permissions.managed_enterprises.select("enterprises.id") + ) + else + Spree::LineItem.where(order_id: editable_orders.select(:id)) + end end private diff --git a/app/views/spree/admin/orders/_shipment_manifest.html.haml b/app/views/spree/admin/orders/_shipment_manifest.html.haml index fd95d95dbc..d08cf371c9 100644 --- a/app/views/spree/admin/orders/_shipment_manifest.html.haml +++ b/app/views/spree/admin/orders/_shipment_manifest.html.haml @@ -1,4 +1,4 @@ -- shipment.manifest.each do |item| +- prepare_shipment_manifest(shipment).each do |item| - line_item = order.find_line_item_by_variant(item.variant) - if line_item.present? From d9308799b00dfa04130732f4298859ec189146f4 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Sun, 16 Feb 2025 01:51:13 +0500 Subject: [PATCH 04/26] add ability search supplier products in orders --- app/controllers/spree/admin/variants_controller.rb | 5 ++++- lib/open_food_network/scope_variants_for_search.rb | 10 ++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/app/controllers/spree/admin/variants_controller.rb b/app/controllers/spree/admin/variants_controller.rb index 765a011dff..deda271867 100644 --- a/app/controllers/spree/admin/variants_controller.rb +++ b/app/controllers/spree/admin/variants_controller.rb @@ -64,7 +64,10 @@ module Spree end def search - scoper = OpenFoodNetwork::ScopeVariantsForSearch.new(variant_search_params) + scoper = OpenFoodNetwork::ScopeVariantsForSearch.new( + variant_search_params, + spree_current_user + ) @variants = scoper.search render json: @variants, each_serializer: ::Api::Admin::VariantSerializer end diff --git a/lib/open_food_network/scope_variants_for_search.rb b/lib/open_food_network/scope_variants_for_search.rb index 7c51cd8b61..465e501c10 100644 --- a/lib/open_food_network/scope_variants_for_search.rb +++ b/lib/open_food_network/scope_variants_for_search.rb @@ -9,8 +9,9 @@ require 'open_food_network/scope_variant_to_hub' module OpenFoodNetwork class ScopeVariantsForSearch - def initialize(params) + def initialize(params, spree_current_user) @params = params + @spree_current_user = spree_current_user end def search @@ -20,13 +21,14 @@ module OpenFoodNetwork scope_to_schedule if params[:schedule_id] scope_to_order_cycle if params[:order_cycle_id] scope_to_distributor if params[:distributor_id] + scope_to_supplier if spree_current_user.can_manage_line_items_in_orders_only? @variants end private - attr_reader :params + attr_reader :params, :spree_current_user def search_params { product_name_cont: params[:q], sku_cont: params[:q], product_sku_cont: params[:q] } @@ -96,5 +98,9 @@ module OpenFoodNetwork # Filtering could be a problem on scoped variants. variants.each { |v| scoper.scope(v) } end + + def scope_to_supplier + @variants = @variants.where(supplier_id: spree_current_user.enterprises.ids) + end end end From 3e71f8293c940b2f680a0e2875d3b09288fd311e Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Sun, 16 Feb 2025 01:53:47 +0500 Subject: [PATCH 05/26] add bulk_management ability --- app/models/spree/ability.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/spree/ability.rb b/app/models/spree/ability.rb index 2572697ca9..14a895bd7c 100644 --- a/app/models/spree/ability.rb +++ b/app/models/spree/ability.rb @@ -360,7 +360,7 @@ module Spree order.variants.any? { |variant| user.enterprises.ids.include?(variant.supplier_id) } end - can [:admin, :read, :index, :edit, :update], Spree::Order do |order| + can [:admin, :read, :index, :edit, :update, :bulk_management], Spree::Order do |order| can_edit_order_lambda.call(order) end can [:admin, :index, :create, :destroy, :update], Spree::LineItem do |item| @@ -370,6 +370,7 @@ module Spree can_edit_order_lambda.call(shipment.order) end + can [:visible], Enterprise end def add_relationship_management_abilities(user) From 4bc578f38f755f956b444cecd422cd72ec49b27e Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Sun, 16 Feb 2025 01:54:56 +0500 Subject: [PATCH 06/26] restrict page sections from supplier --- app/views/spree/admin/orders/_form.html.haml | 27 ++++++++++--------- .../spree/admin/orders/_shipment.html.haml | 4 +-- .../spree/admin/orders/_table_row.html.haml | 2 +- app/views/spree/admin/orders/edit.html.haml | 6 +++-- 4 files changed, 21 insertions(+), 18 deletions(-) diff --git a/app/views/spree/admin/orders/_form.html.haml b/app/views/spree/admin/orders/_form.html.haml index 79b86cbbfe..4f1866f330 100644 --- a/app/views/spree/admin/orders/_form.html.haml +++ b/app/views/spree/admin/orders/_form.html.haml @@ -8,23 +8,24 @@ - if @order.shipments.any? = render :partial => "spree/admin/orders/shipment", :collection => @order.shipments, :locals => { :order => @order } - - if @order.line_items.exists? - = render partial: "spree/admin/orders/note", locals: { order: @order } + - if spree_current_user.can_manage_orders? + - if @order.line_items.exists? + = render partial: "spree/admin/orders/note", locals: { order: @order } - = render :partial => "spree/admin/orders/_form/adjustments", :locals => { :adjustments => @order.line_item_adjustments, :title => t(".line_item_adjustments")} - = render :partial => "spree/admin/orders/_form/adjustments", :locals => { :adjustments => order_adjustments_for_display(@order), :title => t(".order_adjustments")} + = render :partial => "spree/admin/orders/_form/adjustments", :locals => { :adjustments => @order.line_item_adjustments, :title => t(".line_item_adjustments")} + = render :partial => "spree/admin/orders/_form/adjustments", :locals => { :adjustments => order_adjustments_for_display(@order), :title => t(".order_adjustments")} - - if @order.line_items.exists? - %fieldset#order-total.no-border-bottom.order-details-total - %legend{ align: 'center' }= t(".order_total") - %span.order-total= @order.display_total + - if @order.line_items.exists? + %fieldset#order-total.no-border-bottom.order-details-total + %legend{ align: 'center' }= t(".order_total") + %span.order-total= @order.display_total - = form_for @order, url: spree.admin_order_url(@order), method: :put do |f| - = render partial: 'spree/admin/orders/_form/distribution_fields' + = form_for @order, url: spree.admin_order_url(@order), method: :put do |f| + = render partial: 'spree/admin/orders/_form/distribution_fields' - .filter-actions.actions{"ng-show" => "distributionChosen()"} - = button t(:update_and_recalculate_fees), 'icon-refresh' - = link_to_with_icon 'button icon-arrow-left', t(:back), spree.admin_orders_url + .filter-actions.actions{"ng-show" => "distributionChosen()"} + = button t(:update_and_recalculate_fees), 'icon-refresh' + = link_to_with_icon 'button icon-arrow-left', t(:back), spree.admin_orders_url = javascript_tag do var order_number = '#{@order.number}'; diff --git a/app/views/spree/admin/orders/_shipment.html.haml b/app/views/spree/admin/orders/_shipment.html.haml index a992fe38fd..6ed1042554 100644 --- a/app/views/spree/admin/orders/_shipment.html.haml +++ b/app/views/spree/admin/orders/_shipment.html.haml @@ -62,7 +62,7 @@ - if shipment.fee_adjustment.present? && shipment.can_modify? %td.actions - - if can? :update, shipment + - if can? :update, shipment.shipping_method = link_to '', '', :class => 'edit-method icon_link icon-edit no-text with-tip', :data => { :action => 'edit' }, :title => Spree.t('edit') %tr.edit-tracking.hidden.total @@ -86,7 +86,7 @@ = Spree.t(:no_tracking_present) %td.actions - - if can?(:update, shipment) && shipment.can_modify? + - if spree_current_user.can_manage_orders? && can?(:update, shipment) && shipment.can_modify? = link_to '', '', :class => 'edit-tracking icon_link icon-edit no-text with-tip', :data => { :action => 'edit' }, :title => Spree.t('edit') - if shipment.tracking.present? = link_to '', '', :class => 'delete-tracking icon_link icon-trash no-text with-tip', :data => { 'shipment-number' => shipment.number, :action => 'remove' }, :title => Spree.t('delete') diff --git a/app/views/spree/admin/orders/_table_row.html.haml b/app/views/spree/admin/orders/_table_row.html.haml index fd202f76d7..fa4916c729 100644 --- a/app/views/spree/admin/orders/_table_row.html.haml +++ b/app/views/spree/admin/orders/_table_row.html.haml @@ -51,5 +51,5 @@ = render ShipOrderComponent.new(order: order) = render partial: 'admin/shared/tooltip_button', locals: {button_class: "icon-road icon_link with-tip no-text", reflex_data_id: order.id.to_s, tooltip_text: t('spree.admin.orders.index.ship'), shipment: true} - - if order.payment_required? && order.pending_payments.reject(&:requires_authorization?).any? + - if can?(:update, Spree::Payment) && order.payment_required? && order.pending_payments.reject(&:requires_authorization?).any? = render partial: 'admin/shared/tooltip_button', locals: {button_class: "icon-capture icon_link no-text", button_reflex: "click->Admin::OrdersReflex#capture", reflex_data_id: order.id.to_s, tooltip_text: t('spree.admin.orders.index.capture')} diff --git a/app/views/spree/admin/orders/edit.html.haml b/app/views/spree/admin/orders/edit.html.haml index d1d0dc73a9..8d9023581d 100644 --- a/app/views/spree/admin/orders/edit.html.haml +++ b/app/views/spree/admin/orders/edit.html.haml @@ -6,14 +6,16 @@ - content_for :page_actions do - if can?(:fire, @order) %li= event_links(@order) - = render partial: 'spree/admin/shared/order_links' + - if spree_current_user.can_manage_orders? + = render partial: 'spree/admin/shared/order_links' - if can?(:admin, Spree::Order) %li %a.button.icon-arrow-left{icon: 'icon-arrow-left', href: admin_orders_path } = t(:back_to_orders_list) = render partial: "spree/admin/shared/order_page_title" -= render partial: "spree/admin/shared/order_tabs", locals: { current: 'Order Details' } +- if spree_current_user.can_manage_orders? + = render partial: "spree/admin/shared/order_tabs", locals: { current: 'Order Details' } %div = render partial: "spree/shared/error_messages", locals: { target: @order } From bc3917ebc1a3e2633ea49283ae41b1c89c101745 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Sun, 16 Feb 2025 01:55:38 +0500 Subject: [PATCH 07/26] incorporate show_customer_names_to_suppliers setting --- app/helpers/spree/admin/orders_helper.rb | 5 +++++ app/views/spree/admin/orders/_table_row.html.haml | 7 ++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/app/helpers/spree/admin/orders_helper.rb b/app/helpers/spree/admin/orders_helper.rb index 65ac3021fc..cd0afab2f7 100644 --- a/app/helpers/spree/admin/orders_helper.rb +++ b/app/helpers/spree/admin/orders_helper.rb @@ -159,6 +159,11 @@ module Spree spree_current_user.can_manage_line_items_in_orders_only? end + def display_value_for_producer(order, value) + return value unless distributor_allows_order_editing?(order) + + order.distributor&.show_customer_names_to_suppliers ? value : t("admin.reports.hidden") + end end end end diff --git a/app/views/spree/admin/orders/_table_row.html.haml b/app/views/spree/admin/orders/_table_row.html.haml index fa4916c729..405efb6857 100644 --- a/app/views/spree/admin/orders/_table_row.html.haml +++ b/app/views/spree/admin/orders/_table_row.html.haml @@ -34,10 +34,11 @@ %span.state{ class: order.shipment_state.to_s} = t('js.admin.orders.shipment_states.' + order.shipment_state.to_s) %td - %a{ href: "mailto:#{order.email}", target: "_blank" } - = order.email + - email_value = display_value_for_producer(order, order.email) + %a{ href: "mailto:#{email_value}", target: "_blank" } + = email_value %td - = order&.bill_address&.full_name_for_sorting + = display_value_for_producer(order, order.bill_address&.full_name_for_sorting) %td.align-left %span = order.display_total From 20146a8e113910aeb913ee852ff07621d4c5bd55 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Sun, 16 Feb 2025 01:56:10 +0500 Subject: [PATCH 08/26] update respective specs --- .../admin/bulk_line_items_controller_spec.rb | 20 +- .../api/v0/orders_controller_spec.rb | 20 +- .../admin/mail_methods_controller_spec.rb | 3 +- .../spree/admin/variants_controller_spec.rb | 273 ++++++++++-------- .../scope_variants_for_search_spec.rb | 15 +- .../orders_and_distributors_report_spec.rb | 1 + spec/models/spree/ability_spec.rb | 19 ++ spec/services/permissions/order_spec.rb | 2 +- .../spree/admin/orders/edit.html.haml_spec.rb | 2 +- 9 files changed, 229 insertions(+), 126 deletions(-) diff --git a/spec/controllers/admin/bulk_line_items_controller_spec.rb b/spec/controllers/admin/bulk_line_items_controller_spec.rb index 89303ca6d4..8c62de79bd 100644 --- a/spec/controllers/admin/bulk_line_items_controller_spec.rb +++ b/spec/controllers/admin/bulk_line_items_controller_spec.rb @@ -123,8 +123,24 @@ RSpec.describe Admin::BulkLineItemsController, type: :controller do get :index, as: :json end - it "does not display line items for which my enterprise is a supplier" do - expect(response).to redirect_to unauthorized_path + context "with no distributor allows to edit orders" do + before { get :index, as: :json } + + it "does not display line items for which my enterprise is a supplier" do + expect(response).to redirect_to unauthorized_path + end + end + + context "with distributor allows to edit orders" do + before do + distributor1.update_columns(enable_producers_to_edit_orders: true) + get :index, as: :json + end + + it "retrieves a list of line_items from the supplier" do + keys = json_response['line_items'].first.keys.map(&:to_sym) + expect(line_item_attributes.all?{ |attr| keys.include? attr }).to eq(true) + end end end diff --git a/spec/controllers/api/v0/orders_controller_spec.rb b/spec/controllers/api/v0/orders_controller_spec.rb index a6d15774fa..558f14696b 100644 --- a/spec/controllers/api/v0/orders_controller_spec.rb +++ b/spec/controllers/api/v0/orders_controller_spec.rb @@ -84,11 +84,25 @@ module Api context 'producer enterprise' do before do allow(controller).to receive(:spree_current_user) { supplier.owner } - get :index end - it "does not display line items for which my enterprise is a supplier" do - assert_unauthorized! + context "with no distributor allows to edit orders" do + before { get :index } + + it "does not display line items for which my enterprise is a supplier" do + assert_unauthorized! + end + end + + context "with distributor allows to edit orders" do + before do + distributor.update_columns(enable_producers_to_edit_orders: true) + get :index + end + + it "retrieves a list of orders which have my supplied products" do + returns_orders(json_response) + end end end diff --git a/spec/controllers/spree/admin/mail_methods_controller_spec.rb b/spec/controllers/spree/admin/mail_methods_controller_spec.rb index 5221204c73..25592aa25b 100644 --- a/spec/controllers/spree/admin/mail_methods_controller_spec.rb +++ b/spec/controllers/spree/admin/mail_methods_controller_spec.rb @@ -22,7 +22,8 @@ RSpec.describe Spree::Admin::MailMethodsController do owned_groups: nil) allow(user).to receive_messages(enterprises: [create(:enterprise)], admin?: true, - locale: nil) + locale: nil, + can_manage_orders?: true) allow(controller).to receive_messages(spree_current_user: user) expect { diff --git a/spec/controllers/spree/admin/variants_controller_spec.rb b/spec/controllers/spree/admin/variants_controller_spec.rb index a443e4d449..6ec3bb6720 100644 --- a/spec/controllers/spree/admin/variants_controller_spec.rb +++ b/spec/controllers/spree/admin/variants_controller_spec.rb @@ -5,151 +5,192 @@ require 'spec_helper' module Spree module Admin RSpec.describe VariantsController, type: :controller do - before { controller_login_as_admin } + context "log in as admin user" do + before { controller_login_as_admin } - describe "#index" do - describe "deleted variants" do - let(:product) { create(:product, name: 'Product A') } - let(:deleted_variant) do - deleted_variant = product.variants.create( - unit_value: "2", variant_unit: "weight", variant_unit_scale: 1, price: 1, - primary_taxon: create(:taxon), supplier: create(:supplier_enterprise) - ) - deleted_variant.delete - deleted_variant - end + describe "#index" do + describe "deleted variants" do + let(:product) { create(:product, name: 'Product A') } + let(:deleted_variant) do + deleted_variant = product.variants.create( + unit_value: "2", variant_unit: "weight", variant_unit_scale: 1, price: 1, + primary_taxon: create(:taxon), supplier: create(:supplier_enterprise) + ) + deleted_variant.delete + deleted_variant + end - it "lists only non-deleted variants with params[:deleted] == off" do - spree_get :index, product_id: product.id, deleted: "off" - expect(assigns(:variants)).to eq(product.variants) - end + it "lists only non-deleted variants with params[:deleted] == off" do + spree_get :index, product_id: product.id, deleted: "off" + expect(assigns(:variants)).to eq(product.variants) + end - it "lists only deleted variants with params[:deleted] == on" do - spree_get :index, product_id: product.id, deleted: "on" - expect(assigns(:variants)).to eq([deleted_variant]) + it "lists only deleted variants with params[:deleted] == on" do + spree_get :index, product_id: product.id, deleted: "on" + expect(assigns(:variants)).to eq([deleted_variant]) + end end end - end - describe "#update" do - let!(:variant) { create(:variant, display_name: "Tomatoes", sku: 123, supplier: producer) } - let(:producer) { create(:enterprise) } + describe "#update" do + let!(:variant) { create(:variant, display_name: "Tomatoes", sku: 123, supplier: producer) } + let(:producer) { create(:enterprise) } - it "updates the variant" do - expect { - spree_put( - :update, - id: variant.id, - product_id: variant.product.id, - variant: { display_name: "Better tomatoes", sku: 456 } - ) - variant.reload - }.to change { variant.display_name }.to("Better tomatoes") - .and change { variant.sku }.to(456.to_s) - end - - context "when updating supplier" do - let(:new_producer) { create(:enterprise) } - - it "updates the supplier" do + it "updates the variant" do expect { + spree_put( + :update, + id: variant.id, + product_id: variant.product.id, + variant: { display_name: "Better tomatoes", sku: 456 } + ) + variant.reload + }.to change { variant.display_name }.to("Better tomatoes") + .and change { variant.sku }.to(456.to_s) + end + + context "when updating supplier" do + let(:new_producer) { create(:enterprise) } + + it "updates the supplier" do + expect { + spree_put( + :update, + id: variant.id, + product_id: variant.product.id, + variant: { supplier_id: new_producer.id } + ) + variant.reload + }.to change { variant.supplier_id }.to(new_producer.id) + end + + it "removes associated product from existing Order Cycles" do + distributor = create(:distributor_enterprise) + order_cycle = create( + :simple_order_cycle, + variants: [variant], + coordinator: distributor, + distributors: [distributor] + ) + spree_put( :update, id: variant.id, product_id: variant.product.id, variant: { supplier_id: new_producer.id } ) - variant.reload - }.to change { variant.supplier_id }.to(new_producer.id) + + expect(order_cycle.reload.distributed_variants).not_to include variant + end + end + end + + describe "#search" do + let(:supplier) { create(:supplier_enterprise) } + let!(:p1) { create(:simple_product, name: 'Product 1', supplier_id: supplier.id) } + let!(:p2) { create(:simple_product, name: 'Product 2', supplier_id: supplier.id) } + let!(:v1) { p1.variants.first } + let!(:v2) { p2.variants.first } + let!(:vo) { create(:variant_override, variant: v1, hub: d, count_on_hand: 44) } + let!(:d) { create(:distributor_enterprise) } + let!(:oc) { create(:simple_order_cycle, distributors: [d], variants: [v1]) } + + it "filters by distributor" do + spree_get :search, q: 'Prod', distributor_id: d.id.to_s + expect(assigns(:variants)).to eq([v1]) end - it "removes associated product from existing Order Cycles" do - distributor = create(:distributor_enterprise) - order_cycle = create( - :simple_order_cycle, - variants: [variant], - coordinator: distributor, - distributors: [distributor] - ) + it "applies variant overrides" do + spree_get :search, q: 'Prod', distributor_id: d.id.to_s + expect(assigns(:variants)).to eq([v1]) + expect(assigns(:variants).first.on_hand).to eq(44) + end - spree_put( - :update, - id: variant.id, - product_id: variant.product.id, - variant: { supplier_id: new_producer.id } - ) + it "filters by order cycle" do + spree_get :search, q: 'Prod', order_cycle_id: oc.id.to_s + expect(assigns(:variants)).to eq([v1]) + end - expect(order_cycle.reload.distributed_variants).not_to include variant + it "does not filter when no distributor or order cycle is specified" do + spree_get :search, q: 'Prod' + expect(assigns(:variants)).to match_array [v1, v2] + end + end + + describe '#destroy' do + let(:variant) { create(:variant) } + + context 'when requesting with html' do + before do + allow(Spree::Variant).to receive(:find).with(variant.id.to_s) { variant } + allow(variant).to receive(:destroy).and_call_original + end + + it 'deletes the variant' do + spree_delete :destroy, id: variant.id, product_id: variant.product.id, + format: 'html' + expect(variant).to have_received(:destroy) + end + + it 'shows a success flash message' do + spree_delete :destroy, id: variant.id, product_id: variant.product.id, + format: 'html' + expect(flash[:success]).to be + end + + it 'redirects to admin_product_variants_url' do + spree_delete :destroy, id: variant.id, product_id: variant.product.id, + format: 'html' + expect(response).to redirect_to spree.admin_product_variants_url(variant.product.id) + end + + it 'destroys all its exchanges' do + exchange = create(:exchange) + variant.exchanges << exchange + + spree_delete :destroy, id: variant.id, product_id: variant.product.id, + format: 'html' + expect(variant.exchanges.reload).to be_empty + end end end end - describe "#search" do - let(:supplier) { create(:supplier_enterprise) } - let!(:p1) { create(:simple_product, name: 'Product 1', supplier_id: supplier.id) } - let!(:p2) { create(:simple_product, name: 'Product 2', supplier_id: supplier.id) } + context "log in as supplier and distributor enable_producers_to_edit_orders" do + let(:supplier1) { create(:supplier_enterprise) } + let(:supplier2) { create(:supplier_enterprise) } + let!(:p1) { create(:simple_product, name: 'Product 1', supplier_id: supplier1.id) } + let!(:p2) { create(:simple_product, name: 'Product 2', supplier_id: supplier2.id) } let!(:v1) { p1.variants.first } let!(:v2) { p2.variants.first } - let!(:vo) { create(:variant_override, variant: v1, hub: d, count_on_hand: 44) } - let!(:d) { create(:distributor_enterprise) } - let!(:oc) { create(:simple_order_cycle, distributors: [d], variants: [v1]) } + let!(:d) { create(:distributor_enterprise, enable_producers_to_edit_orders: true) } + let!(:oc) { create(:simple_order_cycle, distributors: [d], variants: [v1, v2]) } - it "filters by distributor" do - spree_get :search, q: 'Prod', distributor_id: d.id.to_s - expect(assigns(:variants)).to eq([v1]) + before do + order = create(:order_with_line_items, distributor: d, line_items_count: 1) + order.line_items.take.variant.update_attribute(:supplier_id, supplier1.id) + controller_login_as_enterprise_user([supplier1]) end - it "applies variant overrides" do - spree_get :search, q: 'Prod', distributor_id: d.id.to_s - expect(assigns(:variants)).to eq([v1]) - expect(assigns(:variants).first.on_hand).to eq(44) + describe "#search" do + it "filters by distributor and supplier1 products" do + spree_get :search, q: 'Prod', distributor_id: d.id.to_s + expect(assigns(:variants)).to eq([v1]) + end end - it "filters by order cycle" do - spree_get :search, q: 'Prod', order_cycle_id: oc.id.to_s - expect(assigns(:variants)).to eq([v1]) - end - - it "does not filter when no distributor or order cycle is specified" do - spree_get :search, q: 'Prod' - expect(assigns(:variants)).to match_array [v1, v2] - end - end - - describe '#destroy' do - let(:variant) { create(:variant) } - - context 'when requesting with html' do - before do - allow(Spree::Variant).to receive(:find).with(variant.id.to_s) { variant } - allow(variant).to receive(:destroy).and_call_original - end - - it 'deletes the variant' do - spree_delete :destroy, id: variant.id, product_id: variant.product.id, - format: 'html' - expect(variant).to have_received(:destroy) - end - - it 'shows a success flash message' do - spree_delete :destroy, id: variant.id, product_id: variant.product.id, - format: 'html' - expect(flash[:success]).to be - end - - it 'redirects to admin_product_variants_url' do - spree_delete :destroy, id: variant.id, product_id: variant.product.id, - format: 'html' - expect(response).to redirect_to spree.admin_product_variants_url(variant.product.id) - end - - it 'destroys all its exchanges' do - exchange = create(:exchange) - variant.exchanges << exchange - - spree_delete :destroy, id: variant.id, product_id: variant.product.id, - format: 'html' - expect(variant.exchanges.reload).to be_empty + describe "#update" do + it "updates the variant" do + expect { + spree_put( + :update, + id: v1.id, + product_id: v1.product.id, + variant: { display_name: "Better tomatoes", sku: 456 } + ) + v1.reload + }.to change { v1.display_name }.to("Better tomatoes") + .and change { v1.sku }.to(456.to_s) end end end diff --git a/spec/lib/open_food_network/scope_variants_for_search_spec.rb b/spec/lib/open_food_network/scope_variants_for_search_spec.rb index fc16580d15..277aaa6680 100644 --- a/spec/lib/open_food_network/scope_variants_for_search_spec.rb +++ b/spec/lib/open_food_network/scope_variants_for_search_spec.rb @@ -19,8 +19,9 @@ RSpec.describe OpenFoodNetwork::ScopeVariantsForSearch do let!(:oc3) { create(:simple_order_cycle, distributors: [d2], variants: [v4]) } let!(:s1) { create(:schedule, order_cycles: [oc1]) } let!(:s2) { create(:schedule, order_cycles: [oc2]) } + let(:spree_current_user) { create(:user) } - let(:scoper) { OpenFoodNetwork::ScopeVariantsForSearch.new(params) } + let(:scoper) { OpenFoodNetwork::ScopeVariantsForSearch.new(params, spree_current_user) } describe "search" do let(:result) { scoper.search } @@ -66,10 +67,20 @@ RSpec.describe OpenFoodNetwork::ScopeVariantsForSearch do it "returns all products distributed through that distributor" do expect{ result }.to query_database [ + "TRANSACTION", + "Spree::User Exists?", + "Spree::User Create", + "Customer Load", + "Customer Load", + "Spree::Order Load", + "TRANSACTION", "Enterprise Load", "VariantOverride Load", - "SQL" + "SQL", + "Enterprise Pluck", + "Enterprise Load" ] + expect(result).to include v4 expect(result).not_to include v1, v2, v3 end diff --git a/spec/lib/reports/orders_and_distributors_report_spec.rb b/spec/lib/reports/orders_and_distributors_report_spec.rb index 98f4943f29..07c68b55ea 100644 --- a/spec/lib/reports/orders_and_distributors_report_spec.rb +++ b/spec/lib/reports/orders_and_distributors_report_spec.rb @@ -151,6 +151,7 @@ RSpec.describe Reporting::Reports::OrdersAndDistributors::Base do subject # build context first expect { subject.table_rows }.to query_database [ + "Enterprise Pluck", "SQL", "Spree::LineItem Load", "Spree::PaymentMethod Load", diff --git a/spec/models/spree/ability_spec.rb b/spec/models/spree/ability_spec.rb index 1339a39328..6060d7bfc5 100644 --- a/spec/models/spree/ability_spec.rb +++ b/spec/models/spree/ability_spec.rb @@ -240,6 +240,24 @@ RSpec.describe Spree::Ability do it { expect(subject.can_manage_enterprises?(user)).to be true } it { expect(subject.can_manage_orders?(user)).to be false } it { expect(subject.can_manage_order_cycles?(user)).to be false } + + context "with no distributor allows me to edit orders" do + it { expect(subject.can_manage_orders?(user)).to be false } + it { expect(subject.can_manage_line_items_in_orders?(user)).to be false } + end + + context "with any distributor allows me to edit orders containing my product" do + before do + order = create( + :order_with_line_items, + line_items_count: 1, + distributor: create(:distributor_enterprise, enable_producers_to_edit_orders: true) + ) + order.line_items.first.variant.update!(supplier_id: enterprise_none_producer.id) + end + + it { expect(subject.can_manage_line_items_in_orders?(user)).to be true } + end end context "as a profile" do @@ -260,6 +278,7 @@ RSpec.describe Spree::Ability do it { expect(subject.can_manage_products?(user)).to be false } it { expect(subject.can_manage_enterprises?(user)).to be false } it { expect(subject.can_manage_orders?(user)).to be false } + it { expect(subject.can_manage_line_items_in_orders?(user)).to be false } it { expect(subject.can_manage_order_cycles?(user)).to be false } it "can create enterprises straight off the bat" do diff --git a/spec/services/permissions/order_spec.rb b/spec/services/permissions/order_spec.rb index d85777a583..f0cba2275b 100644 --- a/spec/services/permissions/order_spec.rb +++ b/spec/services/permissions/order_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' module Permissions RSpec.describe Order do - let(:user) { double(:user) } + let(:user) { double(:user, can_manage_line_items_in_orders_only?: false) } let(:permissions) { Permissions::Order.new(user) } let!(:basic_permissions) { OpenFoodNetwork::Permissions.new(user) } let(:distributor) { create(:distributor_enterprise) } diff --git a/spec/views/spree/admin/orders/edit.html.haml_spec.rb b/spec/views/spree/admin/orders/edit.html.haml_spec.rb index 644af45b88..ca22a3c226 100644 --- a/spec/views/spree/admin/orders/edit.html.haml_spec.rb +++ b/spec/views/spree/admin/orders/edit.html.haml_spec.rb @@ -21,7 +21,7 @@ RSpec.describe "spree/admin/orders/edit.html.haml" do end end - allow(view).to receive_messages spree_current_user: create(:user) + allow(view).to receive_messages spree_current_user: create(:admin_user) end context "when order is complete" do From b803e18f425adfcf4a11e835c079095ec4834850 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Mon, 17 Feb 2025 04:27:58 +0500 Subject: [PATCH 09/26] fix lint issues --- spec/controllers/spree/admin/variants_controller_spec.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/spec/controllers/spree/admin/variants_controller_spec.rb b/spec/controllers/spree/admin/variants_controller_spec.rb index 6ec3bb6720..dda0c01dab 100644 --- a/spec/controllers/spree/admin/variants_controller_spec.rb +++ b/spec/controllers/spree/admin/variants_controller_spec.rb @@ -33,7 +33,14 @@ module Spree end describe "#update" do - let!(:variant) { create(:variant, display_name: "Tomatoes", sku: 123, supplier: producer) } + let!(:variant) { + create( + :variant, + display_name: "Tomatoes", + sku: 123, + supplier: producer + ) + } let(:producer) { create(:enterprise) } it "updates the variant" do From bf6934db944b523bfa21f57b684451b2678335cb Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Mon, 3 Mar 2025 05:52:26 +0500 Subject: [PATCH 10/26] add specs --- app/services/search_orders.rb | 3 +- app/views/spree/admin/orders/index.html.haml | 2 +- .../scope_variants_for_search_spec.rb | 16 + spec/models/enterprise_spec.rb | 23 ++ spec/models/spree/user_spec.rb | 35 ++ spec/services/permissions/order_spec.rb | 345 ++++++++++-------- .../admin/orders/producer_actions_spec.rb | 164 +++++++++ 7 files changed, 434 insertions(+), 154 deletions(-) create mode 100644 spec/system/admin/orders/producer_actions_spec.rb diff --git a/app/services/search_orders.rb b/app/services/search_orders.rb index e8992d1b90..fe839fafe0 100644 --- a/app/services/search_orders.rb +++ b/app/services/search_orders.rb @@ -28,8 +28,7 @@ class SearchOrders end def search_query - base_query = ::Permissions::Order.new(current_user).editable_orders.not_empty - .or(::Permissions::Order.new(current_user).editable_orders.finalized) + base_query = ::Permissions::Order.new(current_user).editable_orders.not_empty.or(::Permissions::Order.new(current_user).editable_orders.finalized) return base_query if params[:shipping_method_id].blank? diff --git a/app/views/spree/admin/orders/index.html.haml b/app/views/spree/admin/orders/index.html.haml index 0deea9b1ab..26d9c002ac 100644 --- a/app/views/spree/admin/orders/index.html.haml +++ b/app/views/spree/admin/orders/index.html.haml @@ -3,7 +3,7 @@ - content_for :minimal_js, true -- if can?(:create, Spree::Order) +- if can?(:create, Spree::Order) && spree_current_user.can_manage_orders? - content_for :page_actions do %li = button_link_to t('.new_order'), spree.new_admin_order_url, icon: 'icon-plus', id: 'admin_new_order' diff --git a/spec/lib/open_food_network/scope_variants_for_search_spec.rb b/spec/lib/open_food_network/scope_variants_for_search_spec.rb index 277aaa6680..5c72fc4012 100644 --- a/spec/lib/open_food_network/scope_variants_for_search_spec.rb +++ b/spec/lib/open_food_network/scope_variants_for_search_spec.rb @@ -190,6 +190,22 @@ RSpec.describe OpenFoodNetwork::ScopeVariantsForSearch do to eq(["Product 1", "Product a", "Product b", "Product c"]) end end + + context "when search is done by the producer allowing to edit orders" do + let(:params) { { q: "product" } } + let(:producer) { create(:supplier_enterprise) } + let(:spree_current_user) { instance_double('Spree::User', enterprises: Enterprise.where(id: producer.id), can_manage_line_items_in_orders_only?: true) } + + it "returns all products distributed through distributors allowing producers to edit orders" do + v1.supplier_id = producer.id + v2.supplier_id = producer.id + v1.save! + v2.save! + + expect(result).to include v1, v2 + expect(result).not_to include v3, v4 + end + end end private diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb index 1d05f99bd6..4c57b9d69c 100644 --- a/spec/models/enterprise_spec.rb +++ b/spec/models/enterprise_spec.rb @@ -1012,6 +1012,29 @@ RSpec.describe Enterprise do expect(expected).to include(sender) end end + + describe "#is_producer" do + context "when enterprise is_primary_producer and sells none" do + it "returns true" do + enterprise = build(:supplier_enterprise) + expect(enterprise.is_producer).to be true + end + end + + context "when enterprise is_primary_producer and sells any" do + it "returns false" do + enterprise = build(:enterprise, is_primary_producer: true, sells: "any") + expect(enterprise.is_producer).to be false + end + end + + context "when enterprise is_primary_producer and sells own" do + it "returns false" do + enterprise = build(:enterprise, is_primary_producer: true, sells: "own") + expect(enterprise.is_producer).to be false + end + end + end end def enterprise_name_error(owner_email) diff --git a/spec/models/spree/user_spec.rb b/spec/models/spree/user_spec.rb index ea9531f707..dc69933211 100644 --- a/spec/models/spree/user_spec.rb +++ b/spec/models/spree/user_spec.rb @@ -298,4 +298,39 @@ RSpec.describe Spree::User do end end end + + describe "#can_manage_line_items_in_orders_only?" do + let(:producer) { create(:supplier_enterprise) } + let(:order) { create(:order, distributor: distributor) } + + subject { user.can_manage_line_items_in_orders_only? } + + context "when user has producer" do + let(:user) { create(:user, enterprises: [producer]) } + + context "order containing their product" do + before do + order.line_items << create(:line_item, product: create(:product, supplier_id: producer.id)) + end + context "order distributor allow producer to edit orders" do + let(:distributor) do + create(:distributor_enterprise, enable_producers_to_edit_orders: true) + end + + it { is_expected.to be_truthy } + end + + context "order distributor doesn't allow producer to edit orders" do + let(:distributor) { create(:distributor_enterprise) } + it { is_expected.to be_falsey } + end + end + end + + context "no order containing their product" do + let(:user) { create(:user, enterprises: [create(:distributor_enterprise)]) } + + it { is_expected.to be_falsey } + end + end end diff --git a/spec/services/permissions/order_spec.rb b/spec/services/permissions/order_spec.rb index f0cba2275b..fbf727b792 100644 --- a/spec/services/permissions/order_spec.rb +++ b/spec/services/permissions/order_spec.rb @@ -4,7 +4,6 @@ require 'spec_helper' module Permissions RSpec.describe Order do - let(:user) { double(:user, can_manage_line_items_in_orders_only?: false) } let(:permissions) { Permissions::Order.new(user) } let!(:basic_permissions) { OpenFoodNetwork::Permissions.new(user) } let(:distributor) { create(:distributor_enterprise) } @@ -28,68 +27,24 @@ module Permissions before { allow(OpenFoodNetwork::Permissions).to receive(:new) { basic_permissions } } - describe "finding orders that are visible in reports" do - let(:random_enterprise) { create(:distributor_enterprise) } - let(:order) { create(:order, order_cycle:, distributor: ) } - let!(:line_item) { create(:line_item, order:) } - let!(:producer) { create(:supplier_enterprise) } + context "with user cannot only manage line_items in orders" do + let(:user) { instance_double('Spree::User', can_manage_line_items_in_orders_only?: false) } - before do - allow(basic_permissions).to receive(:coordinated_order_cycles) { Enterprise.where("1=0") } - end + describe "finding orders that are visible in reports" do + let(:random_enterprise) { create(:distributor_enterprise) } + let(:order) { create(:order, order_cycle:, distributor: ) } + let!(:line_item) { create(:line_item, order:) } + let!(:producer) { create(:supplier_enterprise) } - context "as the hub through which the order was placed" do before do - allow(basic_permissions).to receive(:managed_enterprises) { - Enterprise.where(id: distributor) - } + allow(basic_permissions).to receive(:coordinated_order_cycles) { Enterprise.where("1=0") } end - it "should let me see the order" do - expect(permissions.visible_orders).to include order - end - end - - context "as the coordinator of the order cycle through which the order was placed" do - before do - allow(basic_permissions).to receive(:managed_enterprises) { - Enterprise.where(id: coordinator) - } - allow(basic_permissions).to receive(:coordinated_order_cycles) { - OrderCycle.where(id: order_cycle) - } - end - - it "should let me see the order" do - expect(permissions.visible_orders).to include order - end - - context "with search params" do - let(:search_params) { { completed_at_gt: Time.zone.now.yesterday.strftime('%Y-%m-%d') } } - let(:permissions) { Permissions::Order.new(user, search_params) } - - it "only returns completed, non-cancelled orders within search filter range" do - expect(permissions.visible_orders).to include order_completed - expect(permissions.visible_orders).not_to include order_cancelled - expect(permissions.visible_orders).not_to include order_cart - expect(permissions.visible_orders).not_to include order_from_last_year - end - end - end - - context "as a producer which has granted P-OC to the distributor of an order" do - before do - allow(basic_permissions).to receive(:managed_enterprises) { - Enterprise.where(id: producer) - } - create(:enterprise_relationship, parent: producer, child: distributor, - permissions_list: [:add_to_order_cycle]) - end - - context "which contains my products" do + context "as the hub through which the order was placed" do before do - line_item.variant.supplier = producer - line_item.variant.save + allow(basic_permissions).to receive(:managed_enterprises) { + Enterprise.where(id: distributor) + } end it "should let me see the order" do @@ -97,118 +52,206 @@ module Permissions end end - context "which does not contain my products" do + context "as the coordinator of the order cycle through which the order was placed" do + before do + allow(basic_permissions).to receive(:managed_enterprises) { + Enterprise.where(id: coordinator) + } + allow(basic_permissions).to receive(:coordinated_order_cycles) { + OrderCycle.where(id: order_cycle) + } + end + + it "should let me see the order" do + expect(permissions.visible_orders).to include order + end + + context "with search params" do + let(:search_params) { { completed_at_gt: Time.zone.now.yesterday.strftime('%Y-%m-%d') } } + let(:permissions) { Permissions::Order.new(user, search_params) } + + it "only returns completed, non-cancelled orders within search filter range" do + expect(permissions.visible_orders).to include order_completed + expect(permissions.visible_orders).not_to include order_cancelled + expect(permissions.visible_orders).not_to include order_cart + expect(permissions.visible_orders).not_to include order_from_last_year + end + end + end + + context "as a producer which has granted P-OC to the distributor of an order" do + before do + allow(basic_permissions).to receive(:managed_enterprises) { + Enterprise.where(id: producer) + } + create(:enterprise_relationship, parent: producer, child: distributor, + permissions_list: [:add_to_order_cycle]) + end + + context "which contains my products" do + before do + line_item.variant.supplier = producer + line_item.variant.save + end + + it "should let me see the order" do + expect(permissions.visible_orders).to include order + end + end + + context "which does not contain my products" do + it "should not let me see the order" do + expect(permissions.visible_orders).not_to include order + end + end + end + + context "as an enterprise that is a distributor in the order cycle, " \ + "but not the distributor of the order" do + before do + allow(basic_permissions).to receive(:managed_enterprises) { + Enterprise.where(id: random_enterprise) + } + end + it "should not let me see the order" do expect(permissions.visible_orders).not_to include order end end end - context "as an enterprise that is a distributor in the order cycle, " \ - "but not the distributor of the order" do + describe "finding line items that are visible in reports" do + let(:random_enterprise) { create(:distributor_enterprise) } + let(:order) { create(:order, order_cycle:, distributor: ) } + let!(:line_item1) { create(:line_item, order:) } + let!(:line_item2) { create(:line_item, order:) } + let!(:producer) { create(:supplier_enterprise) } + before do - allow(basic_permissions).to receive(:managed_enterprises) { - Enterprise.where(id: random_enterprise) - } + allow(basic_permissions).to receive(:coordinated_order_cycles) { Enterprise.where("1=0") } end - it "should not let me see the order" do - expect(permissions.visible_orders).not_to include order + context "as the hub through which the parent order was placed" do + before do + allow(basic_permissions).to receive(:managed_enterprises) { + Enterprise.where(id: distributor) + } + end + + it "should let me see the line_items" do + expect(permissions.visible_line_items).to include line_item1, line_item2 + end + end + + context "as the coordinator of the order cycle through which the parent order was placed" do + before do + allow(basic_permissions).to receive(:managed_enterprises) { + Enterprise.where(id: coordinator) + } + allow(basic_permissions).to receive(:coordinated_order_cycles) { + OrderCycle.where(id: order_cycle) + } + end + + it "should let me see the line_items" do + expect(permissions.visible_line_items).to include line_item1, line_item2 + end + end + + context "as the manager producer which has granted P-OC to the distributor " \ + "of the parent order" do + before do + allow(basic_permissions).to receive(:managed_enterprises) { + Enterprise.where(id: producer) + } + create(:enterprise_relationship, parent: producer, child: distributor, + permissions_list: [:add_to_order_cycle]) + + line_item1.variant.supplier = producer + line_item1.variant.save + end + + it "should let me see the line_items pertaining to variants I produce" do + ps = permissions.visible_line_items + expect(ps).to include line_item1 + expect(ps).not_to include line_item2 + end + end + + context "as an enterprise that is a distributor in the order cycle, " \ + "but not the distributor of the parent order" do + before do + allow(basic_permissions).to receive(:managed_enterprises) { + Enterprise.where(id: random_enterprise) + } + end + + it "should not let me see the line_items" do + expect(permissions.visible_line_items).not_to include line_item1, line_item2 + end + end + + context "with search params" do + let!(:line_item3) { create(:line_item, order: order_completed) } + let!(:line_item4) { create(:line_item, order: order_cancelled) } + let!(:line_item5) { create(:line_item, order: order_cart) } + let!(:line_item6) { create(:line_item, order: order_from_last_year) } + + let(:search_params) { { completed_at_gt: Time.zone.now.yesterday.strftime('%Y-%m-%d') } } + let(:permissions) { Permissions::Order.new(user, search_params) } + + before do + allow(user).to receive(:admin?) { "admin" } + end + + it "only returns line items from completed, " \ + "non-cancelled orders within search filter range" do + expect(permissions.visible_line_items).to include order_completed.line_items.first + expect(permissions.visible_line_items).not_to include order_cancelled.line_items.first + expect(permissions.visible_line_items).not_to include order_cart.line_items.first + expect(permissions.visible_line_items) + .not_to include order_from_last_year.line_items.first + end end end end - describe "finding line items that are visible in reports" do - let(:random_enterprise) { create(:distributor_enterprise) } - let(:order) { create(:order, order_cycle:, distributor: ) } - let!(:line_item1) { create(:line_item, order:) } - let!(:line_item2) { create(:line_item, order:) } - let!(:producer) { create(:supplier_enterprise) } - - before do - allow(basic_permissions).to receive(:coordinated_order_cycles) { Enterprise.where("1=0") } + context "with user can only manage line_items in orders" do + let(:producer) { create(:supplier_enterprise) } + let(:user) do + create(:user, enterprises: [producer]) end + let!(:order_by_distributor_allow_edits) do + order = create( + :order_with_line_items, + distributor: create( + :distributor_enterprise, + enable_producers_to_edit_orders: true + ), + line_items_count: 1 + ) + order.line_items.first.variant.update_attribute(:supplier_id, producer.id) - context "as the hub through which the parent order was placed" do - before do - allow(basic_permissions).to receive(:managed_enterprises) { - Enterprise.where(id: distributor) - } - end - - it "should let me see the line_items" do - expect(permissions.visible_line_items).to include line_item1, line_item2 + order + end + let!(:order_by_distributor_disallow_edits) do + create( + :order_with_line_items, + distributor: create(:distributor_enterprise), + line_items_count: 1 + ) + end + describe "#editable_orders" do + it "returns orders where the distributor allows producers to edit" do + expect(permissions.editable_orders.count).to eq 1 + expect(permissions.editable_orders).to include order_by_distributor_allow_edits end end - context "as the coordinator of the order cycle through which the parent order was placed" do - before do - allow(basic_permissions).to receive(:managed_enterprises) { - Enterprise.where(id: coordinator) - } - allow(basic_permissions).to receive(:coordinated_order_cycles) { - OrderCycle.where(id: order_cycle) - } - end - - it "should let me see the line_items" do - expect(permissions.visible_line_items).to include line_item1, line_item2 - end - end - - context "as the manager producer which has granted P-OC to the distributor " \ - "of the parent order" do - before do - allow(basic_permissions).to receive(:managed_enterprises) { - Enterprise.where(id: producer) - } - create(:enterprise_relationship, parent: producer, child: distributor, - permissions_list: [:add_to_order_cycle]) - - line_item1.variant.supplier = producer - line_item1.variant.save - end - - it "should let me see the line_items pertaining to variants I produce" do - ps = permissions.visible_line_items - expect(ps).to include line_item1 - expect(ps).not_to include line_item2 - end - end - - context "as an enterprise that is a distributor in the order cycle, " \ - "but not the distributor of the parent order" do - before do - allow(basic_permissions).to receive(:managed_enterprises) { - Enterprise.where(id: random_enterprise) - } - end - - it "should not let me see the line_items" do - expect(permissions.visible_line_items).not_to include line_item1, line_item2 - end - end - - context "with search params" do - let!(:line_item3) { create(:line_item, order: order_completed) } - let!(:line_item4) { create(:line_item, order: order_cancelled) } - let!(:line_item5) { create(:line_item, order: order_cart) } - let!(:line_item6) { create(:line_item, order: order_from_last_year) } - - let(:search_params) { { completed_at_gt: Time.zone.now.yesterday.strftime('%Y-%m-%d') } } - let(:permissions) { Permissions::Order.new(user, search_params) } - - before do - allow(user).to receive(:admin?) { "admin" } - end - - it "only returns line items from completed, " \ - "non-cancelled orders within search filter range" do - expect(permissions.visible_line_items).to include order_completed.line_items.first - expect(permissions.visible_line_items).not_to include order_cancelled.line_items.first - expect(permissions.visible_line_items).not_to include order_cart.line_items.first - expect(permissions.visible_line_items) - .not_to include order_from_last_year.line_items.first + describe "#editable_line_items" do + it "returns line items from orders where the distributor allows producers to edit" do + expect(permissions.editable_line_items.count).to eq 1 + expect(permissions.editable_line_items.first.order).to eq order_by_distributor_allow_edits end end end diff --git a/spec/system/admin/orders/producer_actions_spec.rb b/spec/system/admin/orders/producer_actions_spec.rb new file mode 100644 index 0000000000..026bae260a --- /dev/null +++ b/spec/system/admin/orders/producer_actions_spec.rb @@ -0,0 +1,164 @@ +# frozen_string_literal: true + +require 'system_helper' + +RSpec.describe 'As a producer who have the ability to update orders' do + include AdminHelper + include AuthenticationHelper + include WebHelper + + let!(:supplier1) { create(:supplier_enterprise, name: 'My supplier1') } + let!(:supplier2) { create(:supplier_enterprise, name: 'My supplier2') } + let!(:supplier1_v1) { create(:variant, supplier_id: supplier1.id) } + let!(:supplier1_v2) { create(:variant, supplier_id: supplier1.id) } + let!(:supplier2_v1) { create(:variant, supplier_id: supplier2.id) } + let(:order_cycle) do + create(:simple_order_cycle, distributors: [distributor], variants: [supplier1_v1, supplier1_v2]) + end + let!(:order_containing_supplier1_products) do + o = create( + :completed_order_with_totals, + distributor:, order_cycle:, + user: supplier1_ent_user, line_items_count: 1 + ) + o.line_items.first.update_columns(variant_id: supplier1_v1.id) + o + end + let!(:order_containing_supplier2_v1_products) do + o = create( + :completed_order_with_totals, + distributor:, order_cycle:, + user: supplier2_ent_user, line_items_count: 1 + ) + o.line_items.first.update_columns(variant_id: supplier2_v1.id) + o + end + let(:supplier1_ent_user) { create(:user, enterprises: [supplier1]) } + let(:supplier2_ent_user) { create(:user, enterprises: [supplier2]) } + + context "As supplier1 enterprise user" do + before { login_as(supplier1_ent_user) } + let(:order) { order_containing_supplier1_products } + let(:user) { supplier1_ent_user } + + describe 'orders index page' do + before { visit spree.admin_orders_path } + + context "when no distributor allow the producer to edit orders" do + let(:distributor) { create(:distributor_enterprise) } + + it "should not allow producer to view orders page" do + expect(page).to have_content 'Unauthorized' + end + end + + context "when distributor allows the producer to edit orders" do + let(:distributor) { create(:distributor_enterprise, enable_producers_to_edit_orders: true) } + it "should not allow to add new orders" do + expect(page).not_to have_link('New Order') + end + + context "when distributor doesn't allow to view customer details" do + it "should allow producer to view orders page with HIDDEN customer details" do + within('#listing_orders tbody') do + expect(page).to have_selector('tr', count: 1) # Only one order + # One for Email, one for Name + expect(page).to have_selector('td', text: 'HIDDEN', count: 2) + end + end + end + + context "when distributor allows to view customer details" do + let(:distributor) do + create( + :distributor_enterprise, + enable_producers_to_edit_orders: true, + show_customer_names_to_suppliers: true + ) + end + it "should allow producer to view orders page with customer details" do + within('#listing_orders tbody') do + name = order.bill_address&.full_name_for_sorting + email = order.email + expect(page).to have_selector('tr', count: 1) # Only one order + expect(page).to have_selector('td', text: name, count: 1) + expect(page).to have_selector('td', text: email, count: 1) + end + end + end + end + end + + describe 'orders edit page' do + before { visit spree.edit_admin_order_path(order) } + + context "when no distributor allow the producer to edit orders" do + let(:distributor) { create(:distributor_enterprise) } + + it "should not allow producer to view orders page" do + expect(page).to have_content 'Unauthorized' + end + end + + context "when distributor allows to edit orders" do + let(:distributor) { create(:distributor_enterprise, enable_producers_to_edit_orders: true) } + let(:product) { supplier1_v2.product } + + it "should allow me to manage my products in the order" do + expect(page).to have_content 'Add Product' + + # Add my product + add_product(product) + expect_product_change(product, :add) + + # Edit my product + edit_product(product) + expect_product_change(product, :update, 2) + + # Delete my product + delete_product(product) + expect_product_change(product, :remove) + end + end + + + def expect_product_change(product, action, expected_qty = 0) + within('table.index') do + # JS for this page sometimes take more than 2 seconds (default timeout for cappybara) + timeout = 5 + item_name_selector = 'tbody tr td.item-name' + quantity_selector = 'tbody tr td.item-qty-show' + + case action + when :add + expect(page).to have_selector item_name_selector, text: product.name, wait: timeout + when :update + expect(page).to have_selector quantity_selector, text: expected_qty, wait: timeout + when :remove + expect(page).not_to have_selector item_name_selector, text: product.name, wait: timeout + else + raise 'Invalid action' + end + end + end + + def add_product(product) + select2_select product.name, from: 'add_variant_id', search: true + find('button.add_variant').click + end + + def edit_product(product) + find('a.edit-item.icon_link.icon-edit.no-text.with-tip').click + fill_in 'quantity', with: 2 + find("a[data-variant-id='#{product.variants.last.id}'][data-action='save']").click + end + + def delete_product(product) + find("a[data-variant-id='#{product.variants.last.id}'][data-action='remove']").click + click_button 'OK' + end + + end + end + +end From ccfd0edbf163fa91e07a8cc3b72b73d50fa4c392 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Mon, 3 Mar 2025 06:15:09 +0500 Subject: [PATCH 11/26] fix lint issues --- app/models/spree/ability.rb | 19 +++++++++---------- app/services/search_orders.rb | 3 ++- .../scope_variants_for_search_spec.rb | 7 +++++-- spec/models/spree/user_spec.rb | 5 +++-- spec/services/permissions/order_spec.rb | 4 +++- .../admin/orders/producer_actions_spec.rb | 3 --- 6 files changed, 22 insertions(+), 19 deletions(-) diff --git a/app/models/spree/ability.rb b/app/models/spree/ability.rb index 14a895bd7c..ce87891d13 100644 --- a/app/models/spree/ability.rb +++ b/app/models/spree/ability.rb @@ -353,23 +353,22 @@ module Spree end end + def can_edit_order(order, user) + return unless order.distributor&.enable_producers_to_edit_orders + + order.variants.any? { |variant| user.enterprises.ids.include?(variant.supplier_id) } + end + def add_manage_line_items_abilities(user) - can_edit_order_lambda = lambda do |order| - return unless order.distributor&.enable_producers_to_edit_orders - - order.variants.any? { |variant| user.enterprises.ids.include?(variant.supplier_id) } - end - can [:admin, :read, :index, :edit, :update, :bulk_management], Spree::Order do |order| - can_edit_order_lambda.call(order) + can_edit_order(order, user) end can [:admin, :index, :create, :destroy, :update], Spree::LineItem do |item| - can_edit_order_lambda.call(item.order) + can_edit_order(item.order, user) end can [:index, :create, :add, :read, :edit, :update], Spree::Shipment do |shipment| - can_edit_order_lambda.call(shipment.order) + can_edit_order(shipment.order, user) end - can [:visible], Enterprise end diff --git a/app/services/search_orders.rb b/app/services/search_orders.rb index fe839fafe0..e8992d1b90 100644 --- a/app/services/search_orders.rb +++ b/app/services/search_orders.rb @@ -28,7 +28,8 @@ class SearchOrders end def search_query - base_query = ::Permissions::Order.new(current_user).editable_orders.not_empty.or(::Permissions::Order.new(current_user).editable_orders.finalized) + base_query = ::Permissions::Order.new(current_user).editable_orders.not_empty + .or(::Permissions::Order.new(current_user).editable_orders.finalized) return base_query if params[:shipping_method_id].blank? diff --git a/spec/lib/open_food_network/scope_variants_for_search_spec.rb b/spec/lib/open_food_network/scope_variants_for_search_spec.rb index 5c72fc4012..6e55c99db7 100644 --- a/spec/lib/open_food_network/scope_variants_for_search_spec.rb +++ b/spec/lib/open_food_network/scope_variants_for_search_spec.rb @@ -194,9 +194,12 @@ RSpec.describe OpenFoodNetwork::ScopeVariantsForSearch do context "when search is done by the producer allowing to edit orders" do let(:params) { { q: "product" } } let(:producer) { create(:supplier_enterprise) } - let(:spree_current_user) { instance_double('Spree::User', enterprises: Enterprise.where(id: producer.id), can_manage_line_items_in_orders_only?: true) } + let(:spree_current_user) { + instance_double('Spree::User', enterprises: Enterprise.where(id: producer.id), + can_manage_line_items_in_orders_only?: true) + } - it "returns all products distributed through distributors allowing producers to edit orders" do + it "returns products distributed by distributors allowing producers to edit orders" do v1.supplier_id = producer.id v2.supplier_id = producer.id v1.save! diff --git a/spec/models/spree/user_spec.rb b/spec/models/spree/user_spec.rb index dc69933211..6d123b6990 100644 --- a/spec/models/spree/user_spec.rb +++ b/spec/models/spree/user_spec.rb @@ -301,7 +301,7 @@ RSpec.describe Spree::User do describe "#can_manage_line_items_in_orders_only?" do let(:producer) { create(:supplier_enterprise) } - let(:order) { create(:order, distributor: distributor) } + let(:order) { create(:order, distributor:) } subject { user.can_manage_line_items_in_orders_only? } @@ -310,7 +310,8 @@ RSpec.describe Spree::User do context "order containing their product" do before do - order.line_items << create(:line_item, product: create(:product, supplier_id: producer.id)) + order.line_items << create(:line_item, + product: create(:product, supplier_id: producer.id)) end context "order distributor allow producer to edit orders" do let(:distributor) do diff --git a/spec/services/permissions/order_spec.rb b/spec/services/permissions/order_spec.rb index fbf727b792..5f7246d75b 100644 --- a/spec/services/permissions/order_spec.rb +++ b/spec/services/permissions/order_spec.rb @@ -67,7 +67,9 @@ module Permissions end context "with search params" do - let(:search_params) { { completed_at_gt: Time.zone.now.yesterday.strftime('%Y-%m-%d') } } + let(:search_params) { + { completed_at_gt: Time.zone.now.yesterday.strftime('%Y-%m-%d') } + } let(:permissions) { Permissions::Order.new(user, search_params) } it "only returns completed, non-cancelled orders within search filter range" do diff --git a/spec/system/admin/orders/producer_actions_spec.rb b/spec/system/admin/orders/producer_actions_spec.rb index 026bae260a..bfebbcfee3 100644 --- a/spec/system/admin/orders/producer_actions_spec.rb +++ b/spec/system/admin/orders/producer_actions_spec.rb @@ -121,7 +121,6 @@ RSpec.describe 'As a producer who have the ability to update orders' do end end - def expect_product_change(product, action, expected_qty = 0) within('table.index') do # JS for this page sometimes take more than 2 seconds (default timeout for cappybara) @@ -157,8 +156,6 @@ RSpec.describe 'As a producer who have the ability to update orders' do find("a[data-variant-id='#{product.variants.last.id}'][data-action='remove']").click click_button 'OK' end - end end - end From 9867d0bc3a1a74a15415ff3c05d6fd8da3f4d280 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Mon, 3 Mar 2025 07:04:40 +0500 Subject: [PATCH 12/26] fix flaky spec error: Ferrum::BrowserError: Argument should belong to the same JavaScript world as target object --- spec/system/admin/orders/producer_actions_spec.rb | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/spec/system/admin/orders/producer_actions_spec.rb b/spec/system/admin/orders/producer_actions_spec.rb index bfebbcfee3..f62067086b 100644 --- a/spec/system/admin/orders/producer_actions_spec.rb +++ b/spec/system/admin/orders/producer_actions_spec.rb @@ -122,19 +122,17 @@ RSpec.describe 'As a producer who have the ability to update orders' do end def expect_product_change(product, action, expected_qty = 0) - within('table.index') do - # JS for this page sometimes take more than 2 seconds (default timeout for cappybara) - timeout = 5 - item_name_selector = 'tbody tr td.item-name' - quantity_selector = 'tbody tr td.item-qty-show' + # JS for this page sometimes take more than 2 seconds (default timeout for cappybara) + timeout = 5 + within('table.index tbody tr', wait: timeout) do case action when :add - expect(page).to have_selector item_name_selector, text: product.name, wait: timeout + expect(page).to have_text(product.name, wait: timeout) when :update - expect(page).to have_selector quantity_selector, text: expected_qty, wait: timeout + expect(page).to have_text(expected_qty.to_s, wait: timeout) when :remove - expect(page).not_to have_selector item_name_selector, text: product.name, wait: timeout + expect(page).not_to have_text(product.name, wait: timeout) else raise 'Invalid action' end From f132d99df463428b743eebbe71d5b187a6aef6c8 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Wed, 5 Mar 2025 03:27:00 +0500 Subject: [PATCH 13/26] remove ids usage --- app/models/spree/order.rb | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 8b95992855..9d9bd0ac7e 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -140,6 +140,15 @@ module Spree end } + scope :editable_by_producers, ->(enterprises) { + joins( + :distributor, line_items: :supplier + ).where( + supplier: enterprises, + distributor: { enable_producers_to_edit_orders: true } + ) + } + scope :distributed_by_user, lambda { |user| if user.admin? where(nil) @@ -171,14 +180,6 @@ module Spree scope :invoiceable, -> { where(state: [:complete, :resumed]) } scope :by_state, lambda { |state| where(state:) } scope :not_state, lambda { |state| where.not(state:) } - scope :editable_by_producers, ->(enterprises) { - joins( - :distributor, line_items: :supplier - ).where( - supplier: { id: enterprises.ids }, - distributor: { enable_producers_to_edit_orders: true } - ) - } def initialize(*_args) @checkout_processing = nil From adbdc64c13f2121d6e810bd3a11f2c38a526252d Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Wed, 5 Mar 2025 03:28:44 +0500 Subject: [PATCH 14/26] remove subquery for optimization --- app/services/permissions/order.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/services/permissions/order.rb b/app/services/permissions/order.rb index 29d63b5a2f..aa6610ddd4 100644 --- a/app/services/permissions/order.rb +++ b/app/services/permissions/order.rb @@ -24,8 +24,7 @@ module Permissions # Any orders that the user can edit def editable_orders orders = if @user.can_manage_line_items_in_orders_only? - Spree::Order.joins(:distributor).where( - id: produced_orders.select(:id), + produced_orders.joins(:distributor).where( distributor: { enable_producers_to_edit_orders: true } ) else From a4f1f542bee0d6ac63bbe6790d1d6d48d34116c9 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Wed, 5 Mar 2025 03:29:58 +0500 Subject: [PATCH 15/26] rename distributor_allows_order_editing to filter_by_supplier --- app/helpers/spree/admin/orders_helper.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/helpers/spree/admin/orders_helper.rb b/app/helpers/spree/admin/orders_helper.rb index cd0afab2f7..58e1af234f 100644 --- a/app/helpers/spree/admin/orders_helper.rb +++ b/app/helpers/spree/admin/orders_helper.rb @@ -146,7 +146,7 @@ module Spree def prepare_shipment_manifest(shipment) manifest = shipment.manifest - if distributor_allows_order_editing?(shipment.order) + if filter_by_supplier?(shipment.order) supplier_ids = spree_current_user.enterprises.ids manifest.select! { |mi| supplier_ids.include?(mi.variant.supplier_id) } end @@ -154,13 +154,13 @@ module Spree manifest end - def distributor_allows_order_editing?(order) + def filter_by_supplier?(order) order.distributor&.enable_producers_to_edit_orders && spree_current_user.can_manage_line_items_in_orders_only? end def display_value_for_producer(order, value) - return value unless distributor_allows_order_editing?(order) + return value unless filter_by_supplier?(order) order.distributor&.show_customer_names_to_suppliers ? value : t("admin.reports.hidden") end From 0500294480b8a86267ab04e2b619ab303690a931 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Wed, 5 Mar 2025 03:32:03 +0500 Subject: [PATCH 16/26] rename is_producer to is_producer_only --- app/models/enterprise.rb | 2 +- app/models/spree/user.rb | 6 +++--- spec/models/enterprise_spec.rb | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index d0f7c30ced..049eefef65 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -381,7 +381,7 @@ class Enterprise < ApplicationRecord sells == 'any' end - def is_producer + def is_producer_only is_primary_producer && sells == 'none' end diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb index f174722b36..f9550daf70 100644 --- a/app/models/spree/user.rb +++ b/app/models/spree/user.rb @@ -156,9 +156,9 @@ module Spree # any of order distributors allow them to edit their orders. def can_manage_line_items_in_orders? @can_manage_line_items_in_orders ||= begin - has_any_producer = enterprises.any?(&:is_producer) - has_producer_editable_orders = Spree::Order.editable_by_producers(enterprises).exists? - has_any_producer && has_producer_editable_orders + return unless enterprises.any?(&:is_producer_only) + + Spree::Order.editable_by_producers(enterprises).exists? end end diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb index 4c57b9d69c..7f790e096f 100644 --- a/spec/models/enterprise_spec.rb +++ b/spec/models/enterprise_spec.rb @@ -1013,25 +1013,25 @@ RSpec.describe Enterprise do end end - describe "#is_producer" do + describe "#is_producer_only" do context "when enterprise is_primary_producer and sells none" do it "returns true" do enterprise = build(:supplier_enterprise) - expect(enterprise.is_producer).to be true + expect(enterprise.is_producer_only).to be true end end context "when enterprise is_primary_producer and sells any" do it "returns false" do enterprise = build(:enterprise, is_primary_producer: true, sells: "any") - expect(enterprise.is_producer).to be false + expect(enterprise.is_producer_only).to be false end end context "when enterprise is_primary_producer and sells own" do it "returns false" do enterprise = build(:enterprise, is_primary_producer: true, sells: "own") - expect(enterprise.is_producer).to be false + expect(enterprise.is_producer_only).to be false end end end From 3474734418285248c726dd999a41a56611ff86d4 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Wed, 5 Mar 2025 03:34:50 +0500 Subject: [PATCH 17/26] implement spree_current_user let! so that user creation doesn't get captured in the scoper queries --- .../scope_variants_for_search_spec.rb | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/spec/lib/open_food_network/scope_variants_for_search_spec.rb b/spec/lib/open_food_network/scope_variants_for_search_spec.rb index 6e55c99db7..35453826bf 100644 --- a/spec/lib/open_food_network/scope_variants_for_search_spec.rb +++ b/spec/lib/open_food_network/scope_variants_for_search_spec.rb @@ -19,7 +19,7 @@ RSpec.describe OpenFoodNetwork::ScopeVariantsForSearch do let!(:oc3) { create(:simple_order_cycle, distributors: [d2], variants: [v4]) } let!(:s1) { create(:schedule, order_cycles: [oc1]) } let!(:s2) { create(:schedule, order_cycles: [oc2]) } - let(:spree_current_user) { create(:user) } + let!(:spree_current_user) { create(:user) } let(:scoper) { OpenFoodNetwork::ScopeVariantsForSearch.new(params, spree_current_user) } @@ -67,13 +67,6 @@ RSpec.describe OpenFoodNetwork::ScopeVariantsForSearch do it "returns all products distributed through that distributor" do expect{ result }.to query_database [ - "TRANSACTION", - "Spree::User Exists?", - "Spree::User Create", - "Customer Load", - "Customer Load", - "Spree::Order Load", - "TRANSACTION", "Enterprise Load", "VariantOverride Load", "SQL", @@ -194,7 +187,7 @@ RSpec.describe OpenFoodNetwork::ScopeVariantsForSearch do context "when search is done by the producer allowing to edit orders" do let(:params) { { q: "product" } } let(:producer) { create(:supplier_enterprise) } - let(:spree_current_user) { + let!(:spree_current_user) { instance_double('Spree::User', enterprises: Enterprise.where(id: producer.id), can_manage_line_items_in_orders_only?: true) } From acdffb0aa15f069eb4897a42de3df94f431dac09 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Wed, 5 Mar 2025 04:14:51 +0500 Subject: [PATCH 18/26] revert "remove subquery for optimization" - we are using OR between two queries here: https://github.com/openfoodfoundation/openfoodnetwork/blob/53ec6621bc30aa86f64440c9b1f9f9357da9a1b1/app/services/search_orders.rb#L31-L32 - so to make it compatible with this, had to revert Throws following error: Relation passed to #or must be structurally compatible. Incompatible values: [:left_outer_joins] --- app/services/permissions/order.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/services/permissions/order.rb b/app/services/permissions/order.rb index aa6610ddd4..29d63b5a2f 100644 --- a/app/services/permissions/order.rb +++ b/app/services/permissions/order.rb @@ -24,7 +24,8 @@ module Permissions # Any orders that the user can edit def editable_orders orders = if @user.can_manage_line_items_in_orders_only? - produced_orders.joins(:distributor).where( + Spree::Order.joins(:distributor).where( + id: produced_orders.select(:id), distributor: { enable_producers_to_edit_orders: true } ) else From f2adcbf2b80a2e5f34b4bd0f9dc72c84d3f31589 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Wed, 5 Mar 2025 04:16:40 +0500 Subject: [PATCH 19/26] fix syntax error --- app/models/spree/order.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 9d9bd0ac7e..9e1cc83c4a 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -144,7 +144,7 @@ module Spree joins( :distributor, line_items: :supplier ).where( - supplier: enterprises, + supplier: { id: enterprises }, distributor: { enable_producers_to_edit_orders: true } ) } From 293b30cfa6527793619e0569a09511bf6598b6e8 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Wed, 5 Mar 2025 04:48:43 +0500 Subject: [PATCH 20/26] simplify block --- app/models/spree/user.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb index f9550daf70..94ad89ab15 100644 --- a/app/models/spree/user.rb +++ b/app/models/spree/user.rb @@ -155,11 +155,11 @@ module Spree # Users can manage line items in orders if they have producer enterprise and # any of order distributors allow them to edit their orders. def can_manage_line_items_in_orders? - @can_manage_line_items_in_orders ||= begin - return unless enterprises.any?(&:is_producer_only) + return @can_manage_line_items_in_orders if defined? @can_manage_line_items_in_orders + @can_manage_line_items_in_orders = + enterprises.any?(&:is_producer_only) && Spree::Order.editable_by_producers(enterprises).exists? - end end def can_manage_line_items_in_orders_only? From eee5d5c8ad3ffc25a080ebc9b13c56fb3d6d79e1 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Wed, 5 Mar 2025 05:32:28 +0500 Subject: [PATCH 21/26] fix buggy spec: - this before block was causing multiple nevigation to the index - one from the spec itself, one from here. --- spec/controllers/admin/bulk_line_items_controller_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/controllers/admin/bulk_line_items_controller_spec.rb b/spec/controllers/admin/bulk_line_items_controller_spec.rb index 8c62de79bd..23e42284ac 100644 --- a/spec/controllers/admin/bulk_line_items_controller_spec.rb +++ b/spec/controllers/admin/bulk_line_items_controller_spec.rb @@ -120,7 +120,6 @@ RSpec.describe Admin::BulkLineItemsController, type: :controller do context "producer enterprise" do before do allow(controller).to receive_messages spree_current_user: supplier.owner - get :index, as: :json end context "with no distributor allows to edit orders" do From 4e2198cd4f5ae4655b5a1de7e7d2f677c7b02755 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Sun, 23 Mar 2025 16:15:52 +0500 Subject: [PATCH 22/26] hide shipping icons from suppliers --- app/views/spree/admin/orders/_table_row.html.haml | 2 +- spec/system/admin/orders/producer_actions_spec.rb | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/app/views/spree/admin/orders/_table_row.html.haml b/app/views/spree/admin/orders/_table_row.html.haml index 405efb6857..6705719168 100644 --- a/app/views/spree/admin/orders/_table_row.html.haml +++ b/app/views/spree/admin/orders/_table_row.html.haml @@ -47,7 +47,7 @@ - if local_assigns[:success] %i.success.icon-ok-sign{"data-controller": "ephemeral"} = render AdminTooltipComponent.new(text: t('spree.admin.orders.index.edit'), link_text: "", link: edit_admin_order_path(order), link_class: "icon_link with-tip icon-edit no-text") - - if order.ready_to_ship? + - if spree_current_user.can_manage_orders? && order.ready_to_ship? %form = render ShipOrderComponent.new(order: order) = render partial: 'admin/shared/tooltip_button', locals: {button_class: "icon-road icon_link with-tip no-text", reflex_data_id: order.id.to_s, tooltip_text: t('spree.admin.orders.index.ship'), shipment: true} diff --git a/spec/system/admin/orders/producer_actions_spec.rb b/spec/system/admin/orders/producer_actions_spec.rb index f62067086b..26c2c63ad5 100644 --- a/spec/system/admin/orders/producer_actions_spec.rb +++ b/spec/system/admin/orders/producer_actions_spec.rb @@ -83,6 +83,12 @@ RSpec.describe 'As a producer who have the ability to update orders' do expect(page).to have_selector('tr', count: 1) # Only one order expect(page).to have_selector('td', text: name, count: 1) expect(page).to have_selector('td', text: email, count: 1) + within 'td.actions' do + # to have edit button + expect(page).to have_selector("a.icon-edit") + # not to have ship button + expect(page).not_to have_selector('button.icon-road') + end end end end From 213209b460cc7ed0f614bae652ebfc3202b83e6b Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Sun, 13 Apr 2025 21:28:14 +0500 Subject: [PATCH 23/26] hide customer info on bulk orders page --- app/controllers/api/v0/orders_controller.rb | 3 +- app/helpers/spree/admin/orders_helper.rb | 2 +- app/models/spree/ability.rb | 3 + app/serializers/api/admin/order_serializer.rb | 23 +++++- .../admin/orders/producer_actions_spec.rb | 2 +- .../admin/producer_bulk_order_management.rb | 76 +++++++++++++++++++ 6 files changed, 103 insertions(+), 6 deletions(-) create mode 100644 spec/system/admin/producer_bulk_order_management.rb diff --git a/app/controllers/api/v0/orders_controller.rb b/app/controllers/api/v0/orders_controller.rb index 2ffb4304b3..d3fe7070c4 100644 --- a/app/controllers/api/v0/orders_controller.rb +++ b/app/controllers/api/v0/orders_controller.rb @@ -67,7 +67,8 @@ module Api def serialized_orders(orders) ActiveModel::ArraySerializer.new( orders, - each_serializer: Api::Admin::OrderSerializer + each_serializer: Api::Admin::OrderSerializer, + current_user: current_api_user ) end diff --git a/app/helpers/spree/admin/orders_helper.rb b/app/helpers/spree/admin/orders_helper.rb index 58e1af234f..7a474332a6 100644 --- a/app/helpers/spree/admin/orders_helper.rb +++ b/app/helpers/spree/admin/orders_helper.rb @@ -162,7 +162,7 @@ module Spree def display_value_for_producer(order, value) return value unless filter_by_supplier?(order) - order.distributor&.show_customer_names_to_suppliers ? value : t("admin.reports.hidden") + order.distributor&.show_customer_names_to_suppliers ? value : t("admin.reports.hidden_field") end end end diff --git a/app/models/spree/ability.rb b/app/models/spree/ability.rb index ce87891d13..08e27d60d7 100644 --- a/app/models/spree/ability.rb +++ b/app/models/spree/ability.rb @@ -369,6 +369,9 @@ module Spree can [:index, :create, :add, :read, :edit, :update], Spree::Shipment do |shipment| can_edit_order(shipment.order, user) end + can [:admin, :index], OrderCycle do |order_cycle| + can_edit_order(order_cycle.order, user) + end can [:visible], Enterprise end diff --git a/app/serializers/api/admin/order_serializer.rb b/app/serializers/api/admin/order_serializer.rb index db3a72f0b5..9d28da3344 100644 --- a/app/serializers/api/admin/order_serializer.rb +++ b/app/serializers/api/admin/order_serializer.rb @@ -15,8 +15,14 @@ module Api has_one :distributor, serializer: Api::Admin::IdSerializer has_one :order_cycle, serializer: Api::Admin::IdSerializer + def full_name_for_sorting + value = [last_name, first_name].compact_blank.join(", ") + display_value_for_producer(object, value) + end + def full_name - object.billing_address.nil? ? "" : ( object.billing_address.full_name || "" ) + value = object.billing_address.nil? ? "" : ( object.billing_address.full_name || "" ) + display_value_for_producer(object, value) end def first_name @@ -65,11 +71,12 @@ module Api end def email - object.email || "" + display_value_for_producer(object, object.email || "") end def phone - object.billing_address.nil? ? "a" : ( object.billing_address.phone || "" ) + value = object.billing_address.nil? ? "a" : ( object.billing_address.phone || "" ) + display_value_for_producer(object, value) end def created_at @@ -93,6 +100,16 @@ module Api def spree_routes_helper Spree::Core::Engine.routes.url_helpers end + + def display_value_for_producer(order, value) + filter_by_supplier = ( + order.distributor&.enable_producers_to_edit_orders && + options[:current_user]&.can_manage_line_items_in_orders_only? + ) + return value unless filter_by_supplier + + order.distributor&.show_customer_names_to_suppliers ? value : I18n.t("admin.reports.hidden_field") + end end end end diff --git a/spec/system/admin/orders/producer_actions_spec.rb b/spec/system/admin/orders/producer_actions_spec.rb index 26c2c63ad5..88e8d5bd34 100644 --- a/spec/system/admin/orders/producer_actions_spec.rb +++ b/spec/system/admin/orders/producer_actions_spec.rb @@ -63,7 +63,7 @@ RSpec.describe 'As a producer who have the ability to update orders' do within('#listing_orders tbody') do expect(page).to have_selector('tr', count: 1) # Only one order # One for Email, one for Name - expect(page).to have_selector('td', text: 'HIDDEN', count: 2) + expect(page).to have_selector('td', text: '< Hidden >', count: 2) end end end diff --git a/spec/system/admin/producer_bulk_order_management.rb b/spec/system/admin/producer_bulk_order_management.rb new file mode 100644 index 0000000000..51d80a64b8 --- /dev/null +++ b/spec/system/admin/producer_bulk_order_management.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +require 'system_helper' + +RSpec.describe 'As a producer who have the ability to update orders' do + include AdminHelper + include AuthenticationHelper + include WebHelper + + let!(:supplier1) { create(:supplier_enterprise, name: 'My supplier1') } + let!(:supplier2) { create(:supplier_enterprise, name: 'My supplier2') } + let!(:supplier1_v1) { create(:variant, supplier_id: supplier1.id) } + let!(:supplier1_v2) { create(:variant, supplier_id: supplier1.id) } + let!(:supplier2_v1) { create(:variant, supplier_id: supplier2.id) } + let(:order_cycle) do + create(:simple_order_cycle, distributors: [distributor], variants: [supplier1_v1, supplier1_v2]) + end + let!(:order_containing_supplier1_products) do + o = create( + :completed_order_with_totals, + distributor:, order_cycle:, + user: supplier1_ent_user, line_items_count: 1 + ) + o.line_items.first.update_columns(variant_id: supplier1_v1.id) + o + end + + let(:supplier1_ent_user) { create(:user, enterprises: [supplier1]) } + + context "As supplier1 enterprise user" do + before { login_as(supplier1_ent_user) } + let(:order) { order_containing_supplier1_products } + let(:user) { supplier1_ent_user } + + describe 'bulk orders index page' do + before { visit spree.admin_bulk_order_management_path } + + context "when no distributor allow the producer to edit orders" do + let(:distributor) { create(:distributor_enterprise) } + + it "should not allow producer to view orders page" do + expect(page).to have_content 'Unauthorized' + end + end + + context "when distributor allows the producer to edit orders" do + let(:distributor) { create(:distributor_enterprise, enable_producers_to_edit_orders: true) } + + context "when distributor doesn't allow to view customer details" do + it "should allow producer to view bulk orders page with HIDDEN customer details" do + within('tbody') do + expect(page).to have_selector('tr', count: 1) + expect(page).to have_selector('td', text: '< Hidden >', count: 1) + end + end + end + + context "when distributor allows to view customer details" do + let(:distributor) do + create( + :distributor_enterprise, + enable_producers_to_edit_orders: true, + show_customer_names_to_suppliers: true + ) + end + it "should allow producer to view bulk orders page with customer details" do + within('tbody') do + expect(page).to have_selector('tr', count: 1) + expect(page).to have_selector('td', text: order.bill_address.full_name_for_sorting, count: 1) + end + end + end + end + end + end +end From 87c957541d3c3d68c9f8800cd55d656483ae5575 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Sun, 13 Apr 2025 21:31:45 +0500 Subject: [PATCH 24/26] refactor: fix rubocop lint issues --- app/helpers/spree/admin/orders_helper.rb | 6 +++++- app/serializers/api/admin/order_serializer.rb | 9 ++++++--- spec/system/admin/producer_bulk_order_management.rb | 3 ++- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/app/helpers/spree/admin/orders_helper.rb b/app/helpers/spree/admin/orders_helper.rb index 7a474332a6..e80b6d9d13 100644 --- a/app/helpers/spree/admin/orders_helper.rb +++ b/app/helpers/spree/admin/orders_helper.rb @@ -162,7 +162,11 @@ module Spree def display_value_for_producer(order, value) return value unless filter_by_supplier?(order) - order.distributor&.show_customer_names_to_suppliers ? value : t("admin.reports.hidden_field") + if order.distributor&.show_customer_names_to_suppliers + value + else + t("admin.reports.hidden_field") + end end end end diff --git a/app/serializers/api/admin/order_serializer.rb b/app/serializers/api/admin/order_serializer.rb index 9d28da3344..b1b4740786 100644 --- a/app/serializers/api/admin/order_serializer.rb +++ b/app/serializers/api/admin/order_serializer.rb @@ -102,13 +102,16 @@ module Api end def display_value_for_producer(order, value) - filter_by_supplier = ( + filter_by_supplier = order.distributor&.enable_producers_to_edit_orders && options[:current_user]&.can_manage_line_items_in_orders_only? - ) return value unless filter_by_supplier - order.distributor&.show_customer_names_to_suppliers ? value : I18n.t("admin.reports.hidden_field") + if order.distributor&.show_customer_names_to_suppliers + value + else + I18n.t("admin.reports.hidden_field") + end end end end diff --git a/spec/system/admin/producer_bulk_order_management.rb b/spec/system/admin/producer_bulk_order_management.rb index 51d80a64b8..575bef0ffb 100644 --- a/spec/system/admin/producer_bulk_order_management.rb +++ b/spec/system/admin/producer_bulk_order_management.rb @@ -66,7 +66,8 @@ RSpec.describe 'As a producer who have the ability to update orders' do it "should allow producer to view bulk orders page with customer details" do within('tbody') do expect(page).to have_selector('tr', count: 1) - expect(page).to have_selector('td', text: order.bill_address.full_name_for_sorting, count: 1) + expect(page).to have_selector('td', text: order.bill_address.full_name_for_sorting, + count: 1) end end end From d12455ccbebf5dc2f9312a8764495dffd55ae7fe Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Sun, 13 Apr 2025 21:45:04 +0500 Subject: [PATCH 25/26] update schema --- db/schema.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/db/schema.rb b/db/schema.rb index ad2af62ff7..69728aa208 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -230,6 +230,8 @@ ActiveRecord::Schema[7.0].define(version: 2025_03_04_234657) do t.text "white_label_logo_link" t.boolean "hide_groups_tab", default: false t.string "external_billing_id", limit: 128 + t.boolean "enable_producers_to_edit_orders", default: false, null: false + t.boolean "show_customer_contacts_to_suppliers", default: false, null: false t.index ["address_id"], name: "index_enterprises_on_address_id" t.index ["is_primary_producer", "sells"], name: "index_enterprises_on_is_primary_producer_and_sells" t.index ["name"], name: "index_enterprises_on_name", unique: true From 7642c54667252349a36185141654cd4d984a8283 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Mon, 21 Apr 2025 03:38:06 +0500 Subject: [PATCH 26/26] remove the disabled button --- app/views/admin/enterprises/form/_shop_preferences.html.haml | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/views/admin/enterprises/form/_shop_preferences.html.haml b/app/views/admin/enterprises/form/_shop_preferences.html.haml index 096753b879..ce36dc1c08 100644 --- a/app/views/admin/enterprises/form/_shop_preferences.html.haml +++ b/app/views/admin/enterprises/form/_shop_preferences.html.haml @@ -123,8 +123,6 @@ .five.columns.omega = f.radio_button :show_customer_contacts_to_suppliers, false = f.label :show_customer_contacts_to_suppliers, t('.customer_contacts_false'), value: :false - = radio_button :enterprise, :show_customer_names_to_suppliers, false - = label :enterprise_show_customer_names_to_suppliers, t('.customer_names_false'), value: :false .row .three.columns.alpha