Use static error message and don't give booleans

in validation. Feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9219#note_23437431
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9219#note_23437868

1 files changed, 4 insertions, 9 deletions

diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 6d5f02a1011..e865da0ce7a 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -306,17 +306,12 @@ class ApplicationSetting < ActiveRecord::Base
   end
 
   def check_default_artifacts_expire_in
-    return true unless default_artifacts_expire_in
-
-    if ChronicDuration.parse(default_artifacts_expire_in).nil?
-      errors.add(:default_artifacts_expire_in,
+    if default_artifacts_expire_in &&
+      ChronicDuration.parse(default_artifacts_expire_in).nil?
+      errors.add(:default_artifacts_expiration,
         "can't be 0. Leave it blank for no expiration")
-      false
-    else
-      true
     end
   rescue ChronicDuration::DurationParseError => e
-    errors.add(:default_artifacts_expire_in, ": #{e.message}")
-    false
+    errors.add(:default_artifacts_expiration, "is invalid")
   end
 end