From b154d87070570269f7b165007f19eda783153aca Mon Sep 17 00:00:00 2001
From: Marcelo Briones <ing@marcelobriones.com.ar>
Date: Thu, 19 Feb 2015 00:04:25 -0300
Subject: [PATCH] Exports user photos as zip file

---
 app/controllers/users_controller.rb           |  9 ++++-
 app/mailers/export_mailer.rb                  | 17 +++++++++
 app/models/user.rb                            | 38 ++++++++++++++++++-
 app/uploaders/exported_photos.rb              | 21 ++++++++++
 app/views/users/edit.html.haml                | 25 ++++++++----
 app/views/users/edit.mobile.haml              | 29 +++++++++-----
 app/views/users/export_photos_email.markerb   |  1 +
 .../users/export_photos_failure_email.markerb |  1 +
 app/workers/export_photos.rb                  | 21 ++++++++++
 config/initializers/load_libraries.rb         |  1 -
 config/locales/diaspora/en.yml                | 26 +++++++++++++
 config/routes.rb                              |  1 +
 ...0150220001357_add_photos_export_to_user.rb | 13 +++++++
 db/schema.rb                                  |  5 ++-
 lib/collect_user_photos.rb                    | 25 ------------
 spec/controllers/users_controller_spec.rb     | 14 ++++++-
 spec/mailers/export_spec.rb                   | 30 +++++++++++++++
 spec/models/user_spec.rb                      | 35 +++++++++++++++++
 spec/workers/export_photos_spec.rb            | 26 +++++++++++++
 19 files changed, 288 insertions(+), 50 deletions(-)
 create mode 100644 app/uploaders/exported_photos.rb
 create mode 100644 app/views/users/export_photos_email.markerb
 create mode 100644 app/views/users/export_photos_failure_email.markerb
 create mode 100644 app/workers/export_photos.rb
 create mode 100644 db/migrate/20150220001357_add_photos_export_to_user.rb
 delete mode 100644 lib/collect_user_photos.rb
 create mode 100644 spec/workers/export_photos_spec.rb

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 1161a391a..72dbea5c1 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -148,10 +148,15 @@ class UsersController < ApplicationController
   end
 
   def export_photos
-    tar_path = PhotoMover::move_photos(current_user)
-    send_data( File.open(tar_path).read, :filename => "#{current_user.id}.tar" )
+    current_user.queue_export_photos
+    flash[:notice] = I18n.t('users.edit.export_photos_in_progress')
+    redirect_to edit_user_path
   end
 
+  def download_photos
+    redirect_to current_user.exported_photos_file.url
+  end
+  
   def user_photo
     username = params[:username].split('@')[0]
     user = User.find_by_username(username)
diff --git a/app/mailers/export_mailer.rb b/app/mailers/export_mailer.rb
index 24e34d0f6..a0bc8b5e5 100644
--- a/app/mailers/export_mailer.rb
+++ b/app/mailers/export_mailer.rb
@@ -19,4 +19,21 @@ class ExportMailer < ActionMailer::Base
     end
   end
 
+  def export_photos_complete_for(user)
+    @user = user
+
+    mail(to: @user.email, subject: I18n.t('notifier.export_photos_email.subject', name: @user.name)) do |format|
+      format.html { render 'users/export_photos_email' }
+      format.text { render 'users/export_photos_email' }
+    end
+  end
+
+  def export_photos_failure_for(user)
+    @user = user
+
+    mail(to: @user.email, subject: I18n.t('notifier.export_photos_failure_email.subject', name: @user.name)) do |format|
+      format.html { render 'users/export_photos_failure_email' }
+      format.text { render 'users/export_photos_failure_email' }
+    end
+  end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index 91bf3214d..17448f126 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -314,6 +314,40 @@ class User < ActiveRecord::Base
     ActiveSupport::Gzip.compress Diaspora::Exporter.new(self).execute
   end
 
+  ######### Photos export ##################
+  mount_uploader :exported_photos_file, ExportedPhotos
+
+  def queue_export_photos
+    update exporting_photos: true
+    Workers::ExportPhotos.perform_async(id)
+  end
+
+  def perform_export_photos!
+    temp_zip = Tempfile.new([username, '_photos.zip'])
+    begin
+      Zip::ZipOutputStream.open(temp_zip.path) do |zos|
+        photos.each do |photo|
+          begin
+            photo_data = photo.unprocessed_image.file.read
+            zos.put_next_entry(photo.remote_photo_name)
+            zos.print(photo_data)
+          rescue Errno::ENOENT
+            logger.info "Export photos error: #{photo.unprocessed_image.file.path} not found"
+          end
+        end
+      end
+    ensure
+      temp_zip.close
+    end
+
+    begin
+      update exported_photos_file: temp_zip, exported_photos_at: Time.zone.now if temp_zip.present?
+    ensure
+      restore_attributes if invalid? || temp_zip.present?
+      update exporting_photos: false
+    end
+  end
+  
   ######### Mailer #######################
   def mail(job, *args)
     pref = job.to_s.gsub('Workers::Mail::', '').underscore
@@ -534,6 +568,8 @@ class User < ActiveRecord::Base
                             "created_at", "updated_at", "locked_at",
                             "serialized_private_key", "getting_started",
                             "disable_mail", "show_community_spotlight_in_stream",
-                            "strip_exif", "email", "remove_after", "export", "exporting", "exported_at"]
+                            "strip_exif", "email", "remove_after",
+                            "export", "exporting", "exported_at",
+                            "exported_photos_file", "exporting_photos", "exported_photos_at"]
   end
 end
diff --git a/app/uploaders/exported_photos.rb b/app/uploaders/exported_photos.rb
new file mode 100644
index 000000000..d68f94e0e
--- /dev/null
+++ b/app/uploaders/exported_photos.rb
@@ -0,0 +1,21 @@
+#   Copyright (c) 2010-2011, Diaspora Inc.  This file is
+#   licensed under the Affero General Public License version 3 or later.  See
+#   the COPYRIGHT file.
+
+class ExportedPhotos < CarrierWave::Uploader::Base
+
+  def store_dir
+    "uploads/users"
+  end
+
+  def filename
+    "#{model.username}_photos_#{secure_token}.zip" if original_filename.present?
+  end
+
+  protected
+  def secure_token(bytes = 16)
+    var = :"@#{mounted_as}_secure_token"
+    model.instance_variable_get(var) or model.instance_variable_set(var, SecureRandom.urlsafe_base64(bytes))
+  end
+
+end
diff --git a/app/views/users/edit.html.haml b/app/views/users/edit.html.haml
index b3bf48d8e..f31ee00b4 100644
--- a/app/views/users/edit.html.haml
+++ b/app/views/users/edit.html.haml
@@ -182,23 +182,32 @@
           %h3 
             = t('.export_data')
           - if current_user.exporting
-            .small-horizontal-spacer
-              .export-in-progress= t('.export_in_progress')
+            .export-in-progress= t('.export_in_progress')
           - elsif current_user.export.present?
-            .small-horizontal-spacer
-              = link_to t('.download_export'), download_profile_user_path, class: "button"
-            .small-horizontal-spacer
+            = link_to t('.download_export'), download_profile_user_path, class: "btn btn-success"
+            %h6
               = t('.last_exported_at', timestamp: current_user.exported_at)
+            = link_to t('.request_export_update'), export_profile_user_path, class: "btn"
+          - else
+            = link_to t('.request_export'), export_profile_user_path, :class => "btn"
+              
+          - if current_user.exporting_photos
             .small-horizontal-spacer
-              = link_to t('.request_export_update'), export_profile_user_path
+            .export-in-progress= t('.export_photos_in_progress')
+          - elsif current_user.exported_photos_file.present?
+            .small-horizontal-spacer
+            = link_to t('.download_export_photos'), download_photos_user_path, class: "btn  btn-success"
+            %h6
+              = t('.last_exported_at', timestamp: current_user.exported_photos_at)
+            = link_to t('.request_export_photos_update'), export_photos_user_path, class: "btn"
           - else
             .small-horizontal-spacer
-              = link_to t('.request_export'), export_profile_user_path, :class => "button"
+            = link_to t('.request_export_photos'), export_photos_user_path, :class => "btn"
 
         .span6
           %h3
             = t('.close_account_text')
-          =link_to t('.close_account_text'), '#close_account_pane', :rel => 'facebox', :class => "button", :id => "close_account"
+          =link_to t('.close_account_text'), '#close_account_pane', :rel => 'facebox', :class => "btn btn-danger", :id => "close_account"
 
           .hidden#close_account_pane{:rel => 'facebox'}
             #inner_account_delete
diff --git a/app/views/users/edit.mobile.haml b/app/views/users/edit.mobile.haml
index a3f51f281..688db8b18 100644
--- a/app/views/users/edit.mobile.haml
+++ b/app/views/users/edit.mobile.haml
@@ -162,23 +162,31 @@
     %h4
       = t('.export_data')
     - if current_user.exporting
-      .small-horizontal-spacer
-        .export-in-progress= t('.export_in_progress')
+      .export-in-progress= t('.export_in_progress')
     - elsif current_user.export.present?
-      .small-horizontal-spacer
-        = link_to t('.download_export'), download_profile_user_path, class: "button"
-      .small-horizontal-spacer
+      = link_to t('.download_export'), download_profile_user_path, class: "btn btn-success"
+      %h6
         = t('.last_exported_at', timestamp: current_user.exported_at)
-      .small-horizontal-spacer
-        = link_to t('.request_export_update'), export_profile_user_path
+      = link_to t('.request_export_update'), export_profile_user_path, class: "btn"
     - else
-      .small-horizontal-spacer
-        = link_to t('.request_export'), export_profile_user_path, :class => "button"
+      = link_to t('.request_export'), export_profile_user_path, :class => "btn"
+    %br
+    %br
+    - if current_user.exporting_photos
+      .export-in-progress= t('.export_photos_in_progress')
+    - elsif current_user.exported_photos_file.present?
+      = link_to t('.download_export_photos'), download_photos_user_path, class: "btn btn-success"
+      %h6
+        = t('.last_exported_at', timestamp: current_user.exported_photos_at)
+      = link_to t('.request_export_photos_update'), export_photos_user_path, class: "btn"
+    - else
+      = link_to t('.request_export_photos'), export_photos_user_path, :class => "btn"
 
+  %hr
   .span-5.last
     %h4
       = t('.close_account_text')
-    =link_to t('.close_account_text'), '#close_account_pane', :rel => 'facebox', :class => "btn", :id => "close_account"
+    =link_to t('.close_account_text'), '#close_account_pane', :rel => 'facebox', :class => "btn btn-danger", :id => "close_account"
 
     .hidden#close_account_pane{:rel => 'facebox'}
       #inner_account_delete
@@ -213,3 +221,4 @@
               = f.password_field :current_password, :id => :close_account_password
             %p
               = f.submit t('.close_account_text'), :id => "close_account_confirm", :data => { :confirm => t('are_you_sure_delete_account') }
+  %br
diff --git a/app/views/users/export_photos_email.markerb b/app/views/users/export_photos_email.markerb
new file mode 100644
index 000000000..24bc4fd1d
--- /dev/null
+++ b/app/views/users/export_photos_email.markerb
@@ -0,0 +1 @@
+<%= t('notifier.export_photos_email.body', url: download_photos_user_url, name: @user.first_name) %>
diff --git a/app/views/users/export_photos_failure_email.markerb b/app/views/users/export_photos_failure_email.markerb
new file mode 100644
index 000000000..2067327c9
--- /dev/null
+++ b/app/views/users/export_photos_failure_email.markerb
@@ -0,0 +1 @@
+<%= t('notifier.export_photos_failure_email.body', name: @user.first_name) %>
diff --git a/app/workers/export_photos.rb b/app/workers/export_photos.rb
new file mode 100644
index 000000000..b49935c11
--- /dev/null
+++ b/app/workers/export_photos.rb
@@ -0,0 +1,21 @@
+#   Copyright (c) 2010-2011, Diaspora Inc.  This file is
+#   licensed under the Affero General Public License version 3 or later.  See
+#   the COPYRIGHT file.
+
+
+module Workers
+  class ExportPhotos < Base
+    sidekiq_options queue: :export_photos
+
+    def perform(user_id)
+      @user = User.find(user_id)
+      @user.perform_export_photos!
+
+      if @user.reload.exported_photos_file.present?
+        ExportMailer.export_photos_complete_for(@user)
+      else
+        ExportMailer.export_photos_failure_for(@user)
+      end
+    end
+  end
+end
diff --git a/config/initializers/load_libraries.rb b/config/initializers/load_libraries.rb
index 87f46ccad..b83911b87 100644
--- a/config/initializers/load_libraries.rb
+++ b/config/initializers/load_libraries.rb
@@ -13,7 +13,6 @@ require 'typhoeus'
 require 'post_presenter'
 
 # Our libs
-require 'collect_user_photos'
 require 'diaspora'
 require 'direction_detector'
 require 'email_inviter'
diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml
index 2f97d1ac4..159a9da47 100644
--- a/config/locales/diaspora/en.yml
+++ b/config/locales/diaspora/en.yml
@@ -817,8 +817,29 @@ en:
         We’ve encountered an issue while processing your personal data for download.
         Please try again!
 
+        Sorry,
+
+        The diaspora* email robot!
+    export_photos_email:
+      subject: "Your photos are ready for download, %{name}"
+      body: |-
+        Hello %{name},
+
+        Your photos have been processed and are ready for download by following [this link](%{url}).
+
         Cheers,
 
+        The diaspora* email robot!
+    export_photos_failure_email:
+      subject: "There was an issue with your photos, %{name}"
+      body: |-
+        Hello %{name}
+
+        We’ve encountered an issue while processing your photos for download.
+        Please try again!
+
+        Sorry,
+
         The diaspora* email robot!
     accept_invite: "Accept your diaspora* invite!"
     invited_you: "%{name} invited you to diaspora*"
@@ -1319,6 +1340,11 @@ en:
       export_data: "Export data"
       export_in_progress: "We are currently processing your data. Please check back in a few moments."
       last_exported_at: "(Last updated at %{timestamp})"
+      download_export_photos: "Download my photos"
+      request_export_photos: "Request my photos"
+      request_export_photos_update: "Refresh my photos"
+      download_photos: "Download my photos"
+      export_photos_in_progress: "We are currently processing your photos. Please check back in a few moments."
 
       close_account:
         dont_go: "Hey, please don’t go!"
diff --git a/config/routes.rb b/config/routes.rb
index e31d8409d..a43effa5c 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -104,6 +104,7 @@ Diaspora::Application.routes.draw do
     get :export_profile
     get :download_profile
     get :export_photos
+    get :download_photos
   end
 
   controller :users do
diff --git a/db/migrate/20150220001357_add_photos_export_to_user.rb b/db/migrate/20150220001357_add_photos_export_to_user.rb
new file mode 100644
index 000000000..f19349723
--- /dev/null
+++ b/db/migrate/20150220001357_add_photos_export_to_user.rb
@@ -0,0 +1,13 @@
+class AddPhotosExportToUser < ActiveRecord::Migration
+  def up
+    add_column :users, :exported_photos_file, :string
+    add_column :users, :exported_photos_at, :datetime
+    add_column :users, :exporting_photos, :boolean, default: false
+  end
+
+  def down
+    remove_column :users, :exported_photos_file
+    remove_column :users, :exported_photos_at
+    remove_column :users, :exporting_photos
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 49eb595e4..6bdcab9ca 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20150209230946) do
+ActiveRecord::Schema.define(version: 20150220001357) do
 
   create_table "account_deletions", force: :cascade do |t|
     t.string   "diaspora_handle", limit: 255
@@ -562,6 +562,9 @@ ActiveRecord::Schema.define(version: 20150209230946) do
     t.datetime "exported_at"
     t.boolean  "exporting",                                        default: false
     t.boolean  "strip_exif",                                       default: true
+    t.string   "exported_photos_file",               limit: 255
+    t.datetime "exported_photos_at"
+    t.boolean  "exporting_photos",                                 default: false
   end
 
   add_index "users", ["authentication_token"], name: "index_users_on_authentication_token", unique: true, using: :btree
diff --git a/lib/collect_user_photos.rb b/lib/collect_user_photos.rb
deleted file mode 100644
index 8ea909a64..000000000
--- a/lib/collect_user_photos.rb
+++ /dev/null
@@ -1,25 +0,0 @@
-module PhotoMover
-  def self.move_photos(user)
-    Dir.chdir Rails.root
-    temp_dir = "tmp/exports/#{user.id}"
-    FileUtils::mkdir_p temp_dir
-    Dir.chdir 'tmp/exports'
-
-    photos = user.visible_shareables(Post).where(:author_id => user.person_id, :type => 'Photo')
-
-    photos_dir = "#{user.id}/photos"
-    FileUtils::mkdir_p photos_dir
-
-    photos.each do |photo|
-      current_photo_location = "#{Rails.root}/public/uploads/images/#{photo.remote_photo_name}"
-      new_photo_location = "#{photos_dir}/#{photo.remote_photo_name}"
-      FileUtils::cp current_photo_location, new_photo_location
-    end
-
-    `tar c #{user.id} > #{user.id}.tar`
-    #system("tar", "c", "#{user.id}",">", "#{user.id}.tar")
-    FileUtils::rm_r "#{user.id.to_s}/", :secure => true, :force => true
-
-    "#{Rails.root}/#{temp_dir}.tar"
-  end
-end
diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index daf46e1e1..9b339135c 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -30,9 +30,19 @@ describe UsersController, :type => :controller do
   end
 
   describe '#export_photos' do
-    it 'returns a tar file'  do
+    it 'queues an export photos job' do
+      expect(@user).to receive :queue_export_photos
       get :export_photos
-      expect(response.header["Content-Type"]).to include "application/octet-stream"
+      expect(request.flash[:notice]).to eql(I18n.t('users.edit.export_photos_in_progress'))
+      expect(response).to redirect_to(edit_user_path)
+    end
+  end
+  
+  describe '#download_photos' do
+    it "redirects to user's photos zip file"  do
+      @user.perform_export_photos!
+      get :download_photos
+      expect(response).to redirect_to(@user.exported_photos_file.url)
     end
   end
 
diff --git a/spec/mailers/export_spec.rb b/spec/mailers/export_spec.rb
index 16d5ac71c..24b10753b 100644
--- a/spec/mailers/export_spec.rb
+++ b/spec/mailers/export_spec.rb
@@ -34,4 +34,34 @@ describe ExportMailer, :type => :mailer do
       expect(ActionMailer::Base.deliveries[0].to[0]).to include(alice.email)
     end
   end
+
+  describe '#export_photos_complete_for' do
+    it "should deliver successfully" do
+      expect { ExportMailer.export_photos_complete_for(alice).deliver_now }.to_not raise_error
+    end
+
+    it "should be added to the delivery queue" do
+      expect { ExportMailer.export_photos_complete_for(alice).deliver_now }.to change(ActionMailer::Base.deliveries, :size).by(1)
+    end
+
+    it "should include correct recipient" do
+      ExportMailer.export_photos_complete_for(alice).deliver_now
+      expect(ActionMailer::Base.deliveries[0].to[0]).to include(alice.email)
+    end
+  end
+
+  describe '#export_photos_failure_for' do
+    it "should deliver successfully" do
+      expect { ExportMailer.export_photos_failure_for(alice).deliver_now }.to_not raise_error
+    end
+
+    it "should be added to the delivery queue" do
+      expect { ExportMailer.export_photos_failure_for(alice).deliver_now }.to change(ActionMailer::Base.deliveries, :size).by(1)
+    end
+
+    it "should include correct recipient" do
+      ExportMailer.export_photos_failure_for(alice).deliver_now
+      expect(ActionMailer::Base.deliveries[0].to[0]).to include(alice.email)
+    end
+  end
 end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index d766874be..fc5a94404 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1035,6 +1035,41 @@ describe User, :type => :model do
     end
   end
 
+  describe "queue_export_photos" do
+    it "queues up a job to perform the export photos" do
+      user = FactoryGirl.create :user
+      expect(Workers::ExportPhotos).to receive(:perform_async).with(user.id)
+      user.queue_export_photos
+      expect(user.exporting_photos).to be_truthy
+    end
+  end
+
+  describe "perform_export_photos!" do
+    before do
+      @user = alice
+      filename  = 'button.png'
+      image = File.join(File.dirname(__FILE__), '..', 'fixtures', filename)
+      @saved_image = @user.build_post(:photo, :user_file => File.open(image), :to => alice.aspects.first.id)
+      @saved_image.save!
+    end
+
+    it "saves a zip export to the user" do
+      @user.perform_export_photos!
+      expect(@user.exported_photos_file).to be_present
+      expect(@user.exported_photos_at).to be_present
+      expect(@user.exporting_photos).to be_falsey
+      expect(@user.exported_photos_file.filename).to match /.zip/
+      expect(Zip::ZipFile.open(@user.exported_photos_file.path).entries.count).to eq(1)
+    end
+
+    it "does not add empty entries when photo not found" do
+      File.unlink @user.photos.first.unprocessed_image.path
+      @user.perform_export_photos!
+      expect(@user.exported_photos_file.filename).to match /.zip/
+      expect(Zip::ZipFile.open(@user.exported_photos_file.path).entries.count).to eq(0)
+    end
+  end
+
   describe "sign up" do
     before do
       params = {:username => "ohai",
diff --git a/spec/workers/export_photos_spec.rb b/spec/workers/export_photos_spec.rb
new file mode 100644
index 000000000..15db9eee4
--- /dev/null
+++ b/spec/workers/export_photos_spec.rb
@@ -0,0 +1,26 @@
+require 'spec_helper'
+
+describe Workers::ExportPhotos do
+
+  before do
+    allow(User).to receive(:find).with(alice.id).and_return(alice)
+  end
+
+  it 'calls export_photos! on user with given id' do
+    expect(alice).to receive(:perform_export_photos!)
+    Workers::ExportPhotos.new.perform(alice.id)
+  end
+
+  it 'sends a success message when the export photos is successful' do
+    allow(alice).to receive(:exported_photos_file).and_return(OpenStruct.new)
+    expect(ExportMailer).to receive(:export_photos_complete_for).with(alice).and_call_original
+    Workers::ExportPhotos.new.perform(alice.id)
+  end
+
+  it 'sends a failure message when the export photos fails' do
+    allow(alice).to receive(:exported_photos_file).and_return(nil)
+    expect(alice).to receive(:perform_export_photos!).and_return(false)
+    expect(ExportMailer).to receive(:export_photos_failure_for).with(alice).and_call_original
+    Workers::ExportPhotos.new.perform(alice.id)
+  end
+end