0 for unlimited, disallow blank, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/issues/27762#note_23520780

app/models/application_setting.rb

@@ -269,14 +269,6 @@ class ApplicationSetting < ActiveRecord::Base
     self.repository_storages = [value]
   end
 
-  def default_artifacts_expire_in=(value)
-    if value.present?
-      super(value.squish)
-    else
-      super(nil)
-    end
-  end
-
   # Choose one of the available repository storage options. Currently all have
   # equal weighting.
   def pick_repository_storage
@@ -306,10 +298,10 @@ class ApplicationSetting < ActiveRecord::Base
   end
 
   def check_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")
+    if default_artifacts_expire_in.blank?
+      errors.add(:default_artifacts_expiration, "is not presented")
+    else
+      ChronicDuration.parse(default_artifacts_expire_in)
     end
   rescue ChronicDuration::DurationParseError
     errors.add(:default_artifacts_expiration, "is invalid")

app/models/ci/build.rb

@@ -509,7 +509,7 @@ module Ci
     def artifacts_expire_in=(value)
       self.artifacts_expire_at =
         if value
-          Time.now + ChronicDuration.parse(value)
+          ChronicDuration.parse(value)&.seconds&.from_now
         end
     end
 

app/views/admin/application_settings/_form.html.haml

@@ -219,10 +219,11 @@
       .col-sm-10
         = f.text_field :default_artifacts_expire_in, class: 'form-control'
         .help-block
-          Set the default expiration time for each job's artifacts
+          Set the default expiration time for each job's artifacts.
+          0 for unlimited.
           = surround '(', ')' do
             = link_to 'syntax', help_page_path('ci/yaml/README', anchor: 'artifactsexpire_in')
-          = link_to icon('question-circle'), help_page_path('user/admin_area/settings/continuous_integration', anchor: 'default-artifacts-expiration-time')
+          = link_to icon('question-circle'), help_page_path('user/admin_area/settings/continuous_integration', anchor: 'default-artifacts-expiration')
 
   - if Gitlab.config.registry.enabled
     %fieldset

db/migrate/20170214084746_add_default_artifacts_expiration_to_application_settings.rb

@@ -5,7 +5,7 @@ class AddDefaultArtifactsExpirationToApplicationSettings < ActiveRecord::Migrati
 
   def change
     add_column :application_settings,
-      :default_artifacts_expire_in,
-      :string, null: true
+      :default_artifacts_expire_in, :string,
+      null: false, default: '0'
   end
 end

db/schema.rb

@@ -111,7 +111,7 @@ ActiveRecord::Schema.define(version: 20170214111112) do
     t.boolean "plantuml_enabled"
     t.integer "max_pages_size", default: 100, null: false
     t.integer "terminal_max_session_time", default: 0, null: false
-    t.string "default_artifacts_expire_in", limit: 255
+    t.string "default_artifacts_expire_in", default: '0', null: false
   end
 
   create_table "audit_events", force: :cascade do |t|
@@ -1352,4 +1352,4 @@ ActiveRecord::Schema.define(version: 20170214111112) do
   add_foreign_key "timelogs", "merge_requests", name: "fk_timelogs_merge_requests_merge_request_id", on_delete: :cascade
   add_foreign_key "trending_projects", "projects", on_delete: :cascade
   add_foreign_key "u2f_registrations", "users"
-end
+end

doc/user/admin_area/settings/continuous_integration.md

@@ -18,13 +18,13 @@ that this setting is set for each job.
 
 [art-yml]: ../../../administration/build_artifacts
 
-## Default artifacts expiration time
+## Default artifacts expiration
 
-The default expiration time of the [build artifacts][art-yml] can be set in
+The default expiration time of the [job artifacts][art-yml] can be set in
 the Admin area of your GitLab instance. The syntax of duration is described
 in [artifacts:expire_in][duration-syntax]. The default is `30 days`. Note that
-this setting is set for each job. Leave it blank if you don't want to set
-default expiration time.
+this setting is set for each job. Set it to 0 if you don't want default
+expiration.
 
 1. Go to **Admin area > Settings** (`/admin/application_settings`).
 

spec/models/application_setting_spec.rb

@@ -30,20 +30,16 @@ describe ApplicationSetting, models: true do
     end
 
     describe 'default_artifacts_expire_in' do
-      it 'sets an error if it is invalid' do
+      it 'sets an error if it cannot parse' do
         setting.update(default_artifacts_expire_in: 'a')
 
-        expect(setting).to be_invalid
-        expect(setting.errors.messages)
-          .to have_key(:default_artifacts_expiration)
+        expect_invalid
       end
 
-      it 'does not allow 0' do
-        setting.update(default_artifacts_expire_in: '0')
+      it 'sets an error if it is blank' do
+        setting.update(default_artifacts_expire_in: ' ')
 
-        expect(setting).to be_invalid
-        expect(setting.errors.messages)
-          .to have_key(:default_artifacts_expiration)
+        expect_invalid
       end
 
       it 'sets the value if it is valid' do
@@ -53,11 +49,17 @@ describe ApplicationSetting, models: true do
         expect(setting.default_artifacts_expire_in).to eq('30 days')
       end
 
-      it 'does not set it if it is blank' do
-        setting.update(default_artifacts_expire_in: ' ')
+      it 'sets the value if it is 0' do
+        setting.update(default_artifacts_expire_in: '0')
 
         expect(setting).to be_valid
-        expect(setting.default_artifacts_expire_in).to be_nil
+        expect(setting.default_artifacts_expire_in).to eq('0')
+      end
+
+      def expect_invalid
+        expect(setting).to be_invalid
+        expect(setting.errors.messages)
+          .to have_key(:default_artifacts_expiration)
       end
     end
 

spec/models/ci/build_spec.rb

@@ -166,6 +166,12 @@ describe Ci::Build, :models do
 
       is_expected.to be_nil
     end
+
+    it 'when setting to 0' do
+      build.artifacts_expire_in = '0'
+
+      is_expected.to be_nil
+    end
   end
 
   describe '#commit' do