Cleanup!

Co-authored-by: Sébastien Gaya <sebastien.gaya@gmail.com>
Co-authored-by: Pierre-André Boissinot <pierreandre.boissinot@noesya.coop>

app/models/communication/block.rb

@@ -72,7 +72,6 @@ class Communication::Block < ApplicationRecord
 
   scope :published, -> { where(published: true) }
 
-  after_save :sync_if_about_is_direct
   before_save :attach_template_blobs
   before_validation :set_university_from_about, on: :create
 
@@ -146,10 +145,6 @@ class Communication::Block < ApplicationRecord
     "Communication::Block::Template::#{template_kind.classify}".constantize
   end
 
-  def sync_if_about_is_direct
-    about.save_and_sync if about.respond_to? :save_and_sync
-  end
-
   # FIXME @sebou
   # Could not find or build blob: expected attachable, got #<ActiveStorage::Blob id: "f4c78657-5062-416b-806f-0b80fb66f9cd", key: "gri33wtop0igur8w3a646llel3sd", filename: "logo.svg", content_type: "image/svg+xml", metadata: {"identified"=>true, "width"=>709, "height"=>137, "analyzed"=>true}, service_name: "scaleway", byte_size: 4137, checksum: "aZqqTYabP5+72ZeddcZ/2Q==", created_at: "2022-05-05 12:17:33.941505000 +0200", university_id: "ebf2d273-ffc9-4d9f-a4ee-a2146913d617">
   def attach_template_blobs

app/models/communication/website/with_connected_objects.rb

@@ -68,6 +68,14 @@ module Communication::Website::WithConnectedObjects
     University::Organization.where(id: ids)
   end
 
+  def is_direct_object?
+    true
+  end
+
+  def is_indirect_object?
+    false
+  end
+
   protected
 
   def connect_about

app/models/concerns/as_direct_object.rb

@@ -8,7 +8,7 @@ module AsDirectObject
   extend ActiveSupport::Concern
 
   included do
-    include WithDependenciesSynchronization
+    include WithDependencies
     include WithGit
     include WithGitFiles
     include WithReferences

app/models/concerns/as_indirect_object.rb

@@ -7,7 +7,7 @@ module AsIndirectObject
 
   included do
     # Les blocs sont des objets indirects, mais n'ont pas de GitFiles, on n'inclut donc pas WithGitFiles ici
-    include WithDependenciesSynchronization
+    include WithDependencies
     include WithReferences
 
     has_many  :connections,
@@ -83,7 +83,7 @@ module AsIndirectObject
   def sync_connections
     direct_sources.each do |direct_source|
       direct_source.website.connect self, direct_source
-      direct_source.save_and_sync
+      direct_source.sync_with_git
     end
   end
 end
\ No newline at end of file

app/models/concerns/with_dependencies.rb

@@ -5,6 +5,17 @@
 module WithDependencies
   extend ActiveSupport::Concern
 
+  included do
+    attr_accessor :previous_dependencies
+
+    if self < ActiveRecord::Base
+      before_save :snapshot_dependencies
+      after_save :clean_websites_if_necessary
+      after_destroy :clean_websites
+    end
+  end
+
+
   # Cette méthode doit être définie dans chaque objet,
   # et renvoyer un tableau de ses références directes.
   # Jamais de référence indirecte !
@@ -24,10 +35,10 @@ module WithDependencies
     end
   end
 
+  # On ne liste pas les objets en cours de suppression
+  # return array if respond_to?(:mark_for_destruction?) && mark_for_destruction
+  # On renvoie l'array tel quel, non modifié, si on demande les contenus syncable_only et que le contenu ne l'est pas
   def recursive_dependencies(array: [], syncable_only: false)
-    # On ne liste pas les objets en cours de suppression
-    # return array if respond_to?(:mark_for_destruction?) && mark_for_destruction
-    # On renvoie l'array tel quel, non modifié, si on demande les contenus syncable_only et que le contenu ne l'est pas
     return array unless dependency_should_be_synced?(self, syncable_only)
     dependencies.each do |dependency|
       # Si l'objet ne doit pas être ajouté on n'ajoute pas non plus ses dépendances récursives
@@ -46,15 +57,49 @@ module WithDependencies
 
   protected
   
+  # Si l'objet est déjà là, on ne doit pas l'ajouter
+  # Si l'objet n'est pas syncable, on ne doit pas l'ajouter non plus
   def dependency_should_be_added?(array, dependency, syncable_only)
-    # Si l'objet est déjà là, on ne doit pas l'ajouter
-    # Si l'objet n'est pas syncable, on ne doit pas l'ajouter non plus
     !dependency.in?(array) && dependency_should_be_synced?(dependency, syncable_only)
   end
   
+  # Si on n'est pas en syncable only on liste tout, sinon, il faut analyser
   def dependency_should_be_synced?(dependency, syncable_only)
-    # Si on n'est pas en syncable only on liste tout, sinon, il faut analyser
     !syncable_only || (dependency.respond_to?(:syncable?) && dependency.syncable?)
   end
 
+  # Stockage en RAM des dépendances avant enregistrement
+  def snapshot_dependencies
+    @previous_dependencies = persisted? ? reloaded_recursive_dependencies_syncable_filtered : []
+  end
+
+  def clean_websites_if_necessary
+    # Debug :)
+    # puts self
+    # puts "  previous_dependencies           #{ @previous_dependencies }"
+    # puts "  recursive_dependencies_syncable #{ reloaded_recursive_dependencies_syncable_filtered }"
+    # puts "  missing_dependencies_after_save #{ missing_dependencies_after_save }"
+    # puts
+    clean_websites if missing_dependencies_after_save.any?
+  end
+
+  def clean_websites
+    return unless respond_to?(:is_direct_object?)
+    
+    if is_direct_object?
+      website.destroy_obsolete_git_files
+    elsif is_indirect_object?
+      websites.each(&:destroy_obsolete_git_files)
+    end
+  end
+
+  def missing_dependencies_after_save
+    @previous_dependencies - reloaded_recursive_dependencies_syncable_filtered
+  end
+
+  def reloaded_recursive_dependencies_syncable_filtered
+    reloaded_object = self.class.unscoped.find(id)
+    reloaded_dependencies = reloaded_object.recursive_dependencies_syncable
+    DependenciesFilter.filtered(reloaded_dependencies)
+  end
 end
\ No newline at end of file

app/models/concerns/with_dependencies_synchronization.rb

-module WithDependenciesSynchronization
-  extend ActiveSupport::Concern
-
-  included do
-    include WithDependencies
-
-    attr_accessor :dependencies_before_save
-
-    # TODO
-    # before_save :compute_dependencies_before_save
-    # after_save :cleanup_websites, if: :lost_dependencies_after_save?
-    after_destroy :cleanup_websites
-  end
-
-  protected
-
-  def compute_dependencies_before_save
-    @dependencies_before_save = begin
-      array = []
-      array = self.class.find(id).recursive_dependencies_syncable if persisted?
-      array.select { |dependency| dependency.respond_to?(:git_files) }
-    end
-  end
-
-  def lost_dependencies_after_save?
-    lost_dependencies_after_save = @dependencies_before_save - recursive_dependencies_syncable
-    lost_dependencies_after_save.any?
-  end
-
-  def cleanup_websites
-    # byebug unless is_a?(Communication::Block)
-    if is_direct_object?
-      website.destroy_obsolete_git_files
-    else
-      websites.each(&:destroy_obsolete_git_files)
-    end
-  end
-
-end

app/services/dependencies_filter.rb

+class DependenciesFilter
+
+  # [
+  #   "gid://osuny/Communication::Block/4ac9f6fe-80cd-46f6-becc-fc14b3fecea0",
+  #   "gid://osuny/University::Person/36501bf0-99b9-58f9-b269-bf161b451c43"
+  # ]
+  def self.filtered(dependencies)
+    dependencies.select { |dependency| dependency.is_a?(ActiveRecord::Base) }
+                .map    { |dependency| dependency.to_global_id.to_s }
+                .uniq
+  end
+end
\ No newline at end of file

test/models/communication/website/dependency_test.rb

@@ -2,6 +2,8 @@ require "test_helper"
 
 # rails test test/models/communication/website/dependency_test.rb
 class Communication::Website::DependencyTest < ActiveSupport::TestCase
+  include ActiveJob::TestHelper
+
   def test_page_dependencies
     # Rien : 0 dépendances
     page = communication_website_pages(:page_with_no_dependency)
@@ -19,17 +21,68 @@ class Communication::Website::DependencyTest < ActiveSupport::TestCase
     # - le block Personnes (1)
     # - 4 composants du template du block + 1 élément (5)
     # - 2 composants de l'élément du template (2)
-    # - La personne en dépendance du composant Person (1)
+    # - la personne en dépendance du composant Person (1)
     block = page.blocks.create(position: 1, published: true, template_kind: :organization_chart)
     block.data = "{ \"elements\": [ { \"id\": \"#{arnaud.id}\" } ] }"
     block.save
-
+    
     page = page.reload
-
+    
     assert_equal 9, page.recursive_dependencies.count
-
+    
     # On modifie le target du block
+    Delayed::Job.destroy_all
     block.data = "{ \"elements\": [ { \"id\": \"#{olivia.id}\" } ] }"
     block.save
+    # On vérifie qu'on appelle bien la méthode destroy_obsolete_git_files sur le site de la page
+    job = find_performable_method_job(:destroy_obsolete_git_files_without_delay, 
+                                      page.communication_website_id)
+    assert(job)
+
+    # Arnaud est remplacé par Olivia, le nombre de dépendances reste le même
+    assert_equal 9, page.recursive_dependencies.count
+  end
+  
+  def test_change_website_dependencies
+    dependencies_before_count = website_with_github.recursive_dependencies.count
+    
+    # On modifie l'about du website en ajoutant une école
+    website_with_github.update(about: default_school)
+    job = find_performable_method_job(:destroy_obsolete_git_files_without_delay, website_with_github.id)
+    refute(job)
+    delta = website_with_github.recursive_dependencies.count - dependencies_before_count
+    assert_equal 12, delta
+    
+    Delayed::Job.destroy_all
+
+    # On enlève l'about du website
+    website_with_github.update(about: nil)
+
+    # On vérifie qu'on appelle bien la méthode destroy_obsolete_git_files sur le site
+    job = find_performable_method_job(:destroy_obsolete_git_files_without_delay, website_with_github.id)
+    assert(job)
+  end
+
+  def test_change_menu_item_dependencies
+    website_with_github.save
+    menu = website_with_github.menus.first
+
+    item = menu.items.create(university: default_university, website: website_with_github, kind: :blank, title: 'Test')
+    assert_equal 2, item.recursive_dependencies.count
+    
+    item.kind = :page
+    item.about = communication_website_pages(:page_with_no_dependency)
+    item.save
+    assert_equal 2, item.recursive_dependencies.count
+  end
+
+  protected
+
+  # On ne peut pas utiliser assert_enqueued_jobs sur les méthodes asynchrones gérées avec handle_asynchronously
+  def find_performable_method_job(method, id)
+    Delayed::Job.all.detect { |job|
+      job.payload_object.method_name == method &&
+        job.payload_object.object.id == id
+    }
   end
 end