From b5147c8f76d0244859b2b304c0573e5935b3358c Mon Sep 17 00:00:00 2001 From: Vasilii Iakliushin <viakliushin@gitlab.com> Date: Tue, 24 Sep 2024 11:19:29 +0000 Subject: [PATCH] Add emails notification for RewriteHistory worker Contributes to https://gitlab.com/gitlab-org/gitlab/-/issues/483085 **Problem** Synchrounous rewriting of repository history is too slow. We are going to make it asynchronous instead. But it's currently missing a notification channel for users to receive updates about the processing status. **Solution** Define email templates that is going to be called from RewriteHistory Sidekiq job. Changelog: other --- app/mailers/emails/projects.rb | 19 +++++++++ app/mailers/previews/notify_preview.rb | 8 ++++ app/services/notification_service.rb | 12 ++++++ ...ry_rewrite_history_failure_email.html.haml | 4 ++ ...ory_rewrite_history_failure_email.text.erb | 3 ++ ...ry_rewrite_history_success_email.html.haml | 4 ++ ...ory_rewrite_history_success_email.text.erb | 3 ++ locale/gitlab.pot | 9 +++++ spec/mailers/emails/projects_spec.rb | 40 +++++++++++++++++++ spec/services/notification_service_spec.rb | 30 ++++++++++++++ 10 files changed, 132 insertions(+) create mode 100644 app/views/notify/repository_rewrite_history_failure_email.html.haml create mode 100644 app/views/notify/repository_rewrite_history_failure_email.text.erb create mode 100644 app/views/notify/repository_rewrite_history_success_email.html.haml create mode 100644 app/views/notify/repository_rewrite_history_success_email.text.erb diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb index 63eb4eb8fd83f..a8c14c8c91edd 100644 --- a/app/mailers/emails/projects.rb +++ b/app/mailers/emails/projects.rb @@ -48,6 +48,25 @@ def repository_cleanup_failure_email(project, user, error) mail_with_locale(to: user.notification_email_for(project.group), subject: subject("Project cleanup failure")) end + def repository_rewrite_history_success_email(project, user) + @project = project + + email_with_layout( + to: user.notification_email_for(project.group), + subject: subject("Project history rewrite has completed") + ) + end + + def repository_rewrite_history_failure_email(project, user, error) + @project = project + @error = error + + email_with_layout( + to: user.notification_email_for(project.group), + subject: subject("Project history rewrite failure") + ) + end + def repository_push_email(project_id, opts = {}) @message = Gitlab::Email::Message::RepositoryPush.new(self, project_id, opts) diff --git a/app/mailers/previews/notify_preview.rb b/app/mailers/previews/notify_preview.rb index 73ed87a5c255a..091daaff1a73e 100644 --- a/app/mailers/previews/notify_preview.rb +++ b/app/mailers/previews/notify_preview.rb @@ -379,6 +379,14 @@ def import_source_user_rejected Notify.import_source_user_rejected(source_user.id) end + def repository_rewrite_history_success_email + Notify.repository_rewrite_history_success_email(project, user) + end + + def repository_rewrite_history_failure_email + Notify.repository_rewrite_history_failure_email(project, user, 'Error message') + end + private def project diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 4d3bf1efedca6..0155d009722e9 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -700,6 +700,18 @@ def repository_cleanup_failure(project, user, error) mailer.send(:repository_cleanup_failure_email, project, user, error).deliver_later end + def repository_rewrite_history_success(project, user) + return if project.emails_disabled? + + mailer.repository_rewrite_history_success_email(project, user).deliver_later + end + + def repository_rewrite_history_failure(project, user, error) + return if project.emails_disabled? + + mailer.repository_rewrite_history_failure_email(project, user, error).deliver_later + end + def remote_mirror_update_failed(remote_mirror) recipients = project_maintainers_recipients(remote_mirror, action: 'update_failed') diff --git a/app/views/notify/repository_rewrite_history_failure_email.html.haml b/app/views/notify/repository_rewrite_history_failure_email.html.haml new file mode 100644 index 0000000000000..323c0038e39bc --- /dev/null +++ b/app/views/notify/repository_rewrite_history_failure_email.html.haml @@ -0,0 +1,4 @@ +%p + = s_('Notify|Repository history rewrite failed on %{project_url}.') % { project_url: @project.web_url } +%p + = @error diff --git a/app/views/notify/repository_rewrite_history_failure_email.text.erb b/app/views/notify/repository_rewrite_history_failure_email.text.erb new file mode 100644 index 0000000000000..7a35e29a3a9af --- /dev/null +++ b/app/views/notify/repository_rewrite_history_failure_email.text.erb @@ -0,0 +1,3 @@ +Repository history rewrite failed on <%= @project.web_url %> + +<%= @error %> diff --git a/app/views/notify/repository_rewrite_history_success_email.html.haml b/app/views/notify/repository_rewrite_history_success_email.html.haml new file mode 100644 index 0000000000000..c982f5ff19957 --- /dev/null +++ b/app/views/notify/repository_rewrite_history_success_email.html.haml @@ -0,0 +1,4 @@ +%p + = s_('Notify|Repository history rewrite succeeded on %{project_url}.') % { project_url: @project.web_url } +%p + = s_('Notify|Repository size is now %{repository_size} MiB.') % { repository_size: "%.1f" % (@project.repository.size || 0) } diff --git a/app/views/notify/repository_rewrite_history_success_email.text.erb b/app/views/notify/repository_rewrite_history_success_email.text.erb new file mode 100644 index 0000000000000..2ee613b5cc47d --- /dev/null +++ b/app/views/notify/repository_rewrite_history_success_email.text.erb @@ -0,0 +1,3 @@ +Repository history rewrite succeeded on <%= @project.web_url %> + +Repository size is now <%= "%.1f" % (@project.repository.size || 0) %> MiB diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d7aecf8d85f0f..5fc129c2f899e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -36905,6 +36905,15 @@ msgstr "" msgid "Notify|Remote mirror" msgstr "" +msgid "Notify|Repository history rewrite failed on %{project_url}." +msgstr "" + +msgid "Notify|Repository history rewrite succeeded on %{project_url}." +msgstr "" + +msgid "Notify|Repository size is now %{repository_size} MiB." +msgstr "" + msgid "Notify|Review changes" msgstr "" diff --git a/spec/mailers/emails/projects_spec.rb b/spec/mailers/emails/projects_spec.rb index 94ef98dafe2b2..8062a49140416 100644 --- a/spec/mailers/emails/projects_spec.rb +++ b/spec/mailers/emails/projects_spec.rb @@ -140,6 +140,46 @@ end end + describe '#repository_rewrite_history_success_email' do + let(:recipient) { user } + + subject { Notify.repository_rewrite_history_success_email(project, user) } + + it_behaves_like 'an email sent to a user' + it_behaves_like 'an email sent from GitLab' + it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like 'a user cannot unsubscribe through footer link' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' + + it 'has the correct subject and body' do + is_expected.to have_subject("#{project.name} | Project history rewrite has completed") + is_expected.to have_body_text(project.full_path) + is_expected.to have_body_text("Repository history rewrite succeeded on") + end + end + + describe '#repository_rewrite_history_failure_email' do + let(:recipient) { user } + let(:error) { 'Some error' } + + subject { Notify.repository_rewrite_history_failure_email(project, user, error) } + + it_behaves_like 'an email sent to a user' + it_behaves_like 'an email sent from GitLab' + it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like 'a user cannot unsubscribe through footer link' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' + + it 'has the correct subject and body' do + is_expected.to have_subject("#{project.name} | Project history rewrite failure") + is_expected.to have_body_text(project.full_path) + is_expected.to have_body_text("Repository history rewrite failed on") + is_expected.to have_body_text(error) + end + end + describe '.inactive_project_deletion_warning_email' do let(:recipient) { user } let(:deletion_date) { "2022-01-10" } diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index e1a77582aba65..1b47c73e9d3da 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -4105,6 +4105,36 @@ def create_pipeline(user, status) end end + describe 'Repository rewrite history', :deliver_mails_inline do + let(:user) { create(:user) } + + describe '#repository_rewrite_history_success' do + it 'emails the specified user only' do + notification.repository_rewrite_history_success(project, user) + + should_email(user) + end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { project } + let(:notification_trigger) { notification.repository_rewrite_history_success(project, user) } + end + end + + describe '#repository_rewrite_history_failure' do + it 'emails the specified user only' do + notification.repository_rewrite_history_failure(project, user, 'Some error') + + should_email(user) + end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { project } + let(:notification_trigger) { notification.repository_rewrite_history_failure(project, user, 'Some error') } + end + end + end + describe 'Repository cleanup', :deliver_mails_inline do let(:user) { create(:user) } -- GitLab