Merge pull request #9789 from alphagov/content-modelling/782-pull-use…

…r-details-from-signon

(782) Pull user details from Signon

Gemfile.lock

@@ -248,7 +248,7 @@ GEM
       et-orbi (~> 1, >= 1.2.11)
       raabro (~> 1.4)
     fuzzy_match (2.1.0)
-    gds-api-adapters (98.1.0)
+    gds-api-adapters (98.2.0)
       addressable
       link_header
       null_logger

lib/engines/content_block_manager/app/components/content_block_manager/content_block/document/show/host_editions_table_component.rb

@@ -71,15 +71,6 @@ def sort_link(param)
     helpers.content_block_manager.url_for(only_path: false, params: { order: param }, anchor: TABLE_ID)
   end
 
-  # TODO: Currently, we're only fetching Users from the local Whitehall database, which means
-  # content updated outside Whitehall with a `last_edited_by_editor_id` where the user
-  # does not have Whitehall access will show up as an unknown user. We are looking to
-  # fix this by possibly adding an endpoint to Signon, but this gets us part of the way
-  # there. Card for this work is here: https://trello.com/c/jVvs4nAP/640-get-author-information-from-signon
-  def users
-    @users ||= User.where(uid: host_content_items.map(&:last_edited_by_editor_id))
-  end
-
   def frontend_path(content_item)
     if @is_preview
       helpers.content_block_manager.content_block_manager_content_block_host_content_preview_path(id: content_block_edition.id, host_content_id: content_item.host_content_id)
@@ -122,8 +113,7 @@ def all_publishing_organisations
   end
 
   def updated_field_for(content_item)
-    last_updated_by_user = content_item.last_edited_by_editor_id && users.find { |u| u.uid == content_item.last_edited_by_editor_id }
-    user_copy = last_updated_by_user ? mail_to(last_updated_by_user.email, last_updated_by_user.name, { class: "govuk-link" }) : "Unknown user"
+    user_copy = content_item.last_edited_by_editor ? mail_to(content_item.last_edited_by_editor.email, content_item.last_edited_by_editor.name, { class: "govuk-link" }) : "Unknown user"
     "#{time_ago_in_words(content_item.last_edited_at)} ago by #{user_copy}".html_safe
   end
 end

lib/engines/content_block_manager/app/models/content_block_manager/host_content_item.rb

@@ -5,7 +5,7 @@ class HostContentItem < Data.define(
     :document_type,
     :publishing_organisation,
     :publishing_app,
-    :last_edited_by_editor_id,
+    :last_edited_by_editor,
     :last_edited_at,
     :unique_pageviews,
     :instances,

lib/engines/content_block_manager/app/models/content_block_manager/host_content_item/editor.rb

@@ -0,0 +1,16 @@
+module ContentBlockManager
+  class HostContentItem
+    class Editor < Data.define(:uid, :name, :email, :organisation)
+      def self.with_uuids(uuids)
+        Services.signon_api_client.get_users(uuids:).map do |user|
+          new(
+            uid: user["uid"],
+            name: user["name"],
+            email: user["email"],
+            organisation: ContentBlockManager::HostContentItem::Editor::Organisation.from_user_hash(user),
+          )
+        end
+      end
+    end
+  end
+end

lib/engines/content_block_manager/app/models/content_block_manager/host_content_item/editor/organisation.rb

@@ -0,0 +1,17 @@
+module ContentBlockManager
+  class HostContentItem
+    class Editor
+      class Organisation < Data.define(:content_id, :name, :slug)
+        def self.from_user_hash(user)
+          if user["organisation"].present?
+            new(
+              content_id: user["organisation"]["content_id"],
+              name: user["organisation"]["name"],
+              slug: user["organisation"]["slug"],
+            )
+          end
+        end
+      end
+    end
+  end
+end

lib/engines/content_block_manager/app/services/content_block_manager/get_host_content_items.rb

@@ -26,7 +26,7 @@ def items
           document_type: item["document_type"],
           publishing_organisation: item["primary_publishing_organisation"],
           publishing_app: item["publishing_app"],
-          last_edited_by_editor_id: item["last_edited_by_editor_id"],
+          last_edited_by_editor: editor_for_uid(item["last_edited_by_editor_id"]),
           last_edited_at: item["last_edited_at"],
           unique_pageviews: item["unique_pageviews"],
           instances: item["instances"],
@@ -61,5 +61,17 @@ def content_items
         response.parsed_content
       end
     end
+
+    def editor_for_uid(uid)
+      editors.find { |editor| editor.uid == uid }
+    end
+
+    def editors
+      @editors ||= editor_uuids.present? ? ContentBlockManager::HostContentItem::Editor.with_uuids(editor_uuids) : []
+    end
+
+    def editor_uuids
+      content_items["results"].map { |c| c["last_edited_by_editor_id"] }.compact
+    end
   end
 end

lib/engines/content_block_manager/features/step_definitions/content_block_manager_steps.rb

@@ -541,6 +541,10 @@ def should_show_edit_form_for_email_address_content_block(document_title, email_
   )
 
   stub_publishing_api_has_embedded_content_details(@dependent_content.first)
+
+  stub_request(:get, "#{Plek.find('signon', external: true)}/api/users")
+    .with(query: { uuids: @dependent_content.map { |item| item["last_edited_by_editor_id"] } })
+    .to_return(body: [].to_json)
 end
 
 Then(/^I should see the dependent content listed$/) do

lib/engines/content_block_manager/test/components/content_block/document/show/host_editions_table_component_test.rb

@@ -6,7 +6,6 @@ class ContentBlockManager::ContentBlock::Document::Show::HostEditionsTableCompon
   include ActionView::Helpers::DateHelper
 
   let(:described_class) { ContentBlockManager::ContentBlock::Document::Show::HostEditionsTableComponent }
-  let(:user) { create(:user) }
   let(:caption) { "Some caption" }
   let(:publishing_organisation) do
     {
@@ -15,16 +14,16 @@ class ContentBlockManager::ContentBlock::Document::Show::HostEditionsTableCompon
       "base_path" => "/bar",
     }
   end
-  let(:last_edited_by_editor_id) { user.uid }
   let(:unique_pageviews) { 1_200_000 }
 
+  let(:last_edited_by_editor) { build(:host_content_item_editor) }
   let(:host_content_item) do
     ContentBlockManager::HostContentItem.new(
       "title" => "Some title",
       "base_path" => "/foo",
       "document_type" => "document_type",
       "publishing_app" => "publisher",
-      "last_edited_by_editor_id" => last_edited_by_editor_id,
+      "last_edited_by_editor" => last_edited_by_editor,
       "last_edited_at" => Time.zone.now.to_s,
       "publishing_organisation" => publishing_organisation,
       "unique_pageviews" => unique_pageviews,
@@ -88,8 +87,8 @@ def self.it_returns_unknown_user
       assert_selector "tbody .govuk-table__cell", text: host_content_item.document_type.humanize
       assert_selector "tbody .govuk-table__cell", text: "1.2m"
       assert_selector "tbody .govuk-table__cell", text: host_content_item.publishing_organisation["title"]
-      assert_selector "tbody .govuk-table__cell", text: "#{time_ago_in_words(host_content_item.last_edited_at)} ago by #{user.name}"
-      assert_link user.name, { href: "mailto:#{user.email}" }
+      assert_selector "tbody .govuk-table__cell", text: "#{time_ago_in_words(host_content_item.last_edited_at)} ago by #{last_edited_by_editor.name}"
+      assert_link last_edited_by_editor.name, { href: "mailto:#{last_edited_by_editor.email}" }
     end
 
     context "when the organisation does NOT exist within Whitehall" do
@@ -147,22 +146,8 @@ def self.it_returns_unknown_user
       end
     end
 
-    context "when last_edited_by_editor_id is nil" do
-      let(:last_edited_by_editor_id) { nil }
-
-      it_returns_unknown_user
-
-      context "and a user exists with a nil uuid" do
-        before do
-          create(:user, uid: nil)
-        end
-
-        it_returns_unknown_user
-      end
-    end
-
-    context "when last_edited_by_editor_id refers to a user id which is not present in Whitehall" do
-      let(:last_edited_by_editor_id) { SecureRandom.uuid }
+    context "when last_edited_by_editor is nil" do
+      let(:last_edited_by_editor) { nil }
 
       it_returns_unknown_user
     end

lib/engines/content_block_manager/test/factories/host_content_item.rb

@@ -5,7 +5,7 @@
     document_type { "something" }
     publishing_organisation { { name: "organisation", content_id: SecureRandom.uuid } }
     publishing_app { "publishing_app" }
-    last_edited_by_editor_id { SecureRandom.uuid }
+    last_edited_by_editor { build(:host_content_item_editor) }
     last_edited_at { 2.days.ago.to_s }
     unique_pageviews { 123 }
     instances { 1 }
@@ -17,7 +17,7 @@
           document_type:,
           publishing_organisation:,
           publishing_app:,
-          last_edited_by_editor_id:,
+          last_edited_by_editor:,
           last_edited_at:,
           unique_pageviews:,
           host_content_id:,

lib/engines/content_block_manager/test/factories/host_content_item_editor.rb

@@ -0,0 +1,17 @@
+FactoryBot.define do
+  factory :host_content_item_editor, class: "ContentBlockManager::HostContentItem::Editor" do
+    uid { SecureRandom.uuid }
+    sequence(:name) { |i| "Someone #{i}" }
+    sequence(:email) { |i| "someone-#{i}@example.com" }
+    organisation { build(:host_content_item_editor_organisation) }
+
+    initialize_with do
+      new(
+        uid:,
+        name:,
+        email:,
+        organisation:,
+      )
+    end
+  end
+end

lib/engines/content_block_manager/test/factories/host_content_item_editor_organisation.rb

@@ -0,0 +1,15 @@
+FactoryBot.define do
+  factory :host_content_item_editor_organisation, class: "ContentBlockManager::HostContentItem::Editor::Organisation" do
+    content_id { SecureRandom.uuid }
+    sequence(:name) { |i| "organisation #{i}" }
+    sequence(:slug) { |i| "organisation-#{i}" }
+
+    initialize_with do
+      new(
+        content_id:,
+        name:,
+        slug:,
+      )
+    end
+  end
+end

lib/engines/content_block_manager/test/support/embedded_content_helpers.rb

@@ -19,12 +19,16 @@ def self.it_returns_embedded_content(&block)
       end
     end
 
+    let(:host_content_item_users) { build_list(:host_content_item_editor, 10) }
+
     before do
       stub_publishing_api_has_embedded_content_for_any_content_id(
         results: host_content_items,
         total: host_content_items.length,
         order: ContentBlockManager::GetHostContentItems::DEFAULT_ORDER,
       )
+
+      ContentBlockManager::HostContentItem::Editor.stubs(:with_uuids).with(host_content_items.map { |i| i["last_edited_by_editor_id"] }).returns(host_content_item_users)
     end
 
     it "returns host content items" do

lib/engines/content_block_manager/test/unit/app/models/host_content_item/editor_test.rb

@@ -0,0 +1,57 @@
+require "test_helper"
+
+class ContentBlockManager::HostContentItem::EditorTest < ActiveSupport::TestCase
+  extend Minitest::Spec::DSL
+
+  describe ".with_uuids" do
+    let(:signon_api_stub) { stub }
+
+    before do
+      Services.expects(:signon_api_client).returns(signon_api_stub)
+    end
+
+    it "returns an empty array when no UUIDs are provided" do
+      signon_api_stub.expects(:get_users).with(uuids: []).returns([])
+
+      result = ContentBlockManager::HostContentItem::Editor.with_uuids([])
+
+      assert_equal [], result
+    end
+
+    it "fetches users for a given list of UUIDs" do
+      uuids = [SecureRandom.uuid, SecureRandom.uuid]
+      api_response = [
+        {
+          "uid" => uuids[0],
+          "name" => "Someone",
+          "email" => "[email protected]",
+        },
+        {
+          "uid" => uuids[1],
+          "name" => "Someone else",
+          "email" => "[email protected]",
+          "organisation" => {
+            "content_id" => SecureRandom.uuid,
+            "name" => "Organisation",
+            "slug" => "organisation",
+          },
+        },
+      ]
+      signon_api_stub.expects(:get_users).with(uuids:).returns(api_response)
+
+      result = ContentBlockManager::HostContentItem::Editor.with_uuids(uuids)
+
+      assert_equal result[0].uid, api_response[0]["uid"]
+      assert_equal result[0].name, api_response[0]["name"]
+      assert_equal result[0].email, api_response[0]["email"]
+      assert_nil result[0].organisation
+
+      assert_equal result[1].uid, api_response[1]["uid"]
+      assert_equal result[1].name, api_response[1]["name"]
+      assert_equal result[1].email, api_response[1]["email"]
+      assert_equal result[1].organisation.content_id, api_response[1]["organisation"]["content_id"]
+      assert_equal result[1].organisation.name, api_response[1]["organisation"]["name"]
+      assert_equal result[1].organisation.slug, api_response[1]["organisation"]["slug"]
+    end
+  end
+end