Merge branch 'master' into 'api-remove-owned-groups'

app/assets/javascripts/vue_pipelines_index/stage.js.es6

@@ -23,6 +23,13 @@
         required: true,
       },
     },
+
+    updated() {
+      if (this.builds) {
+        this.stopDropdownClickPropagation();
+      }
+    },
+
     methods: {
       fetchBuilds(e) {
         const areaExpanded = e.currentTarget.attributes['aria-expanded'];
@@ -37,17 +44,19 @@
             return flash;
           });
       },
-      keepGraph(e) {
-        const { target } = e;
-
-        if (target.className.indexOf('js-ci-action-icon') >= 0) return null;
-
-        if (
-          target.parentElement &&
-          (target.parentElement.className.indexOf('js-ci-action-icon') >= 0)
-        ) return null;
 
-        return e.stopPropagation();
+      /**
+       * When the user right clicks or cmd/ctrl + click in the job name
+       * the dropdown should not be closed and the link should open in another tab,
+       * so we stop propagation of the click event inside the dropdown.
+       *
+       * Since this component is rendered multiple times per page we need to guarantee we only
+       * target the click event of this component.
+       */
+      stopDropdownClickPropagation() {
+        $(this.$el.querySelectorAll('.js-builds-dropdown-list a.mini-pipeline-graph-dropdown-item')).on('click', (e) => {
+          e.stopPropagation();
+        });
       },
     },
     computed: {
@@ -76,13 +85,13 @@
     template: `
       <div>
         <button
-          @click='fetchBuilds($event)'
+          @click="fetchBuilds($event)"
           :class="triggerButtonClass"
-          :title='stage.title'
+          :title="stage.title"
           data-placement="top"
           data-toggle="dropdown"
           type="button"
-          :aria-label='stage.title'
+          :aria-label="stage.title"
         >
           <span v-html="svg" aria-hidden="true"></span>
           <i class="fa fa-caret-down" aria-hidden="true"></i>
@@ -90,7 +99,6 @@
         <ul class="dropdown-menu mini-pipeline-graph-dropdown-menu js-builds-dropdown-container">
           <div class="arrow-up" aria-hidden="true"></div>
           <div
-            @click='keepGraph($event)'
             :class="dropdownClass"
             class="js-builds-dropdown-list scrollable-menu"
             v-html="buildsOrSpinner"

app/controllers/admin/application_settings_controller.rb

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

app/models/application_setting.rb

@@ -76,6 +76,12 @@ class ApplicationSetting < ActiveRecord::Base
             presence: true,
             numericality: { only_integer: true, greater_than: 0 }
 
+  validates :max_artifacts_size,
+            presence: true,
+            numericality: { only_integer: true, greater_than: 0 }
+
+  validates :default_artifacts_expire_in, presence: true, duration: true
+
   validates :container_registry_token_expire_delay,
             presence: true,
             numericality: { only_integer: true, greater_than: 0 }
@@ -168,6 +174,7 @@ class ApplicationSetting < ActiveRecord::Base
       after_sign_up_text: nil,
       akismet_enabled: false,
       container_registry_token_expire_delay: 5,
+      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'],
@@ -201,9 +208,9 @@ class ApplicationSetting < ActiveRecord::Base
       sign_in_text: nil,
       signin_enabled: Settings.gitlab['signin_enabled'],
       signup_enabled: Settings.gitlab['signup_enabled'],
+      terminal_max_session_time: 0,
       two_factor_grace_period: 48,
-      user_default_external: false,
-      terminal_max_session_time: 0
+      user_default_external: false
     }
   end
 
@@ -215,6 +222,14 @@ class ApplicationSetting < ActiveRecord::Base
     create(defaults)
   end
 
+  def self.human_attribute_name(attr, _options = {})
+    if attr == :default_artifacts_expire_in
+      'Default artifacts expiration'
+    else
+      super
+    end
+  end
+
   def home_page_url_column_exist
     ActiveRecord::Base.connection.column_exists?(:application_settings, :home_page_url)
   end

app/models/ci/build.rb

@@ -484,7 +484,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/validators/duration_validator.rb

+# DurationValidator
+#
+# Validate the format conforms with ChronicDuration
+#
+# Example:
+#
+#   class ApplicationSetting < ActiveRecord::Base
+#     validates :default_artifacts_expire_in, presence: true, duration: true
+#   end
+#
+class DurationValidator < ActiveModel::EachValidator
+  def validate_each(record, attribute, value)
+    ChronicDuration.parse(value)
+  rescue ChronicDuration::DurationParseError
+    record.errors.add(attribute, "is not a correct duration")
+  end
+end

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

@@ -212,8 +212,16 @@
       .col-sm-10
         = f.number_field :max_artifacts_size, class: 'form-control'
         .help-block
-          Set the maximum file size each jobs's artifacts can have
-          = link_to "(?)", help_page_path("user/admin_area/settings/continuous_integration", anchor: "maximum-artifacts-size")
+          Set the maximum file size for each job's artifacts
+          = link_to icon('question-circle'), help_page_path('user/admin_area/settings/continuous_integration', anchor: 'maximum-artifacts-size')
+    .form-group
+      = f.label :default_artifacts_expire_in, 'Default artifacts expiration', class: 'control-label col-sm-2'
+      .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.
+          0 for unlimited.
+          = link_to icon('question-circle'), help_page_path('user/admin_area/settings/continuous_integration', anchor: 'default-artifacts-expiration')
 
   - if Gitlab.config.registry.enabled
     %fieldset

changelogs/unreleased/27530-fix-job-dropdown-pipeline-console-error.yml

+---
+title: Fixes job dropdown action throws error in js console
+merge_request: 9182
+author:

changelogs/unreleased/27762-add-default-artifacts-expiration.yml

+---
+title: Add admin setting for default artifacts expiration
+merge_request: 9219
+author:

changelogs/unreleased/3874-correctly-return-json-on-delete-responses.yml

+---
+title: Return 202 with JSON body on async removals on V4 API
+merge_request:
+author:

db/migrate/20170214084746_add_default_artifacts_expiration_to_application_settings.rb

+class AddDefaultArtifactsExpirationToApplicationSettings < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :application_settings,
+      :default_artifacts_expire_in, :string,
+      null: false, default: '0'
+  end
+end

db/schema.rb

@@ -111,6 +111,7 @@ ActiveRecord::Schema.define(version: 20170216141440) 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", default: '0', null: false
   end
 
   create_table "audit_events", force: :cascade do |t|

doc/api/v3_to_v4.md

@@ -42,3 +42,4 @@ changes are in V4:
 - Remove `public` param from create and edit actions of projects [!8736](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8736)
 - Notes do not return deprecated field `upvote` and `downvote` [!9384](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9384)
 - Remove `GET /groups/owned`. Use `GET /groups?owned=true` instead [!9505](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9505)
+- Return 202 with JSON body on async removals on V4 API (DELETE `/projects/:id/repository/merged_branches` and DELETE `/projects/:id`) [!9449](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9449)
\ No newline at end of file

doc/ci/variables/README.md

@@ -148,7 +148,8 @@ available in the build environment. It's the recommended method to use for
 storing things like passwords, secret keys and credentials.
 
 Secret variables can be added by going to your project's
-**Settings ➔ Variables ➔ Add variable**.
+**Settings ➔ CI/CD Pipelines**, then finding the section called
+**Secret Variables**.
 
 Once you set them, they will be available for all subsequent jobs.
 

doc/user/admin_area/settings/continuous_integration.md

@@ -3,18 +3,38 @@
 ## Maximum artifacts size
 
 The maximum size of the [job artifacts][art-yml] can be set in the Admin area
-of your GitLab instance. The value is in MB and the default is 100MB. Note that
-this setting is set for each job.
+of your GitLab instance. The value is in *MB* and the default is 100MB. Note
+that this setting is set for each job.
 
 1. Go to **Admin area > Settings** (`/admin/application_settings`).
 
     ![Admin area settings button](img/admin_area_settings_button.png)
 
-1. Change the value of the maximum artifacts size (in MB):
+1. Change the value of maximum artifacts size (in MB):
 
     ![Admin area maximum artifacts size](img/admin_area_maximum_artifacts_size.png)
 
 1. Hit **Save** for the changes to take effect.
 
+[art-yml]: ../../../administration/build_artifacts
 
-[art-yml]: ../../../administration/job_artifacts.md
+## Default artifacts expiration
+
+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. Set it to 0 if you don't want default
+expiration.
+
+1. Go to **Admin area > Settings** (`/admin/application_settings`).
+
+    ![Admin area settings button](img/admin_area_settings_button.png)
+
+1. Change the value of default expiration time ([syntax][duration-syntax]):
+
+    ![Admin area default artifacts expiration](img/admin_area_default_artifacts_expiration.png)
+
+1. Hit **Save** for the changes to take effect.
+
+[art-yml]: ../../../administration/job_artifacts
+[duration-syntax]: ../../../ci/yaml/README#artifactsexpire_in

lib/api/branches.rb

@@ -137,7 +137,7 @@ module API
       delete ":id/repository/merged_branches" do
         DeleteMergedBranchesService.new(user_project, current_user).async_execute
 
-        status(200)
+        accepted!
       end
     end
   end

lib/api/entities.rb

@@ -557,6 +557,7 @@ module API
       expose :default_project_visibility
       expose :default_snippet_visibility
       expose :default_group_visibility
+      expose :default_artifacts_expire_in
       expose :domain_whitelist
       expose :domain_blacklist_enabled
       expose :domain_blacklist

lib/api/helpers.rb

@@ -209,6 +209,10 @@ module API
       render_api_error!('204 No Content', 204)
     end
 
+    def accepted!
+      render_api_error!('202 Accepted', 202)
+    end
+
     def render_validation_error!(model)
       if model.errors.any?
         render_api_error!(model.errors.messages || '400 Bad Request', 400)

lib/api/projects.rb

@@ -282,6 +282,8 @@ module API
       delete ":id" do
         authorize! :remove_project, user_project
         ::Projects::DestroyService.new(user_project, current_user, {}).async_execute
+
+        accepted!
       end
 
       desc 'Mark this project as forked from another'

lib/api/settings.rb

@@ -56,7 +56,8 @@ module API
       given shared_runners_enabled: ->(val) { val } do
         requires :shared_runners_text, type: String, desc: 'Shared runners text '
       end
-      optional :max_artifacts_size, type: Integer, desc: "Set the maximum file size each build's artifacts can have"
+      optional :max_artifacts_size, type: Integer, desc: "Set the maximum file size for each job's artifacts"
+      optional :default_artifacts_expire_in, type: String, 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'
@@ -117,7 +118,9 @@ module API
                       :send_user_confirmation_email, :domain_whitelist, :domain_blacklist_enabled,
                       :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, :max_pages_size, :container_registry_token_expire_delay,
+                      :shared_runners_enabled, :max_artifacts_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,
                       :repository_storage, :repository_checks_enabled, :koding_enabled, :plantuml_enabled,

lib/api/v3/branches.rb

@@ -18,6 +18,13 @@ module API
 
           present branches, with: ::API::Entities::RepoBranch, project: user_project
         end
+
+        desc 'Delete all merged branches'
+        delete ":id/repository/merged_branches" do
+          DeleteMergedBranchesService.new(user_project, current_user).async_execute
+
+          status(200)
+        end
       end
     end
   end

lib/ci/api/builds.rb

@@ -167,7 +167,10 @@ module Ci
 
           build.artifacts_file = artifacts
           build.artifacts_metadata = metadata
-          build.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/features/projects/pipelines/pipelines_spec.rb

@@ -328,6 +328,18 @@ describe 'Pipelines', :feature, :js do
             expect(build.reload).to be_canceled
           end
         end
+
+        context 'dropdown jobs list' do
+          it 'should keep the dropdown open when the user ctr/cmd + clicks in the job name' do
+            find('.js-builds-dropdown-button').trigger('click')
+
+            execute_script('var e = $.Event("keydown", { keyCode: 64 }); $("body").trigger(e);')
+
+            find('.mini-pipeline-graph-dropdown-item').trigger('click')
+
+            expect(page).to have_selector('.js-ci-action-icon')
+          end
+        end
       end
 
       context 'with pagination' do

spec/models/application_setting_spec.rb

@@ -29,6 +29,40 @@ describe ApplicationSetting, models: true do
       it { is_expected.not_to allow_value(['test']).for(:disabled_oauth_sign_in_sources) }
     end
 
+    describe 'default_artifacts_expire_in' do
+      it 'sets an error if it cannot parse' do
+        setting.update(default_artifacts_expire_in: 'a')
+
+        expect_invalid
+      end
+
+      it 'sets an error if it is blank' do
+        setting.update(default_artifacts_expire_in: ' ')
+
+        expect_invalid
+      end
+
+      it 'sets the value if it is valid' do
+        setting.update(default_artifacts_expire_in: '30 days')
+
+        expect(setting).to be_valid
+        expect(setting.default_artifacts_expire_in).to eq('30 days')
+      end
+
+      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 eq('0')
+      end
+
+      def expect_invalid
+        expect(setting).to be_invalid
+        expect(setting.errors.messages)
+          .to have_key(:default_artifacts_expire_in)
+      end
+    end
+
     it { is_expected.to validate_presence_of(:max_attachment_size) }
 
     it do

spec/models/ci/build_spec.rb

@@ -162,11 +162,17 @@ describe Ci::Build, :models do
       is_expected.to be_nil
     end
 
-    it 'when resseting value' do
+    it 'when resetting value' do
       build.artifacts_expire_in = nil
 
       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

spec/requests/api/branches_spec.rb

@@ -360,9 +360,11 @@ describe API::Branches, api: true  do
       allow_any_instance_of(Repository).to receive(:rm_branch).and_return(true)
     end
 
-    it 'returns 200' do
+    it 'returns 202 with json body' do
       delete api("/projects/#{project.id}/repository/merged_branches", user)
-      expect(response).to have_http_status(200)
+
+      expect(response).to have_http_status(202)
+      expect(json_response['message']).to eql('202 Accepted')
     end
 
     it 'returns a 403 error if guest' do

spec/requests/api/projects_spec.rb

@@ -1263,7 +1263,9 @@ describe API::Projects, api: true  do
     context 'when authenticated as user' do
       it 'removes project' do
         delete api("/projects/#{project.id}", user)
-        expect(response).to have_http_status(200)
+
+        expect(response).to have_http_status(202)
+        expect(json_response['message']).to eql('202 Accepted')
       end
 
       it 'does not remove a project if not an owner' do
@@ -1287,7 +1289,9 @@ describe API::Projects, api: true  do
     context 'when authenticated as admin' do
       it 'removes any existing project' do
         delete api("/projects/#{project.id}", admin)
-        expect(response).to have_http_status(200)
+
+        expect(response).to have_http_status(202)
+        expect(json_response['message']).to eql('202 Accepted')
       end
 
       it 'does not remove a non existing project' do

spec/requests/api/settings_spec.rb

@@ -30,8 +30,14 @@ describe API::Settings, 'Settings', api: true  do
 
       it "updates application settings" do
         put api("/application/settings", admin),
-          default_projects_limit: 3, signin_enabled: false, repository_storage: 'custom', koding_enabled: true, koding_url: 'http://koding.example.com',
-          plantuml_enabled: true, plantuml_url: 'http://plantuml.example.com'
+          default_projects_limit: 3,
+          signin_enabled: false,
+          repository_storage: 'custom',
+          koding_enabled: true,
+          koding_url: 'http://koding.example.com',
+          plantuml_enabled: true,
+          plantuml_url: 'http://plantuml.example.com',
+          default_artifacts_expire_in: '2 days'
         expect(response).to have_http_status(200)
         expect(json_response['default_projects_limit']).to eq(3)
         expect(json_response['signin_enabled']).to be_falsey
@@ -41,6 +47,7 @@ describe API::Settings, 'Settings', api: true  do
         expect(json_response['koding_url']).to eq('http://koding.example.com')
         expect(json_response['plantuml_enabled']).to be_truthy
         expect(json_response['plantuml_url']).to eq('http://plantuml.example.com')
+        expect(json_response['default_artifacts_expire_in']).to eq('2 days')
       end
     end
 

spec/requests/api/v3/branches_spec.rb

@@ -20,4 +20,25 @@ describe API::V3::Branches, api: true  do
       expect(branch_names).to match_array(project.repository.branch_names)
     end
   end
+
+  describe "DELETE /projects/:id/repository/merged_branches" do
+    before do
+      allow_any_instance_of(Repository).to receive(:rm_branch).and_return(true)
+    end
+
+    it 'returns 200' do
+      delete v3_api("/projects/#{project.id}/repository/merged_branches", user)
+
+      expect(response).to have_http_status(200)
+    end
+
+    it 'returns a 403 error if guest' do
+      user_b = create :user
+      create(:project_member, :guest, user: user_b, project: project)
+
+      delete v3_api("/projects/#{project.id}/repository/merged_branches", user_b)
+
+      expect(response).to have_http_status(403)
+    end
+  end
 end

spec/requests/ci/api/builds_spec.rb

@@ -630,6 +630,7 @@ describe Ci::API::Builds do
 
           context 'with an expire date' do
             let!(:artifacts) { file_upload }
+            let(:default_artifacts_expire_in) {}
 
             let(:post_data) do
               { 'file.path' => artifacts.path,
@@ -638,6 +639,9 @@ describe Ci::API::Builds do
             end
 
             before do
+              stub_application_setting(
+                default_artifacts_expire_in: default_artifacts_expire_in)
+
               post(post_url, post_data, headers_with_token)
             end
 
@@ -648,7 +652,8 @@ describe Ci::API::Builds do
                 build.reload
                 expect(response).to have_http_status(201)
                 expect(json_response['artifacts_expire_at']).not_to be_empty
-                expect(build.artifacts_expire_at).to be_within(5.minutes).of(Time.now + 7.days)
+                expect(build.artifacts_expire_at).
+                  to be_within(5.minutes).of(7.days.from_now)
               end
             end
 
@@ -661,6 +666,32 @@ describe Ci::API::Builds do
                 expect(json_response['artifacts_expire_at']).to be_nil
                 expect(build.artifacts_expire_at).to be_nil
               end
+
+              context 'with application default' do
+                context 'default to 5 days' do
+                  let(:default_artifacts_expire_in) { '5 days' }
+
+                  it 'sets to application default' do
+                    build.reload
+                    expect(response).to have_http_status(201)
+                    expect(json_response['artifacts_expire_at'])
+                      .not_to be_empty
+                    expect(build.artifacts_expire_at)
+                      .to be_within(5.minutes).of(5.days.from_now)
+                  end
+                end
+
+                context 'default to 0' do
+                  let(:default_artifacts_expire_in) { '0' }
+
+                  it 'does not set expire_in' do
+                    build.reload
+                    expect(response).to have_http_status(201)
+                    expect(json_response['artifacts_expire_at']).to be_nil
+                    expect(build.artifacts_expire_at).to be_nil
+                  end
+                end
+              end
             end
           end