From 83b4b3a9fa97fac22c880ef78c14db1a3fcd21a8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?V=C3=ADtor=20Avelino?= <vavelino@suse.com>
Date: Thu, 19 Apr 2018 08:16:55 -0300
Subject: [PATCH] Fixed hostname copied to clipboard on tags
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The internal hostname was being referring as the hostname to be copied
to the clipboard when the user clicked on the labeled tags. This patch
fixes that by checking if external hostname is set or not. If set,
external hostname will be used instead of internal hostname.

This change affects the API directly because the repository entity is
now using the #reacheable_hostname method that tried to prioritize the
external hostname if set.

Signed-off-by: Vítor Avelino <vavelino@suse.com>
---
 app/models/registry.rb       |  7 +++++++
 lib/api/entities.rb          |  5 +++--
 spec/models/registry_spec.rb | 13 +++++++++++++
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/app/models/registry.rb b/app/models/registry.rb
index 34decae07..6ea79b0ae 100644
--- a/app/models/registry.rb
+++ b/app/models/registry.rb
@@ -130,6 +130,13 @@ def reachable?
     e.message
   end
 
+  # Returns the hostname value that should be priotized and used by the user.
+  # In other words, whenever external hostname is present, that would be returned.
+  # Otherwise the internal hostname is returned.1
+  def reachable_hostname
+    external_hostname.presence || hostname
+  end
+
   protected
 
   # Fetch the tag being pushed through the given target object.
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index d36f9771f..027703aa9 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -120,9 +120,10 @@ class Repositories < Grape::Entity
       end
       expose :registry_hostname, documentation: {
         type: Integer,
-        desc: "The repository's registry hostname"
+        desc: "The repository's registry hostname. Prioritizes external hostname value" \
+              "if present, otherwise internal hostname is shown"
       }, if: { type: :internal } do |repository|
-        repository.registry.hostname
+        repository.registry.reachable_hostname
       end
       expose :stars, documentation: {
         type: Integer,
diff --git a/spec/models/registry_spec.rb b/spec/models/registry_spec.rb
index 3d9e84c27..0dc3cf596 100644
--- a/spec/models/registry_spec.rb
+++ b/spec/models/registry_spec.rb
@@ -107,6 +107,19 @@ def create_empty_namespace
     end
   end
 
+  describe "#reachable_hostname" do
+    let!(:registry) { create(:registry, use_ssl: true) }
+    let!(:registry_external) { create(:registry, external_hostname: "external", use_ssl: true) }
+
+    it "returns internal hostname when external not present" do
+      expect(registry.reachable_hostname).to eq registry.hostname
+    end
+
+    it "returns external hostname whenever is present" do
+      expect(registry_external.reachable_hostname).to eq registry_external.external_hostname
+    end
+  end
+
   describe "#reachable" do
     after :each do
       allow_any_instance_of(::Portus::RegistryClient).to receive(:perform_request).and_call_original