Add MenuItem#children and #data_hook

Use MenuItem instances for second level backend menus instead of
partials.

backend/app/views/spree/admin/shared/_product_sub_menu.html.erb

@@ -1,3 +1,5 @@
+<% Spree::Deprecation.warn "Using the #{@virtual_path.inspect} partial is deprecated, please use MenuItem#children instead." %>
+
 <ul class="admin-subnav" data-hook="admin_product_sub_tabs">
   <% if can? :admin, Spree::Product %>
     <%= tab label: :products, match_path: '/products' %>

backend/app/views/spree/admin/shared/_promotion_sub_menu.html.erb

@@ -1,3 +1,5 @@
+<% Spree::Deprecation.warn "Using the #{@virtual_path.inspect} partial is deprecated, please use MenuItem#children instead." %>
+
 <ul class="admin-subnav" data-hook="admin_promotion_sub_tabs">
   <% if can? :admin, Spree::Promotion %>
     <%= tab label: :promotions %>

backend/app/views/spree/admin/shared/_settings_sub_menu.html.erb

@@ -1,3 +1,5 @@
+<% Spree::Deprecation.warn "Using the #{@virtual_path.inspect} partial is deprecated, please use MenuItem#children instead." %>
+
 <ul class="admin-subnav" data-hook="admin_settings_sub_tabs">
   <% if can?(:admin, Spree::Store) %>
     <%= tab label: :stores, url: spree.admin_stores_path %>

backend/app/views/spree/admin/shared/_tabs.html.erb

@@ -5,10 +5,23 @@
         icon: menu_item.icon,
         label: menu_item.label,
         url: menu_item.url,
-        match_path: menu_item.match_path,
+        selected: menu_item.match_path?(request) || menu_item.children.any? { _1.match_path?(request) },
       ) do
     %>
-      <%- render partial: menu_item.partial if menu_item.partial %>
+      <% if menu_item.render_partial? %>
+        <%- render partial: menu_item.partial %>
+      <% elsif menu_item.children.present? %>
+        <ul class="admin-subnav" data-hook="<%= menu_item.data_hook %>">
+          <%- menu_item.children.each do |child| %>
+            <%= tab(
+              icon: child.icon,
+              label: child.label,
+              url: child.url,
+              selected: child.match_path?(request),
+            ) if instance_exec(&child.condition) %>
+          <% end %>
+        </ul>
+      <% end %>
     <%- end %>
   <% end %>
 <% end %>

backend/lib/spree/backend_configuration.rb

@@ -32,6 +32,10 @@ def theme_path(user_theme = nil)
         }
       }
 
+    # @!attribute [rw] prefer_menu_item_partials
+    #   @return [Boolean] Whether or not to prefer menu item partials when both a partial and children are present.
+    preference :prefer_menu_item_partials, :boolean, default: false
+
     autoload :ORDER_TABS, 'spree/backend_configuration/deprecated_tab_constants'
     autoload :PRODUCT_TABS, 'spree/backend_configuration/deprecated_tab_constants'
     autoload :CONFIGURATION_TABS, 'spree/backend_configuration/deprecated_tab_constants'
@@ -98,7 +102,34 @@ def menu_items
             variants
           )}x,
           partial: 'spree/admin/shared/product_sub_menu',
-          position: 1
+          position: 1,
+          data_hook: :admin_product_sub_tabs,
+          children: [
+            MenuItem.new(
+              label: :products,
+              condition: -> { can? :admin, Spree::Product },
+              match_path: '/products',
+            ),
+            MenuItem.new(
+              label: :option_types,
+              condition: -> { can? :admin, Spree::OptionType },
+              match_path: '/option_types',
+            ),
+            MenuItem.new(
+              label: :properties,
+              condition: -> { can? :admin, Spree::Property },
+            ),
+            MenuItem.new(
+              label: :taxonomies,
+              condition: -> { can? :admin, Spree::Taxonomy },
+            ),
+            MenuItem.new(
+              url: :admin_taxons_path,
+              condition: -> { can? :admin, Spree::Taxon },
+              label: :display_order,
+              match_path: '/taxons',
+            ),
+          ],
         ),
         MenuItem.new(
           label: :promotions,
@@ -107,23 +138,34 @@ def menu_items
           partial: 'spree/admin/shared/promotion_sub_menu',
           condition: -> { can?(:admin, Spree::Promotion) },
           url: :admin_promotions_path,
-          position: 2
+          data_hook: :admin_promotion_sub_tabs,
+          position: 2,
+          children: [
+            MenuItem.new(
+              label: :promotions,
+              condition: -> { can?(:admin, Spree::Promotion) },
+            ),
+            MenuItem.new(
+              label: :promotion_categories,
+              condition: -> { can?(:admin, Spree::PromotionCategory) },
+            ),
+          ],
         ),
         MenuItem.new(
           label: :stock,
           icon: 'cubes',
           match_path: %r{/(stock_items)},
           condition: -> { can?(:admin, Spree::StockItem) },
           url: :admin_stock_items_path,
-          position: 3
+          position: 3,
         ),
         MenuItem.new(
           label: :users,
           icon: 'user',
           match_path: %r{/(users|store_credits)},
           condition: -> { Spree.user_class && can?(:admin, Spree.user_class) },
           url: :admin_users_path,
-          position: 4
+          position: 4,
         ),
         MenuItem.new(
           label: :settings,
@@ -143,6 +185,7 @@ def menu_items
             tax_rates|
             zones
           )}x,
+          data_hook: :admin_settings_sub_tabs,
           condition: -> {
             can?(:admin, Spree::Store) ||
             can?(:admin, Spree::AdjustmentReason) ||
@@ -159,7 +202,52 @@ def menu_items
           },
           partial: 'spree/admin/shared/settings_sub_menu',
           url: :admin_stores_path,
-          position: 5
+          position: 5,
+          children: [
+            MenuItem.new(
+              label: :stores,
+              condition: -> { can? :admin, Spree::Store },
+              url: :admin_stores_path,
+            ),
+            MenuItem.new(
+              label: :payments,
+              condition: -> { can? :admin, Spree::PaymentMethod },
+              url: :admin_payment_methods_path,
+            ),
+
+            MenuItem.new(
+              label: :taxes,
+              condition: -> { can?(:admin, Spree::TaxCategory) || can?(:admin, Spree::TaxRate) },
+              url: :admin_tax_categories_path,
+              match_path: %r(tax_categories|tax_rates),
+            ),
+            MenuItem.new(
+              label: :checkout,
+              condition: -> {
+                can?(:admin, Spree::RefundReason) ||
+                can?(:admin, Spree::ReimbursementType) ||
+                can?(:show, Spree::ReturnReason) ||
+                can?(:show, Spree::AdjustmentReason)
+              },
+              url: :admin_refund_reasons_path,
+              match_path: %r(refund_reasons|reimbursement_types|return_reasons|adjustment_reasons|store_credit_reasons)
+            ),
+            MenuItem.new(
+              label: :shipping,
+              condition: -> {
+                can?(:admin, Spree::ShippingMethod) ||
+                  can?(:admin, Spree::ShippingCategory) ||
+                  can?(:admin, Spree::StockLocation)
+              },
+              url: :admin_shipping_methods_path,
+              match_path: %r(shipping_methods|shipping_categories|stock_locations),
+            ),
+            MenuItem.new(
+              label: :zones,
+              condition: -> { can?(:admin, Spree::Zone) },
+              url: :admin_zones_path,
+            ),
+          ],
         )
       ]
     end

backend/lib/spree/backend_configuration/menu_item.rb

@@ -4,7 +4,7 @@ module Spree
   class BackendConfiguration < Preferences::Configuration
     # An item which should be drawn in the admin menu
     class MenuItem
-      attr_reader :icon, :label, :partial, :condition, :match_path
+      attr_reader :icon, :label, :partial, :children, :condition, :data_hook, :match_path
 
       def sections # rubocop:disable Style/TrivialAccessors
         @sections
@@ -19,8 +19,7 @@ def sections # rubocop:disable Style/TrivialAccessors
       #   returns true.
       # @param label [Symbol] The translation key for a label to use for this
       #   menu item.
-      # @param partial [String] A partial to draw within this menu item for use
-      #   in declaring a submenu
+      # @param children [Array<Spree::BackendConfiguration::MenuItem>] An array
       # @param url [String|Symbol] A url where this link should send the user to or a Symbol representing a route name
       # @param position [Integer] The position in which the menu item should render
       #   nil will cause the item to render last
@@ -33,8 +32,10 @@ def initialize(
         condition: nil,
         label: nil,
         partial: nil,
+        children: [],
         url: nil,
         position: nil,
+        data_hook: nil,
         match_path: nil
       )
         if args.length == 2
@@ -46,16 +47,30 @@ def initialize(
           raise ArgumentError, "wrong number of arguments (given #{args.length}, expected 0..2)"
         end
 
+        if partial.present? && children.blank?
+          # We only show the deprecation if there are no children, because if there are children,
+          # then the menu item is already future-proofed.
+          Spree::Deprecation.warn "Passing a partial to #{self.class.name} is deprecated. Please use the children keyword argument instead."
+        end
+
         @condition = condition || -> { true }
         @sections = sections
         @icon = icon
         @label = label
         @partial = partial
+        @children = children
         @url = url
         @position = position
+        @data_hook = data_hook
         @match_path = match_path
       end
 
+      def render_partial?
+        return false if partial.blank?
+
+        children.blank? || Spree::Backend::Config.prefer_menu_item_partials
+      end
+
       def match_path?(request)
         if match_path.is_a? Regexp
           request.fullpath =~ match_path

backend/spec/lib/spree/backend_configuration/menu_item_spec.rb

@@ -3,6 +3,14 @@
 require 'spec_helper'
 
 RSpec.describe Spree::BackendConfiguration::MenuItem do
+  describe '#children' do
+    it 'is the replacement for the deprecated #partial method' do
+      expect(Spree::Deprecation).to receive(:warn).with(a_string_matching(/partial/))
+
+      described_class.new(partial: 'foo')
+    end
+  end
+
   describe '#match_path?' do
     it 'matches a string using the admin path prefix' do
       described_class.new(match_path: '/stock_items')