diff options
Diffstat (limited to 'app/models')
| -rw-r--r-- | app/models/commit_status.rb | 2 | ||||
| -rw-r--r-- | app/models/concerns/issuable.rb | 2 | ||||
| -rw-r--r-- | app/models/concerns/storage/legacy_namespace.rb | 16 | ||||
| -rw-r--r-- | app/models/merge_request_diff.rb | 11 | ||||
| -rw-r--r-- | app/models/namespace.rb | 28 | ||||
| -rw-r--r-- | app/models/pages_domain.rb | 4 | ||||
| -rw-r--r-- | app/models/project.rb | 4 | ||||
| -rw-r--r-- | app/models/remote_mirror.rb | 8 | ||||
| -rw-r--r-- | app/models/storage/legacy_project.rb | 2 |
9 files changed, 46 insertions, 31 deletions
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index f97dc38dab7..be6f3e9c5b0 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -140,7 +140,7 @@ class CommitStatus < ApplicationRecord end def locking_enabled? - status_changed? + will_save_change_to_status? end def before_sha diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 775aaa44e5b..127430cc68f 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -117,7 +117,7 @@ module Issuable # We want to use optimistic lock for cases when only title or description are involved # http://api.rubyonrails.org/classes/ActiveRecord/Locking/Optimistic.html def locking_enabled? - title_changed? || description_changed? + will_save_change_to_title? || will_save_change_to_description? end def allows_multiple_assignees? diff --git a/app/models/concerns/storage/legacy_namespace.rb b/app/models/concerns/storage/legacy_namespace.rb index 1cbe27ad03a..a15dc19e07a 100644 --- a/app/models/concerns/storage/legacy_namespace.rb +++ b/app/models/concerns/storage/legacy_namespace.rb @@ -13,20 +13,20 @@ module Storage raise Gitlab::UpdatePathError.new("Namespace #{name} (#{id}) cannot be moved because at least one project (e.g. #{proj_with_tags.name} (#{proj_with_tags.id})) has tags in container registry") end - parent_was = if parent_changed? && parent_id_before_last_save.present? + parent_was = if saved_change_to_parent? && parent_id_before_last_save.present? Namespace.find(parent_id_before_last_save) # raise NotFound early if needed end move_repositories - if parent_changed? + if saved_change_to_parent? former_parent_full_path = parent_was&.full_path parent_full_path = parent&.full_path Gitlab::UploadsTransfer.new.move_namespace(path, former_parent_full_path, parent_full_path) Gitlab::PagesTransfer.new.move_namespace(path, former_parent_full_path, parent_full_path) else - Gitlab::UploadsTransfer.new.rename_namespace(full_path_was, full_path) - Gitlab::PagesTransfer.new.rename_namespace(full_path_was, full_path) + Gitlab::UploadsTransfer.new.rename_namespace(full_path_before_last_save, full_path) + Gitlab::PagesTransfer.new.rename_namespace(full_path_before_last_save, full_path) end # If repositories moved successfully we need to @@ -38,7 +38,7 @@ module Storage write_projects_repository_config rescue => e # Raise if development/test environment, else just notify Sentry - Gitlab::Sentry.track_exception(e, extra: { full_path_was: full_path_was, full_path: full_path, action: 'move_dir' }) + Gitlab::Sentry.track_exception(e, extra: { full_path_before_last_save: full_path_before_last_save, full_path: full_path, action: 'move_dir' }) end true # false would cancel later callbacks but not rollback @@ -57,14 +57,14 @@ module Storage # Move the namespace directory in all storages used by member projects repository_storages.each do |repository_storage| # Ensure old directory exists before moving it - gitlab_shell.add_namespace(repository_storage, full_path_was) + gitlab_shell.add_namespace(repository_storage, full_path_before_last_save) # Ensure new directory exists before moving it (if there's a parent) gitlab_shell.add_namespace(repository_storage, parent.full_path) if parent - unless gitlab_shell.mv_namespace(repository_storage, full_path_was, full_path) + unless gitlab_shell.mv_namespace(repository_storage, full_path_before_last_save, full_path) - Rails.logger.error "Exception moving path #{repository_storage} from #{full_path_was} to #{full_path}" + Rails.logger.error "Exception moving path #{repository_storage} from #{full_path_before_last_save} to #{full_path}" # if we cannot move namespace directory we should rollback # db changes in order to prevent out of sync between db and fs diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 0b787217410..5f5d92bc2f0 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -134,7 +134,7 @@ class MergeRequestDiff < ApplicationRecord # It allows you to override variables like head_commit_sha before getting diff. after_create :save_git_content, unless: :importing? - after_save :update_external_diff_store, if: -> { !importing? && external_diff_changed? } + after_save :update_external_diff_store, if: -> { !importing? && saved_change_to_external_diff? } def self.find_by_diff_refs(diff_refs) find_by(start_commit_sha: diff_refs.start_sha, head_commit_sha: diff_refs.head_sha, base_commit_sha: diff_refs.base_sha) @@ -154,7 +154,14 @@ class MergeRequestDiff < ApplicationRecord ensure_commit_shas save_commits save_diffs + + # Another set of `after_save` hooks will be called here when we update the record save + # We need to reset so that dirty tracking is reset when running the original set + # of `after_save` hooks that come after this `after_create` hook. Otherwise, the + # hooks that run when an attribute was changed are run twice. + reset + keep_around_commits end @@ -348,7 +355,7 @@ class MergeRequestDiff < ApplicationRecord has_attribute?(:external_diff_store) end - def external_diff_changed? + def saved_change_to_external_diff? super if has_attribute?(:external_diff) end diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 7228aab2c2e..168f6bedd63 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -63,7 +63,7 @@ class Namespace < ApplicationRecord # Legacy Storage specific hooks - after_update :move_dir, if: :path_or_parent_changed? + after_update :move_dir, if: :saved_change_to_path_or_parent? before_destroy(prepend: true) { prepare_for_destroy } after_destroy :rm_dir @@ -144,7 +144,7 @@ class Namespace < ApplicationRecord def send_update_instructions projects.each do |project| - project.send_move_instructions("#{full_path_was}/#{project.path}") + project.send_move_instructions("#{full_path_before_last_save}/#{project.path}") end end @@ -229,10 +229,6 @@ class Namespace < ApplicationRecord [owner_id] end - def parent_changed? - parent_id_changed? - end - # Includes projects from this namespace and projects from all subgroups # that belongs to this namespace def all_projects @@ -262,12 +258,12 @@ class Namespace < ApplicationRecord false end - def full_path_was - if parent_id_was.nil? - path_was + def full_path_before_last_save + if parent_id_before_last_save.nil? + path_before_last_save else - previous_parent = Group.find_by(id: parent_id_was) - previous_parent.full_path + '/' + path_was + previous_parent = Group.find_by(id: parent_id_before_last_save) + previous_parent.full_path + '/' + path_before_last_save end end @@ -293,7 +289,15 @@ class Namespace < ApplicationRecord private - def path_or_parent_changed? + def parent_changed? + parent_id_changed? + end + + def saved_change_to_parent? + saved_change_to_parent_id? + end + + def saved_change_to_path_or_parent? saved_change_to_path? || saved_change_to_parent_id? end diff --git a/app/models/pages_domain.rb b/app/models/pages_domain.rb index 9e806b2e232..407d85b1520 100644 --- a/app/models/pages_domain.rb +++ b/app/models/pages_domain.rb @@ -26,7 +26,7 @@ class PagesDomain < ApplicationRecord after_initialize :set_verification_code after_create :update_daemon - after_update :update_daemon, if: :pages_config_changed? + after_update :update_daemon, if: :saved_change_to_pages_config? after_destroy :update_daemon scope :enabled, -> { where('enabled_until >= ?', Time.now ) } @@ -148,7 +148,7 @@ class PagesDomain < ApplicationRecord end # rubocop: enable CodeReuse/ServiceClass - def pages_config_changed? + def saved_change_to_pages_config? saved_change_to_project_id? || saved_change_to_domain? || saved_change_to_certificate? || diff --git a/app/models/project.rb b/app/models/project.rb index a29100405f9..b0dbf09ecbf 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1912,8 +1912,8 @@ class Project < ApplicationRecord false end - def full_path_was - File.join(namespace.full_path, previous_changes['path'].first) + def full_path_before_last_save + File.join(namespace.full_path, path_before_last_save) end alias_method :name_with_namespace, :full_name diff --git a/app/models/remote_mirror.rb b/app/models/remote_mirror.rb index 535f772b6a1..cbfc1a7c1b2 100644 --- a/app/models/remote_mirror.rb +++ b/app/models/remote_mirror.rb @@ -22,8 +22,8 @@ class RemoteMirror < ApplicationRecord before_save :set_new_remote_name, if: :mirror_url_changed? after_save :set_override_remote_mirror_available, unless: -> { Gitlab::CurrentSettings.current_application_settings.mirror_available } - after_save :refresh_remote, if: :mirror_url_changed? - after_update :reset_fields, if: :mirror_url_changed? + after_save :refresh_remote, if: :saved_change_to_mirror_url? + after_update :reset_fields, if: :saved_change_to_mirror_url? after_commit :remove_remote, on: :destroy @@ -265,4 +265,8 @@ class RemoteMirror < ApplicationRecord def mirror_url_changed? url_changed? || credentials_changed? end + + def saved_change_to_mirror_url? + saved_change_to_url? || saved_change_to_credentials? + end end diff --git a/app/models/storage/legacy_project.rb b/app/models/storage/legacy_project.rb index 76ac5c13c18..b483c677be9 100644 --- a/app/models/storage/legacy_project.rb +++ b/app/models/storage/legacy_project.rb @@ -30,7 +30,7 @@ module Storage end def rename_repo(old_full_path: nil, new_full_path: nil) - old_full_path ||= project.full_path_was + old_full_path ||= project.full_path_before_last_save new_full_path ||= project.build_full_path if gitlab_shell.mv_repository(repository_storage, old_full_path, new_full_path) |