From 33c9cf20d1accf5ab89bcbe58a4ce6d1a59ed97a Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 30 May 2017 13:27:57 +0000 Subject: Merge branch '33048-markdown-rendering-of-md-files-has-ceased-to-display-latex-equations' into 'master' Fix math rendering on blob pages Closes #33048 See merge request !11793 Conflicts: app/assets/javascripts/blob/viewer/index.js --- app/assets/javascripts/blob/viewer/index.js | 39 ++++++++++++++++++++++ ...files-has-ceased-to-display-latex-equations.yml | 4 +++ 2 files changed, 43 insertions(+) create mode 100644 changelogs/unreleased/33048-markdown-rendering-of-md-files-has-ceased-to-display-latex-equations.yml diff --git a/app/assets/javascripts/blob/viewer/index.js b/app/assets/javascripts/blob/viewer/index.js index d06387c0f4d..abf2f6aab6a 100644 --- a/app/assets/javascripts/blob/viewer/index.js +++ b/app/assets/javascripts/blob/viewer/index.js @@ -116,6 +116,45 @@ export default class BlobViewer { this.toggleCopyButtonState(); +<<<<<<< HEAD this.loadViewer(newViewer); +======= + BlobViewer.loadViewer(newViewer) + .then((viewer) => { + $(viewer).renderGFM(); + + this.$fileHolder.trigger('highlight:line'); + gl.utils.handleLocationHash(); + + this.toggleCopyButtonState(); + }) + .catch(() => new Flash('Error loading viewer')); + } + + static loadViewer(viewerParam) { + const viewer = viewerParam; + const url = viewer.getAttribute('data-url'); + + return new Promise((resolve, reject) => { + if (!url || viewer.getAttribute('data-loaded') || viewer.getAttribute('data-loading')) { + resolve(viewer); + return; + } + + viewer.setAttribute('data-loading', 'true'); + + $.ajax({ + url, + dataType: 'JSON', + }) + .fail(reject) + .done((data) => { + viewer.innerHTML = data.html; + viewer.setAttribute('data-loaded', 'true'); + + resolve(viewer); + }); + }); +>>>>>>> 83550c0... Merge branch '33048-markdown-rendering-of-md-files-has-ceased-to-display-latex-equations' into 'master' } } diff --git a/changelogs/unreleased/33048-markdown-rendering-of-md-files-has-ceased-to-display-latex-equations.yml b/changelogs/unreleased/33048-markdown-rendering-of-md-files-has-ceased-to-display-latex-equations.yml new file mode 100644 index 00000000000..5648e013e75 --- /dev/null +++ b/changelogs/unreleased/33048-markdown-rendering-of-md-files-has-ceased-to-display-latex-equations.yml @@ -0,0 +1,4 @@ +--- +title: Fix math rendering on blob pages +merge_request: +author: -- cgit v1.2.1 From 03742e2908996567d8bc4e5f40cdb807c9f59726 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Thu, 8 Jun 2017 11:06:08 +0100 Subject: Fix conflict in app/assets/javascripts/blob/viewer/index.js --- app/assets/javascripts/blob/viewer/index.js | 41 +---------------------------- 1 file changed, 1 insertion(+), 40 deletions(-) diff --git a/app/assets/javascripts/blob/viewer/index.js b/app/assets/javascripts/blob/viewer/index.js index abf2f6aab6a..3eddf595f30 100644 --- a/app/assets/javascripts/blob/viewer/index.js +++ b/app/assets/javascripts/blob/viewer/index.js @@ -78,7 +78,7 @@ export default class BlobViewer { .fail(() => new Flash('Error loading source view')) .done((data) => { viewer.innerHTML = data.html; - $(viewer).syntaxHighlight(); + $(viewer).renderGFM(); viewer.setAttribute('data-loaded', 'true'); @@ -116,45 +116,6 @@ export default class BlobViewer { this.toggleCopyButtonState(); -<<<<<<< HEAD this.loadViewer(newViewer); -======= - BlobViewer.loadViewer(newViewer) - .then((viewer) => { - $(viewer).renderGFM(); - - this.$fileHolder.trigger('highlight:line'); - gl.utils.handleLocationHash(); - - this.toggleCopyButtonState(); - }) - .catch(() => new Flash('Error loading viewer')); - } - - static loadViewer(viewerParam) { - const viewer = viewerParam; - const url = viewer.getAttribute('data-url'); - - return new Promise((resolve, reject) => { - if (!url || viewer.getAttribute('data-loaded') || viewer.getAttribute('data-loading')) { - resolve(viewer); - return; - } - - viewer.setAttribute('data-loading', 'true'); - - $.ajax({ - url, - dataType: 'JSON', - }) - .fail(reject) - .done((data) => { - viewer.innerHTML = data.html; - viewer.setAttribute('data-loaded', 'true'); - - resolve(viewer); - }); - }); ->>>>>>> 83550c0... Merge branch '33048-markdown-rendering-of-md-files-has-ceased-to-display-latex-equations' into 'master' } } -- cgit v1.2.1 From 8531cd2c6490d9d5398ceafaf25cc97a5a43d0ad Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 26 May 2017 16:13:00 +0000 Subject: Merge branch 'fix_counter_cache_invalidation' into 'master' Fix counter cache invalidation Closes #32854 and #32870 See merge request !11736 --- app/models/user.rb | 4 ++-- app/services/issues/close_service.rb | 1 + app/services/issues/reopen_service.rb | 1 + app/services/merge_requests/close_service.rb | 1 + app/services/merge_requests/post_merge_service.rb | 1 + app/services/merge_requests/reopen_service.rb | 1 + spec/services/issues/close_service_spec.rb | 6 ++++++ spec/services/issues/reopen_service_spec.rb | 7 +++++++ spec/services/merge_requests/close_service_spec.rb | 2 ++ spec/services/merge_requests/post_merge_service_spec.rb | 15 +++++++++++++++ spec/services/merge_requests/reopen_service_spec.rb | 2 ++ spec/support/issuable_shared_examples.rb | 7 +++++++ 12 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 spec/services/merge_requests/post_merge_service_spec.rb create mode 100644 spec/support/issuable_shared_examples.rb diff --git a/app/models/user.rb b/app/models/user.rb index 157e0948cd3..1d83f5ed9e1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -914,13 +914,13 @@ class User < ActiveRecord::Base end def assigned_open_merge_requests_count(force: false) - Rails.cache.fetch(['users', id, 'assigned_open_merge_requests_count'], force: force) do + Rails.cache.fetch(['users', id, 'assigned_open_merge_requests_count'], force: force, expires_in: 20.minutes) do MergeRequestsFinder.new(self, assignee_id: self.id, state: 'opened').execute.count end end def assigned_open_issues_count(force: false) - Rails.cache.fetch(['users', id, 'assigned_open_issues_count'], force: force) do + Rails.cache.fetch(['users', id, 'assigned_open_issues_count'], force: force, expires_in: 20.minutes) do IssuesFinder.new(self, assignee_id: self.id, state: 'opened').execute.count end end diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb index f1030912c68..85c616ca576 100644 --- a/app/services/issues/close_service.rb +++ b/app/services/issues/close_service.rb @@ -28,6 +28,7 @@ module Issues notification_service.close_issue(issue, current_user) if notifications todo_service.close_issue(issue, current_user) execute_hooks(issue, 'close') + invalidate_cache_counts(issue.assignees, issue) end issue diff --git a/app/services/issues/reopen_service.rb b/app/services/issues/reopen_service.rb index 40fbe354492..80ea6312768 100644 --- a/app/services/issues/reopen_service.rb +++ b/app/services/issues/reopen_service.rb @@ -8,6 +8,7 @@ module Issues create_note(issue) notification_service.reopen_issue(issue, current_user) execute_hooks(issue, 'reopen') + invalidate_cache_counts(issue.assignees, issue) end issue diff --git a/app/services/merge_requests/close_service.rb b/app/services/merge_requests/close_service.rb index f2053bda83a..2ffc989ed71 100644 --- a/app/services/merge_requests/close_service.rb +++ b/app/services/merge_requests/close_service.rb @@ -13,6 +13,7 @@ module MergeRequests notification_service.close_mr(merge_request, current_user) todo_service.close_merge_request(merge_request, current_user) execute_hooks(merge_request, 'close') + invalidate_cache_counts(merge_request.assignees, merge_request) end merge_request diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb index e8fb1b59752..f0d998731d7 100644 --- a/app/services/merge_requests/post_merge_service.rb +++ b/app/services/merge_requests/post_merge_service.rb @@ -13,6 +13,7 @@ module MergeRequests create_note(merge_request) notification_service.merge_mr(merge_request, current_user) execute_hooks(merge_request, 'merge') + invalidate_cache_counts(merge_request.assignees, merge_request) end private diff --git a/app/services/merge_requests/reopen_service.rb b/app/services/merge_requests/reopen_service.rb index fadcce5d9b6..b016e5235cd 100644 --- a/app/services/merge_requests/reopen_service.rb +++ b/app/services/merge_requests/reopen_service.rb @@ -10,6 +10,7 @@ module MergeRequests execute_hooks(merge_request, 'reopen') merge_request.reload_diff merge_request.mark_as_unchecked + invalidate_cache_counts(merge_request.assignees, merge_request) end merge_request diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb index 51840531711..52f2066d9c5 100644 --- a/spec/services/issues/close_service_spec.rb +++ b/spec/services/issues/close_service_spec.rb @@ -41,6 +41,12 @@ describe Issues::CloseService, services: true do service.execute(issue) end + + it 'invalidates counter cache for assignees' do + expect_any_instance_of(User).to receive(:invalidate_issue_cache_counts) + + service.execute(issue) + end end describe '#close_issue' do diff --git a/spec/services/issues/reopen_service_spec.rb b/spec/services/issues/reopen_service_spec.rb index 93a8270fd16..391ecad303a 100644 --- a/spec/services/issues/reopen_service_spec.rb +++ b/spec/services/issues/reopen_service_spec.rb @@ -27,6 +27,13 @@ describe Issues::ReopenService, services: true do project.team << [user, :master] end + it 'invalidates counter cache for assignees' do + issue.assignees << user + expect_any_instance_of(User).to receive(:invalidate_issue_cache_counts) + + described_class.new(project, user).execute(issue) + end + context 'when issue is not confidential' do it 'executes issue hooks' do expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :issue_hooks) diff --git a/spec/services/merge_requests/close_service_spec.rb b/spec/services/merge_requests/close_service_spec.rb index d55a7657c0e..154f30aac3b 100644 --- a/spec/services/merge_requests/close_service_spec.rb +++ b/spec/services/merge_requests/close_service_spec.rb @@ -15,6 +15,8 @@ describe MergeRequests::CloseService, services: true do end describe '#execute' do + it_behaves_like 'cache counters invalidator' + context 'valid params' do let(:service) { described_class.new(project, user, {}) } diff --git a/spec/services/merge_requests/post_merge_service_spec.rb b/spec/services/merge_requests/post_merge_service_spec.rb new file mode 100644 index 00000000000..a20b32eda5f --- /dev/null +++ b/spec/services/merge_requests/post_merge_service_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +describe MergeRequests::PostMergeService, services: true do + let(:user) { create(:user) } + let(:merge_request) { create(:merge_request, assignee: user) } + let(:project) { merge_request.project } + + before do + project.team << [user, :master] + end + + describe '#execute' do + it_behaves_like 'cache counters invalidator' + end +end diff --git a/spec/services/merge_requests/reopen_service_spec.rb b/spec/services/merge_requests/reopen_service_spec.rb index a99d4eac9bd..b6d4db2f922 100644 --- a/spec/services/merge_requests/reopen_service_spec.rb +++ b/spec/services/merge_requests/reopen_service_spec.rb @@ -14,6 +14,8 @@ describe MergeRequests::ReopenService, services: true do end describe '#execute' do + it_behaves_like 'cache counters invalidator' + context 'valid params' do let(:service) { described_class.new(project, user, {}) } diff --git a/spec/support/issuable_shared_examples.rb b/spec/support/issuable_shared_examples.rb new file mode 100644 index 00000000000..03011535351 --- /dev/null +++ b/spec/support/issuable_shared_examples.rb @@ -0,0 +1,7 @@ +shared_examples 'cache counters invalidator' do + it 'invalidates counter cache for assignees' do + expect_any_instance_of(User).to receive(:invalidate_merge_request_cache_counts) + + described_class.new(project, user, {}).execute(merge_request) + end +end -- cgit v1.2.1 From 3c42294bb79fe349d655afee1dc2c0b39e22d556 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 30 May 2017 07:36:10 +0000 Subject: Merge branch '32954-loading-icon' into 'master' Displays loading icon in async buttons inline Closes #32954 See merge request !11735 --- app/assets/stylesheets/pages/environments.scss | 4 ++++ app/assets/stylesheets/pages/pipelines.scss | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/app/assets/stylesheets/pages/environments.scss b/app/assets/stylesheets/pages/environments.scss index 026d35295d7..40a1a6ce82a 100644 --- a/app/assets/stylesheets/pages/environments.scss +++ b/app/assets/stylesheets/pages/environments.scss @@ -69,6 +69,10 @@ } } + .btn .text-center { + display: inline; + } + .commit-title { margin: 0; } diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 4304e736b58..03020a7e1fd 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -98,6 +98,10 @@ } } + .btn .text-center { + display: inline; + } + .tooltip { white-space: nowrap; } -- cgit v1.2.1 From 4d49b7dcf1c8575ccb6ef282fe97e4a610e574b8 Mon Sep 17 00:00:00 2001 From: Fatih Acet Date: Tue, 30 May 2017 11:40:21 +0000 Subject: Merge branch '32916-browser-notifications-for-pipeline-running-in-a-mr-is-gone' into 'master' Resolve "Browser notifications for pipeline running in a MR is gone" Closes #32916 See merge request !11734 --- app/assets/javascripts/lib/utils/notify.js | 85 +++++++++++----------- app/assets/javascripts/main.js | 1 - .../vue_merge_request_widget/dependencies.js | 1 + .../javascripts/vue_merge_request_widget/index.js | 2 + .../vue_merge_request_widget/mr_widget_options.js | 12 +++ .../stores/mr_widget_store.js | 2 + lib/gitlab/gon_helper.rb | 3 + .../vue_mr_widget/mr_widget_options_spec.js | 37 ++++++++++ 8 files changed, 100 insertions(+), 43 deletions(-) diff --git a/app/assets/javascripts/lib/utils/notify.js b/app/assets/javascripts/lib/utils/notify.js index 66f39122a66..973d6119158 100644 --- a/app/assets/javascripts/lib/utils/notify.js +++ b/app/assets/javascripts/lib/utils/notify.js @@ -1,47 +1,48 @@ /* eslint-disable func-names, space-before-function-paren, wrap-iife, no-var, one-var, one-var-declaration-per-line, consistent-return, prefer-arrow-callback, no-return-assign, object-shorthand, comma-dangle, no-param-reassign, max-len */ -(function() { - (function(w) { - var notificationGranted, notifyMe, notifyPermissions; - notificationGranted = function(message, opts, onclick) { - var notification; - notification = new Notification(message, opts); - setTimeout(function() { - return notification.close(); - // Hide the notification after X amount of seconds - }, 8000); - if (onclick) { - return notification.onclick = onclick; - } - }; - notifyPermissions = function() { - if ('Notification' in window) { - return Notification.requestPermission(); - } - }; - notifyMe = function(message, body, icon, onclick) { - var opts; - opts = { - body: body, - icon: icon - }; - // Let's check if the browser supports notifications - if (!('Notification' in window)) { +function notificationGranted(message, opts, onclick) { + var notification; + notification = new Notification(message, opts); + setTimeout(function() { + // Hide the notification after X amount of seconds + return notification.close(); + }, 8000); + + return notification.onclick = onclick || notification.close; +} - // do nothing - } else if (Notification.permission === 'granted') { - // If it's okay let's create a notification +function notifyPermissions() { + if ('Notification' in window) { + return Notification.requestPermission(); + } +} + +function notifyMe(message, body, icon, onclick) { + var opts; + opts = { + body: body, + icon: icon + }; + // Let's check if the browser supports notifications + if (!('Notification' in window)) { + // do nothing + } else if (Notification.permission === 'granted') { + // If it's okay let's create a notification + return notificationGranted(message, opts, onclick); + } else if (Notification.permission !== 'denied') { + return Notification.requestPermission(function(permission) { + // If the user accepts, let's create a notification + if (permission === 'granted') { return notificationGranted(message, opts, onclick); - } else if (Notification.permission !== 'denied') { - return Notification.requestPermission(function(permission) { - // If the user accepts, let's create a notification - if (permission === 'granted') { - return notificationGranted(message, opts, onclick); - } - }); } - }; - w.notify = notifyMe; - return w.notifyPermissions = notifyPermissions; - })(window); -}).call(window); + }); + } +} + +const notify = { + notificationGranted, + notifyPermissions, + notifyMe, +}; + +export default notify; diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index a07aa047293..f3f2f2262fd 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -56,7 +56,6 @@ import './lib/utils/animate'; import './lib/utils/bootstrap_linked_tabs'; import './lib/utils/common_utils'; import './lib/utils/datetime_utility'; -import './lib/utils/notify'; import './lib/utils/pretty_time'; import './lib/utils/text_utility'; import './lib/utils/type_utility'; diff --git a/app/assets/javascripts/vue_merge_request_widget/dependencies.js b/app/assets/javascripts/vue_merge_request_widget/dependencies.js index bfe30ee4c08..fe5e1bbb55c 100644 --- a/app/assets/javascripts/vue_merge_request_widget/dependencies.js +++ b/app/assets/javascripts/vue_merge_request_widget/dependencies.js @@ -41,3 +41,4 @@ export { default as getStateKey } from './stores/get_state_key'; export { default as mrWidgetOptions } from './mr_widget_options'; export { default as stateMaps } from './stores/state_maps'; export { default as SquashBeforeMerge } from './components/states/mr_widget_squash_before_merge'; +export { default as notify } from '../lib/utils/notify'; diff --git a/app/assets/javascripts/vue_merge_request_widget/index.js b/app/assets/javascripts/vue_merge_request_widget/index.js index cd65ac069c5..43ef468c303 100644 --- a/app/assets/javascripts/vue_merge_request_widget/index.js +++ b/app/assets/javascripts/vue_merge_request_widget/index.js @@ -4,6 +4,8 @@ import { } from './dependencies'; document.addEventListener('DOMContentLoaded', () => { + gl.mrWidgetData.gitlabLogo = gon.gitlab_logo; + const vm = new Vue(mrWidgetOptions); window.gl.mrWidget = { diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js index 99600b6664e..2339a00ddd0 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js +++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js @@ -29,6 +29,7 @@ import { eventHub, stateMaps, SquashBeforeMerge, + notify, } from './dependencies'; export default { @@ -77,8 +78,10 @@ export default { this.service.checkStatus() .then(res => res.json()) .then((res) => { + this.handleNotification(res); this.mr.setData(res); this.setFavicon(); + if (cb) { cb.call(null, res); } @@ -136,6 +139,15 @@ export default { new Flash('Something went wrong. Please try again.'); // eslint-disable-line }); }, + handleNotification(data) { + if (data.ci_status === this.mr.ciStatus) return; + + const label = data.pipeline.details.status.label; + const title = `Pipeline ${label}`; + const message = `Pipeline ${label} for "${data.title}"`; + + notify.notifyMe(title, message, this.mr.gitlabLogo); + }, resumePolling() { this.pollingInterval.resume(); }, diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js index 1533c857863..27399308260 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js @@ -5,6 +5,8 @@ export default class MergeRequestStore { constructor(data) { this.sha = data.diff_head_sha; + this.gitlabLogo = data.gitlabLogo; + this.setData(data); } diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb index 1e09cb5ca11..cad873b92a3 100644 --- a/lib/gitlab/gon_helper.rb +++ b/lib/gitlab/gon_helper.rb @@ -1,3 +1,5 @@ +# rubocop:disable Metrics/AbcSize + module Gitlab module GonHelper def add_gon_variables @@ -13,6 +15,7 @@ module Gitlab gon.sentry_dsn = current_application_settings.clientside_sentry_dsn if current_application_settings.clientside_sentry_enabled gon.gitlab_url = Gitlab.config.gitlab.url gon.revision = Gitlab::REVISION + gon.gitlab_logo = ActionController::Base.helpers.asset_path('gitlab_logo.png') if current_user gon.current_user_id = current_user.id diff --git a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js index bdc18243a15..3a0c50b750f 100644 --- a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js +++ b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js @@ -2,6 +2,7 @@ import Vue from 'vue'; import MRWidgetService from '~/vue_merge_request_widget/services/mr_widget_service'; import mrWidgetOptions from '~/vue_merge_request_widget/mr_widget_options'; import eventHub from '~/vue_merge_request_widget/event_hub'; +import notify from '~/lib/utils/notify'; import mockData from './mock_data'; const createComponent = () => { @@ -107,6 +108,8 @@ describe('mrWidgetOptions', () => { it('should tell service to check status', (done) => { spyOn(vm.service, 'checkStatus').and.returnValue(returnPromise(mockData)); spyOn(vm.mr, 'setData'); + spyOn(vm, 'handleNotification'); + let isCbExecuted = false; const cb = () => { isCbExecuted = true; @@ -117,6 +120,7 @@ describe('mrWidgetOptions', () => { setTimeout(() => { expect(vm.service.checkStatus).toHaveBeenCalled(); expect(vm.mr.setData).toHaveBeenCalled(); + expect(vm.handleNotification).toHaveBeenCalledWith(mockData); expect(isCbExecuted).toBeTruthy(); done(); }, 333); @@ -254,6 +258,39 @@ describe('mrWidgetOptions', () => { }); }); + describe('handleNotification', () => { + const data = { + ci_status: 'running', + title: 'title', + pipeline: { details: { status: { label: 'running-label' } } }, + }; + + beforeEach(() => { + spyOn(notify, 'notifyMe'); + + vm.mr.ciStatus = 'failed'; + vm.mr.gitlabLogo = 'logo.png'; + }); + + it('should call notifyMe', () => { + vm.handleNotification(data); + + expect(notify.notifyMe).toHaveBeenCalledWith( + 'Pipeline running-label', + 'Pipeline running-label for "title"', + 'logo.png', + ); + }); + + it('should not call notifyMe if the status has not changed', () => { + vm.mr.ciStatus = data.ci_status; + + vm.handleNotification(data); + + expect(notify.notifyMe).not.toHaveBeenCalled(); + }); + }); + describe('resumePolling', () => { it('should call stopTimer on pollingInterval', () => { spyOn(vm.pollingInterval, 'resume'); -- cgit v1.2.1 From 9da703283ae918d83e1a48c030ae40aaa3297058 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Gray Date: Fri, 26 May 2017 14:13:18 +0000 Subject: Merge branch '32683-resolved-discussions-icon-is-misaligned' into 'master' Resolve "Resolved discussions icon is misaligned" Closes #32683 See merge request !11711 --- app/assets/stylesheets/pages/notes.scss | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 99bcf612e8f..71cf448121d 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -665,7 +665,7 @@ ul.notes { .line-resolve-all { vertical-align: middle; display: inline-block; - padding: 6px 10px; + padding: 5px 10px 6px; background-color: $gray-light; border: 1px solid $border-color; border-radius: $border-radius-default; @@ -678,6 +678,10 @@ ul.notes { .line-resolve-btn { margin-right: 5px; + + svg { + vertical-align: middle; + } } } @@ -714,6 +718,10 @@ ul.notes { } } +.line-resolve-text { + vertical-align: middle; +} + .discussion-next-btn { svg { margin: 0; -- cgit v1.2.1 From 000f42176845409e5b5725bd701438e50d29ba63 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Gray Date: Thu, 25 May 2017 16:24:04 +0000 Subject: Merge branch '32801-fix-discussion-header-wrapping-in-parallel-diff' into 'master' Fix note header author and time ago wrapping in parallel diff Closes #32801 See merge request !11702 Conflicts: app/assets/stylesheets/pages/notes.scss --- app/assets/stylesheets/pages/notes.scss | 16 ++++++++++++++-- app/views/shared/notes/_note.html.haml | 2 +- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 71cf448121d..49db0ef9961 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -394,6 +394,19 @@ ul.notes { padding-bottom: 8px; } +<<<<<<< HEAD +======= +.system-note .note-header-info { + padding-bottom: 0; +} + +.note-header-author-name { + @media (max-width: $screen-xs-max) { + display: none; + } +} + +>>>>>>> 4144144... Merge branch '32801-fix-discussion-header-wrapping-in-parallel-diff' into 'master' .note-headline-light { display: inline; @@ -739,9 +752,8 @@ ul.notes { // Merge request notes in diffs .diff-file { // Diff is side by side - .notes_content.parallel .note-header .note-headline-light { + .notes_content.parallel .note-header .note-header-author-name { display: block; - position: relative; } // Diff is inline .notes_content .note-header .note-headline-light { diff --git a/app/views/shared/notes/_note.html.haml b/app/views/shared/notes/_note.html.haml index a7bf610b9c7..1e34b7c1e76 100644 --- a/app/views/shared/notes/_note.html.haml +++ b/app/views/shared/notes/_note.html.haml @@ -18,7 +18,7 @@ .note-header .note-header-info %a{ href: user_path(note.author) } - %span.hidden-xs + %span.note-header-author-name = sanitize(note.author.name) %span.note-headline-light = note.author.to_reference -- cgit v1.2.1 From 6f1ce7660e9888b195d2a09783f8261964a6549b Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Gray Date: Fri, 26 May 2017 14:18:52 +0000 Subject: Merge branch 'fix-emoji-menu-z-index' into 'master' Set emoji-menu z-index to 200 Closes #32632 See merge request !11686 --- app/assets/stylesheets/framework/awards.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/framework/awards.scss b/app/assets/stylesheets/framework/awards.scss index 0db3ac1a60e..df24a02f068 100644 --- a/app/assets/stylesheets/framework/awards.scss +++ b/app/assets/stylesheets/framework/awards.scss @@ -10,7 +10,7 @@ top: 0; margin-top: 3px; padding: $gl-padding; - z-index: 9; + z-index: 300; width: 300px; font-size: 14px; background-color: $white-light; -- cgit v1.2.1 From d9b4e4c8651367d13700eadeb5cedd6dd36db9f5 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 25 May 2017 18:50:02 +0000 Subject: Merge branch 'winh-label-textcolor-default' into 'master' Provide default for calculating label text color Closes #32728 See merge request !11681 --- app/helpers/labels_helper.rb | 5 ++--- app/models/label.rb | 4 ++++ spec/models/label_spec.rb | 17 +++++++++++++++++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb index e5b1e6e8bc7..4e6e6805920 100644 --- a/app/helpers/labels_helper.rb +++ b/app/helpers/labels_helper.rb @@ -69,13 +69,12 @@ module LabelsHelper end def render_colored_label(label, label_suffix = '', tooltip: true) - label_color = label.color || Label::DEFAULT_COLOR - text_color = text_color_for_bg(label_color) + text_color = text_color_for_bg(label.color) # Intentionally not using content_tag here so that this method can be called # by LabelReferenceFilter span = %() + %(#{escape_once(label.name)}#{label_suffix}) diff --git a/app/models/label.rb b/app/models/label.rb index ddddb6bdf8f..074239702f8 100644 --- a/app/models/label.rb +++ b/app/models/label.rb @@ -133,6 +133,10 @@ class Label < ActiveRecord::Base template end + def color + super || DEFAULT_COLOR + end + def text_color LabelsHelper.text_color_for_bg(self.color) end diff --git a/spec/models/label_spec.rb b/spec/models/label_spec.rb index 80ca19acdda..84867e3d96b 100644 --- a/spec/models/label_spec.rb +++ b/spec/models/label_spec.rb @@ -49,6 +49,23 @@ describe Label, models: true do expect(label.color).to eq('#abcdef') end + + it 'uses default color if color is missing' do + label = described_class.new(color: nil) + + expect(label.color).to be(Label::DEFAULT_COLOR) + end + end + + describe '#text_color' do + it 'uses default color if color is missing' do + expect(LabelsHelper).to receive(:text_color_for_bg).with(Label::DEFAULT_COLOR). + and_return(spy) + + label = described_class.new(color: nil) + + label.text_color + end end describe '#title' do -- cgit v1.2.1 From 516fd01fd75ff45ea1f191eff007687db3ecf0b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= Date: Tue, 23 May 2017 20:34:42 +0000 Subject: Merge branch '32509-next-run-in-pipeline-schedules-shows-past-time' into 'master' Use #real_next_run in pipelines table Closes #32509 See merge request !11660 --- app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml | 2 +- spec/features/projects/pipeline_schedules_spec.rb | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml index 075ecee4343..2f686988cb5 100644 --- a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml +++ b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml @@ -15,7 +15,7 @@ None %td.next-run-cell - if pipeline_schedule.active? - = time_ago_with_tooltip(pipeline_schedule.next_run_at) + = time_ago_with_tooltip(pipeline_schedule.real_next_run) - else Inactive %td diff --git a/spec/features/projects/pipeline_schedules_spec.rb b/spec/features/projects/pipeline_schedules_spec.rb index 1211b17b3d8..a521222fc9c 100644 --- a/spec/features/projects/pipeline_schedules_spec.rb +++ b/spec/features/projects/pipeline_schedules_spec.rb @@ -5,7 +5,7 @@ feature 'Pipeline Schedules', :feature do include WaitForAjax let!(:project) { create(:project) } - let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project) } + let!(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly, project: project ) } let!(:pipeline) { create(:ci_pipeline, pipeline_schedule: pipeline_schedule) } let(:scope) { nil } let!(:user) { create(:user) } @@ -32,6 +32,7 @@ feature 'Pipeline Schedules', :feature do it 'displays the required information description' do page.within('.pipeline-schedule-table-row') do expect(page).to have_content('pipeline schedule') + expect(page).to have_content(pipeline_schedule.real_next_run.strftime('%b %d, %Y')) expect(page).to have_link('master') expect(page).to have_link("##{pipeline.id}") end -- cgit v1.2.1 From 2226b5a1555b42461045410ef978a7216afab48a Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 25 May 2017 13:49:39 +0000 Subject: Merge branch 'fix-kubernetes-namespace' into 'master' Fix terminals support for Kubernetes service Closes #31754 See merge request !11653 --- app/models/project_services/kubernetes_service.rb | 24 ++++++++--------- ...ix-terminals-support-for-kubernetes-service.yml | 4 +++ spec/factories/services.rb | 1 - .../project_services/kubernetes_service_spec.rb | 31 +++++++++++++++++++++- spec/support/kubernetes_helpers.rb | 2 +- 5 files changed, 47 insertions(+), 15 deletions(-) create mode 100644 changelogs/unreleased/fix-terminals-support-for-kubernetes-service.yml diff --git a/app/models/project_services/kubernetes_service.rb b/app/models/project_services/kubernetes_service.rb index 9c56518c991..504f986817a 100644 --- a/app/models/project_services/kubernetes_service.rb +++ b/app/models/project_services/kubernetes_service.rb @@ -77,6 +77,14 @@ class KubernetesService < DeploymentService ] end + def actual_namespace + if namespace.present? + namespace + else + default_namespace + end + end + # Check we can connect to the Kubernetes API def test(*args) kubeclient = build_kubeclient! @@ -91,7 +99,7 @@ class KubernetesService < DeploymentService variables = [ { key: 'KUBE_URL', value: api_url, public: true }, { key: 'KUBE_TOKEN', value: token, public: false }, - { key: 'KUBE_NAMESPACE', value: namespace_variable, public: true } + { key: 'KUBE_NAMESPACE', value: actual_namespace, public: true } ] if ca_pem.present? @@ -110,7 +118,7 @@ class KubernetesService < DeploymentService with_reactive_cache do |data| pods = data.fetch(:pods, nil) filter_pods(pods, app: environment.slug). - flat_map { |pod| terminals_for_pod(api_url, namespace, pod) }. + flat_map { |pod| terminals_for_pod(api_url, actual_namespace, pod) }. each { |terminal| add_terminal_auth(terminal, terminal_auth) } end end @@ -124,7 +132,7 @@ class KubernetesService < DeploymentService # Store as hashes, rather than as third-party types pods = begin - kubeclient.get_pods(namespace: namespace).as_json + kubeclient.get_pods(namespace: actual_namespace).as_json rescue KubeException => err raise err unless err.error_code == 404 [] @@ -142,20 +150,12 @@ class KubernetesService < DeploymentService default_namespace || TEMPLATE_PLACEHOLDER end - def namespace_variable - if namespace.present? - namespace - else - default_namespace - end - end - def default_namespace "#{project.path}-#{project.id}" if project.present? end def build_kubeclient!(api_path: 'api', api_version: 'v1') - raise "Incomplete settings" unless api_url && namespace && token + raise "Incomplete settings" unless api_url && actual_namespace && token ::Kubeclient::Client.new( join_api_url(api_path), diff --git a/changelogs/unreleased/fix-terminals-support-for-kubernetes-service.yml b/changelogs/unreleased/fix-terminals-support-for-kubernetes-service.yml new file mode 100644 index 00000000000..fb91da9510c --- /dev/null +++ b/changelogs/unreleased/fix-terminals-support-for-kubernetes-service.yml @@ -0,0 +1,4 @@ +--- +title: Fix terminals support for Kubernetes Service +merge_request: +author: diff --git a/spec/factories/services.rb b/spec/factories/services.rb index 62aa71ae8d8..ffdc492e7ab 100644 --- a/spec/factories/services.rb +++ b/spec/factories/services.rb @@ -20,7 +20,6 @@ FactoryGirl.define do project factory: :empty_project active true properties({ - namespace: 'somepath', api_url: 'https://kubernetes.example.com', token: 'a' * 40, }) diff --git a/spec/models/project_services/kubernetes_service_spec.rb b/spec/models/project_services/kubernetes_service_spec.rb index e69eb0098dd..79d19265a2c 100644 --- a/spec/models/project_services/kubernetes_service_spec.rb +++ b/spec/models/project_services/kubernetes_service_spec.rb @@ -13,7 +13,7 @@ describe KubernetesService, models: true, caching: true do let(:discovery_url) { service.api_url + '/api/v1' } let(:discovery_response) { { body: kube_discovery_body.to_json } } - let(:pods_url) { service.api_url + "/api/v1/namespaces/#{service.namespace}/pods" } + let(:pods_url) { service.api_url + "/api/v1/namespaces/#{service.actual_namespace}/pods" } let(:pods_response) { { body: kube_pods_body(kube_pod).to_json } } def stub_kubeclient_discover @@ -105,6 +105,34 @@ describe KubernetesService, models: true, caching: true do end end + describe '#actual_namespace' do + subject { service.actual_namespace } + + it "returns the default namespace" do + is_expected.to eq(service.send(:default_namespace)) + end + + context 'when namespace is specified' do + before do + service.namespace = 'my-namespace' + end + + it "returns the user-namespace" do + is_expected.to eq('my-namespace') + end + end + + context 'when service is not assigned to project' do + before do + service.project = nil + end + + it "does not return namespace" do + is_expected.to be_nil + end + end + end + describe '#test' do before do stub_kubeclient_discover @@ -194,6 +222,7 @@ describe KubernetesService, models: true, caching: true do describe '#terminals' do let(:environment) { build(:environment, project: project, name: "env", slug: "env-000000") } + subject { service.terminals(environment) } context 'with invalid pods' do diff --git a/spec/support/kubernetes_helpers.rb b/spec/support/kubernetes_helpers.rb index b5ed71ba3be..912f1daa378 100644 --- a/spec/support/kubernetes_helpers.rb +++ b/spec/support/kubernetes_helpers.rb @@ -41,7 +41,7 @@ module KubernetesHelpers containers.map do |container| terminal = { selectors: { pod: pod_name, container: container['name'] }, - url: container_exec_url(service.api_url, service.namespace, pod_name, container['name']), + url: container_exec_url(service.api_url, service.actual_namespace, pod_name, container['name']), subprotocols: ['channel.k8s.io'], headers: { 'Authorization' => ["Bearer #{service.token}"] }, created_at: DateTime.parse(pod['metadata']['creationTimestamp']), -- cgit v1.2.1 From 47420a2d6222b36bb58599b181075e2a356665ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= Date: Wed, 31 May 2017 13:37:49 +0000 Subject: Merge branch 'fix/gb/use-merge-ability-for-protected-manual-actions' into 'master' Check only a merge ability for protected actions Closes #32618 See merge request !11648 --- app/policies/ci/build_policy.rb | 2 +- app/views/projects/deployments/_actions.haml | 1 + app/views/projects/environments/show.html.haml | 2 +- ...-merge-ability-for-protected-manual-actions.yml | 4 ++ doc/ci/yaml/README.md | 2 +- .../controllers/projects/builds_controller_spec.rb | 6 ++- .../projects/environments/environment_spec.rb | 57 ++++++++++++++-------- spec/lib/gitlab/chat_commands/command_spec.rb | 7 ++- spec/lib/gitlab/chat_commands/deploy_spec.rb | 7 ++- spec/lib/gitlab/ci/status/build/factory_spec.rb | 5 +- spec/lib/gitlab/ci/status/build/play_spec.rb | 10 +++- spec/models/environment_spec.rb | 5 +- spec/serializers/build_entity_spec.rb | 6 ++- spec/services/ci/play_build_service_spec.rb | 17 +++++-- spec/services/ci/process_pipeline_service_spec.rb | 7 +-- spec/services/ci/retry_pipeline_service_spec.rb | 7 ++- 16 files changed, 104 insertions(+), 41 deletions(-) create mode 100644 changelogs/unreleased/fix-gb-use-merge-ability-for-protected-manual-actions.yml diff --git a/app/policies/ci/build_policy.rb b/app/policies/ci/build_policy.rb index d4af4490608..2d7405dc240 100644 --- a/app/policies/ci/build_policy.rb +++ b/app/policies/ci/build_policy.rb @@ -23,7 +23,7 @@ module Ci !::Gitlab::UserAccess .new(user, project: build.project) - .can_push_to_branch?(build.ref) + .can_merge_to_branch?(build.ref) end end end diff --git a/app/views/projects/deployments/_actions.haml b/app/views/projects/deployments/_actions.haml index 506246f2ee6..e2baaa625ae 100644 --- a/app/views/projects/deployments/_actions.haml +++ b/app/views/projects/deployments/_actions.haml @@ -8,6 +8,7 @@ = icon('caret-down') %ul.dropdown-menu.dropdown-menu-align-right - actions.each do |action| + - next unless can?(current_user, :update_build, action) %li = link_to [:play, @project.namespace.becomes(Namespace), @project, action], method: :post, rel: 'nofollow' do = custom_icon('icon_play') diff --git a/app/views/projects/environments/show.html.haml b/app/views/projects/environments/show.html.haml index 7315e671056..9e221240cf2 100644 --- a/app/views/projects/environments/show.html.haml +++ b/app/views/projects/environments/show.html.haml @@ -13,7 +13,7 @@ = render 'projects/environments/metrics_button', environment: @environment - if can?(current_user, :update_environment, @environment) = link_to 'Edit', edit_namespace_project_environment_path(@project.namespace, @project, @environment), class: 'btn' - - if can?(current_user, :create_deployment, @environment) && @environment.can_stop? + - if can?(current_user, :stop_environment, @environment) = link_to 'Stop', stop_namespace_project_environment_path(@project.namespace, @project, @environment), data: { confirm: 'Are you sure you want to stop this environment?' }, class: 'btn btn-danger', method: :post .environments-container diff --git a/changelogs/unreleased/fix-gb-use-merge-ability-for-protected-manual-actions.yml b/changelogs/unreleased/fix-gb-use-merge-ability-for-protected-manual-actions.yml new file mode 100644 index 00000000000..43c18502cd6 --- /dev/null +++ b/changelogs/unreleased/fix-gb-use-merge-ability-for-protected-manual-actions.yml @@ -0,0 +1,4 @@ +--- +title: Respect merge, instead of push, permissions for protected actions +merge_request: 11648 +author: diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index e542b1119ea..0a0be717fa8 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -587,7 +587,7 @@ Optional manual actions have `allow_failure: true` set by default. **Manual actions are considered to be write actions, so permissions for protected branches are used when user wants to trigger an action. In other words, in order to trigger a manual action assigned to a branch that the -pipeline is running for, user needs to have ability to push to this branch.** +pipeline is running for, user needs to have ability to merge to this branch.** ### environment diff --git a/spec/controllers/projects/builds_controller_spec.rb b/spec/controllers/projects/builds_controller_spec.rb index 3ce23c17cdc..025b845de30 100644 --- a/spec/controllers/projects/builds_controller_spec.rb +++ b/spec/controllers/projects/builds_controller_spec.rb @@ -261,7 +261,11 @@ describe Projects::BuildsController do describe 'POST play' do before do - project.add_master(user) + project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: 'master', project: project) + sign_in(user) post_play diff --git a/spec/features/projects/environments/environment_spec.rb b/spec/features/projects/environments/environment_spec.rb index 86ce50c976f..18b608c863e 100644 --- a/spec/features/projects/environments/environment_spec.rb +++ b/spec/features/projects/environments/environment_spec.rb @@ -12,6 +12,7 @@ feature 'Environment', :feature do feature 'environment details page' do given!(:environment) { create(:environment, project: project) } + given!(:permissions) { } given!(:deployment) { } given!(:action) { } @@ -62,20 +63,31 @@ feature 'Environment', :feature do name: 'deploy to production') end - given(:role) { :master } + context 'when user has ability to trigger deployment' do + given(:permissions) do + create(:protected_branch, :developers_can_merge, + name: action.ref, project: project) + end - scenario 'does show a play button' do - expect(page).to have_link(action.name.humanize) - end + it 'does show a play button' do + expect(page).to have_link(action.name.humanize) + end + + it 'does allow to play manual action' do + expect(action).to be_manual - scenario 'does allow to play manual action' do - expect(action).to be_manual + expect { click_link(action.name.humanize) } + .not_to change { Ci::Pipeline.count } - expect { click_link(action.name.humanize) } - .not_to change { Ci::Pipeline.count } + expect(page).to have_content(action.name) + expect(action.reload).to be_pending + end + end - expect(page).to have_content(action.name) - expect(action.reload).to be_pending + context 'when user has no ability to trigger a deployment' do + it 'does not show a play button' do + expect(page).not_to have_link(action.name.humanize) + end end context 'with external_url' do @@ -134,12 +146,23 @@ feature 'Environment', :feature do on_stop: 'close_app') end - given(:role) { :master } + context 'when user has ability to stop environment' do + given(:permissions) do + create(:protected_branch, :developers_can_merge, + name: action.ref, project: project) + end - scenario 'does allow to stop environment' do - click_link('Stop') + it 'allows to stop environment' do + click_link('Stop') - expect(page).to have_content('close_app') + expect(page).to have_content('close_app') + end + end + + context 'when user has no ability to stop environment' do + it 'does not allow to stop environment' do + expect(page).to have_no_link('Stop') + end end context 'for reporter' do @@ -150,12 +173,6 @@ feature 'Environment', :feature do end end end - - context 'without stop action' do - scenario 'does allow to stop environment' do - click_link('Stop') - end - end end context 'when environment is stopped' do diff --git a/spec/lib/gitlab/chat_commands/command_spec.rb b/spec/lib/gitlab/chat_commands/command_spec.rb index eb4f06b371c..13e6953147b 100644 --- a/spec/lib/gitlab/chat_commands/command_spec.rb +++ b/spec/lib/gitlab/chat_commands/command_spec.rb @@ -58,9 +58,12 @@ describe Gitlab::ChatCommands::Command, service: true do end end - context 'and user does have deployment permission' do + context 'and user has deployment permission' do before do - build.project.add_master(user) + build.project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: build.ref, project: project) end it 'returns action' do diff --git a/spec/lib/gitlab/chat_commands/deploy_spec.rb b/spec/lib/gitlab/chat_commands/deploy_spec.rb index b33389d959e..46dbdeae37c 100644 --- a/spec/lib/gitlab/chat_commands/deploy_spec.rb +++ b/spec/lib/gitlab/chat_commands/deploy_spec.rb @@ -7,7 +7,12 @@ describe Gitlab::ChatCommands::Deploy, service: true do let(:regex_match) { described_class.match('deploy staging to production') } before do - project.add_master(user) + # Make it possible to trigger protected manual actions for developers. + # + project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: 'master', project: project) end subject do diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb index 185bb9098da..3f30b2c38f2 100644 --- a/spec/lib/gitlab/ci/status/build/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb @@ -224,7 +224,10 @@ describe Gitlab::Ci::Status::Build::Factory do context 'when user has ability to play action' do before do - build.project.add_master(user) + project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: build.ref, project: project) end it 'fabricates status that has action' do diff --git a/spec/lib/gitlab/ci/status/build/play_spec.rb b/spec/lib/gitlab/ci/status/build/play_spec.rb index f5d0f977768..0e15a5f3c6b 100644 --- a/spec/lib/gitlab/ci/status/build/play_spec.rb +++ b/spec/lib/gitlab/ci/status/build/play_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe Gitlab::Ci::Status::Build::Play do let(:user) { create(:user) } + let(:project) { build.project } let(:build) { create(:ci_build, :manual) } let(:status) { Gitlab::Ci::Status::Core.new(build, user) } @@ -15,8 +16,13 @@ describe Gitlab::Ci::Status::Build::Play do describe '#has_action?' do context 'when user is allowed to update build' do - context 'when user can push to branch' do - before { build.project.add_master(user) } + context 'when user is allowed to trigger protected action' do + before do + project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: build.ref, project: project) + end it { is_expected.to have_action } end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 28e5c3f80f4..1772014f8c2 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -227,7 +227,10 @@ describe Environment, models: true do context 'when user is allowed to stop environment' do before do - project.add_master(user) + project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: 'master', project: project) end context 'when action did not yet finish' do diff --git a/spec/serializers/build_entity_spec.rb b/spec/serializers/build_entity_spec.rb index b5eb84ae43b..6d5e1046e86 100644 --- a/spec/serializers/build_entity_spec.rb +++ b/spec/serializers/build_entity_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' describe BuildEntity do let(:user) { create(:user) } let(:build) { create(:ci_build) } + let(:project) { build.project } let(:request) { double('request') } before do @@ -52,7 +53,10 @@ describe BuildEntity do context 'when user is allowed to trigger action' do before do - build.project.add_master(user) + project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: 'master', project: project) end it 'contains path to play action' do diff --git a/spec/services/ci/play_build_service_spec.rb b/spec/services/ci/play_build_service_spec.rb index d6f9fa42045..ea211de1f82 100644 --- a/spec/services/ci/play_build_service_spec.rb +++ b/spec/services/ci/play_build_service_spec.rb @@ -13,8 +13,11 @@ describe Ci::PlayBuildService, '#execute', :services do context 'when project does not have repository yet' do let(:project) { create(:empty_project) } - it 'allows user with master role to play build' do - project.add_master(user) + it 'allows user to play build if protected branch rules are met' do + project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: build.ref, project: project) service.execute(build) @@ -45,7 +48,10 @@ describe Ci::PlayBuildService, '#execute', :services do let(:build) { create(:ci_build, :manual, pipeline: pipeline) } before do - project.add_master(user) + project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: build.ref, project: project) end it 'enqueues the build' do @@ -64,7 +70,10 @@ describe Ci::PlayBuildService, '#execute', :services do let(:build) { create(:ci_build, when: :manual, pipeline: pipeline) } before do - project.add_master(user) + project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: build.ref, project: project) end it 'duplicates the build' do diff --git a/spec/services/ci/process_pipeline_service_spec.rb b/spec/services/ci/process_pipeline_service_spec.rb index fc5de5d069a..1557cb3c938 100644 --- a/spec/services/ci/process_pipeline_service_spec.rb +++ b/spec/services/ci/process_pipeline_service_spec.rb @@ -333,10 +333,11 @@ describe Ci::ProcessPipelineService, '#execute', :services do context 'when pipeline is promoted sequentially up to the end' do before do - # We are using create(:empty_project), and users has to be master in - # order to execute manual action when repository does not exist. + # Users need ability to merge into a branch in order to trigger + # protected manual actions. # - project.add_master(user) + create(:protected_branch, :developers_can_merge, + name: 'master', project: project) end it 'properly processes entire pipeline' do diff --git a/spec/services/ci/retry_pipeline_service_spec.rb b/spec/services/ci/retry_pipeline_service_spec.rb index d941d56c0d8..3e860203063 100644 --- a/spec/services/ci/retry_pipeline_service_spec.rb +++ b/spec/services/ci/retry_pipeline_service_spec.rb @@ -6,9 +6,12 @@ describe Ci::RetryPipelineService, '#execute', :services do let(:pipeline) { create(:ci_pipeline, project: project) } let(:service) { described_class.new(project, user) } - context 'when user has ability to modify pipeline' do + context 'when user has full ability to modify pipeline' do before do - project.add_master(user) + project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: pipeline.ref, project: project) end context 'when there are already retried jobs present' do -- cgit v1.2.1 From 65144f701c137a07f686e228a375121cd1fc0061 Mon Sep 17 00:00:00 2001 From: Fatih Acet Date: Fri, 26 May 2017 19:42:02 +0000 Subject: Merge branch '32447-remove-source-branch-is-displayed-to-mergers-who-cannot-actually-delete-the-source-branch-from-a-forked-project' into 'master' Disable "Remove source branch" in MR Widget for users who can't remove, and re-add checkbox to MR form Closes #32447 and #32907 See merge request !11558 --- .../components/states/mr_widget_ready_to_merge.js | 8 +++-- .../stores/mr_widget_store.js | 2 +- app/serializers/merge_request_entity.rb | 1 + .../shared/issuable/form/_merge_params.html.haml | 10 ++++++ features/project/merge_requests/accept.feature | 3 +- .../steps/project/merge_requests/acceptance.rb | 6 +++- spec/features/merge_requests/edit_mr_spec.rb | 13 ++++++++ .../merge_when_pipeline_succeeds_spec.rb | 10 +++--- spec/features/merge_requests/widget_spec.rb | 21 ++++++++++++ .../api/schemas/entities/merge_request.json | 3 +- .../states/mr_widget_ready_to_merge_spec.js | 37 ++++++++++++++++++++-- 11 files changed, 102 insertions(+), 12 deletions(-) diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js index d866d4e94b0..fcd4fdaf09f 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js @@ -13,7 +13,7 @@ export default { }, data() { return { - removeSourceBranch: true, + removeSourceBranch: this.mr.shouldRemoveSourceBranch, mergeWhenBuildSucceeds: false, useCommitMessageWithDescription: false, setToMergeWhenPipelineSucceeds: false, @@ -69,6 +69,9 @@ export default { || this.isMakingRequest || this.mr.preventMerge); }, + isRemoveSourceBranchButtonDisabled() { + return this.isMergeButtonDisabled || !this.mr.canRemoveSourceBranch; + }, shouldShowSquashBeforeMerge() { const { commitsCount, enableSquashBeforeMerge } = this.mr; return enableSquashBeforeMerge && commitsCount > 1; @@ -252,8 +255,9 @@ export default {