Update status embeds (#4742)

- Use statuses controller for embeds instead of stream entries controller
- Prefer /@:username/:id/embed URL for embeds
- Use /@:username as author_url in OEmbed
- Add follow link to embeds which opens web intent in new window
- Use redis cache in development
- Cache entire embed

app/controllers/api/oembed_controller.rb

@@ -4,14 +4,14 @@ class Api::OEmbedController < Api::BaseController
   respond_to :json
 
   def show
-    @stream_entry = find_stream_entry.stream_entry
-    render json: @stream_entry, serializer: OEmbedSerializer, width: maxwidth_or_default, height: maxheight_or_default
+    @status = status_finder.status
+    render json: @status, serializer: OEmbedSerializer, width: maxwidth_or_default, height: maxheight_or_default
   end
 
   private
 
-  def find_stream_entry
-    StreamEntryFinder.new(params[:url])
+  def status_finder
+    StatusFinder.new(params[:url])
   end
 
   def maxwidth_or_default

app/controllers/statuses_controller.rb

@@ -30,6 +30,11 @@ class StatusesController < ApplicationController
     render json: @status, serializer: ActivityPub::ActivitySerializer, adapter: ActivityPub::Adapter, content_type: 'application/activity+json'
   end
 
+  def embed
+    response.headers['X-Frame-Options'] = 'ALLOWALL'
+    render 'stream_entries/embed', layout: 'embedded'
+  end
+
   private
 
   def set_account

app/controllers/stream_entries_controller.rb

@@ -25,10 +25,7 @@ class StreamEntriesController < ApplicationController
   end
 
   def embed
-    response.headers['X-Frame-Options'] = 'ALLOWALL'
-    return gone if @stream_entry.activity.nil?
-
-    render layout: 'embedded'
+    redirect_to embed_short_account_status_url(@account, @stream_entry.activity), status: 301
   end
 
   private

app/helpers/stream_entries_helper.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 module StreamEntriesHelper
-  EMBEDDED_CONTROLLER = 'stream_entries'
+  EMBEDDED_CONTROLLER = 'statuses'
   EMBEDDED_ACTION = 'embed'
 
   def display_name(account)

app/javascript/packs/public.js

@@ -38,6 +38,13 @@ function main() {
       content.title = dateTimeFormat.format(datetime);
       content.textContent = relativeFormat.format(datetime);
     });
+
+    [].forEach.call(document.querySelectorAll('.logo-button'), (content) => {
+      content.addEventListener('click', (e) => {
+        e.preventDefault();
+        window.open(e.target.href, 'mastodon-intent', 'width=400,height=400,resizable=no,menubar=no,status=no,scrollbars=yes');
+      });
+    });
   });
 
   delegate(document, '.video-player video', 'click', ({ target }) => {

app/javascript/styles/stream_entries.scss

@@ -421,3 +421,33 @@
     }
   }
 }
+
+.button.button-secondary.logo-button {
+  position: absolute;
+  right: 14px;
+  top: 14px;
+  font-size: 14px;
+
+  svg {
+    width: 20px;
+    height: auto;
+    vertical-align: middle;
+    margin-right: 5px;
+
+    path:first-child {
+      fill: $ui-primary-color;
+    }
+
+    path:last-child {
+      fill: $simple-background-color;
+    }
+  }
+
+  &:active,
+  &:focus,
+  &:hover {
+    svg path:first-child {
+      fill: lighten($ui-primary-color, 4%);
+    }
+  }
+}

app/lib/stream_entry_finder.rb → app/lib/status_finder.rb

@@ -1,20 +1,20 @@
 # frozen_string_literal: true
 
-class StreamEntryFinder
+class StatusFinder
   attr_reader :url
 
   def initialize(url)
     @url = url
   end
 
-  def stream_entry
+  def status
     verify_action!
 
     case recognized_params[:controller]
     when 'stream_entries'
-      StreamEntry.find(recognized_params[:id])
+      StreamEntry.find(recognized_params[:id]).status
     when 'statuses'
-      Status.find(recognized_params[:id]).stream_entry
+      Status.find(recognized_params[:id])
     else
       raise ActiveRecord::RecordNotFound
     end

app/serializers/oembed_serializer.rb

@@ -21,7 +21,7 @@ class OEmbedSerializer < ActiveModel::Serializer
   end
 
   def author_url
-    account_url(object.account)
+    short_account_url(object.account)
   end
 
   def provider_name
@@ -38,7 +38,7 @@ class OEmbedSerializer < ActiveModel::Serializer
 
   def html
     tag :iframe,
-        src: embed_account_stream_entry_url(object.account, object),
+        src: embed_short_account_status_url(object.account, object),
         style: 'width: 100%; overflow: hidden',
         frameborder: '0',
         scrolling: 'no',

app/views/stream_entries/_detailed_status.html.haml

@@ -1,4 +1,9 @@
 .detailed-status.light
+  - if embedded_view?
+    = link_to "web+mastodon://follow?uri=#{status.account.local_username_and_domain}", class: 'button button-secondary logo-button', target: '_new' do
+      = render file: Rails.root.join('app', 'javascript', 'images', 'logo.svg')
+      = t('accounts.follow')
+
   = 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
       .avatar

app/views/stream_entries/embed.html.haml

@@ -1,2 +1,3 @@
-.activity-stream.activity-stream-headless
-  = render @type, @type.to_sym => @stream_entry.activity, centered: true
+- cache @stream_entry.activity do
+  .activity-stream.activity-stream-headless
+    = render "stream_entries/#{@type}", @type.to_sym => @stream_entry.activity, centered: true

config/brakeman.ignore

@@ -3,14 +3,33 @@
     {
       "warning_type": "Dynamic Render Path",
       "warning_code": 15,
+      "fingerprint": "44d3f14e05d8fbb5b23e13ac02f15aa38b2a2f0f03b9ba76bab7f98e155a4a4e",
+      "check_name": "Render",
+      "message": "Render path contains parameter value",
+      "file": "app/views/stream_entries/embed.html.haml",
+      "line": 3,
+      "link": "http://brakemanscanner.org/docs/warning_types/dynamic_render_path/",
+      "code": "render(action => \"stream_entries/#{Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity_type.downcase}\", { Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity_type.downcase.to_sym => Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity, :centered => true })",
+      "render_path": [{"type":"controller","class":"StatusesController","method":"embed","line":35,"file":"app/controllers/statuses_controller.rb"}],
+      "location": {
+        "type": "template",
+        "template": "stream_entries/embed"
+      },
+      "user_input": "params[:id]",
+      "confidence": "Weak",
+      "note": ""
+    },
+    {
+      "warning_type": "Dynamic Render Path",
+      "warning_code": 15,
       "fingerprint": "9f31d941f3910dba2e9bfcd81aef4513249bd24c02d0f98e13ad44fdeeccd0e8",
       "check_name": "Render",
       "message": "Render path contains parameter value",
       "file": "app/views/admin/accounts/index.html.haml",
-      "line": 32,
+      "line": 63,
       "link": "http://brakemanscanner.org/docs/warning_types/dynamic_render_path/",
       "code": "render(action => filtered_accounts.page(params[:page]), {})",
-      "render_path": [{"type":"controller","class":"Admin::AccountsController","method":"index","line":7,"file":"app/controllers/admin/accounts_controller.rb"}],
+      "render_path": [{"type":"controller","class":"Admin::AccountsController","method":"index","line":10,"file":"app/controllers/admin/accounts_controller.rb"}],
       "location": {
         "type": "template",
         "template": "admin/accounts/index"
@@ -42,25 +61,6 @@
     {
       "warning_type": "Dynamic Render Path",
       "warning_code": 15,
-      "fingerprint": "c417f9d44ab05dd9cf3d5ec9df2324a5036774c151181787b32c4c940623191b",
-      "check_name": "Render",
-      "message": "Render path contains parameter value",
-      "file": "app/views/stream_entries/embed.html.haml",
-      "line": 2,
-      "link": "http://brakemanscanner.org/docs/warning_types/dynamic_render_path/",
-      "code": "render(action => Account.find_local!(params[:account_username]).stream_entries.where(:activity_type => \"Status\").find(params[:id]).activity_type.downcase, { Account.find_local!(params[:account_username]).stream_entries.where(:activity_type => \"Status\").find(params[:id]).activity_type.downcase.to_sym => Account.find_local!(params[:account_username]).stream_entries.where(:activity_type => \"Status\").find(params[:id]).activity, :centered => true })",
-      "render_path": [{"type":"controller","class":"StreamEntriesController","method":"embed","line":32,"file":"app/controllers/stream_entries_controller.rb"}],
-      "location": {
-        "type": "template",
-        "template": "stream_entries/embed"
-      },
-      "user_input": "params[:id]",
-      "confidence": "Weak",
-      "note": ""
-    },
-    {
-      "warning_type": "Dynamic Render Path",
-      "warning_code": 15,
       "fingerprint": "c5d6945d63264af106d49367228d206aa2f176699ecdce2b98fac101bc6a96cf",
       "check_name": "Render",
       "message": "Render path contains parameter value",
@@ -84,10 +84,10 @@
       "check_name": "Render",
       "message": "Render path contains parameter value",
       "file": "app/views/stream_entries/show.html.haml",
-      "line": 19,
+      "line": 23,
       "link": "http://brakemanscanner.org/docs/warning_types/dynamic_render_path/",
       "code": "render(partial => \"stream_entries/#{Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity_type.downcase}\", { :locals => ({ Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity_type.downcase.to_sym => Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity, :include_threads => true }) })",
-      "render_path": [{"type":"controller","class":"StatusesController","method":"show","line":15,"file":"app/controllers/statuses_controller.rb"}],
+      "render_path": [{"type":"controller","class":"StatusesController","method":"show","line":20,"file":"app/controllers/statuses_controller.rb"}],
       "location": {
         "type": "template",
         "template": "stream_entries/show"
@@ -97,6 +97,6 @@
       "note": ""
     }
   ],
-  "updated": "2017-05-07 08:26:06 +0900",
-  "brakeman_version": "3.6.1"
+  "updated": "2017-08-30 05:14:04 +0200",
+  "brakeman_version": "3.7.2"
 }

config/environments/development.rb

@@ -16,9 +16,10 @@ Rails.application.configure do
   if Rails.root.join('tmp/caching-dev.txt').exist?
     config.action_controller.perform_caching = true
 
-    config.cache_store = :memory_store
+    config.cache_store = :redis_store, ENV['REDIS_URL'], REDIS_CACHE_PARAMS
+
     config.public_file_server.headers = {
-      'Cache-Control' => "public, max-age=#{2.days.seconds.to_i}"
+      'Cache-Control' => "public, max-age=#{2.days.seconds.to_i}",
     }
   else
     config.action_controller.perform_caching = false

config/routes.rb

@@ -44,6 +44,7 @@ Rails.application.routes.draw do
     resources :statuses, only: [:show] do
       member do
         get :activity
+        get :embed
       end
     end
 
@@ -59,6 +60,7 @@ Rails.application.routes.draw do
   get '/@:username/with_replies', to: 'accounts#show', as: :short_account_with_replies
   get '/@:username/media', to: 'accounts#show', as: :short_account_media
   get '/@:account_username/:id', to: 'statuses#show', as: :short_account_status
+  get '/@:account_username/:id/embed', to: 'statuses#embed', as: :embed_short_account_status
 
   namespace :settings do
     resource :profile, only: [:show, :update]

spec/controllers/stream_entries_controller_spec.rb

@@ -88,14 +88,12 @@ RSpec.describe StreamEntriesController, type: :controller do
   describe 'GET #embed' do
     include_examples 'before_action', :embed
 
-    it 'returns embedded view of status' do
+    it 'redirects to new embed page' do
       status = Fabricate(:status)
 
       get :embed, params: { account_username: status.account.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')
+      expect(response).to redirect_to(embed_short_account_status_url(status.account, status))
     end
   end
 end

spec/lib/stream_entry_finder_spec.rb → spec/lib/status_finder_spec.rb

@@ -2,17 +2,17 @@
 
 require 'rails_helper'
 
-describe StreamEntryFinder do
+describe StatusFinder do
   include RoutingHelper
 
-  describe '#stream_entry' do
+  describe '#status' do
     context 'with a status url' do
       let(:status) { Fabricate(:status) }
       let(:url) { short_account_status_url(account_username: status.account.username, id: status.id) }
       subject { described_class.new(url) }
 
       it 'finds the stream entry' do
-        expect(subject.stream_entry).to eq(status.stream_entry)
+        expect(subject.status).to eq(status)
       end
 
       it 'raises an error if action is not :show' do
@@ -20,7 +20,7 @@ describe StreamEntryFinder do
         expect(recognized).to receive(:[]).with(:action).and_return(:create)
         expect(Rails.application.routes).to receive(:recognize_path).with(url).and_return(recognized)
 
-        expect { subject.stream_entry }.to raise_error(ActiveRecord::RecordNotFound)
+        expect { subject.status }.to raise_error(ActiveRecord::RecordNotFound)
       end
     end
 
@@ -30,7 +30,7 @@ describe StreamEntryFinder do
       subject { described_class.new(url) }
 
       it 'finds the stream entry' do
-        expect(subject.stream_entry).to eq(stream_entry)
+        expect(subject.status).to eq(stream_entry.status)
       end
     end
 
@@ -39,7 +39,7 @@ describe StreamEntryFinder do
       subject { described_class.new(url) }
 
       it 'raises an error' do
-        expect { subject.stream_entry }.to raise_error(ActiveRecord::RecordNotFound)
+        expect { subject.status }.to raise_error(ActiveRecord::RecordNotFound)
       end
     end
 
@@ -48,7 +48,7 @@ describe StreamEntryFinder do
       subject { described_class.new(url) }
 
       it 'raises an error' do
-        expect { subject.stream_entry }.to raise_error(ActiveRecord::RecordNotFound)
+        expect { subject.status }.to raise_error(ActiveRecord::RecordNotFound)
       end
     end
   end