Merge pull request #1717 from okriuchykhin/ok_SCI_3368

Improve clean up of Export All files [SCI-3368]

app/controllers/zip_exports_controller.rb

@@ -1,7 +1,8 @@
+# frozen_string_literal: true
+
 class ZipExportsController < ApplicationController
-  before_action :load_var, only: :download
-  before_action :load_var_export_all, only: :download_export_all_zip
-  before_action :check_edit_permissions, only: :download
+  before_action :load_var, only: %i(download download_export_all_zip)
+  before_action :check_download_permissions, except: :file_expired
 
   def download
     if @zip_export.stored_on_s3?
@@ -22,16 +23,11 @@ class ZipExportsController < ApplicationController
   private
 
   def load_var
-    @zip_export = ZipExport.find_by_id(params[:id])
-    redirect_to(file_expired_url, status: 301) and return unless @zip_export
+    @zip_export = current_user.zip_exports.find_by_id(params[:id])
+    redirect_to(file_expired_url, status: 301) and return unless @zip_export&.zip_file&.exists?
   end
 
-  def load_var_export_all
-    @zip_export = TeamZipExport.find_by_id(params[:id])
-    redirect_to(file_expired_url, status: 301) and return unless @zip_export
-  end
-
-  def check_edit_permissions
+  def check_download_permissions
     render_403 unless @zip_export.user == current_user
   end
 end

app/models/zip_export.rb

@@ -24,6 +24,8 @@ class ZipExport < ApplicationRecord
   validates_attachment :zip_file,
                        content_type: { content_type: 'application/zip' }
 
+  after_create :self_destruct
+
   # When using S3 file upload, we can limit file accessibility with url signing
   def presigned_url(style = :original,
                     download: false,
@@ -48,6 +50,11 @@ class ZipExport < ApplicationRecord
     zip_file.options[:storage].to_sym == :s3
   end
 
+  def self.delete_expired_export(id)
+    export = find_by_id(id)
+    export.destroy if export
+  end
+
   def generate_exportable_zip(user, data, type, options = {})
     I18n.backend.date_format =
       user.settings[:date_format] || Constants::DEFAULT_DATE_FORMAT
@@ -71,6 +78,11 @@ class ZipExport < ApplicationRecord
 
   private
 
+  def self_destruct
+    ZipExport.delay(run_at: Constants::EXPORTABLE_ZIP_EXPIRATION_DAYS.days.from_now)
+             .delete_expired_export(id)
+  end
+
   def method_missing(m, *args, &block)
     puts 'Method is missing! To use this zip_export you have to ' \
          'define a method: generate_( type )_zip.'

spec/services/repository_zip_export_spec.rb

@@ -59,11 +59,14 @@ describe RepositoryZipExport, type: :background_job do
     end
 
     it 'generates a new zip export object' do
+      ZipExport.skip_callback(:create, :after, :self_destruct)
       RepositoryZipExport.generate_zip(params, repository, user)
       expect(ZipExport.count).to eq 1
+      ZipExport.set_callback(:create, :after, :self_destruct)
     end
 
     it 'generates a zip with csv file with exported rows' do
+      ZipExport.skip_callback(:create, :after, :self_destruct)
       RepositoryZipExport.generate_zip(params, repository, user)
       csv_zip_file = ZipExport.first.zip_file
       parsed_csv_content = Zip::File.open(csv_zip_file.path) do |zip_file|
@@ -80,6 +83,7 @@ describe RepositoryZipExport, type: :background_job do
         expect(row_hash.fetch('Name')).to eq "row #{index}"
         index += 1
       end
+      ZipExport.set_callback(:create, :after, :self_destruct)
     end
   end
 end