Merge branch 'ce-to-ee-2018-03-08' into 'master'

CE upstream - 2018-03-08 12:23 UTC

See merge request gitlab-org/gitlab-ee!4897

PROCESS.md

@@ -53,7 +53,7 @@ Below we describe the contributing process to GitLab for two reasons:
 Several people from the [GitLab team][team] are helping community members to get
 their contributions accepted by meeting our [Definition of done][done].
 
-What you can expect from them is described at https://about.gitlab.com/jobs/merge-request-coach/.
+What you can expect from them is described at https://about.gitlab.com/roles/merge-request-coach/.
 
 ## Assigning issues
 
@@ -202,6 +202,9 @@ you can ask for an exception to be made.
 Go to [Release tasks issue tracker](https://gitlab.com/gitlab-org/release/tasks/issues/new) and create an issue
 using the `Exception-request` issue template.
 
+**Do not** set the relevant `Pick into X.Y` label (see above) before request an
+exception; this should be done after the exception is approved.
+
 You can find who is who on the [team page](https://about.gitlab.com/team/).
 
 Whether an exception is made is determined by weighing the benefit and urgency of the change

app/assets/javascripts/pipelines/components/nav_controls.vue

 <script>
+  import LoadingButton from '../../vue_shared/components/loading_button.vue';
+
   export default {
     name: 'PipelineNavControls',
+    components: {
+      LoadingButton,
+    },
     props: {
       newPipelinePath: {
         type: String,
@@ -19,6 +24,17 @@
         required: false,
         default: null,
       },
+
+      isResetCacheButtonLoading: {
+        type: Boolean,
+        required: false,
+        default: false,
+      },
+    },
+    methods: {
+      onClickResetCache() {
+        this.$emit('resetRunnersCache', this.resetCachePath);
+      },
     },
   };
 </script>
@@ -32,14 +48,13 @@
       {{ s__('Pipelines|Run Pipeline') }}
     </a>
 
-    <a
+    <loading-button
       v-if="resetCachePath"
-      data-method="post"
-      :href="resetCachePath"
+      @click="onClickResetCache"
+      :loading="isResetCacheButtonLoading"
       class="btn btn-default js-clear-cache"
-    >
-      {{ s__('Pipelines|Clear Runner Caches') }}
-    </a>
+      :label="s__('Pipelines|Clear Runner Caches')"
+    />
 
     <a
       v-if="ciLintPath"

app/assets/javascripts/pipelines/components/pipelines.vue

 <script>
   import _ from 'underscore';
   import { __, sprintf, s__ } from '../../locale';
+  import createFlash from '../../flash';
   import PipelinesService from '../services/pipelines_service';
   import pipelinesMixin from '../mixins/pipelines';
   import TablePagination from '../../vue_shared/components/table_pagination.vue';
@@ -92,6 +93,7 @@
         scope: getParameterByName('scope') || 'all',
         page: getParameterByName('page') || '1',
         requestData: {},
+        isResetCacheButtonLoading: false,
       };
     },
     stateMap: {
@@ -265,6 +267,23 @@
             this.poll.restart({ data: this.requestData });
           });
       },
+
+      handleResetRunnersCache(endpoint) {
+        this.isResetCacheButtonLoading = true;
+
+        this.service.postAction(endpoint)
+          .then(() => {
+            this.isResetCacheButtonLoading = false;
+            createFlash(
+              s__('Pipelines|Project cache successfully reset.'),
+              'notice',
+            );
+          })
+          .catch(() => {
+            this.isResetCacheButtonLoading = false;
+            createFlash(s__('Pipelines|Something went wrong while cleaning runners cache.'));
+          });
+      },
     },
   };
 </script>
@@ -301,6 +320,8 @@
         :new-pipeline-path="newPipelinePath"
         :reset-cache-path="resetCachePath"
         :ci-lint-path="ciLintPath"
+        @resetRunnersCache="handleResetRunnersCache"
+        :is-reset-cache-button-loading="isResetCacheButtonLoading"
       />
     </div>
 

app/assets/javascripts/pipelines/mixins/pipelines.js

@@ -51,12 +51,10 @@ export default {
       }
     });
 
-    eventHub.$on('refreshPipelines', this.fetchPipelines);
     eventHub.$on('postAction', this.postAction);
   },
   beforeDestroy() {
-    eventHub.$off('refreshPipelines');
-    eventHub.$on('postAction', this.postAction);
+    eventHub.$off('postAction', this.postAction);
   },
   destroyed() {
     this.poll.stop();
@@ -92,7 +90,7 @@ export default {
     },
     postAction(endpoint) {
       this.service.postAction(endpoint)
-        .then(() => eventHub.$emit('refreshPipelines'))
+        .then(() => this.fetchPipelines())
         .catch(() => Flash(__('An error occurred while making the request.')));
     },
   },

app/controllers/projects/settings/ci_cd_controller.rb

@@ -13,12 +13,14 @@ module Projects
 
       def reset_cache
         if ResetProjectCacheService.new(@project, current_user).execute
-          flash[:notice] = _("Project cache successfully reset.")
+          respond_to do |format|
+            format.json { head :ok }
+          end
         else
-          flash[:error] = _("Unable to reset project cache.")
+          respond_to do |format|
+            format.json { head :bad_request }
+          end
         end
-
-        redirect_to project_pipelines_path(@project)
       end
 
       private

app/models/repository.rb

@@ -42,7 +42,7 @@ class Repository
   CACHED_METHODS = %i(size commit_count rendered_readme contribution_guide
                       changelog license_blob license_key gitignore koding_yml
                       gitlab_ci_yml branch_names tag_names branch_count
-                      tag_count avatar exists? empty? root_ref has_visible_content?
+                      tag_count avatar exists? root_ref has_visible_content?
                       issue_template_names merge_request_template_names).freeze
 
   # Methods that use cache_method but only memoize the value
@@ -367,7 +367,7 @@ class Repository
   def expire_emptiness_caches
     return unless empty?
 
-    expire_method_caches(%i(empty? has_visible_content?))
+    expire_method_caches(%i(has_visible_content?))
   end
 
   def lookup_cache
@@ -513,12 +513,14 @@ class Repository
   end
   cache_method :exists?
 
+  # We don't need to cache the output of this method because both exists? and
+  # has_visible_content? are already memoized and cached. There's no guarantee
+  # that the values are expired and loaded atomically.
   def empty?
     return true unless exists?
 
     !has_visible_content?
   end
-  cache_method :empty?
 
   # The size of this repository in megabytes.
   def size

changelogs/unreleased/ce-jej-github-project-service-for-ci.yml

+---
+title: Hook data for pipelines includes detailed_status
+merge_request: 17607
+author:
+type: changed

changelogs/unreleased/ce-jej-integrations-can-hide-trigger-checkboxes.yml

+---
+title: Avoid showing unnecessary Trigger checkboxes for project Integrations with
+  only one event
+merge_request: 17607
+author:
+type: changed

changelogs/unreleased/fix-mattermost-delete-team.yml

+---
+title: Fixed group deletion linked to Mattermost
+merge_request: 16209
+author: Julien Millau
+type: fixed

changelogs/unreleased/sh-remove-double-caching-repo-empty.yml

+---
+title: Remove double caching of Repository#empty?
+merge_request:
+author:
+type: fixed

lib/mattermost/session.rb

@@ -83,6 +83,12 @@ module Mattermost
       end
     end
 
+    def delete(path, options = {})
+      handle_exceptions do
+        self.class.delete(path, options.merge(headers: @headers))
+      end
+    end
+
     private
 
     def create

lib/mattermost/team.rb

@@ -16,10 +16,9 @@ module Mattermost
     end
 
     # The deletion is done async, so the response is fast.
-    # On the mattermost side, this triggers an soft deletion first, after which
-    # the actuall data is removed
+    # On the mattermost side, this triggers an soft deletion
     def destroy(team_id:)
-      session_delete("/api/v4/teams/#{team_id}?permanent=true")
+      session_delete("/api/v4/teams/#{team_id}")
     end
   end
 end

spec/controllers/projects/settings/ci_cd_controller_spec.rb

@@ -27,7 +27,7 @@ describe Projects::Settings::CiCdController do
       allow(ResetProjectCacheService).to receive_message_chain(:new, :execute).and_return(true)
     end
 
-    subject { post :reset_cache, namespace_id: project.namespace, project_id: project }
+    subject { post :reset_cache, namespace_id: project.namespace, project_id: project, format: :json }
 
     it 'calls reset project cache service' do
       expect(ResetProjectCacheService).to receive_message_chain(:new, :execute)
@@ -35,19 +35,11 @@ describe Projects::Settings::CiCdController do
       subject
     end
 
-    it 'redirects to project pipelines path' do
-      subject
-
-      expect(response).to have_gitlab_http_status(:redirect)
-      expect(response).to redirect_to(project_pipelines_path(project))
-    end
-
     context 'when service returns successfully' do
-      it 'sets the flash notice variable' do
+      it 'returns a success header' do
         subject
 
-        expect(controller).to set_flash[:notice]
-        expect(controller).not_to set_flash[:error]
+        expect(response).to have_gitlab_http_status(:ok)
       end
     end
 
@@ -56,11 +48,10 @@ describe Projects::Settings::CiCdController do
         allow(ResetProjectCacheService).to receive_message_chain(:new, :execute).and_return(false)
       end
 
-      it 'sets the flash error variable' do
+      it 'returns an error header' do
         subject
 
-        expect(controller).not_to set_flash[:notice]
-        expect(controller).to set_flash[:error]
+        expect(response).to have_gitlab_http_status(:bad_request)
       end
     end
   end

spec/features/projects/pipelines/pipelines_spec.rb

@@ -557,7 +557,7 @@ describe 'Pipelines', :js do
       end
 
       it 'has a clear caches button' do
-        expect(page).to have_link 'Clear Runner Caches'
+        expect(page).to have_button 'Clear Runner Caches'
       end
 
       describe 'user clicks the button' do
@@ -567,14 +567,16 @@ describe 'Pipelines', :js do
           end
 
           it 'increments jobs_cache_index' do
-            click_link 'Clear Runner Caches'
+            click_button 'Clear Runner Caches'
+            wait_for_requests
             expect(page.find('.flash-notice')).to have_content 'Project cache successfully reset.'
           end
         end
 
         context 'when project does not have jobs_cache_index' do
           it 'sets jobs_cache_index to 1' do
-            click_link 'Clear Runner Caches'
+            click_button 'Clear Runner Caches'
+            wait_for_requests
             expect(page.find('.flash-notice')).to have_content 'Project cache successfully reset.'
           end
         end

spec/features/projects/services/disable_triggers_spec.rb

+require 'spec_helper'
+
+describe 'Disable individual triggers' do
+  let(:project) { create(:project) }
+  let(:user) { project.owner }
+  let(:checkbox_selector) { 'input[type=checkbox][id$=_events]' }
+
+  before do
+    sign_in(user)
+
+    visit(project_settings_integrations_path(project))
+
+    click_link(service_name)
+  end
+
+  context 'service has multiple supported events' do
+    let(:service_name) { 'HipChat' }
+
+    it 'shows trigger checkboxes' do
+      event_count = HipchatService.supported_events.count
+
+      expect(page).to have_content "Trigger"
+      expect(page).to have_css(checkbox_selector, count: event_count)
+    end
+  end
+
+  context 'services only has one supported event' do
+    let(:service_name) { 'Asana' }
+
+    it "doesn't show unnecessary Trigger checkboxes" do
+      expect(page).not_to have_content "Trigger"
+      expect(page).not_to have_css(checkbox_selector)
+    end
+  end
+end

spec/javascripts/pipelines/nav_controls_spec.js

@@ -39,19 +39,6 @@ describe('Pipelines Nav Controls', () => {
     expect(component.$el.querySelector('.js-run-pipeline')).toEqual(null);
   });
 
-  it('should render link for resetting runner caches', () => {
-    const mockData = {
-      newPipelinePath: 'foo',
-      ciLintPath: 'foo',
-      resetCachePath: 'foo',
-    };
-
-    component = mountComponent(NavControlsComponent, mockData);
-
-    expect(component.$el.querySelector('.js-clear-cache').textContent.trim()).toContain('Clear Runner Caches');
-    expect(component.$el.querySelector('.js-clear-cache').getAttribute('href')).toEqual(mockData.resetCachePath);
-  });
-
   it('should render link for CI lint', () => {
     const mockData = {
       newPipelinePath: 'foo',
@@ -65,4 +52,28 @@ describe('Pipelines Nav Controls', () => {
     expect(component.$el.querySelector('.js-ci-lint').textContent.trim()).toContain('CI Lint');
     expect(component.$el.querySelector('.js-ci-lint').getAttribute('href')).toEqual(mockData.ciLintPath);
   });
+
+  describe('Reset Runners Cache', () => {
+    beforeEach(() => {
+      const mockData = {
+        newPipelinePath: 'foo',
+        ciLintPath: 'foo',
+        resetCachePath: 'foo',
+      };
+
+      component = mountComponent(NavControlsComponent, mockData);
+    });
+
+    it('should render button for resetting runner caches', () => {
+      expect(component.$el.querySelector('.js-clear-cache').textContent.trim()).toContain('Clear Runner Caches');
+    });
+
+    it('should emit postAction event when reset runner cache button is clicked', () => {
+      spyOn(component, '$emit');
+
+      component.$el.querySelector('.js-clear-cache').click();
+
+      expect(component.$emit).toHaveBeenCalledWith('resetRunnersCache', 'foo');
+    });
+  });
 });

spec/javascripts/pipelines/pipelines_spec.js

@@ -95,16 +95,16 @@ describe('Pipelines', () => {
         expect(vm.$el.querySelector('.js-pipelines-tab-all').textContent.trim()).toContain('All');
       });
 
-      it('renders Run Pipeline button', () => {
+      it('renders Run Pipeline link', () => {
         expect(vm.$el.querySelector('.js-run-pipeline').getAttribute('href')).toEqual(paths.newPipelinePath);
       });
 
-      it('renders CI Lint button', () => {
+      it('renders CI Lint link', () => {
         expect(vm.$el.querySelector('.js-ci-lint').getAttribute('href')).toEqual(paths.ciLintPath);
       });
 
       it('renders Clear Runner Cache button', () => {
-        expect(vm.$el.querySelector('.js-clear-cache').getAttribute('href')).toEqual(paths.resetCachePath);
+        expect(vm.$el.querySelector('.js-clear-cache').textContent.trim()).toEqual('Clear Runner Caches');
       });
 
       it('renders pipelines table', () => {
@@ -139,16 +139,16 @@ describe('Pipelines', () => {
         expect(vm.$el.querySelector('.js-pipelines-tab-all').textContent.trim()).toContain('All');
       });
 
-      it('renders Run Pipeline button', () => {
+      it('renders Run Pipeline link', () => {
         expect(vm.$el.querySelector('.js-run-pipeline').getAttribute('href')).toEqual(paths.newPipelinePath);
       });
 
-      it('renders CI Lint button', () => {
+      it('renders CI Lint link', () => {
         expect(vm.$el.querySelector('.js-ci-lint').getAttribute('href')).toEqual(paths.ciLintPath);
       });
 
       it('renders Clear Runner Cache button', () => {
-        expect(vm.$el.querySelector('.js-clear-cache').getAttribute('href')).toEqual(paths.resetCachePath);
+        expect(vm.$el.querySelector('.js-clear-cache').textContent.trim()).toEqual('Clear Runner Caches');
       });
 
       it('renders tab empty state', () => {
@@ -218,7 +218,7 @@ describe('Pipelines', () => {
       it('renders buttons', () => {
         expect(vm.$el.querySelector('.js-run-pipeline').getAttribute('href')).toEqual(paths.newPipelinePath);
         expect(vm.$el.querySelector('.js-ci-lint').getAttribute('href')).toEqual(paths.ciLintPath);
-        expect(vm.$el.querySelector('.js-clear-cache').getAttribute('href')).toEqual(paths.resetCachePath);
+        expect(vm.$el.querySelector('.js-clear-cache').textContent.trim()).toEqual('Clear Runner Caches');
       });
 
       it('renders error state', () => {

spec/lib/mattermost/team_spec.rb

@@ -64,4 +64,108 @@ describe Mattermost::Team do
       end
     end
   end
+
+  describe '#create' do
+    subject { described_class.new(nil).create(name: "devteam", display_name: "Dev Team", type: "O") }
+
+    context 'for a new team' do
+      let(:response) do
+        {
+          "id" => "cuojfcetjty7tb4pxe47pwpndo",
+          "create_at" => 1517688728701,
+          "update_at" => 1517688728701,
+          "delete_at" => 0,
+          "display_name" => "Dev Team",
+          "name" => "devteam",
+          "description" => "",
+          "email" => "admin@example.com",
+          "type" => "O",
+          "company_name" => "",
+          "allowed_domains" => "",
+          "invite_id" => "7mp9d3ayaj833ymmkfnid8js6w",
+          "allow_open_invite" => false
+        }
+      end
+
+      before do
+        stub_request(:post, "http://mattermost.example.com/api/v3/teams/create")
+          .to_return(
+            status: 200,
+            body: response.to_json,
+            headers: { 'Content-Type' => 'application/json' }
+          )
+      end
+
+      it 'returns the new team' do
+        is_expected.to eq(response)
+      end
+    end
+
+    context 'for existing team' do
+      before do
+        stub_request(:post, 'http://mattermost.example.com/api/v3/teams/create')
+          .to_return(
+            status: 400,
+            headers: { 'Content-Type' => 'application/json' },
+            body: {
+                id: "store.sql_team.save.domain_exists.app_error",
+                message: "A team with that name already exists",
+                detailed_error: "",
+                request_id: "1hsb5bxs97r8bdggayy7n9gxaw",
+                status_code: 400
+            }.to_json
+          )
+      end
+
+      it 'raises an error with message' do
+        expect { subject }.to raise_error(Mattermost::Error, 'A team with that name already exists')
+      end
+    end
+  end
+
+  describe '#delete' do
+    subject { described_class.new(nil).destroy(team_id: "cuojfcetjty7tb4pxe47pwpndo") }
+
+    context 'for an existing team' do
+      let(:response) do
+        {
+            "status" => "OK"
+        }
+      end
+
+      before do
+        stub_request(:delete, "http://mattermost.example.com/api/v4/teams/cuojfcetjty7tb4pxe47pwpndo")
+          .to_return(
+            status: 200,
+            body: response.to_json,
+            headers: { 'Content-Type' => 'application/json' }
+          )
+      end
+
+      it 'returns team status' do
+        is_expected.to eq(response)
+      end
+    end
+
+    context 'for an unknown team' do
+      before do
+        stub_request(:delete, "http://mattermost.example.com/api/v4/teams/cuojfcetjty7tb4pxe47pwpndo")
+          .to_return(
+            status: 404,
+            body: {
+              id: "store.sql_team.get.find.app_error",
+              message: "We couldn't find the existing team",
+              detailed_error: "",
+              request_id: "my114ab5nbnui8c9pes4kz8mza",
+              status_code: 404
+            }.to_json,
+            headers: { 'Content-Type' => 'application/json' }
+          )
+      end
+
+      it 'raises an error with message' do
+        expect { subject }.to raise_error(Mattermost::Error, "We couldn't find the existing team")
+      end
+    end
+  end
 end

spec/models/repository_spec.rb

@@ -1479,7 +1479,6 @@ describe Repository do
     it 'expires the caches for an empty repository' do
       allow(repository).to receive(:empty?).and_return(true)
 
-      expect(cache).to receive(:expire).with(:empty?)
       expect(cache).to receive(:expire).with(:has_visible_content?)
 
       repository.expire_emptiness_caches
@@ -1488,7 +1487,6 @@ describe Repository do
     it 'does not expire the cache for a non-empty repository' do
       allow(repository).to receive(:empty?).and_return(false)
 
-      expect(cache).not_to receive(:expire).with(:empty?)
       expect(cache).not_to receive(:expire).with(:has_visible_content?)
 
       repository.expire_emptiness_caches