fix view_on_env spec

4 files changed, 47 insertions, 9 deletions

diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue
index c832898acdd..fa36e4aa1c2 100644
--- a/app/assets/javascripts/diffs/components/diff_file_header.vue
+++ b/app/assets/javascripts/diffs/components/diff_file_header.vue
@@ -236,11 +236,17 @@ export default {
       >
       </a>
 
-      <button
-        v-if="diffFile.environment"
+      <a
+        v-if="diffFile.externalUrl"
+        :href="diffFile.externalUrl"
+        :title="`View on ${diffFile.formattedExternalUrl}`"
+        target="_blank"
+        rel="noopener noreferrer"
+        class="btn btn-file-option"
+        v-tooltip
       >
-        {{ s__('MergeRequests|View on environment') }}
-      </button>
+       <icon name="external-link" />
+      </a>
     </div>
   </div>
 </template>
diff --git a/app/serializers/diff_file_entity.rb b/app/serializers/diff_file_entity.rb
index 6fd8ce03ed1..a893a5a4b7c 100644
--- a/app/serializers/diff_file_entity.rb
+++ b/app/serializers/diff_file_entity.rb
@@ -74,14 +74,13 @@ class DiffFileEntity < Grape::Entity
     end
   end
 
-  # TODO Simon/Fatih/Winnie - Are we using this? - if not please remove
   expose :formatted_external_url, if: -> (_, options) { options[:environment] } do |diff_file|
     options[:environment].formatted_external_url
   end
 
   # TODO imon/Fatih/Winnie - Are we using this? - if not please remove
   expose :external_url, if: -> (_, options) { options[:environment] } do |diff_file|
-    options[:environment].external_url_for(diff_file.content_sha, diff_file.new_path)
+    options[:environment].external_url_for(diff_file.new_path, diff_file.content_sha)
   end
 
   expose :old_path_html do |diff_file|
diff --git a/spec/features/projects/view_on_env_spec.rb b/spec/features/projects/view_on_env_spec.rb
index 61d032ffd2b..84ec32b3fac 100644
--- a/spec/features/projects/view_on_env_spec.rb
+++ b/spec/features/projects/view_on_env_spec.rb
@@ -57,10 +57,11 @@ describe 'View on environment', :js do
           wait_for_requests
         end
 
-        # TODO: https://gitlab.com/gitlab-org/gitlab-ce/issues/45985
-        xit 'has a "View on env" button' do
+        it 'has a "View on env" button' do
           within '.diffs' do
-            expect(page).to have_link('View on feature.review.example.com', href: 'http://feature.review.example.com/ruby/feature')
+            text = 'View on feature.review.example.com'
+            url = 'http://feature.review.example.com/ruby/feature'
+            expect(page).to have_selector("a[data-original-title='#{text}'][href='#{url}']")
           end
         end
       end
diff --git a/spec/javascripts/diffs/components/diff_file_header_spec.js b/spec/javascripts/diffs/components/diff_file_header_spec.js
index 8a6e5e88786..7eb72f4cd01 100644
--- a/spec/javascripts/diffs/components/diff_file_header_spec.js
+++ b/spec/javascripts/diffs/components/diff_file_header_spec.js
@@ -328,6 +328,7 @@ describe('diff_file_header', () => {
       expect(button.dataset.clipboardText).toBe(props.diffFile.filePath);
     });
 
+
     describe('file mode', () => {
       it('it displays old and new file mode if it changed', () => {
         props.diffFile.modeChanged = true;
@@ -373,5 +374,36 @@ describe('diff_file_header', () => {
         expect(lfsLabel()).toBe(null);
       });
     });
+
+    describe('addMergeRequestButtons', () => {
+      beforeEach(() => {
+        props.addMergeRequestButtons = true;
+        props.diffFile.editPath = '';
+      });
+
+      describe('view on environment button', () => {
+        const url = 'some.external.url/';
+        const title = 'url.title';
+
+        it('displays link to external url', () => {
+          props.diffFile.externalUrl = url;
+          props.diffFile.formattedExternalUrl = title;
+
+          vm = mountComponent(Component, props);
+
+          expect(vm.$el.querySelector(`a[href="${url}"]`)).not.toBe(null);
+          expect(vm.$el.querySelector(`a[data-original-title="View on ${title}"]`)).not.toBe(null);
+        });
+
+        it('hides link if no external url', () => {
+          props.diffFile.externalUrl = '';
+          props.diffFile.formattedExternalUrl = title;
+
+          vm = mountComponent(Component, props);
+
+          expect(vm.$el.querySelector(`a[data-original-title="View on ${title}"]`)).toBe(null);
+        });
+      });
+    });
   });
 });