Empty system folders #1491

app/models/dmsf_file.rb

@@ -79,6 +79,8 @@ class DmsfFile < ApplicationRecord
   )
 
   before_create :default_values
+  before_destroy :delete_system_folder_before
+  after_destroy :delete_system_folder_after
 
   attr_writer :last_revision
 
@@ -95,7 +97,6 @@ class DmsfFile < ApplicationRecord
 
   def initialize(*args)
     super
-    @project = nil
     self.watcher_user_ids = [] if new_record?
   end
 
@@ -689,4 +690,14 @@ class DmsfFile < ApplicationRecord
   def to_s
     name
   end
+
+  def delete_system_folder_before
+    @parent_folder = dmsf_folder
+  end
+
+  def delete_system_folder_after
+    return unless @parent_folder&.system && @parent_folder.dmsf_files.empty? && @parent_folder.dmsf_links.empty?
+
+    @parent_folder&.destroy
+  end
 end

app/models/dmsf_folder.rb

@@ -100,6 +100,8 @@ class DmsfFolder < ApplicationRecord
   validates :dmsf_folder, dmsf_folder_parent: true, if: proc { |folder| !folder.new_record? }
 
   before_create :default_values
+  before_destroy :delete_system_folder_before
+  after_destroy :delete_system_folder_after
 
   def visible?(_user = User.current)
     return DmsfFolder.visible.exists?(id: id) if respond_to?(:type) && /^folder/.match?(type)
@@ -607,6 +609,16 @@ class DmsfFolder < ApplicationRecord
     false
   end
 
+  def delete_system_folder_before
+    @parent_folder = dmsf_folder
+  end
+
+  def delete_system_folder_after
+    return unless @parent_folder&.system && @parent_folder.dmsf_files.empty? && @parent_folder.dmsf_links.empty?
+
+    @parent_folder.destroy
+  end
+
   class << self
     def directory_subtree(tree, folder, level)
       folders = folder.dmsf_folders.visible.to_a

app/models/dmsf_link.rb

@@ -39,6 +39,9 @@ class DmsfLink < ApplicationRecord
   scope :visible, -> { where(deleted: STATUS_ACTIVE) }
   scope :deleted, -> { where(deleted: STATUS_DELETED) }
 
+  before_destroy :delete_system_folder_before
+  after_destroy :delete_system_folder_after
+
   def target_folder_id
     if target_type == DmsfFolder.model_name.to_s
       target_id
@@ -134,4 +137,14 @@ class DmsfLink < ApplicationRecord
     self.deleted_by_user = nil
     save validate: false
   end
+
+  def delete_system_folder_before
+    @parent_folder = dmsf_folder
+  end
+
+  def delete_system_folder_after
+    return unless @parent_folder&.system && @parent_folder.dmsf_files.empty? && @parent_folder.dmsf_links.empty?
+
+    @parent_folder&.destroy
+  end
 end

test/fixtures/dmsf_links.yml

@@ -76,4 +76,17 @@ file_link3:
   deleted: 0
   deleted_by_user_id: NULL
   created_at: <%= DateTime.current %>
+  updated_at: <%= DateTime.current %>
+
+file_link4:
+  id: 7
+  target_project_id: 1
+  target_id: 1
+  target_type: DmsfFile
+  name: file1_link
+  project_id: 1
+  dmsf_folder_id: 9
+  deleted: 0
+  deleted_by_user_id: NULL
+  created_at: <%= DateTime.current %>
   updated_at: <%= DateTime.current %>

test/unit/dmsf_folder_test.rb

@@ -315,4 +315,13 @@ class DmsfFolderTest < RedmineDmsf::Test::UnitTest
     assert_not @folder1.any_child?(@folder6)
     assert @folder1.any_child?(@folder2)
   end
+
+  def delete_system_folder
+    # System folders
+    assert DmsfFolder.where(id: [8, 9]).exist?
+    @file_link7.destroy
+    @dmsf_file11.destroy
+    # System folders are empty and should have been deleted
+    assert_not DmsfFolder.where(id: [8, 9]).exist?
+  end
 end

test/unit/dmsf_link_test.rb

@@ -24,12 +24,6 @@ require File.expand_path('../../test_helper', __FILE__)
 class DmsfLinksTest < RedmineDmsf::Test::UnitTest
   fixtures :dmsf_links, :dmsf_folders, :dmsf_files, :dmsf_file_revisions
 
-  def setup
-    super
-    @folder_link = DmsfLink.find 1
-    @file_link = DmsfLink.find 2
-  end
-
   def test_create_folder_link
     folder_link = DmsfLink.new
     folder_link.target_project_id = @project1.id
@@ -67,42 +61,42 @@ class DmsfLinksTest < RedmineDmsf::Test::UnitTest
   end
 
   def test_validate_name_length
-    @folder_link.name = ('a' * 256)
+    @folder_link1.name = ('a' * 256)
-    assert_not @folder_link.save, "Folder link #{@folder_link.name} should have not been saved"
+    assert_not @folder_link1.save, "Folder link #{@folder_link1.name} should have not been saved"
-    assert_equal 1, @folder_link.errors.size
+    assert_equal 1, @folder_link1.errors.size
   end
 
   def test_validate_name_presence
-    @folder_link.name = ''
+    @folder_link1.name = ''
-    assert_not @folder_link.save, "Folder link #{@folder_link.name} should have not been saved"
+    assert_not @folder_link1.save, "Folder link #{@folder_link1.name} should have not been saved"
-    assert_equal 1, @folder_link.errors.size
+    assert_equal 1, @folder_link1.errors.size
   end
 
   def test_validate_external_url_length
-    @file_link.target_type = 'DmsfUrl'
+    @file_link2.target_type = 'DmsfUrl'
-    @file_link.external_url = "https://localhost/#{'a' * 256}"
+    @file_link2.external_url = "https://localhost/#{'a' * 256}"
-    assert_not @file_link.save, "External URL link #{@file_link.name} should have not been saved"
+    assert_not @file_link2.save, "External URL link #{@file_link2.name} should have not been saved"
-    assert_equal 1, @file_link.errors.size
+    assert_equal 1, @file_link2.errors.size
   end
 
   def test_validate_external_url_presence
-    @file_link.target_type = 'DmsfUrl'
+    @file_link2.target_type = 'DmsfUrl'
-    @file_link.external_url = ''
+    @file_link2.external_url = ''
-    assert_not @file_link.save, "External URL link #{@file_link.name} should have not been saved"
+    assert_not @file_link2.save, "External URL link #{@file_link2.name} should have not been saved"
-    assert_equal 1, @file_link.errors.size
+    assert_equal 1, @file_link2.errors.size
   end
 
   def test_validate_external_url_invalid
-    @file_link.target_type = 'DmsfUrl'
+    @file_link2.target_type = 'DmsfUrl'
-    @file_link.external_url = 'htt ps://abc.xyz'
+    @file_link2.external_url = 'htt ps://abc.xyz'
-    assert_not @file_link.save, "External URL link #{@file_link.name} should have not been saved"
+    assert_not @file_link2.save, "External URL link #{@file_link2.name} should have not been saved"
-    assert_equal 1, @file_link.errors.size
+    assert_equal 1, @file_link2.errors.size
   end
 
   def test_validate_external_url_valid
-    @file_link.target_type = 'DmsfUrl'
+    @file_link2.target_type = 'DmsfUrl'
-    @file_link.external_url = 'https://www.google.com/search?q=寿司'
+    @file_link2.external_url = 'https://www.google.com/search?q=寿司'
-    assert @file_link.save
+    assert @file_link2.save
   end
 
   def test_belongs_to_project
@@ -118,118 +112,119 @@ class DmsfLinksTest < RedmineDmsf::Test::UnitTest
   end
 
   def test_target_folder_id
-    assert_equal 2, @file_link.target_folder_id
+    assert_equal 2, @file_link2.target_folder_id
-    assert_equal 1, @folder_link.target_folder_id
+    assert_equal 1, @folder_link1.target_folder_id
   end
 
   def test_target_folder
-    assert_equal @folder2, @file_link.target_folder
+    assert_equal @folder2, @file_link2.target_folder
-    assert_equal @folder1, @folder_link.target_folder
+    assert_equal @folder1, @folder_link1.target_folder
   end
 
   def test_target_file_id
-    assert_equal 4, @file_link.target_file_id
+    assert_equal 4, @file_link2.target_file_id
-    assert_nil @folder_link.target_file
+    assert_nil @folder_link1.target_file
   end
 
   def test_target_file
-    assert_equal @file4, @file_link.target_file
+    assert_equal @file4, @file_link2.target_file
-    assert_nil @folder_link.target_file
+    assert_nil @folder_link1.target_file
   end
 
   def test_target_project
-    assert_equal @project1, @file_link.target_project
+    assert_equal @project1, @file_link2.target_project
-    assert_equal @project1, @folder_link.target_project
+    assert_equal @project1, @folder_link1.target_project
   end
 
   def test_folder
-    assert_equal @folder1, @file_link.dmsf_folder
+    assert_equal @folder1, @file_link2.dmsf_folder
-    assert_nil @folder_link.dmsf_folder
+    assert_nil @folder_link1.dmsf_folder
   end
 
   def test_title
-    assert_equal @file_link.name, @file_link.title
+    assert_equal @file_link2.name, @file_link2.title
-    assert_equal @folder_link.name, @folder_link.title
+    assert_equal @folder_link1.name, @folder_link1.title
   end
 
   def test_find_link_by_file_name
-    file_link = DmsfLink.find_link_by_file_name(@file_link.project, @file_link.dmsf_folder, @file_link.target_file.name)
+    file_link = DmsfLink.find_link_by_file_name(@file_link2.project, @file_link2.dmsf_folder,
+                                                @file_link2.target_file.name)
     assert file_link, 'File link not found by its name'
   end
 
   def test_path
-    assert_equal 'folder1/folder2/test4.txt', @file_link.path
+    assert_equal 'folder1/folder2/test4.txt', @file_link2.path
-    assert_equal 'folder1', @folder_link.path
+    assert_equal 'folder1', @folder_link1.path
   end
 
   def test_file_kink_copy_to
-    file_link_copy = @file_link.copy_to @folder2.project, @folder2
+    file_link_copy = @file_link2.copy_to @folder2.project, @folder2
     assert_not_nil file_link_copy, 'File link copying failed'
-    assert_equal file_link_copy.target_project_id, @file_link.target_project_id
+    assert_equal file_link_copy.target_project_id, @file_link2.target_project_id
-    assert_equal file_link_copy.target_id, @file_link.target_id
+    assert_equal file_link_copy.target_id, @file_link2.target_id
-    assert_equal file_link_copy.target_type, @file_link.target_type
+    assert_equal file_link_copy.target_type, @file_link2.target_type
-    assert_equal file_link_copy.name, @file_link.name
+    assert_equal file_link_copy.name, @file_link2.name
     assert_equal file_link_copy.project_id, @folder2.project.id
     assert_equal file_link_copy.dmsf_folder_id, @folder2.id
   end
 
   def test_folder_link_copy_to
-    folder_link_copy = @folder_link.copy_to @folder2.project, @folder2
+    folder_link_copy = @folder_link1.copy_to @folder2.project, @folder2
     assert_not_nil folder_link_copy, 'Folder link copying failed'
-    assert_equal folder_link_copy.target_project_id, @folder_link.target_project_id
+    assert_equal folder_link_copy.target_project_id, @folder_link1.target_project_id
-    assert_equal folder_link_copy.target_id, @folder_link.target_id
+    assert_equal folder_link_copy.target_id, @folder_link1.target_id
-    assert_equal folder_link_copy.target_type, @folder_link.target_type
+    assert_equal folder_link_copy.target_type, @folder_link1.target_type
-    assert_equal folder_link_copy.name, @folder_link.name
+    assert_equal folder_link_copy.name, @folder_link1.name
     assert_equal folder_link_copy.project_id, @folder2.project.id
     assert_equal folder_link_copy.dmsf_folder_id, @folder2.id
   end
 
   def test_move_to_author
-    assert_equal @admin.id, @file_link.user_id
+    assert_equal @admin.id, @file_link2.user_id
     User.current = @jsmith
-    assert @file_link.move_to(@project1, @folder1)
+    assert @file_link2.move_to(@project1, @folder1)
-    assert_equal @admin.id, @file_link.user_id, "Author mustn't be updated when moving"
+    assert_equal @admin.id, @file_link2.user_id, "Author mustn't be updated when moving"
   end
 
   def test_copy_to_author
-    assert_equal @admin.id, @file_link.user_id
+    assert_equal @admin.id, @file_link2.user_id
     User.current = @jsmith
-    l = @file_link.copy_to(@project1, @folder1)
+    l = @file_link2.copy_to(@project1, @folder1)
     assert l
     assert_equal @jsmith.id, l.user_id, 'Author must be updated when copying'
   end
 
   def test_delete_file_link
-    assert @file_link.delete(commit: false), @file_link.errors.full_messages.to_sentence
+    assert @file_link2.delete(commit: false), @file_link2.errors.full_messages.to_sentence
-    assert @file_link.deleted?, "File link hasn't been deleted"
+    assert @file_link2.deleted?, "File link hasn't been deleted"
   end
 
   def test_restore_file_link
-    assert @file_link.delete(commit: false), @file_link.errors.full_messages.to_sentence
+    assert @file_link2.delete(commit: false), @file_link2.errors.full_messages.to_sentence
-    assert @file_link.deleted?, "File link hasn't been deleted"
+    assert @file_link2.deleted?, "File link hasn't been deleted"
-    assert @file_link.restore, @file_link.errors.full_messages.to_sentence
+    assert @file_link2.restore, @file_link2.errors.full_messages.to_sentence
-    assert_not @file_link.deleted?, "File link hasn't been restored"
+    assert_not @file_link2.deleted?, "File link hasn't been restored"
   end
 
   def test_delete_folder_link
-    assert @folder_link.delete(commit: false), @folder_link.errors.full_messages.to_sentence
+    assert @folder_link1.delete(commit: false), @folder_link1.errors.full_messages.to_sentence
-    assert @folder_link.deleted?, "Folder link hasn't been deleted"
+    assert @folder_link1.deleted?, "Folder link hasn't been deleted"
   end
 
   def test_restore_folder_link
-    assert @folder_link.delete(commit: false), @folder_link.errors.full_messages.to_sentence
+    assert @folder_link1.delete(commit: false), @folder_link1.errors.full_messages.to_sentence
-    assert @folder_link.deleted?, "Folder link hasn't been deleted"
+    assert @folder_link1.deleted?, "Folder link hasn't been deleted"
-    assert @folder_link.restore, @folder_link.errors.full_messages.to_sentence
+    assert @folder_link1.restore, @folder_link1.errors.full_messages.to_sentence
-    assert_not @folder_link.deleted?, "Folder link hasn't been restored"
+    assert_not @folder_link1.deleted?, "Folder link hasn't been restored"
   end
 
   def test_destroy_file_link
-    assert @file_link.delete(commit: true), @file_link.errors.full_messages.to_sentence
+    assert @file_link2.delete(commit: true), @file_link2.errors.full_messages.to_sentence
-    assert_nil DmsfLink.find_by(id: @file_link.id)
+    assert_nil DmsfLink.find_by(id: @file_link2.id)
   end
 
   def test_destroy_folder_link
-    assert @folder_link.delete(commit: true), @folder_link.errors.full_messages.to_sentence
+    assert @folder_link1.delete(commit: true), @folder_link1.errors.full_messages.to_sentence
-    assert_nil DmsfLink.find_by(id: @folder_link.id)
+    assert_nil DmsfLink.find_by(id: @folder_link1.id)
   end
 end

test/unit_test.rb

@@ -52,7 +52,11 @@ module RedmineDmsf
         @file5 = DmsfFile.find 5
         @file7 = DmsfFile.find 7
         @file8 = DmsfFile.find 8
+        @file11 = DmsfFile.find 11
         @file13 = DmsfFile.find 13
+        @folder_link1 = DmsfLink.find 1
+        @file_link2 = DmsfLink.find 2
+        @file_link7 = DmsfLink.find 7
         @folder1 = DmsfFolder.find 1
         @folder2 = DmsfFolder.find 2
         @folder6 = DmsfFolder.find 6