From 4877dc005a10c7cd1e5b869b9aac0f2ddc63153e Mon Sep 17 00:00:00 2001
From: Arnaud Levy <contact@arnaudlevy.com>
Date: Tue, 4 Jan 2022 12:14:15 +0100
Subject: [PATCH] tweaks

---
 app/models/communication/website/git_file.rb |  7 +++----
 app/models/concerns/with_git.rb              |  3 ++-
 app/services/git/providers/github.rb         |  4 ++--
 app/services/git/repository.rb               |  1 +
 docs/websites/export.md                      | 18 +++++++++---------
 5 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/app/models/communication/website/git_file.rb b/app/models/communication/website/git_file.rb
index 5b420edc3..7a85e5532 100644
--- a/app/models/communication/website/git_file.rb
+++ b/app/models/communication/website/git_file.rb
@@ -39,7 +39,7 @@ class Communication::Website::GitFile < ApplicationRecord
   end
 
   def should_update?
-    !synchronized_with_git? || previous_path != path || previous_sha != sha
+    previous_path != path || previous_sha != sha
   end
 
   def should_destroy?
@@ -54,12 +54,11 @@ class Communication::Website::GitFile < ApplicationRecord
   def sha
     # Git SHA-1 is calculated from the String "blob <length>\x00<contents>"
     # Source: https://alblue.bandlem.com/2011/08/git-tip-of-week-objects.html
-    data = to_s
-    OpenSSL::Digest::SHA1.hexdigest "blob #{data.bytesize}\x00#{data}"
+    OpenSSL::Digest::SHA1.hexdigest "blob #{to_s.bytesize}\x00#{to_s}"
   end
 
   def to_s
-    ApplicationController.render(
+    @to_s ||= ApplicationController.render(
       template: "admin/#{about.class.name.underscore.pluralize}/#{identifier}",
       layout: false,
       assigns: { about.class.name.demodulize.downcase => about }
diff --git a/app/models/concerns/with_git.rb b/app/models/concerns/with_git.rb
index 8fba97885..dcd0af344 100644
--- a/app/models/concerns/with_git.rb
+++ b/app/models/concerns/with_git.rb
@@ -8,8 +8,9 @@ module WithGit
               dependent: :destroy
   end
 
+  # Needs override
   def git_path_static
-    ''
+    raise NotImplementedError
   end
 
   # Overridden if websites relation exists
diff --git a/app/services/git/providers/github.rb b/app/services/git/providers/github.rb
index b6daf07d2..b0fdaf150 100644
--- a/app/services/git/providers/github.rb
+++ b/app/services/git/providers/github.rb
@@ -17,6 +17,7 @@ class Git::Providers::Github
 
   def update_file(path, previous_path, content)
     file = find_in_tree previous_path
+    return if file.nil?
     batch << {
       path: previous_path,
       mode: file[:mode],
@@ -43,8 +44,7 @@ class Git::Providers::Github
   end
 
   def push(commit_message)
-    return unless valid?
-    return if batch.empty?
+    return if !valid? || batch.empty?
     new_tree = client.create_tree repository, batch, base_tree: tree[:sha]
     commit = client.create_commit repository, commit_message, new_tree[:sha], branch_sha
     client.update_branch repository, default_branch, commit[:sha]
diff --git a/app/services/git/repository.rb b/app/services/git/repository.rb
index 573c34c51..ae190369f 100644
--- a/app/services/git/repository.rb
+++ b/app/services/git/repository.rb
@@ -22,6 +22,7 @@ class Git::Repository
 
   protected
 
+  # TODO add gitlab
   def provider
     @provider ||= Git::Providers::Github.new(website&.access_token, website&.repository)
   end
diff --git a/docs/websites/export.md b/docs/websites/export.md
index a084cba5a..be773adaf 100644
--- a/docs/websites/export.md
+++ b/docs/websites/export.md
@@ -8,12 +8,12 @@ Les publications doivent se font en asynchrone parce qu'elles peuvent être long
 
 
 Certains objets peuvent appartenir à plusieurs websites, donc plusieurs repositories, comme par exemple les programs.
-Certains objets ont des dépendances, par exemple les pages enfants, les auteurs ou les catégories.
+Certains objets ont des dépendances, par exemple les pages enfants, les auteurs, les catégories ou les médias.
 
 
 Les fichiers renommés doivent être déplacés sur git.
 Les fichiers supprimés ou dépubliés doivent être supprimés sur git.
-Il faut veiller à limiter le nombre de commits, et éviter les commits vides.
+Il faut veiller à limiter le nombre de commits et à éviter les commits vides.
 
 ## Architecture
 
@@ -48,7 +48,7 @@ Ce flux cause un problème majeur : tout ce qui est analysé disparaît en async
 
 ### Version 2
 
-Après l'enregistrement d'un objet, il faut, pour chaque website, lancer une tâche asynchrone de synchronisation.
+Après l'enregistrement d'un objet, il faut lancer une tâche asynchrone de synchronisation.
 Cette tâche est lancée par les controllers, et intégrée dans le partial `WithGit`.
 ```
 def create
@@ -64,7 +64,7 @@ def update
 end
 
 def destroy
-  @page.save_and_sync
+  @page.destroy_and_sync
 end
 ```
 
@@ -76,8 +76,8 @@ def reorder
   pages.first.sync_with_git
 end
 ```
-
-TODO gérer la suppression correctement
+TODO vérifier que tous les cas de déplacement sont correctement gérés.
+TODO gérer la suppression correctement.
 
 ## Code
 
@@ -88,7 +88,7 @@ Le website a un trait WithRepository qui gère son rapport avec le repository Gi
 ### Objets exportables vers Git
 
 Tous les objets qui doivent être exportés vers Git :
-- doivent utiliser le partial `WithGit`, qui gère l'export vers les repositories des objets et de leurs dépendances
+- doivent utiliser le concern `WithGit`, qui gère l'export vers les repositories des objets et de leurs dépendances
 - doivent présenter une méthode `websites`, éventuellement avec un seul website dans un tableau
 - peuvent intégrer le concern `WithMedia` s'il utilise des médias (`featured_image` et/ou images dans des rich texts)
 - peuvent présenter une méthode `static_files` qui liste les identifiants des git_files à générer, pour les objets qui créent plusieurs fichiers
@@ -110,8 +110,8 @@ Pour cela, le git_file dispose des propriétés suivantes :
 
 Pour informer sur les actions à mener, il dispose des méthodes interrogatives suivantes :
 - synchronized_with_git? (pour évaluer l'intégrité vs le repository)
-- should_create? (pour savoir s'il faut regénérer ou pas)
-- should_update? (pour savoir s'il faut regénérer ou pas)
+- should_create? (pour savoir s'il faut créer ou pas)
+- should_update? (pour savoir s'il faut régénérer ou pas)
 - should_destroy? (pour savoir s'il faut supprimer)
 
 
-- 
GitLab