reintroduce hash sorting for expanded content-item and links, but cor…

…rectly this time

enforce internal consistency of sym/string keys for ExpandedLinksPresenter and
ContentItemPresenter

Gemfile

@@ -3,6 +3,7 @@ source "https://rubygems.org"
 gem "rails", "7.0.5"
 
 gem "bootsnap", require: false
+gem "deepsort"
 gem "gds-api-adapters"
 gem "gds-sso"
 gem "govuk_app_config"

Gemfile.lock

@@ -103,6 +103,7 @@ GEM
       database_cleaner-core (~> 2.0.0)
       mongoid
     date (3.3.3)
+    deepsort (0.4.5)
     diff-lcs (1.5.0)
     docile (1.4.0)
     domain_name (0.5.20190701)
@@ -438,6 +439,7 @@ DEPENDENCIES
   ci_reporter_rspec
   climate_control
   database_cleaner-mongoid
+  deepsort
   factory_bot
   gds-api-adapters
   gds-sso

app/lib/hash_sorter.rb

@@ -0,0 +1,10 @@
+require "deepsort"
+
+class HashSorter
+  def self.sort(hash)
+    # Deepsort by default will sort keys in an array too -
+    # we don't want that, as (for instance) order of links
+    # may be important
+    hash.deep_sort(array: false)
+  end
+end

app/presenters/content_item_presenter.rb

@@ -32,13 +32,14 @@ def initialize(item, api_url_method)
   end
 
   def as_json(options = nil)
-    item.as_json(options).slice(*PUBLIC_ATTRIBUTES).merge(
+    hash = item.as_json(options).slice(*PUBLIC_ATTRIBUTES).merge(
       "links" => RESOLVER.resolve(links),
       "description" => RESOLVER.resolve(item.description),
       "details" => RESOLVER.resolve(item.details),
-    ).tap do |i|
+    ).tap { |i|
       i["redirects"] = item["redirects"] if i["schema_name"] == "redirect"
-    end
+    }.deep_stringify_keys
+    HashSorter.sort(hash)
   end
 
 private

app/presenters/expanded_links_presenter.rb

@@ -6,7 +6,7 @@ def initialize(expanded_links)
   end
 
   def present
-    expanded_links.deep_symbolize_keys.each_with_object({}) do |(type, links), memo|
+    result = expanded_links.deep_symbolize_keys.each_with_object({}) do |(type, links), memo|
       links = Array.wrap(links)
       memo[type] = links.map do |link|
         link.dup.except(secret_fields).merge(
@@ -17,6 +17,7 @@ def present
         ).compact
       end
     end
+    HashSorter.sort(result)
   end
 
 private

spec/lib/hash_sorter_spec.rb

@@ -0,0 +1,37 @@
+require "rails_helper"
+
+describe HashSorter do
+  describe ".sort" do
+    let(:links) do
+      {
+        group_2: [
+          { base_path: "/group-1/link-1", api_path: "/api/content/group-1/link-1" },
+          { base_path: "/group-1/link-2", api_path: "/api/content/group-1/link-2" },
+          { base_path: "/group-1/link-3", api_path: "/api/content/group-1/link-3" },
+        ],
+        group_1: [
+          { base_path: "/group-2/link-3", api_path: "/api/content/group-2/link-3" },
+          { base_path: "/group-2/link-2", api_path: "/api/content/group-2/link-2" },
+          { base_path: "/group-2/link-1", api_path: "/api/content/group-2/link-1" },
+        ],
+      }
+    end
+    let(:result) { described_class.sort(links) }
+
+    it "sorts the top-level keys" do
+      expect(result.keys).to eq(%i[group_1 group_2])
+    end
+
+    it "does not sort the arrays" do
+      expect(result[:group_1].map { |e| e[:base_path] }).to eq(links[:group_1].map { |e| e[:base_path] })
+    end
+
+    it "sorts the keys on Hashes within arrays" do
+      %i[group_1 group_2].each do |group|
+        result[group].each do |hash|
+          expect(hash.keys).to eq(hash.keys.sort)
+        end
+      end
+    end
+  end
+end

spec/presenters/content_item_presenter_spec.rb

@@ -66,11 +66,11 @@
     end
 
     it "inlines the 'text/html' content type in the details" do
-      expect(presenter.as_json["details"]).to eq(body: "<p>content</p>")
+      expect(presenter.as_json["details"]).to eq("body" => "<p>content</p>")
     end
 
     it "inlines the 'text/html' content type in the links" do
-      expect(presenter.as_json["links"][:person].first[:details]).to eq(body: "<p>content</p>")
+      expect(presenter.as_json["links"]["person"].first["details"]).to eq("body" => "<p>content</p>")
     end
   end
 

spec/presenters/expanded_links_presenter_spec.rb

@@ -205,4 +205,30 @@
       end
     end
   end
+
+  describe "sorting the hash" do
+    let(:links) do
+      {
+        group_2: [
+          { base_path: "/group-1/link-1", api_path: "/api/content/group-1/link-1" },
+          { base_path: "/group-1/link-2", api_path: "/api/content/group-1/link-2" },
+          { base_path: "/group-1/link-3", api_path: "/api/content/group-1/link-3" },
+        ],
+        group_1: [
+          { base_path: "/group-2/link-3", api_path: "/api/content/group-2/link-3" },
+          { base_path: "/group-2/link-2", api_path: "/api/content/group-2/link-2" },
+          { base_path: "/group-2/link-1", api_path: "/api/content/group-2/link-1" },
+        ],
+      }
+    end
+    let(:result) { described_class.new(links).present }
+
+    it "sorts the results" do
+      expect(result.keys).to eq(result.keys.sort)
+
+      result.each_key do |group|
+        expect(result[group].all? { |link| link.keys == link.keys.sort }).to eq(true)
+      end
+    end
+  end
 end