From 3ddd936b039474259cff3793c767ecb7f74e89e0 Mon Sep 17 00:00:00 2001
From: Matt Jankowski <mjankowski@thoughtbot.com>
Date: Tue, 11 Apr 2017 16:00:43 -0400
Subject: [PATCH] Refactor exports controller (#1567)

* Add basic coverage for settings/exports controller

* Remove unused @account variable from settings/exports controller

* Add coverage for download export actions

* Remove deprecated `render :text` in favor of `send_data` for csv downloads

* Add model to handle exports

* Use Export class in settings/exports controller

* Simplify settings/exports controller methods

* Move settings/export to more restful routes
---
 .../exports/blocked_accounts_controller.rb    | 17 ++++++++++
 .../exports/following_accounts_controller.rb  | 17 ++++++++++
 .../settings/exports_controller.rb            | 33 -------------------
 app/models/export.rb                          | 18 ++++++++++
 app/views/settings/exports/show.html.haml     |  4 +--
 config/routes.rb                              |  9 +++--
 .../blocked_accounts_controller_spec.rb       | 17 ++++++++++
 .../following_accounts_controller_spec.rb     | 17 ++++++++++
 .../settings/exports_controller_spec.rb       | 14 ++++++++
 spec/models/export_spec.rb                    | 17 ++++++++++
 10 files changed, 123 insertions(+), 40 deletions(-)
 create mode 100644 app/controllers/settings/exports/blocked_accounts_controller.rb
 create mode 100644 app/controllers/settings/exports/following_accounts_controller.rb
 create mode 100644 app/models/export.rb
 create mode 100644 spec/controllers/settings/exports/blocked_accounts_controller_spec.rb
 create mode 100644 spec/controllers/settings/exports/following_accounts_controller_spec.rb
 create mode 100644 spec/controllers/settings/exports_controller_spec.rb
 create mode 100644 spec/models/export_spec.rb

diff --git a/app/controllers/settings/exports/blocked_accounts_controller.rb b/app/controllers/settings/exports/blocked_accounts_controller.rb
new file mode 100644
index 000000000..0bf8848b4
--- /dev/null
+++ b/app/controllers/settings/exports/blocked_accounts_controller.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module Settings
+  module Exports
+    class BlockedAccountsController < ApplicationController
+      before_action :authenticate_user!
+
+      def index
+        export_data = Export.new(current_account.blocking).to_csv
+
+        respond_to do |format|
+          format.csv { send_data export_data, filename: 'blocking.csv' }
+        end
+      end
+    end
+  end
+end
diff --git a/app/controllers/settings/exports/following_accounts_controller.rb b/app/controllers/settings/exports/following_accounts_controller.rb
new file mode 100644
index 000000000..a7f4344ca
--- /dev/null
+++ b/app/controllers/settings/exports/following_accounts_controller.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module Settings
+  module Exports
+    class FollowingAccountsController < ApplicationController
+      before_action :authenticate_user!
+
+      def index
+        export_data = Export.new(current_account.following).to_csv
+
+        respond_to do |format|
+          format.csv { send_data export_data, filename: 'following.csv' }
+        end
+      end
+    end
+  end
+end
diff --git a/app/controllers/settings/exports_controller.rb b/app/controllers/settings/exports_controller.rb
index ff688978c..e060f03d3 100644
--- a/app/controllers/settings/exports_controller.rb
+++ b/app/controllers/settings/exports_controller.rb
@@ -1,46 +1,13 @@
 # frozen_string_literal: true
 
-require 'csv'
-
 class Settings::ExportsController < ApplicationController
   layout 'admin'
 
   before_action :authenticate_user!
-  before_action :set_account
 
   def show
     @total_storage = current_account.media_attachments.sum(:file_file_size)
     @total_follows = current_account.following.count
     @total_blocks  = current_account.blocking.count
   end
-
-  def download_following_list
-    @accounts = current_account.following
-
-    respond_to do |format|
-      format.csv { render text: accounts_list_to_csv(@accounts) }
-    end
-  end
-
-  def download_blocking_list
-    @accounts = current_account.blocking
-
-    respond_to do |format|
-      format.csv { render text: accounts_list_to_csv(@accounts) }
-    end
-  end
-
-  private
-
-  def set_account
-    @account = current_user.account
-  end
-
-  def accounts_list_to_csv(list)
-    CSV.generate do |csv|
-      list.each do |account|
-        csv << [(account.local? ? account.local_username_and_domain : account.acct)]
-      end
-    end
-  end
 end
diff --git a/app/models/export.rb b/app/models/export.rb
new file mode 100644
index 000000000..cd1a58eb6
--- /dev/null
+++ b/app/models/export.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+require 'csv'
+
+class Export
+  attr_reader :accounts
+
+  def initialize(accounts)
+    @accounts = accounts
+  end
+
+  def to_csv
+    CSV.generate do |csv|
+      accounts.each do |account|
+        csv << [(account.local? ? account.local_username_and_domain : account.acct)]
+      end
+    end
+  end
+end
diff --git a/app/views/settings/exports/show.html.haml b/app/views/settings/exports/show.html.haml
index 0a0ff8633..432a61b4a 100644
--- a/app/views/settings/exports/show.html.haml
+++ b/app/views/settings/exports/show.html.haml
@@ -10,8 +10,8 @@
     %tr
       %th= t('exports.follows')
       %td= @total_follows
-      %td= table_link_to 'download', t('exports.csv'), follows_settings_export_path(format: :csv)
+      %td= table_link_to 'download', t('exports.csv'), settings_exports_follows_path(format: :csv)
     %tr
       %th= t('exports.blocks')
       %td= @total_blocks
-      %td= table_link_to 'download', t('exports.csv'), blocks_settings_export_path(format: :csv)
+      %td= table_link_to 'download', t('exports.csv'), settings_exports_blocks_path(format: :csv)
diff --git a/config/routes.rb b/config/routes.rb
index 9adcdb862..69f8887b2 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -53,11 +53,10 @@ Rails.application.routes.draw do
     resource :preferences, only: [:show, :update]
     resource :import, only: [:show, :create]
 
-    resource :export, only: [:show] do
-      collection do
-        get :follows, to: 'exports#download_following_list'
-        get :blocks, to: 'exports#download_blocking_list'
-      end
+    resource :export, only: [:show]
+    namespace :exports, constraints: { format: :csv } do
+      resources :follows, only: :index, controller: :following_accounts
+      resources :blocks, only: :index, controller: :blocked_accounts
     end
 
     resource :two_factor_auth, only: [:show, :new, :create] do
diff --git a/spec/controllers/settings/exports/blocked_accounts_controller_spec.rb b/spec/controllers/settings/exports/blocked_accounts_controller_spec.rb
new file mode 100644
index 000000000..574d4d875
--- /dev/null
+++ b/spec/controllers/settings/exports/blocked_accounts_controller_spec.rb
@@ -0,0 +1,17 @@
+require 'rails_helper'
+
+describe Settings::Exports::BlockedAccountsController do
+  before do
+    sign_in Fabricate(:user), scope: :user
+  end
+
+  describe 'GET #index' do
+    it 'returns a csv of the blocking accounts' do
+      get :index, format: :csv
+
+      expect(response).to have_http_status(:success)
+      expect(response.content_type).to eq 'text/csv'
+      expect(response.headers['Content-Disposition']).to eq 'attachment; filename="blocking.csv"'
+    end
+  end
+end
diff --git a/spec/controllers/settings/exports/following_accounts_controller_spec.rb b/spec/controllers/settings/exports/following_accounts_controller_spec.rb
new file mode 100644
index 000000000..bf7680523
--- /dev/null
+++ b/spec/controllers/settings/exports/following_accounts_controller_spec.rb
@@ -0,0 +1,17 @@
+require 'rails_helper'
+
+describe Settings::Exports::FollowingAccountsController do
+  before do
+    sign_in Fabricate(:user), scope: :user
+  end
+
+  describe 'GET #index' do
+    it 'returns a csv of the following accounts' do
+      get :index, format: :csv
+
+      expect(response).to have_http_status(:success)
+      expect(response.content_type).to eq 'text/csv'
+      expect(response.headers['Content-Disposition']).to eq 'attachment; filename="following.csv"'
+    end
+  end
+end
diff --git a/spec/controllers/settings/exports_controller_spec.rb b/spec/controllers/settings/exports_controller_spec.rb
new file mode 100644
index 000000000..ff98f3ad9
--- /dev/null
+++ b/spec/controllers/settings/exports_controller_spec.rb
@@ -0,0 +1,14 @@
+require 'rails_helper'
+
+describe Settings::ExportsController do
+  before do
+    sign_in Fabricate(:user), scope: :user
+  end
+
+  describe 'GET #show' do
+    it 'returns http success' do
+      get :show
+      expect(response).to have_http_status(:success)
+    end
+  end
+end
diff --git a/spec/models/export_spec.rb b/spec/models/export_spec.rb
new file mode 100644
index 000000000..5cc62c266
--- /dev/null
+++ b/spec/models/export_spec.rb
@@ -0,0 +1,17 @@
+require 'rails_helper'
+
+describe Export do
+  describe 'to_csv' do
+    it 'returns a csv of the accounts' do
+      one = Account.new(username: 'one', domain: 'local.host')
+      two = Account.new(username: 'two', domain: 'local.host')
+      accounts = [one, two]
+
+      export = Export.new(accounts).to_csv
+      results = export.strip.split
+
+      expect(results.size).to eq 2
+      expect(results.first).to eq 'one@local.host'
+    end
+  end
+end