Commit 41a14498 authored by James Lopez's avatar James Lopez

update code based on feedback

parent 4131b6e9
...@@ -2,7 +2,7 @@ class EmailsOnPushService < Service ...@@ -2,7 +2,7 @@ class EmailsOnPushService < Service
boolean_accessor :send_from_committer_email boolean_accessor :send_from_committer_email
boolean_accessor :disable_diffs boolean_accessor :disable_diffs
prop_accessor :recipients prop_accessor :recipients
validates :recipients, presence: true, if: :activated?, unless: :importing? validates :recipients, presence: true, if: :valid_recipients?
def title def title
'Emails on push' 'Emails on push'
......
...@@ -4,7 +4,7 @@ class IrkerService < Service ...@@ -4,7 +4,7 @@ class IrkerService < Service
prop_accessor :server_host, :server_port, :default_irc_uri prop_accessor :server_host, :server_port, :default_irc_uri
prop_accessor :recipients, :channels prop_accessor :recipients, :channels
boolean_accessor :colorize_messages boolean_accessor :colorize_messages
validates :recipients, presence: true, if: :activated?, unless: :importing? validates :recipients, presence: true, if: :valid_recipients?
before_validation :get_channels before_validation :get_channels
......
class PipelinesEmailService < Service class PipelinesEmailService < Service
prop_accessor :recipients prop_accessor :recipients
boolean_accessor :notify_only_broken_pipelines boolean_accessor :notify_only_broken_pipelines
validates :recipients, presence: true, if: :activated?, unless: :importing? validates :recipients, presence: true, if: :valid_recipients?
def initialize_properties def initialize_properties
self.properties ||= { notify_only_broken_pipelines: true } self.properties ||= { notify_only_broken_pipelines: true }
......
...@@ -297,4 +297,8 @@ class Service < ActiveRecord::Base ...@@ -297,4 +297,8 @@ class Service < ActiveRecord::Base
project.cache_has_external_wiki project.cache_has_external_wiki
end end
end end
def valid_recipients?
activated? && !importing?
end
end end
--- ---
title: Fixes destination already exists and is not an empty directory Import/Export title: Fixes destination already exists, and some particular service errors on Import/Export
error error
merge_request: 16714 merge_request: 16714
author: author:
......
...@@ -19,8 +19,13 @@ module Gitlab ...@@ -19,8 +19,13 @@ module Gitlab
def error(error) def error(error)
error_out(error.message, caller[0].dup) error_out(error.message, caller[0].dup)
@errors << error.message @errors << error.message
# Debug: # Debug:
Rails.logger.error(error.backtrace&.join("\n")) if error.backtrace
Rails.logger.error("Import/Export backtrace: #{error.backtrace.join("\n")}")
else
Rails.logger.error("No backtrace found")
end
end end
private private
......
...@@ -49,7 +49,7 @@ describe RepositoryImportWorker do ...@@ -49,7 +49,7 @@ describe RepositoryImportWorker do
expect do expect do
subject.perform(project.id) subject.perform(project.id)
end.to raise_error(StandardError, error) end.to raise_error(RuntimeError, error)
expect(project.reload.import_jid).not_to be_nil expect(project.reload.import_jid).not_to be_nil
end end
...@@ -61,7 +61,7 @@ describe RepositoryImportWorker do ...@@ -61,7 +61,7 @@ describe RepositoryImportWorker do
expect do expect do
subject.perform(project.id) subject.perform(project.id)
end.to raise_error(StandardError, error) end.to raise_error(RuntimeError, error)
expect(project.reload.import_error).not_to be_nil expect(project.reload.import_error).not_to be_nil
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment