diff options
author | Yorick Peterse <yorickpeterse@gmail.com> | 2018-10-17 13:33:54 +0200 |
---|---|---|
committer | Yorick Peterse <yorickpeterse@gmail.com> | 2018-10-22 15:12:46 +0200 |
commit | 4b9c17f196bab6075563f62d01f9db65c1a0515c (patch) | |
tree | 03eb38b1e3264568fe049da3790c4d3cef1a3ddc /db | |
parent | da1a2bd6bbabac055c7f4773813d9cddf2c5f708 (diff) | |
download | gitlab-ce-4b9c17f196bab6075563f62d01f9db65c1a0515c.tar.gz |
Move Project#rename_repo to a service classrefactor-project-rename-repo
This moves the logic of Project#rename_repo and all methods _only_ used
by this method into a new service class: Projects::AfterRenameService.
By moving this code into a separate service class we can more easily
refactor it, and we also get rid of some RuboCop "disable" statements
automatically.
During the refactoring of this code, I removed most of the explicit
logging using Gitlab::AppLogger. The data that was logged would not be
useful when debugging renaming issues, as it does not add any value on
top of data provided by users.
I also removed a variety of comments that either mentioned something the
code does in literal form, or contained various grammatical errors.
Instead we now resort to more clearly named methods, removing the need
for code comments.
This method was chosen based on analysis in
https://gitlab.com/gitlab-org/release/framework/issues/28. In this issue
we determined this method has seen a total of 293 lines being changed in
it. We also noticed that RuboCop determined the ABC size
(https://www.softwarerenovation.com/ABCMetric.pdf) was too great.
Diffstat (limited to 'db')
-rw-r--r-- | db/post_migrate/20161221153951_rename_reserved_project_names.rb | 6 | ||||
-rw-r--r-- | db/post_migrate/20170313133418_rename_more_reserved_project_names.rb | 6 |
2 files changed, 8 insertions, 4 deletions
diff --git a/db/post_migrate/20161221153951_rename_reserved_project_names.rb b/db/post_migrate/20161221153951_rename_reserved_project_names.rb index 08d7f499eec..678876e886c 100644 --- a/db/post_migrate/20161221153951_rename_reserved_project_names.rb +++ b/db/post_migrate/20161221153951_rename_reserved_project_names.rb @@ -113,7 +113,9 @@ class RenameReservedProjectNames < ActiveRecord::Migration begin # Because project path update is quite complex operation we can't safely # copy-paste all code from GitLab. As exception we use Rails code here - project.rename_repo if rename_project_row(project, path) + if rename_project_row(project, path) + Projects::AfterRenameService.new(project).execute + end rescue Exception => e # rubocop: disable Lint/RescueException Rails.logger.error "Exception when renaming project #{id}: #{e.message}" end @@ -123,6 +125,6 @@ class RenameReservedProjectNames < ActiveRecord::Migration def rename_project_row(project, path) project.respond_to?(:update_attributes) && project.update(path: path) && - project.respond_to?(:rename_repo) + defined?(Projects::AfterRenameService) end end diff --git a/db/post_migrate/20170313133418_rename_more_reserved_project_names.rb b/db/post_migrate/20170313133418_rename_more_reserved_project_names.rb index 43a37667250..26a67b0f814 100644 --- a/db/post_migrate/20170313133418_rename_more_reserved_project_names.rb +++ b/db/post_migrate/20170313133418_rename_more_reserved_project_names.rb @@ -55,7 +55,9 @@ class RenameMoreReservedProjectNames < ActiveRecord::Migration begin # Because project path update is quite complex operation we can't safely # copy-paste all code from GitLab. As exception we use Rails code here - project.rename_repo if rename_project_row(project, path) + if rename_project_row(project, path) + Projects::AfterRenameService.new(project).execute + end rescue Exception => e # rubocop: disable Lint/RescueException Rails.logger.error "Exception when renaming project #{id}: #{e.message}" end @@ -65,6 +67,6 @@ class RenameMoreReservedProjectNames < ActiveRecord::Migration def rename_project_row(project, path) project.respond_to?(:update_attributes) && project.update(path: path) && - project.respond_to?(:rename_repo) + defined?(Projects::AfterRenameService) end end |