Use the same syntax for default expiration

Feedback:

* https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9219#note_23343951
* https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9219#note_23344036
* https://gitlab.com/gitlab-org/gitlab-ce/issues/27762#note_23344797

app/controllers/admin/application_settings_controller.rb

@@ -83,7 +83,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
       :akismet_api_key,
       :akismet_enabled,
       :container_registry_token_expire_delay,
-      :default_artifacts_expiration,
+      :default_artifacts_expire_in,
       :default_branch_protection,
       :default_group_visibility,
       :default_project_visibility,

app/models/application_setting.rb

@@ -80,9 +80,7 @@ class ApplicationSetting < ActiveRecord::Base
             presence: true,
             numericality: { only_integer: true, greater_than: 0 }
 
-  validates :default_artifacts_expiration,
-            presence: true,
-            numericality: { only_integer: true, greater_than_or_equal_to: 0 }
+  validate  :check_default_artifacts_expire_in
 
   validates :container_registry_token_expire_delay,
             presence: true,
@@ -176,7 +174,7 @@ class ApplicationSetting < ActiveRecord::Base
       after_sign_up_text: nil,
       akismet_enabled: false,
       container_registry_token_expire_delay: 5,
-      default_artifacts_expiration: 30,
+      default_artifacts_expire_in: '30 days',
       default_branch_protection: Settings.gitlab['default_branch_protection'],
       default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'],
       default_projects_limit: Settings.gitlab['default_projects_limit'],
@@ -291,12 +289,6 @@ class ApplicationSetting < ActiveRecord::Base
     sidekiq_throttling_enabled
   end
 
-  def default_artifacts_expire_in
-    if default_artifacts_expiration.nonzero?
-      "#{default_artifacts_expiration} days"
-    end
-  end
-
   private
 
   def check_repository_storages
@@ -304,4 +296,12 @@ class ApplicationSetting < ActiveRecord::Base
     errors.add(:repository_storages, "can't include: #{invalid.join(", ")}") unless
       invalid.empty?
   end
+
+  def check_default_artifacts_expire_in
+    ChronicDuration.parse(default_artifacts_expire_in)
+    true
+  rescue ChronicDuration::DurationParseError => e
+    errors.add(:default_artifacts_expire_in, ": #{e.message}")
+    false
+  end
 end

app/models/ci/build.rb

@@ -513,18 +513,6 @@ module Ci
         end
     end
 
-    def set_artifacts_expire_in(expire_in)
-      value =
-        if expire_in
-          expire_in
-        else
-          Gitlab::CurrentSettings.current_application_settings
-            .default_artifacts_expire_in
-        end
-
-      self.artifacts_expire_in = value
-    end
-
     def has_expiring_artifacts?
       artifacts_expire_at.present?
     end

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

@@ -213,14 +213,14 @@
         = f.number_field :max_artifacts_size, class: 'form-control'
         .help-block
           Set the maximum file size for each job's artifacts
-          = link_to "(?)", help_page_path("user/admin_area/settings/continuous_integration", anchor: "maximum-artifacts-size")
+          = link_to icon('question-circle'), help_page_path('user/admin_area/settings/continuous_integration', anchor: 'maximum-artifacts-size')
     .form-group
-      = f.label :default_artifacts_expiration, 'Default artifacts expiration (days)', class: 'control-label col-sm-2'
+      = f.label :default_artifacts_expire_in, 'Default artifacts expiration', class: 'control-label col-sm-2'
       .col-sm-10
-        = f.number_field :default_artifacts_expiration, class: 'form-control'
+        = f.text_field :default_artifacts_expire_in, class: 'form-control'
         .help-block
-          Set the default expiration time for each job's artifacts (0 as never expired)
-          = link_to "(?)", help_page_path("user/admin_area/settings/continuous_integration", anchor: "default-artifacts-expiration")
+          Set the default expiration time for each job's artifacts
+          = 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_expiration,
-      :integer, default: 0, null: false
+      :default_artifacts_expire_in,
+      :string, null: true
   end
 end

lib/api/settings.rb

@@ -57,7 +57,7 @@ module API
         requires :shared_runners_text, type: String, desc: 'Shared runners text '
       end
       optional :max_artifacts_size, type: Integer, desc: "Set the maximum file size for each job's artifacts"
-      optional :default_artifacts_expiration, type: Integer, desc: "Set the default expiration time for each job's artifacts"
+      optional :default_artifacts_expire_in, type: Integer, desc: "Set the default expiration time for each job's artifacts"
       optional :max_pages_size, type: Integer, desc: 'Maximum size of pages in MB'
       optional :container_registry_token_expire_delay, type: Integer, desc: 'Authorization token duration (minutes)'
       optional :metrics_enabled, type: Boolean, desc: 'Enable the InfluxDB metrics'
@@ -119,7 +119,7 @@ module API
                       :after_sign_up_text, :signin_enabled, :require_two_factor_authentication,
                       :home_page_url, :after_sign_out_path, :sign_in_text, :help_page_text,
                       :shared_runners_enabled, :max_artifacts_size,
-                      :default_artifacts_expiration, :max_pages_size,
+                      :default_artifacts_expire_in, :max_pages_size,
                       :container_registry_token_expire_delay,
                       :metrics_enabled, :sidekiq_throttling_enabled, :recaptcha_enabled,
                       :akismet_enabled, :admin_notification_email, :sentry_enabled,

lib/ci/api/builds.rb

@@ -167,7 +167,10 @@ module Ci
 
           build.artifacts_file = artifacts
           build.artifacts_metadata = metadata
-          build.set_artifacts_expire_in(params['expire_in'])
+          build.artifacts_expire_in =
+            params['expire_in'] ||
+              Gitlab::CurrentSettings.current_application_settings
+                .default_artifacts_expire_in
 
           if build.save
             present(build, with: Entities::BuildDetails)

spec/requests/ci/api/builds_spec.rb

@@ -630,7 +630,7 @@ describe Ci::API::Builds do
 
           context 'with an expire date' do
             let!(:artifacts) { file_upload }
-            let(:default_artifacts_expiration) { 0 }
+            let(:default_artifacts_expire_in) {}
 
             let(:post_data) do
               { 'file.path' => artifacts.path,
@@ -640,7 +640,7 @@ describe Ci::API::Builds do
 
             before do
               stub_application_setting(
-                default_artifacts_expiration: default_artifacts_expiration)
+                default_artifacts_expire_in: default_artifacts_expire_in)
 
               post(post_url, post_data, headers_with_token)
             end
@@ -668,7 +668,7 @@ describe Ci::API::Builds do
               end
 
               context 'with application default' do
-                let(:default_artifacts_expiration) { 5 }
+                let(:default_artifacts_expire_in) { '5 days' }
 
                 it 'sets to application default' do
                   build.reload