From dd1ae3b10924ca239daa17ce2fe739f790e6f96d Mon Sep 17 00:00:00 2001
From: Matt Jankowski <mjankowski@thoughtbot.com>
Date: Wed, 12 Apr 2017 10:12:42 -0400
Subject: [PATCH] Simplify the way the embed view is created (#1590)

* Add coverage for embedded status view

* Refactor embed view to eliminate @external_links variable
---
 app/controllers/stream_entries_controller.rb        |  2 --
 app/helpers/stream_entries_helper.rb                | 12 +++++++++++-
 app/views/stream_entries/_detailed_status.html.haml |  4 ++--
 app/views/stream_entries/_simple_status.html.haml   |  4 ++--
 spec/controllers/stream_entries_controller_spec.rb  | 10 ++++++++++
 5 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/app/controllers/stream_entries_controller.rb b/app/controllers/stream_entries_controller.rb
index 469a8c33e2..a9ee73500a 100644
--- a/app/controllers/stream_entries_controller.rb
+++ b/app/controllers/stream_entries_controller.rb
@@ -27,8 +27,6 @@ class StreamEntriesController < ApplicationController
 
   def embed
     response.headers['X-Frame-Options'] = 'ALLOWALL'
-    @external_links = true
-
     return gone if @stream_entry.activity.nil?
 
     render layout: 'embedded'
diff --git a/app/helpers/stream_entries_helper.rb b/app/helpers/stream_entries_helper.rb
index d5cc004b0b..59aac78416 100644
--- a/app/helpers/stream_entries_helper.rb
+++ b/app/helpers/stream_entries_helper.rb
@@ -5,8 +5,12 @@ module StreamEntriesHelper
     account.display_name.blank? ? account.username : account.display_name
   end
 
+  def stream_link_target
+    embedded_view? ? '_blank' : nil
+  end
+
   def acct(account)
-    "@#{account.acct}#{@external_links && account.local? ? "@#{Rails.configuration.x.local_domain}" : ''}"
+    "@#{account.acct}#{embedded_view? && account.local? ? "@#{Rails.configuration.x.local_domain}" : ''}"
   end
 
   def entry_classes(status, is_predecessor, is_successor, include_threads)
@@ -30,4 +34,10 @@ module StreamEntriesHelper
 
     rtl_size / ltr_size > 0.3
   end
+
+  private
+
+  def embedded_view?
+    params[:controller] == 'stream_entries' && params[:action] == 'embed'
+  end
 end
diff --git a/app/views/stream_entries/_detailed_status.html.haml b/app/views/stream_entries/_detailed_status.html.haml
index 8495f28b9a..5fc6ab42f3 100644
--- a/app/views/stream_entries/_detailed_status.html.haml
+++ b/app/views/stream_entries/_detailed_status.html.haml
@@ -1,5 +1,5 @@
 .detailed-status.light
-  = link_to TagManager.instance.url_for(status.account), class: 'detailed-status__display-name p-author h-card', target: @external_links ? '_blank' : nil, rel: 'noopener' do
+  = link_to TagManager.instance.url_for(status.account), class: 'detailed-status__display-name p-author h-card', target: stream_link_target, rel: 'noopener' do
     %div
       %div.avatar
         = image_tag status.account.avatar.url(:original), width: 48, height: 48, alt: '', class: 'u-photo'
@@ -30,7 +30,7 @@
 
   %div.detailed-status__meta
     %data.dt-published{ value: status.created_at.to_time.iso8601 }
-    = link_to TagManager.instance.url_for(status), class: 'detailed-status__datetime u-url u-uid', target: @external_links ? '_blank' : nil, rel: 'noopener' do
+    = link_to TagManager.instance.url_for(status), class: 'detailed-status__datetime u-url u-uid', target: stream_link_target, rel: 'noopener' do
       %span= l(status.created_at)
     ·
     - if status.application
diff --git a/app/views/stream_entries/_simple_status.html.haml b/app/views/stream_entries/_simple_status.html.haml
index 2eb9bf1667..5b6069e335 100644
--- a/app/views/stream_entries/_simple_status.html.haml
+++ b/app/views/stream_entries/_simple_status.html.haml
@@ -1,10 +1,10 @@
 .status.light
   .status__header
     .status__meta
-      = link_to time_ago_in_words(status.created_at), TagManager.instance.url_for(status), class: 'status__relative-time u-url u-uid', title: l(status.created_at), target: @external_links ? '_blank' : nil, rel: 'noopener'
+      = link_to time_ago_in_words(status.created_at), TagManager.instance.url_for(status), class: 'status__relative-time u-url u-uid', title: l(status.created_at), target: stream_link_target, rel: 'noopener'
       %data.dt-published{ value: status.created_at.to_time.iso8601 }
 
-    = link_to TagManager.instance.url_for(status.account), class: 'status__display-name p-author h-card', target: @external_links ? '_blank' : nil, rel: 'noopener' do
+    = link_to TagManager.instance.url_for(status.account), class: 'status__display-name p-author h-card', target: stream_link_target, rel: 'noopener' do
       .status__avatar
         %div
           = image_tag status.account.avatar(:original), width: 48, height: 48, alt: '', class: 'u-photo'
diff --git a/spec/controllers/stream_entries_controller_spec.rb b/spec/controllers/stream_entries_controller_spec.rb
index 6f270af9d6..71de606041 100644
--- a/spec/controllers/stream_entries_controller_spec.rb
+++ b/spec/controllers/stream_entries_controller_spec.rb
@@ -17,4 +17,14 @@ RSpec.describe StreamEntriesController, type: :controller do
       expect(response).to have_http_status(:success)
     end
   end
+
+  describe 'GET #embed' do
+    it 'returns embedded view of status' do
+      get :embed, params: { account_username: alice.username, id: status.stream_entry.id }
+
+      expect(response).to have_http_status(:success)
+      expect(response.headers['X-Frame-Options']).to eq 'ALLOWALL'
+      expect(response).to render_template(layout: 'embedded')
+    end
+  end
 end