From 951b647ef9370a748739c5ea7475e91f23fdd665 Mon Sep 17 00:00:00 2001
From: Arnaud Levy <contact@arnaudlevy.com>
Date: Fri, 29 Jul 2022 09:41:56 +0200
Subject: [PATCH] Optimize git sync (remove cascading jobs)
---
.../admin/communication/blocks_controller.rb | 8 ++++++--
.../communication/websites/categories_controller.rb | 3 ++-
.../admin/education/programs_controller.rb | 11 +++++------
app/controllers/concerns/admin/reorderable.rb | 1 +
app/models/communication/block.rb | 5 -----
app/models/communication/website/category.rb | 2 +-
.../communication/website/with_special_pages.rb | 2 +-
app/models/education/program.rb | 2 ++
8 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/app/controllers/admin/communication/blocks_controller.rb b/app/controllers/admin/communication/blocks_controller.rb
index fa5431584..cb392a8f9 100644
--- a/app/controllers/admin/communication/blocks_controller.rb
+++ b/app/controllers/admin/communication/blocks_controller.rb
@@ -6,9 +6,10 @@ class Admin::Communication::BlocksController < Admin::Communication::Application
def reorder
ids = params[:ids] || []
ids.each.with_index do |id, index|
- block = current_university.communication_blocks.find(id)
- block.update position: index + 1
+ @block = current_university.communication_blocks.find(id)
+ @block.update position: index + 1
end
+ @block.about.sync_with_git
end
def new
@@ -29,6 +30,7 @@ class Admin::Communication::BlocksController < Admin::Communication::Application
def create
@block.university = current_university
if @block.save
+ # No need to sync as content is empty
redirect_to [:edit, :admin, @block],
notice: t('admin.successfully_created_html', model: @block.to_s)
else
@@ -39,6 +41,7 @@ class Admin::Communication::BlocksController < Admin::Communication::Application
def update
if @block.update(block_params)
+ @block.about.sync_with_git
redirect_to about_path,
notice: t('admin.successfully_updated_html', model: @block.to_s)
else
@@ -56,6 +59,7 @@ class Admin::Communication::BlocksController < Admin::Communication::Application
def destroy
path = about_path
@block.destroy
+ @block.about.sync_with_git
redirect_to path,
notice: t('admin.successfully_destroyed_html', model: @block.to_s)
end
diff --git a/app/controllers/admin/communication/websites/categories_controller.rb b/app/controllers/admin/communication/websites/categories_controller.rb
index 5be1a6b18..580c9497c 100644
--- a/app/controllers/admin/communication/websites/categories_controller.rb
+++ b/app/controllers/admin/communication/websites/categories_controller.rb
@@ -19,11 +19,12 @@ class Admin::Communication::Websites::CategoriesController < Admin::Communicatio
parent_id: parent_id,
position: index + 1
)
- category.sync_with_git unless parent_id
end
if parent_id
parent = @website.categories.find(parent_id)
parent.sync_with_git
+ else
+ first_category&.sync_with_git # Will sync siblings
end
end
diff --git a/app/controllers/admin/education/programs_controller.rb b/app/controllers/admin/education/programs_controller.rb
index 0686e43eb..c537d4942 100644
--- a/app/controllers/admin/education/programs_controller.rb
+++ b/app/controllers/admin/education/programs_controller.rb
@@ -25,21 +25,20 @@ class Admin::Education::ProgramsController < Admin::Education::ApplicationContro
parent_id = params[:parentId].blank? ? nil : params[:parentId]
ids = params[:ids] || []
ids.each.with_index do |id, index|
- program = current_university.education_programs.find(id)
- program.update(
+ @program = current_university.education_programs.find(id)
+ @program.update(
parent_id: parent_id,
position: index + 1,
skip_websites_categories_callback: true
)
- unless parent_id
- program.set_websites_categories
- program.sync_with_git
- end
end
if parent_id
parent = current_university.education_programs.find(parent_id)
parent.set_websites_categories
parent.sync_with_git
+ else
+ @program&.set_websites_categories
+ @program&.sync_with_git
end
end
diff --git a/app/controllers/concerns/admin/reorderable.rb b/app/controllers/concerns/admin/reorderable.rb
index 3c362c1a6..783c36c0a 100644
--- a/app/controllers/concerns/admin/reorderable.rb
+++ b/app/controllers/concerns/admin/reorderable.rb
@@ -9,6 +9,7 @@ module Admin::Reorderable
object.update_column(:position, index + 1) unless object.nil?
end
first_object.sync_with_git if first_object&.respond_to?(:sync_with_git)
+ # Used to add extra code
yield first_object if block_given?
end
diff --git a/app/models/communication/block.rb b/app/models/communication/block.rb
index 2107c8362..bbd8f3b8d 100644
--- a/app/models/communication/block.rb
+++ b/app/models/communication/block.rb
@@ -65,7 +65,6 @@ class Communication::Block < ApplicationRecord
scope :published, -> { where(published: true) }
before_save :attach_template_blobs
- after_commit :save_and_sync_about, on: [:update, :destroy]
# When we set data from json, we pass it to the template.
# The json we save is first sanitized and prepared by the template.
@@ -121,8 +120,4 @@ class Communication::Block < ApplicationRecord
def attach_template_blobs
# self.template_images = template.active_storage_blobs
end
-
- def save_and_sync_about
- about&.save_and_sync unless about&.destroyed?
- end
end
diff --git a/app/models/communication/website/category.rb b/app/models/communication/website/category.rb
index 9a0a29279..1f238b0e0 100644
--- a/app/models/communication/website/category.rb
+++ b/app/models/communication/website/category.rb
@@ -87,7 +87,7 @@ class Communication::Website::Category < ApplicationRecord
end
def git_dependencies(website)
- [self] + descendants + active_storage_blobs + posts + website.menus
+ [self] + siblings + descendants + active_storage_blobs + posts + website.menus
end
def git_destroy_dependencies(website)
diff --git a/app/models/communication/website/with_special_pages.rb b/app/models/communication/website/with_special_pages.rb
index 4e5d446ca..54fdc06bf 100644
--- a/app/models/communication/website/with_special_pages.rb
+++ b/app/models/communication/website/with_special_pages.rb
@@ -47,6 +47,7 @@ module Communication::Website::WithSpecialPages
university_id: university_id
)
if page.new_record?
+ # This is a bit brutal, as it might generate several syncs at the same time
page.save_and_sync
end
page
@@ -56,5 +57,4 @@ module Communication::Website::WithSpecialPages
@special_pages_keys ||= pages.where.not(kind: nil).pluck(:kind).uniq
end
-
end
diff --git a/app/models/education/program.rb b/app/models/education/program.rb
index 2a8a26dc8..9982afbd1 100644
--- a/app/models/education/program.rb
+++ b/app/models/education/program.rb
@@ -151,6 +151,8 @@ class Education::Program < ApplicationRecord
def git_dependencies(website)
[self] +
+ siblings +
+ descendants +
active_storage_blobs +
git_block_dependencies +
university_people_through_involvements +
--
GitLab