From d600e71b3da94c5122bdca1e6df0991fd2cc5850 Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Tue, 5 Jun 2018 09:41:40 +0800
Subject: [PATCH] FIX: Clean up stale `UserExport` records daily.

* Add tests for `UserExport.remove_old_exports`
---
 app/jobs/scheduled/clean_up_exports.rb |  4 ++--
 app/models/user_export.rb              |  7 ++++---
 spec/models/user_export_spec.rb        | 28 ++++++++++++++++++++++++++
 3 files changed, 34 insertions(+), 5 deletions(-)
 create mode 100644 spec/models/user_export_spec.rb

diff --git a/app/jobs/scheduled/clean_up_exports.rb b/app/jobs/scheduled/clean_up_exports.rb
index b439d7f71c7..667d165bfab 100644
--- a/app/jobs/scheduled/clean_up_exports.rb
+++ b/app/jobs/scheduled/clean_up_exports.rb
@@ -1,9 +1,9 @@
 module Jobs
   class CleanUpExports < Jobs::Scheduled
-    every 2.day
+    every 1.day
 
     def execute(args)
-      UserExport.remove_old_exports # delete exported CSV files older than 2 days
+      UserExport.remove_old_exports
     end
   end
 end
diff --git a/app/models/user_export.rb b/app/models/user_export.rb
index 10314c029b5..6db61c37252 100644
--- a/app/models/user_export.rb
+++ b/app/models/user_export.rb
@@ -1,12 +1,13 @@
 class UserExport < ActiveRecord::Base
+  belongs_to :user
 
   def self.remove_old_exports
-    UserExport.where('created_at < ?', 2.days.ago).find_each do |expired_export|
-      file_name = "#{expired_export.file_name}-#{expired_export.id}.csv.gz"
+    UserExport.where('created_at < ?', 2.days.ago).find_each do |user_export|
+      file_name = "#{user_export.file_name}-#{user_export.id}.csv.gz"
       file_path = "#{UserExport.base_directory}/#{file_name}"
 
       File.delete(file_path) if File.exist?(file_path)
-      expired_export.destroy
+      user_export.destroy!
     end
   end
 
diff --git a/spec/models/user_export_spec.rb b/spec/models/user_export_spec.rb
new file mode 100644
index 00000000000..f1d54324b1a
--- /dev/null
+++ b/spec/models/user_export_spec.rb
@@ -0,0 +1,28 @@
+require 'rails_helper'
+
+RSpec.describe UserExport do
+  let(:user) { Fabricate(:user) }
+
+  describe '.remove_old_exports' do
+    it 'should remove the right records' do
+      export = UserExport.create!(
+        file_name: "test",
+        user: user,
+        created_at: 3.days.ago
+      )
+
+      export2 = UserExport.create!(
+        file_name: "test2",
+        user: user,
+        created_at: 1.day.ago
+      )
+
+      expect do
+        UserExport.remove_old_exports
+      end.to change { UserExport.count }.by(-1)
+
+      expect(UserExport.exists?(id: export.id)).to eq(false)
+      expect(UserExport.exists?(id: export2.id)).to eq(true)
+    end
+  end
+end