Adding docs and specs to AdminMenu.nested_menu_items (#16888)

* Adding docs and specs to AdminMenu.nested_menu_items

This relates to PR forem/forem#16847 which addresses issue
forem/forem#16842.

The goal of this PR is to help me develop an understanding of the
AdminMenu so I can further extend it and better understand it.

There's also a bit of knowledge sharing that I'm looking for, and the
comments and tests are there to help "confirm" that knowledge.  There
might be more that I'd consider regarding a refactor, but I want to make
sure I'm on the right path before I go further.

tl;dr - this is the smallest commit I can make to begin to tease out an
understanding of the method I put under test.

* Update app/models/admin_menu.rb

Co-authored-by: Ridhwana <Ridhwana.Khan16@gmail.com>

* Adding more documentation

* Update app/models/admin_menu.rb

Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>

Co-authored-by: Ridhwana <Ridhwana.Khan16@gmail.com>
Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>

app/models/admin_menu.rb

@@ -68,6 +68,21 @@ class AdminMenu
     feature_flagged_menu_items
   end
 
+  # Return the Menu item that corresponds to the nav_item within the given named scope.
+  #
+  # @param scope_name [String] a slug from the request.path, which, by convention maps to the scope
+  #        declarations in {ITEMS}.
+  # @param nav_item [String] a slug from the request.path, which, by convention maps to the item
+  #        declarations that are one level below the scope declarations of {ITEMS}.
+  #
+  # @return [NilClass] when we don't have a menu representation of the nav_item
+  # @return [Hash] the representation of the nav_item
+  #
+  # @todo This method returns the nav_item, but we really only operate on that item's children.
+  #       Consider replacing with a method that is "children of nav_item within scope".
+  #
+  # @see AdminMenu.nested_menu_items_from_request
+  # @see AdminMenu::ITEMS
   def self.nested_menu_items(scope_name, nav_item)
     return unless navigation_items.dig(scope_name.to_sym, :children)
 
@@ -77,11 +92,28 @@ class AdminMenu
       next unless items[:children]&.any?
 
       items[:children].each do |child|
+        # NOTE: [@jeremyf] trying to puzzle this one out.  My read is that if the "grandchild" of the
+        # scope matches the controller, return the parent node (e.g. the item).
         return items if child[:controller] == nav_item
       end
     end
+
+    # Because we're using each loops, with short-circuiting returns, we need to make sure we don't
+    # return the results of `items[:children].each`, which will be `items[:children]`.
+    nil
   end
 
+  # @param request [#path] the request object (which must respond to `#path`)
+  # @return [NilClass] when we don't have a menu representation of the nav_item
+  # @return [Hash] the representation of the nav_item
+  #
+  # @see {.nested_menu_items} for implementation details
+  #
+  # @note This method assumes that the last two slugs of the request's path are the relevant
+  #       information for determining which menu item to return.  In other words, strongly consider
+  #       the impact of having admin routes whose paths are comprised of more than 3 slugs (.e.g. we
+  #       assume /admin/:scope_name/:nav_item but be wary of /admin/something/:scope_name/:nav_item
+  #       or /admin/:scope_name/something/:nav_item).
   def self.nested_menu_items_from_request(request)
     scope, nav_item = request.path.split("/").last(2)
     nested_menu_items(scope, nav_item)

app/views/admin/shared/_tabbed_navbar.html.erb

@@ -1,5 +1,4 @@
-<!-- menu_items.is_a?(Hash) we have this condition because /reports changes to /feedback_messages  -->
-<% if menu_items.present? && menu_items.is_a?(Hash) && menu_items[:children].any? %>
+<% if menu_items.present? && menu_items[:children].any? %>
   <header class="mb-3 admin__tabbed-navbar">
     <h1 class="crayons-title"><%= menu_items[:name].to_s.titleize %></h1>
     <div class="block s:flex items-center space-between">

spec/models/admin_menu_spec.rb

@@ -0,0 +1,28 @@
+require "rails_helper"
+
+RSpec.describe AdminMenu do
+  describe ".nested_menu_items_from_request" do
+    subject(:method_call) { described_class.nested_menu_items_from_request(request) }
+
+    let(:request) { instance_double("ActionDispatch::Request", path: path) }
+    let(:path) { "/admin" }
+
+    context "when path is /admin/moderation/feedback_messages" do
+      let(:path) { "/admin/moderation/feedback_messages" }
+
+      it { is_expected.not_to be_present }
+    end
+
+    context "when path is /admin/content_manager/badge_achievements" do
+      let(:path) { "/admin/content_manager/badge_achievements" }
+
+      let(:badges_node) do
+        described_class::ITEMS[:content_manager]
+          .fetch(:children)
+          .detect { |ch| ch.fetch(:name) == "badges" }
+      end
+
+      it { is_expected.to eq(badges_node) }
+    end
+  end
+end