From f0ef51729dec490ae2f3a57c4e8d459a224a9193 Mon Sep 17 00:00:00 2001 From: Andrea Kao Date: Tue, 10 Apr 2018 11:13:07 -0700 Subject: [PATCH 01/37] edit GitLab license info so that GitHub recognizes it GitHub uses a library called Licensee to identify a project's license type. It shows this information in the status bar and via the API if it can unambiguously identify the license. This commit updates the LICENSE file so that it contains the exact text of the MIT license. It also moves the reference to third-party software licensing to the README. These changes allow Licensee to successfully identify the license type of GitLab's codebase as MIT. Signed-off-by: Andrea Kao --- LICENSE | 26 ++++---------------------- README.md | 6 ++++++ 2 files changed, 10 insertions(+), 22 deletions(-) diff --git a/LICENSE b/LICENSE index 15c423e1416..a76372fad2c 100644 --- a/LICENSE +++ b/LICENSE @@ -1,25 +1,7 @@ -Copyright (c) 2011-2017 GitLab B.V. +Copyright GitLab B.V. -With regard to the GitLab Software: +Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: -Permission is hereby granted, free of charge, to any person obtaining a copy -of this software and associated documentation files (the "Software"), to deal -in the Software without restriction, including without limitation the rights -to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -copies of the Software, and to permit persons to whom the Software is -furnished to do so, subject to the following conditions: +The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. -The above copyright notice and this permission notice shall be included in -all copies or substantial portions of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -THE SOFTWARE. - -For all third party components incorporated into the GitLab Software, those -components are licensed under the original license provided by the owner of the -applicable component. \ No newline at end of file +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. \ No newline at end of file diff --git a/README.md b/README.md index 9ead6d51c5d..9c1aad65307 100644 --- a/README.md +++ b/README.md @@ -67,6 +67,12 @@ You can access a new installation with the login **`root`** and password **`5ive GitLab is an open source project and we are very happy to accept community contributions. Please refer to [CONTRIBUTING.md](/CONTRIBUTING.md) for details. +## Licensing + +GitLab Community Edition (CE) is available freely under the MIT Expat license. + +All third party components incorporated into the GitLab Software are licensed under the original license provided by the owner of the applicable component. + ## Install a development environment To work on GitLab itself, we recommend setting up your development environment with [the GitLab Development Kit](https://gitlab.com/gitlab-org/gitlab-development-kit). From e8221b47ace3c80b8b812c6289c9035ff8e8f5ff Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 11 Apr 2018 12:29:16 +0100 Subject: [PATCH 02/37] Fixed IDE diff markers being cached too long --- .../javascripts/ide/lib/common/model.js | 16 +++-- .../ide/lib/decorations/controller.js | 9 +++ .../javascripts/ide/lib/diff/controller.js | 25 ++++--- spec/javascripts/ide/lib/common/model_spec.js | 17 +++-- .../ide/lib/decorations/controller_spec.js | 29 ++++++++ .../ide/lib/diff/controller_spec.js | 70 +++++++++++++------ 6 files changed, 123 insertions(+), 43 deletions(-) diff --git a/app/assets/javascripts/ide/lib/common/model.js b/app/assets/javascripts/ide/lib/common/model.js index e47adae99ed..b9d5c0be70b 100644 --- a/app/assets/javascripts/ide/lib/common/model.js +++ b/app/assets/javascripts/ide/lib/common/model.js @@ -31,7 +31,7 @@ export default class Model { ); } - this.events = new Map(); + this.events = new Set(); this.updateContent = this.updateContent.bind(this); this.dispose = this.dispose.bind(this); @@ -73,10 +73,11 @@ export default class Model { } onChange(cb) { - this.events.set( - this.path, - this.disposable.add(this.model.onDidChangeContent(e => cb(this, e))), - ); + this.events.add(this.disposable.add(this.model.onDidChangeContent(e => cb(this, e)))); + } + + onDispose(cb) { + this.events.add(cb); } updateContent(content) { @@ -86,6 +87,11 @@ export default class Model { dispose() { this.disposable.dispose(); + + this.events.forEach(cb => { + if (typeof cb === 'function') cb(); + }); + this.events.clear(); eventHub.$off(`editor.update.model.dispose.${this.file.key}`, this.dispose); diff --git a/app/assets/javascripts/ide/lib/decorations/controller.js b/app/assets/javascripts/ide/lib/decorations/controller.js index 42904774747..13d477bb2cf 100644 --- a/app/assets/javascripts/ide/lib/decorations/controller.js +++ b/app/assets/javascripts/ide/lib/decorations/controller.js @@ -38,6 +38,15 @@ export default class DecorationsController { ); } + hasDecorations(model) { + return this.decorations.has(model.url); + } + + removeDecorations(model) { + this.decorations.delete(model.url); + this.editorDecorations.delete(model.url); + } + dispose() { this.decorations.clear(); this.editorDecorations.clear(); diff --git a/app/assets/javascripts/ide/lib/diff/controller.js b/app/assets/javascripts/ide/lib/diff/controller.js index b136545ad11..f579424cf33 100644 --- a/app/assets/javascripts/ide/lib/diff/controller.js +++ b/app/assets/javascripts/ide/lib/diff/controller.js @@ -3,7 +3,7 @@ import { throttle } from 'underscore'; import DirtyDiffWorker from './diff_worker'; import Disposable from '../common/disposable'; -export const getDiffChangeType = (change) => { +export const getDiffChangeType = change => { if (change.modified) { return 'modified'; } else if (change.added) { @@ -16,12 +16,7 @@ export const getDiffChangeType = (change) => { }; export const getDecorator = change => ({ - range: new monaco.Range( - change.lineNumber, - 1, - change.endLineNumber, - 1, - ), + range: new monaco.Range(change.lineNumber, 1, change.endLineNumber, 1), options: { isWholeLine: true, linesDecorationsClassName: `dirty-diff dirty-diff-${getDiffChangeType(change)}`, @@ -31,6 +26,7 @@ export const getDecorator = change => ({ export default class DirtyDiffController { constructor(modelManager, decorationsController) { this.disposable = new Disposable(); + this.models = new Map(); this.editorSimpleWorker = null; this.modelManager = modelManager; this.decorationsController = decorationsController; @@ -42,7 +38,15 @@ export default class DirtyDiffController { } attachModel(model) { + if (this.models.has(model.url)) return; + model.onChange(() => this.throttledComputeDiff(model)); + model.onDispose(() => { + this.decorationsController.removeDecorations(model); + this.models.delete(model.url); + }); + + this.models.set(model.url, model); } computeDiff(model) { @@ -54,7 +58,11 @@ export default class DirtyDiffController { } reDecorate(model) { - this.decorationsController.decorate(model); + if (this.decorationsController.hasDecorations(model)) { + this.decorationsController.decorate(model); + } else { + this.computeDiff(model); + } } decorate({ data }) { @@ -65,6 +73,7 @@ export default class DirtyDiffController { dispose() { this.disposable.dispose(); + this.models.clear(); this.dirtyDiffWorker.removeEventListener('message', this.decorate); this.dirtyDiffWorker.terminate(); diff --git a/spec/javascripts/ide/lib/common/model_spec.js b/spec/javascripts/ide/lib/common/model_spec.js index 8fc2fccb64c..6fc958c1b98 100644 --- a/spec/javascripts/ide/lib/common/model_spec.js +++ b/spec/javascripts/ide/lib/common/model_spec.js @@ -70,13 +70,6 @@ describe('Multi-file editor library model', () => { }); describe('onChange', () => { - it('caches event by path', () => { - model.onChange(() => {}); - - expect(model.events.size).toBe(1); - expect(model.events.keys().next().value).toBe(model.file.key); - }); - it('calls callback on change', done => { const spy = jasmine.createSpy(); model.onChange(spy); @@ -119,5 +112,15 @@ describe('Multi-file editor library model', () => { jasmine.anything(), ); }); + + it('calls onDispose callback', () => { + const disposeSpy = jasmine.createSpy(); + + model.onDispose(disposeSpy); + + model.dispose(); + + expect(disposeSpy).toHaveBeenCalled(); + }); }); }); diff --git a/spec/javascripts/ide/lib/decorations/controller_spec.js b/spec/javascripts/ide/lib/decorations/controller_spec.js index aec325e26a9..e1c4ca570b6 100644 --- a/spec/javascripts/ide/lib/decorations/controller_spec.js +++ b/spec/javascripts/ide/lib/decorations/controller_spec.js @@ -117,4 +117,33 @@ describe('Multi-file editor library decorations controller', () => { expect(controller.editorDecorations.size).toBe(0); }); }); + + describe('hasDecorations', () => { + it('returns true when decorations are cached', () => { + controller.addDecorations(model, 'key', [{ decoration: 'decorationValue' }]); + + expect(controller.hasDecorations(model)).toBe(true); + }); + + it('returns false when no model decorations exist', () => { + expect(controller.hasDecorations(model)).toBe(false); + }); + }); + + describe('removeDecorations', () => { + beforeEach(() => { + controller.addDecorations(model, 'key', [{ decoration: 'decorationValue' }]); + controller.decorate(model); + }); + + it('removes cached decorations', () => { + expect(controller.decorations.size).not.toBe(0); + expect(controller.editorDecorations.size).not.toBe(0); + + controller.removeDecorations(model); + + expect(controller.decorations.size).toBe(0); + expect(controller.editorDecorations.size).toBe(0); + }); + }); }); diff --git a/spec/javascripts/ide/lib/diff/controller_spec.js b/spec/javascripts/ide/lib/diff/controller_spec.js index ff73240734e..fd8ab3b4f1d 100644 --- a/spec/javascripts/ide/lib/diff/controller_spec.js +++ b/spec/javascripts/ide/lib/diff/controller_spec.js @@ -3,10 +3,7 @@ import monacoLoader from '~/ide/monaco_loader'; import editor from '~/ide/lib/editor'; import ModelManager from '~/ide/lib/common/model_manager'; import DecorationsController from '~/ide/lib/decorations/controller'; -import DirtyDiffController, { - getDiffChangeType, - getDecorator, -} from '~/ide/lib/diff/controller'; +import DirtyDiffController, { getDiffChangeType, getDecorator } from '~/ide/lib/diff/controller'; import { computeDiff } from '~/ide/lib/diff/diff'; import { file } from '../../helpers'; @@ -90,6 +87,14 @@ describe('Multi-file editor library dirty diff controller', () => { expect(model.onChange).toHaveBeenCalled(); }); + it('adds dispose event callback', () => { + spyOn(model, 'onDispose'); + + controller.attachModel(model); + + expect(model.onDispose).toHaveBeenCalled(); + }); + it('calls throttledComputeDiff on change', () => { spyOn(controller, 'throttledComputeDiff'); @@ -99,6 +104,12 @@ describe('Multi-file editor library dirty diff controller', () => { expect(controller.throttledComputeDiff).toHaveBeenCalled(); }); + + it('caches model', () => { + controller.attachModel(model); + + expect(controller.models.has(model.url)).toBe(true); + }); }); describe('computeDiff', () => { @@ -116,14 +127,22 @@ describe('Multi-file editor library dirty diff controller', () => { }); describe('reDecorate', () => { - it('calls decorations controller decorate', () => { - spyOn(controller.decorationsController, 'decorate'); + it('calls computeDiff when no decorations are cached', () => { + spyOn(controller, 'computeDiff'); controller.reDecorate(model); - expect(controller.decorationsController.decorate).toHaveBeenCalledWith( - model, - ); + expect(controller.computeDiff).toHaveBeenCalledWith(model); + }); + + it('calls decorate when decorations are cached', () => { + spyOn(controller.decorationsController, 'decorate'); + + controller.decorationsController.decorations.set(model.url, 'test'); + + controller.reDecorate(model); + + expect(controller.decorationsController.decorate).toHaveBeenCalledWith(model); }); }); @@ -133,16 +152,15 @@ describe('Multi-file editor library dirty diff controller', () => { controller.decorate({ data: { changes: [], path: model.path } }); - expect( - controller.decorationsController.addDecorations, - ).toHaveBeenCalledWith(model, 'dirtyDiff', jasmine.anything()); + expect(controller.decorationsController.addDecorations).toHaveBeenCalledWith( + model, + 'dirtyDiff', + jasmine.anything(), + ); }); it('adds decorations into editor', () => { - const spy = spyOn( - controller.decorationsController.editor.instance, - 'deltaDecorations', - ); + const spy = spyOn(controller.decorationsController.editor.instance, 'deltaDecorations'); controller.decorate({ data: { changes: computeDiff('123', '1234'), path: model.path }, @@ -181,16 +199,22 @@ describe('Multi-file editor library dirty diff controller', () => { }); it('removes worker event listener', () => { - spyOn( - controller.dirtyDiffWorker, - 'removeEventListener', - ).and.callThrough(); + spyOn(controller.dirtyDiffWorker, 'removeEventListener').and.callThrough(); controller.dispose(); - expect( - controller.dirtyDiffWorker.removeEventListener, - ).toHaveBeenCalledWith('message', jasmine.anything()); + expect(controller.dirtyDiffWorker.removeEventListener).toHaveBeenCalledWith( + 'message', + jasmine.anything(), + ); + }); + + it('clears cached models', () => { + controller.attachModel(model); + + model.dispose(); + + expect(controller.models.size).toBe(0); }); }); }); From f2bf23feb908020a0861608ce856da1548a9694e Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 11 Apr 2018 13:10:12 +0100 Subject: [PATCH 03/37] fixed spec with checking cached events --- spec/javascripts/ide/components/repo_editor_spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/javascripts/ide/components/repo_editor_spec.js b/spec/javascripts/ide/components/repo_editor_spec.js index 310d222377f..5f479aa3a8d 100644 --- a/spec/javascripts/ide/components/repo_editor_spec.js +++ b/spec/javascripts/ide/components/repo_editor_spec.js @@ -222,7 +222,7 @@ describe('RepoEditor', () => { vm.setupEditor(); expect(vm.editor.onPositionChange).toHaveBeenCalled(); - expect(vm.model.events.size).toBe(1); + expect(vm.model.events.size).toBe(2); }); it('updates state when model content changed', done => { From 7e3b4764cfefe760cb0fc408c2a27e7a054e2b2f Mon Sep 17 00:00:00 2001 From: Mek Stittri Date: Fri, 13 Apr 2018 00:41:09 -0700 Subject: [PATCH 04/37] Defined priority label for bugs - Clearly define new priority issue for bugs - Clarrified existing priority label => Milestone labels --- CONTRIBUTING.md | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9c8fdc1275b..918e02893e3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -126,7 +126,7 @@ Most issues will have labels for at least one of the following: - Type: ~"feature proposal", ~bug, ~customer, etc. - Subject: ~wiki, ~"container registry", ~ldap, ~api, ~frontend, etc. - Team: ~"CI/CD", ~Discussion, ~Edge, ~Platform, etc. -- Priority: ~Deliverable, ~Stretch, ~"Next Patch Release" +- Milestone: ~Deliverable, ~Stretch, ~"Next Patch Release" All labels, their meaning and priority are defined on the [labels page][labels-page]. @@ -185,10 +185,10 @@ indicate if an issue needs backend work, frontend work, or both. Team labels are always capitalized so that they show up as the first label for any issue. -### Priority labels (~Deliverable, ~Stretch, ~"Next Patch Release") +### Milestone labels (~Deliverable, ~Stretch, ~"Next Patch Release") -Priority labels help us clearly communicate expectations of the work for the -release. There are two levels of priority labels: +Milestone labels help us clearly communicate expectations of the work for the +release. There are three levels of Milestone labels: - ~Deliverable: Issues that are expected to be delivered in the current milestone. @@ -203,16 +203,29 @@ Each issue scheduled for the current milestone should be labeled ~Deliverable or ~"Stretch". Any open issue for a previous milestone should be labeled ~"Next Patch Release", or otherwise rescheduled to a different milestone. -### Severity labels (~S1, ~S2, etc.) +### Bug Priority labels (~P1, ~P2, ~P3 & etc.) + +Bug Priority labels help us define the time a ~bug fix should be completed. +This label documents the planned timeline & urgency which is used to measure against our actual SLA on delivering ~bug fixes. + +| Label | Estimate time to fix | Guidance | +|-------|--------------------------------------------------|----------| +| ~P1 | Immediate hotfix to production | This would normally correspond to a S1 severity below | +| ~P2 | The current milestone (regular & patch releases) | The issue is (almost) guaranteed to occur in the near future | +| ~P3 | The next milestone (regular & patch releases) | The issue is likely to occur in the near future | +| ~P4 | The next 2 to 4 milestones (regular & patch releases) | The issue _may_ occur but it's not likely | +| ~P5 | Anything we know will not be done within the next quarter | The issue is prominent but does not impact user workflow and a workaround if any is well documented | + +### Bug Severity labels (~S1, ~S2, ~S3 & etc.) Severity labels help us clearly communicate the impact of a ~bug on users. -| Label | Meaning | Example | -|-------|------------------------------------------|---------| -| ~S1 | Feature broken, no workaround | Unable to create an issue | -| ~S2 | Feature broken, workaround unacceptable | Can push commits, but only via the command line | -| ~S3 | Feature broken, workaround acceptable | Can create merge requests only from the Merge Requests page, not through the Issue | -| ~S4 | Cosmetic issue | Label colors are incorrect / not being displayed | +| Label | Meaning | Example | +|-------|-------------------------------------------------------|---------| +| ~S1 | Outage, broken feature with no workaround | Unable to create an issue. Data corruption/loss. Security breach. | +| ~S2 | Broken Feature, workaround too complex & unacceptable | Can push commits, but only via the command line. | +| ~S3 | Broken Feature, workaround acceptable | Can create merge requests only from the Merge Requests page, not through the Issue. | +| ~S4 | Functionality inconvenience or cosmetic issue | Label colors are incorrect / not being displayed. | ### Label for community contributors (~"Accepting Merge Requests") From d897c2a2fce7d70777dcb15e5d5d740d7d7c92a8 Mon Sep 17 00:00:00 2001 From: Mek Stittri Date: Fri, 13 Apr 2018 13:10:03 -0700 Subject: [PATCH 05/37] Revised definitions for clarrity --- CONTRIBUTING.md | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 918e02893e3..633e0b7777e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -205,27 +205,27 @@ or ~"Stretch". Any open issue for a previous milestone should be labeled ### Bug Priority labels (~P1, ~P2, ~P3 & etc.) -Bug Priority labels help us define the time a ~bug fix should be completed. +Bug Priority labels help us define the time a ~bug fix should be completed. Priority determines how quickly the defect turnaround time must be. If there are multiple defects, the priority decides which defect has to be fixed immediately versus later. This label documents the planned timeline & urgency which is used to measure against our actual SLA on delivering ~bug fixes. -| Label | Estimate time to fix | Guidance | -|-------|--------------------------------------------------|----------| -| ~P1 | Immediate hotfix to production | This would normally correspond to a S1 severity below | -| ~P2 | The current milestone (regular & patch releases) | The issue is (almost) guaranteed to occur in the near future | -| ~P3 | The next milestone (regular & patch releases) | The issue is likely to occur in the near future | -| ~P4 | The next 2 to 4 milestones (regular & patch releases) | The issue _may_ occur but it's not likely | -| ~P5 | Anything we know will not be done within the next quarter | The issue is prominent but does not impact user workflow and a workaround if any is well documented | +| Label | Meaning | Estimate time to fix | Guidance | +|-------|-----------------|------------------------------------------------------------------------|----------| +| ~P1 | Immediate | Immediate hotfix to production | This would normally correspond to a S1 severity below | +| ~P2 | Urgent Priority | The current release (regular & patch) | The issue is (almost) guaranteed to occur in the near future | +| ~P3 | High Priority | The next release (regular & patch) | The issue is likely to occur in the near future | +| ~P4 | Medium Priority | Within the next 3 major releases (1 quarter duration) | The issue _may_ occur but it's not likely | +| ~P5 | Low Priority | Anything outside the next 3 major releases (the next quarter duration) | The issue is prominent but does not impact user workflow and a workaround if any is well documented | ### Bug Severity labels (~S1, ~S2, ~S3 & etc.) -Severity labels help us clearly communicate the impact of a ~bug on users. +Severity labels help us clearly communicate the impact of a ~bug on users. -| Label | Meaning | Example | -|-------|-------------------------------------------------------|---------| -| ~S1 | Outage, broken feature with no workaround | Unable to create an issue. Data corruption/loss. Security breach. | -| ~S2 | Broken Feature, workaround too complex & unacceptable | Can push commits, but only via the command line. | -| ~S3 | Broken Feature, workaround acceptable | Can create merge requests only from the Merge Requests page, not through the Issue. | -| ~S4 | Functionality inconvenience or cosmetic issue | Label colors are incorrect / not being displayed. | +| Label | Meaning | Impact of the defect | Example | +|-------|-------------------|-------------------------------------------------------|---------| +| ~S1 | Blocker | Outage, broken feature with no workaround | Unable to create an issue. Data corruption/loss. Security breach. | +| ~S2 | Critical Severity | Broken Feature, workaround too complex & unacceptable | Can push commits, but only via the command line. | +| ~S3 | Major Severity | Broken Feature, workaround acceptable | Can create merge requests only from the Merge Requests page, not through the Issue. | +| ~S4 | Low Severity | Functionality inconvenience or cosmetic issue | Label colors are incorrect / not being displayed. | ### Label for community contributors (~"Accepting Merge Requests") From 3394d9538b70209ec6ccbafebeff377d82149e59 Mon Sep 17 00:00:00 2001 From: Mek Stittri Date: Mon, 16 Apr 2018 23:00:20 -0700 Subject: [PATCH 06/37] =?UTF-8?q?Address=20Douwe,=20Sean=20and=20R=C3=A9my?= =?UTF-8?q?=20comments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CONTRIBUTING.md | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 633e0b7777e..3a13b804319 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -208,13 +208,23 @@ or ~"Stretch". Any open issue for a previous milestone should be labeled Bug Priority labels help us define the time a ~bug fix should be completed. Priority determines how quickly the defect turnaround time must be. If there are multiple defects, the priority decides which defect has to be fixed immediately versus later. This label documents the planned timeline & urgency which is used to measure against our actual SLA on delivering ~bug fixes. -| Label | Meaning | Estimate time to fix | Guidance | -|-------|-----------------|------------------------------------------------------------------------|----------| -| ~P1 | Immediate | Immediate hotfix to production | This would normally correspond to a S1 severity below | -| ~P2 | Urgent Priority | The current release (regular & patch) | The issue is (almost) guaranteed to occur in the near future | -| ~P3 | High Priority | The next release (regular & patch) | The issue is likely to occur in the near future | -| ~P4 | Medium Priority | Within the next 3 major releases (1 quarter duration) | The issue _may_ occur but it's not likely | -| ~P5 | Low Priority | Anything outside the next 3 major releases (the next quarter duration) | The issue is prominent but does not impact user workflow and a workaround if any is well documented | +| Label | Meaning | Estimate time to fix | Guidance | +|-------|-----------------|------------------------------------------------------------------|----------| +| ~P1 | Immediate | Immediate hotfix to production | This would normally correspond to a S1 severity below | +| ~P2 | Urgent Priority | The current release | | +| ~P3 | High Priority | The next release | | +| ~P4 | Medium Priority | Within the next 3 releases (1 quarter duration) | | +| ~P5 | Low Priority | Anything outside the next 3 releases (the next quarter duration) | The issue is prominent but does not impact user workflow and a workaround if any is well documented | + +#### Team specific priority guidance + +| Label | Availability / Performance | Security | +|-------|--------------------------------------------------------------|----------| +| ~P1 | | | +| ~P2 | The issue is (almost) guaranteed to occur in the near future | | +| ~P3 | The issue is likely to occur in the near future | | +| ~P4 | The issue _may_ occur but it's not likely | | +| ~P5 | | | ### Bug Severity labels (~S1, ~S2, ~S3 & etc.) From 5fbfa62b07ea500f9068b80af961c6f3a2ede9e0 Mon Sep 17 00:00:00 2001 From: Mek Stittri Date: Tue, 17 Apr 2018 14:25:29 -0700 Subject: [PATCH 07/37] Added severity guidelines for security --- CONTRIBUTING.md | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3a13b804319..65dc2281dd5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -210,21 +210,21 @@ This label documents the planned timeline & urgency which is used to measure aga | Label | Meaning | Estimate time to fix | Guidance | |-------|-----------------|------------------------------------------------------------------|----------| -| ~P1 | Immediate | Immediate hotfix to production | This would normally correspond to a S1 severity below | +| ~P1 | Immediate | Immediate hotfix (outside of the normal release process) | This would normally correspond to a S1 severity below | | ~P2 | Urgent Priority | The current release | | -| ~P3 | High Priority | The next release | | -| ~P4 | Medium Priority | Within the next 3 releases (1 quarter duration) | | -| ~P5 | Low Priority | Anything outside the next 3 releases (the next quarter duration) | The issue is prominent but does not impact user workflow and a workaround if any is well documented | +| ~P3 | High Priority | The next release after the current release | | +| ~P4 | Medium Priority | Within the next 3 releases (approx one quarter) | | +| ~P5 | Low Priority | Anything outside the next 3 releases (approx beyond one quarter) | The issue is prominent but does not impact user workflow and a workaround is documented | -#### Team specific priority guidance +#### Specific Priority guidance -| Label | Availability / Performance | Security | -|-------|--------------------------------------------------------------|----------| -| ~P1 | | | -| ~P2 | The issue is (almost) guaranteed to occur in the near future | | -| ~P3 | The issue is likely to occur in the near future | | -| ~P4 | The issue _may_ occur but it's not likely | | -| ~P5 | | | +| Label | Availability / Performance | +|-------|--------------------------------------------------------------| +| ~P1 | | +| ~P2 | The issue is (almost) guaranteed to occur in the near future | +| ~P3 | The issue is likely to occur in the near future | +| ~P4 | The issue _may_ occur but it's not likely | +| ~P5 | | ### Bug Severity labels (~S1, ~S2, ~S3 & etc.) @@ -237,6 +237,15 @@ Severity labels help us clearly communicate the impact of a ~bug on users. | ~S3 | Major Severity | Broken Feature, workaround acceptable | Can create merge requests only from the Merge Requests page, not through the Issue. | | ~S4 | Low Severity | Functionality inconvenience or cosmetic issue | Label colors are incorrect / not being displayed. | +#### Specific Severity guidance + +| Label | Security Impact | +|-------|-------------------------------------------------------------------| +| ~S1 | >50% customers impacted (possible company extinction level event) | +| ~S2 | Multiple customers impacted (but not apocalyptic) | +| ~S3 | A single customer impacted | +| ~S4 | No customer impact, or expected impact within 30 days | + ### Label for community contributors (~"Accepting Merge Requests") Issues that are beneficial to our users, 'nice to haves', that we currently do From c55685d265b628d457fc065df2559fc89996cef9 Mon Sep 17 00:00:00 2001 From: Alexander Tanayno Date: Wed, 18 Apr 2018 12:04:40 +0000 Subject: [PATCH 08/37] add restart of gitlab-pages to apply gitlab pages settings --- doc/administration/pages/index.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/doc/administration/pages/index.md b/doc/administration/pages/index.md index 00c631fdaae..c342a82ef7b 100644 --- a/doc/administration/pages/index.md +++ b/doc/administration/pages/index.md @@ -124,6 +124,12 @@ The Pages daemon doesn't listen to the outside world. ``` 1. [Reconfigure GitLab][reconfigure] +1. Restart gitlab-pages by running the following command: + + ```ruby + sudo gitlab-ctl restart gitlab-pages + ``` + Watch the [video tutorial][video-admin] for this configuration. @@ -155,6 +161,11 @@ outside world. respectively. 1. [Reconfigure GitLab][reconfigure] +1. Restart gitlab-pages by running the following command: + + ```ruby + sudo gitlab-ctl restart gitlab-pages + ``` ## Advanced configuration @@ -192,6 +203,11 @@ world. Custom domains are supported, but no TLS. listens on. If you don't have IPv6, you can omit the IPv6 address. 1. [Reconfigure GitLab][reconfigure] +1. Restart gitlab-pages by running the following command: + + ```ruby + sudo gitlab-ctl restart gitlab-pages + ``` ### Custom domains with TLS support @@ -225,6 +241,11 @@ world. Custom domains and TLS are supported. listens on. If you don't have IPv6, you can omit the IPv6 address. 1. [Reconfigure GitLab][reconfigure] +1. Restart gitlab-pages by running the following command: + + ```ruby + sudo gitlab-ctl restart gitlab-pages + ``` ### Custom domain verification @@ -252,6 +273,11 @@ are stored. ``` 1. [Reconfigure GitLab][reconfigure] +1. Restart gitlab-pages by running the following command: + + ```ruby + sudo gitlab-ctl restart gitlab-pages + ``` ## Set maximum pages size From 1e2659d69c995f0f7de994dd59dc68f11c304252 Mon Sep 17 00:00:00 2001 From: Mek Stittri Date: Wed, 18 Apr 2018 11:02:52 -0700 Subject: [PATCH 09/37] Removed one priority layer --- CONTRIBUTING.md | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 65dc2281dd5..02599907af7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -210,11 +210,10 @@ This label documents the planned timeline & urgency which is used to measure aga | Label | Meaning | Estimate time to fix | Guidance | |-------|-----------------|------------------------------------------------------------------|----------| -| ~P1 | Immediate | Immediate hotfix (outside of the normal release process) | This would normally correspond to a S1 severity below | -| ~P2 | Urgent Priority | The current release | | -| ~P3 | High Priority | The next release after the current release | | -| ~P4 | Medium Priority | Within the next 3 releases (approx one quarter) | | -| ~P5 | Low Priority | Anything outside the next 3 releases (approx beyond one quarter) | The issue is prominent but does not impact user workflow and a workaround is documented | +| ~P1 | Urgent Priority | The current release | | +| ~P2 | High Priority | The next release | | +| ~P3 | Medium Priority | Within the next 3 releases (approx one quarter) | | +| ~P4 | Low Priority | Anything outside the next 3 releases (approx beyond one quarter) | The issue is prominent but does not impact user workflow and a workaround is documented | #### Specific Priority guidance @@ -224,7 +223,6 @@ This label documents the planned timeline & urgency which is used to measure aga | ~P2 | The issue is (almost) guaranteed to occur in the near future | | ~P3 | The issue is likely to occur in the near future | | ~P4 | The issue _may_ occur but it's not likely | -| ~P5 | | ### Bug Severity labels (~S1, ~S2, ~S3 & etc.) From 0ac5f1279ef45279a06a744e97955143f9b96ee8 Mon Sep 17 00:00:00 2001 From: Ash McKenzie Date: Wed, 18 Apr 2018 15:52:31 +1000 Subject: [PATCH 10/37] Add index to file_store on ci_job_artifacts --- ...ement-timeout-counting-local-job-artifacts.yml | 5 +++++ ...07_add_index_to_ci_job_artifacts_file_store.rb | 15 +++++++++++++++ db/schema.rb | 3 ++- 3 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/45476-geo-statement-timeout-counting-local-job-artifacts.yml create mode 100644 db/migrate/20180418053107_add_index_to_ci_job_artifacts_file_store.rb diff --git a/changelogs/unreleased/45476-geo-statement-timeout-counting-local-job-artifacts.yml b/changelogs/unreleased/45476-geo-statement-timeout-counting-local-job-artifacts.yml new file mode 100644 index 00000000000..763d3a28200 --- /dev/null +++ b/changelogs/unreleased/45476-geo-statement-timeout-counting-local-job-artifacts.yml @@ -0,0 +1,5 @@ +--- +title: Add index to file_store on ci_job_artifacts +merge_request: 18444 +author: +type: performance diff --git a/db/migrate/20180418053107_add_index_to_ci_job_artifacts_file_store.rb b/db/migrate/20180418053107_add_index_to_ci_job_artifacts_file_store.rb new file mode 100644 index 00000000000..1084ca14a34 --- /dev/null +++ b/db/migrate/20180418053107_add_index_to_ci_job_artifacts_file_store.rb @@ -0,0 +1,15 @@ +class AddIndexToCiJobArtifactsFileStore < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :ci_job_artifacts, :file_store + end + + def down + remove_index :ci_job_artifacts, :file_store if index_exists?(:ci_job_artifacts, :file_store) + end +end diff --git a/db/schema.rb b/db/schema.rb index fd75b176318..364166f6c30 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20180405142733) do +ActiveRecord::Schema.define(version: 20180418053107) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -367,6 +367,7 @@ ActiveRecord::Schema.define(version: 20180405142733) do end add_index "ci_job_artifacts", ["expire_at", "job_id"], name: "index_ci_job_artifacts_on_expire_at_and_job_id", using: :btree + add_index "ci_job_artifacts", ["file_store"], name: "index_ci_job_artifacts_on_file_store", using: :btree add_index "ci_job_artifacts", ["job_id", "file_type"], name: "index_ci_job_artifacts_on_job_id_and_file_type", unique: true, using: :btree add_index "ci_job_artifacts", ["project_id"], name: "index_ci_job_artifacts_on_project_id", using: :btree From 90716733522691e964680539e231d3677bafbc51 Mon Sep 17 00:00:00 2001 From: blackst0ne Date: Thu, 19 Apr 2018 15:42:50 +1100 Subject: [PATCH 11/37] [Rails5] Fix `User#manageable_groups` In `arel 7.0` (`7.1.4` version is used for rails5) there were introduced some changes that break our code in the `User#manageable_groups` method. The problem is that `arel_table[:id].in(Arel::Nodes::SqlLiteral)` generates wrong `IN ()` construction. The selection for `IN` is missing: => "\"namespaces\".\"id\" IN (0)" That caused such spec errors for the `rails5` branch: ``` 4) User groups with child groups #manageable_groups does not include duplicates if a membership was added for the subgroup Failure/Error: expect(user.manageable_groups).to contain_exactly(group, subgroup) expected collection contained: [#, #] actual collection contained: [] the missing elements were: [#, #] # ./spec/models/user_spec.rb:699:in `block (5 levels) in ' # ./spec/spec_helper.rb:188:in `block (2 levels) in ' # /var/lib/gems/2.3.0/gems/rspec-retry-0.4.6/lib/rspec/retry.rb:112:in `block in run' # /var/lib/gems/2.3.0/gems/rspec-retry-0.4.6/lib/rspec/retry.rb:101:in `loop' # /var/lib/gems/2.3.0/gems/rspec-retry-0.4.6/lib/rspec/retry.rb:101:in `run' # /var/lib/gems/2.3.0/gems/rspec-retry-0.4.6/lib/rspec_ext/rspec_ext.rb:12:in `run_with_retry' # /var/lib/gems/2.3.0/gems/rspec-retry-0.4.6/lib/rspec/retry.rb:30:in `block (2 levels) in setup' ``` This commit changes `User#manageable_groups` in the way to drop the usage of `Arel::Nodes::SqlLiteral` and adds usage of raw SQL query. This change should be updated when we're migrated to Rails 5.2 because arel was fixed in `9.0.0` (which is used in Rails 5.2). --- app/models/user.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index d5c5c0964c5..cddc0b8d2e9 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -947,10 +947,13 @@ class User < ActiveRecord::Base end def manageable_groups - union = Gitlab::SQL::Union.new([owned_groups.select(:id), - masters_groups.select(:id)]) - arel_union = Arel::Nodes::SqlLiteral.new(union.to_sql) - owned_and_master_groups = Group.where(Group.arel_table[:id].in(arel_union)) + union_sql = Gitlab::SQL::Union.new([owned_groups.select(:id), masters_groups.select(:id)]).to_sql + + # Update this line to not use raw SQL when migrated to Rails 5.2. + # Either ActiveRecord or Arel constructions are fine. + # This was replaced with the raw SQL construction because of bugs in the arel gem. + # Bugs were fixed in arel 9.0.0 (Rails 5.2). + owned_and_master_groups = Group.where("namespaces.id IN (#{union_sql})") # rubocop:disable GitlabSecurity/SqlInjection Gitlab::GroupHierarchy.new(owned_and_master_groups).base_and_descendants end From 775211bc7076bba14d6e268fb324391124a2751f Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Wed, 18 Apr 2018 22:02:04 -0700 Subject: [PATCH 12/37] Fix N+1 queries when loading participants for a commit note We saw about 10,000 SQL queries for some commits in the NewNoteWorker, which stalled the Sidekiq queue for other new notes. The notification service took up to 8 minutes to process the commits. Avoiding this N+1 query brings the time down significantly. Closes #45526 --- app/models/commit.rb | 2 +- ...h-fix-award-emoji-nplus-one-participants.yml | 5 +++++ spec/models/note_spec.rb | 17 +++++++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/sh-fix-award-emoji-nplus-one-participants.yml diff --git a/app/models/commit.rb b/app/models/commit.rb index de860df4b9c..9750e9298ec 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -248,7 +248,7 @@ class Commit end def notes_with_associations - notes.includes(:author) + notes.includes(:author, :award_emoji) end def merge_requests diff --git a/changelogs/unreleased/sh-fix-award-emoji-nplus-one-participants.yml b/changelogs/unreleased/sh-fix-award-emoji-nplus-one-participants.yml new file mode 100644 index 00000000000..aee26f9824a --- /dev/null +++ b/changelogs/unreleased/sh-fix-award-emoji-nplus-one-participants.yml @@ -0,0 +1,5 @@ +--- +title: Fix N+1 queries when loading participants for a commit note +merge_request: +author: +type: performance diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 86962cd8d61..6a6c71e6c82 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -91,6 +91,23 @@ describe Note do it "keeps the commit around" do expect(note.project.repository.kept_around?(commit.id)).to be_truthy end + + it 'does not generate N+1 queries for participants', :request_store do + def retrieve_participants + commit.notes_with_associations.map(&:participants).to_a + end + + # Project authorization checks are cached, establish a baseline + retrieve_participants + + control_count = ActiveRecord::QueryRecorder.new do + retrieve_participants + end + + create(:note_on_commit, project: note.project, note: 'another note', noteable_id: commit.id) + + expect { retrieve_participants }.not_to exceed_query_limit(control_count) + end end describe 'authorization' do From 27a5deea38d92d7810e2e620ca3671b5bf2e8d36 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 19 Apr 2018 09:28:08 +0200 Subject: [PATCH 13/37] Rename #qa slack channel to #quality in docs --- doc/development/testing_guide/end_to_end_tests.md | 6 +++--- qa/qa/page/README.md | 4 ++-- qa/qa/scenario/test/sanity/selectors.rb | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/development/testing_guide/end_to_end_tests.md b/doc/development/testing_guide/end_to_end_tests.md index d10a797a142..21ec926414d 100644 --- a/doc/development/testing_guide/end_to_end_tests.md +++ b/doc/development/testing_guide/end_to_end_tests.md @@ -67,9 +67,9 @@ and examples in [the `qa/` directory][instance-qa-examples]. ## Where can I ask for help? -You can ask question in the `#qa` channel on Slack (GitLab internal) or you can -find an issue you would like to work on in [the issue tracker][gitlab-qa-issues] -and start a new discussion there. +You can ask question in the `#quality` channel on Slack (GitLab internal) or +you can find an issue you would like to work on in +[the issue tracker][gitlab-qa-issues] and start a new discussion there. [omnibus-gitlab]: https://gitlab.com/gitlab-org/omnibus-gitlab [gitlab-qa]: https://gitlab.com/gitlab-org/gitlab-qa diff --git a/qa/qa/page/README.md b/qa/qa/page/README.md index d38223f690d..2dbc59846e7 100644 --- a/qa/qa/page/README.md +++ b/qa/qa/page/README.md @@ -115,8 +115,8 @@ from within the `qa` directory. ## Where to ask for help? -If you need more information, ask for help on `#qa` channel on Slack (GitLab -Team only). +If you need more information, ask for help on `#quality` channel on Slack +(internal, GitLab Team only). If you are not a Team Member, and you still need help to contribute, please open an issue in GitLab QA issue tracker. diff --git a/qa/qa/scenario/test/sanity/selectors.rb b/qa/qa/scenario/test/sanity/selectors.rb index c87eb5f3dfb..cff320cb751 100644 --- a/qa/qa/scenario/test/sanity/selectors.rb +++ b/qa/qa/scenario/test/sanity/selectors.rb @@ -31,7 +31,7 @@ module QA current changes in this merge request. For more help see documentation in `qa/page/README.md` file or - ask for help on #qa channel on Slack (GitLab Team only). + ask for help on #quality channel on Slack (GitLab Team only). If you are not a Team Member, and you still need help to contribute, please open an issue in GitLab QA issue tracker. From bbccd310573bb70e6b413c2bde915fcb8810716e Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Thu, 19 Apr 2018 10:40:28 +0200 Subject: [PATCH 14/37] Check Gitaly when running `rake dev:setup` Before this change, trying to setup dev environment could be tried without having at least one Gitaly running. Cloning the repositories would fail, but not stop the setup. Given this would lead to an inconsistent state, a check was added if we could connect to the server. Output when it fails: ``` $ rake dev:setup Failed to connect to Gitaly... Error: 14:Connect Failed ``` --- lib/tasks/gitlab/setup.rake | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/tasks/gitlab/setup.rake b/lib/tasks/gitlab/setup.rake index 1d903c81358..f71e69987cb 100644 --- a/lib/tasks/gitlab/setup.rake +++ b/lib/tasks/gitlab/setup.rake @@ -1,9 +1,20 @@ namespace :gitlab do desc "GitLab | Setup production application" task setup: :gitlab_environment do + check_gitaly_connection setup_db end + def check_gitaly_connection + Gitlab.config.repositories.storages.each do |name, _details| + Gitlab::GitalyClient::ServerService.new(name).info + end + rescue GRPC::Unavailable => ex + puts "Failed to connect to Gitaly...".color(:red) + puts "Error: #{ex}" + exit 1 + end + def setup_db warn_user_is_not_gitlab From 276d4eb80cade01f1d035d849a90009d02ae926d Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Wed, 18 Apr 2018 19:11:50 +0100 Subject: [PATCH 15/37] Fix specifying a non-default ref when requesting an archive using the legacy URL --- .../projects/repositories_controller.rb | 11 +++++---- .../45507-fix-repository-archive-url.yml | 6 +++++ .../projects/repositories_controller_spec.rb | 24 +++++++++++++++++++ 3 files changed, 36 insertions(+), 5 deletions(-) create mode 100644 changelogs/unreleased/45507-fix-repository-archive-url.yml diff --git a/app/controllers/projects/repositories_controller.rb b/app/controllers/projects/repositories_controller.rb index 937b0e39cbd..d01f324e6fd 100644 --- a/app/controllers/projects/repositories_controller.rb +++ b/app/controllers/projects/repositories_controller.rb @@ -28,11 +28,12 @@ class Projects::RepositoriesController < Projects::ApplicationController end def assign_archive_vars - @id = params[:id] - - return unless @id - - @ref, @filename = extract_ref(@id) + if params[:id] + @ref, @filename = extract_ref(params[:id]) + else + @ref = params[:ref] + @filename = nil + end rescue InvalidPathError render_404 end diff --git a/changelogs/unreleased/45507-fix-repository-archive-url.yml b/changelogs/unreleased/45507-fix-repository-archive-url.yml new file mode 100644 index 00000000000..548c9c38689 --- /dev/null +++ b/changelogs/unreleased/45507-fix-repository-archive-url.yml @@ -0,0 +1,6 @@ +--- +title: Fix specifying a non-default ref when requesting an archive using the legacy + URL +merge_request: 18468 +author: +type: fixed diff --git a/spec/controllers/projects/repositories_controller_spec.rb b/spec/controllers/projects/repositories_controller_spec.rb index c3b71458e38..a102a3a3c8c 100644 --- a/spec/controllers/projects/repositories_controller_spec.rb +++ b/spec/controllers/projects/repositories_controller_spec.rb @@ -40,6 +40,30 @@ describe Projects::RepositoriesController do expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-archive:") end + it 'handles legacy queries with the ref specified as ref in params' do + get :archive, namespace_id: project.namespace, project_id: project, ref: 'feature', format: 'zip' + + expect(response).to have_gitlab_http_status(200) + expect(assigns(:ref)).to eq('feature') + expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-archive:") + end + + it 'handles legacy queries with the ref specified as id in params' do + get :archive, namespace_id: project.namespace, project_id: project, id: 'feature', format: 'zip' + + expect(response).to have_gitlab_http_status(200) + expect(assigns(:ref)).to eq('feature') + expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-archive:") + end + + it 'prioritizes the id param over the ref param when both are specified' do + get :archive, namespace_id: project.namespace, project_id: project, id: 'feature', ref: 'feature_conflict', format: 'zip' + + expect(response).to have_gitlab_http_status(200) + expect(assigns(:ref)).to eq('feature') + expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-archive:") + end + context "when the service raises an error" do before do allow(Gitlab::Workhorse).to receive(:send_git_archive).and_raise("Archive failed") From 5a29a304be5fc86a5bbe61764c47cdd8069e2103 Mon Sep 17 00:00:00 2001 From: Jacopo Date: Thu, 5 Apr 2018 17:17:02 +0200 Subject: [PATCH 16/37] Shows new branch/mr button even when branch exists --- .../create_merge_request_dropdown.js | 33 ++++++---- app/controllers/projects/issues_controller.rb | 6 +- app/models/issue.rb | 16 +++-- .../42803-show-new-branch-mr-button.yml | 5 ++ spec/javascripts/issue_spec.js | 1 + spec/models/issue_spec.rb | 63 +++++++++++++++++++ 6 files changed, 104 insertions(+), 20 deletions(-) create mode 100644 changelogs/unreleased/42803-show-new-branch-mr-button.yml diff --git a/app/assets/javascripts/create_merge_request_dropdown.js b/app/assets/javascripts/create_merge_request_dropdown.js index fb1fc9cd32e..a88b6971f90 100644 --- a/app/assets/javascripts/create_merge_request_dropdown.js +++ b/app/assets/javascripts/create_merge_request_dropdown.js @@ -84,20 +84,21 @@ export default class CreateMergeRequestDropdown { if (data.can_create_branch) { this.available(); this.enable(); + this.updateBranchName(data.suggested_branch_name); if (!this.droplabInitialized) { this.droplabInitialized = true; this.initDroplab(); this.bindEvents(); } - } else if (data.has_related_branch) { + } else { this.hide(); } }) .catch(() => { this.unavailable(); this.disable(); - Flash('Failed to check if a new branch can be created.'); + Flash(__('Failed to check related branches.')); }); } @@ -409,13 +410,16 @@ export default class CreateMergeRequestDropdown { this.unavailableButton.classList.remove('hide'); } + updateBranchName(suggestedBranchName) { + this.branchInput.value = suggestedBranchName; + this.updateCreatePaths('branch', suggestedBranchName); + } + updateInputState(target, ref, result) { // target - 'branch' or 'ref' - which the input field we are searching a ref for. // ref - string - what a user typed. // result - string - what has been found on backend. - const pathReplacement = `$1${ref}`; - // If a found branch equals exact the same text a user typed, // that means a new branch cannot be created as it already exists. if (ref === result) { @@ -426,18 +430,12 @@ export default class CreateMergeRequestDropdown { this.refIsValid = true; this.refInput.dataset.value = ref; this.showAvailableMessage('ref'); - this.createBranchPath = this.createBranchPath.replace(this.regexps.ref.createBranchPath, - pathReplacement); - this.createMrPath = this.createMrPath.replace(this.regexps.ref.createMrPath, - pathReplacement); + this.updateCreatePaths(target, ref); } } else if (target === 'branch') { this.branchIsValid = true; this.showAvailableMessage('branch'); - this.createBranchPath = this.createBranchPath.replace(this.regexps.branch.createBranchPath, - pathReplacement); - this.createMrPath = this.createMrPath.replace(this.regexps.branch.createMrPath, - pathReplacement); + this.updateCreatePaths(target, ref); } else { this.refIsValid = false; this.refInput.dataset.value = ref; @@ -457,4 +455,15 @@ export default class CreateMergeRequestDropdown { this.disableCreateAction(); } } + + // target - 'branch' or 'ref' + // ref - string - the new value to use as branch or ref + updateCreatePaths(target, ref) { + const pathReplacement = `$1${ref}`; + + this.createBranchPath = this.createBranchPath.replace(this.regexps[target].createBranchPath, + pathReplacement); + this.createMrPath = this.createMrPath.replace(this.regexps[target].createMrPath, + pathReplacement); + } } diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 767e492f566..d69015c8665 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -134,11 +134,11 @@ class Projects::IssuesController < Projects::ApplicationController def can_create_branch can_create = current_user && can?(current_user, :push_code, @project) && - @issue.can_be_worked_on?(current_user) + @issue.can_be_worked_on? respond_to do |format| format.json do - render json: { can_create_branch: can_create, has_related_branch: @issue.has_related_branch? } + render json: { can_create_branch: can_create, suggested_branch_name: @issue.suggested_branch_name } end end end @@ -177,7 +177,7 @@ class Projects::IssuesController < Projects::ApplicationController end def authorize_create_merge_request! - render_404 unless can?(current_user, :push_code, @project) && @issue.can_be_worked_on?(current_user) + render_404 unless can?(current_user, :push_code, @project) && @issue.can_be_worked_on? end def render_issue_json diff --git a/app/models/issue.rb b/app/models/issue.rb index 7611e83647c..c34c35bcd34 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -194,6 +194,15 @@ class Issue < ActiveRecord::Base branches_with_iid - branches_with_merge_request end + def suggested_branch_name + return to_branch_name unless project.repository.branch_exists?(to_branch_name) + + index = 2 + index += 1 while project.repository.branch_exists?("#{to_branch_name}-#{index}") + + "#{to_branch_name}-#{index}" + end + # Returns boolean if a related branch exists for the current issue # ignores merge requests branchs def has_related_branch? @@ -248,11 +257,8 @@ class Issue < ActiveRecord::Base end end - def can_be_worked_on?(current_user) - !self.closed? && - !self.project.forked? && - self.related_branches(current_user).empty? && - self.closed_by_merge_requests(current_user).empty? + def can_be_worked_on? + !self.closed? && !self.project.forked? end # Returns `true` if the current issue can be viewed by either a logged in User diff --git a/changelogs/unreleased/42803-show-new-branch-mr-button.yml b/changelogs/unreleased/42803-show-new-branch-mr-button.yml new file mode 100644 index 00000000000..d689ff7f001 --- /dev/null +++ b/changelogs/unreleased/42803-show-new-branch-mr-button.yml @@ -0,0 +1,5 @@ +--- +title: Show new branch/mr button even when branch exists +merge_request: 17712 +author: Jacopo Beschi @jacopo-beschi +type: added diff --git a/spec/javascripts/issue_spec.js b/spec/javascripts/issue_spec.js index f37426a72d4..047ecab27db 100644 --- a/spec/javascripts/issue_spec.js +++ b/spec/javascripts/issue_spec.js @@ -92,6 +92,7 @@ describe('Issue', function() { function mockCanCreateBranch(canCreateBranch) { mock.onGet(/(.*)\/can_create_branch$/).reply(200, { can_create_branch: canCreateBranch, + suggested_branch_name: 'foo-99', }); } diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 11154291368..128acf83686 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -376,6 +376,48 @@ describe Issue do end end + describe '#suggested_branch_name' do + let(:repository) { double } + + subject { build(:issue) } + + before do + allow(subject.project).to receive(:repository).and_return(repository) + end + + context '#to_branch_name does not exists' do + before do + allow(repository).to receive(:branch_exists?).and_return(false) + end + + it 'returns #to_branch_name' do + expect(subject.suggested_branch_name).to eq(subject.to_branch_name) + end + end + + context '#to_branch_name exists not ending with -index' do + before do + allow(repository).to receive(:branch_exists?).and_return(true) + allow(repository).to receive(:branch_exists?).with(/#{subject.to_branch_name}-\d/).and_return(false) + end + + it 'returns #to_branch_name ending with -2' do + expect(subject.suggested_branch_name).to eq("#{subject.to_branch_name}-2") + end + end + + context '#to_branch_name exists ending with -index' do + before do + allow(repository).to receive(:branch_exists?).and_return(true) + allow(repository).to receive(:branch_exists?).with("#{subject.to_branch_name}-3").and_return(false) + end + + it 'returns #to_branch_name ending with max index + 1' do + expect(subject.suggested_branch_name).to eq("#{subject.to_branch_name}-3") + end + end + end + describe '#has_related_branch?' do let(:issue) { create(:issue, title: "Blue Bell Knoll") } subject { issue.has_related_branch? } @@ -425,6 +467,27 @@ describe Issue do end end + describe '#can_be_worked_on?' do + let(:project) { build(:project) } + subject { build(:issue, :opened, project: project) } + + context 'is closed' do + subject { build(:issue, :closed) } + + it { is_expected.not_to be_can_be_worked_on } + end + + context 'project is forked' do + before do + allow(project).to receive(:forked?).and_return(true) + end + + it { is_expected.not_to be_can_be_worked_on } + end + + it { is_expected.to be_can_be_worked_on } + end + describe '#participants' do context 'using a public project' do let(:project) { create(:project, :public) } From 4f2e494772eb5f31929ecfdb439dfa4baa56521c Mon Sep 17 00:00:00 2001 From: Jacopo Date: Fri, 13 Apr 2018 16:21:22 +0200 Subject: [PATCH 17/37] Checked in new translation keys --- locale/gitlab.pot | 3 +++ 1 file changed, 3 insertions(+) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 0eec9793391..cd30783c274 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1776,6 +1776,9 @@ msgstr "" msgid "Failed to change the owner" msgstr "" +msgid "Failed to check related branches." +msgstr "" + msgid "Failed to remove issue from board, please try again." msgstr "" From 6ae3098eb8f01406190942e8952866dd9af81dde Mon Sep 17 00:00:00 2001 From: Jacopo Date: Thu, 19 Apr 2018 08:59:37 +0200 Subject: [PATCH 18/37] Uses Uniquify to calculate Issue#suggested_branch_name --- app/models/concerns/uniquify.rb | 7 +++++-- app/models/issue.rb | 8 ++++---- spec/models/concerns/uniquify_spec.rb | 7 +++++++ 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/app/models/concerns/uniquify.rb b/app/models/concerns/uniquify.rb index a7fe5951b6e..db51ed2dbeb 100644 --- a/app/models/concerns/uniquify.rb +++ b/app/models/concerns/uniquify.rb @@ -3,11 +3,14 @@ class Uniquify # by appending a counter to it. Uniqueness is determined by # repeated calls to the passed block. # + # You can pass an initial value for the counter, if not given + # counting starts from 1. + # # If `base` is a function/proc, we expect that calling it with a # candidate counter returns a string to test/return. - def string(base) + def string(base, counter = nil) @base = base - @counter = nil + @counter = counter increment_counter! while yield(base_string) base_string diff --git a/app/models/issue.rb b/app/models/issue.rb index c34c35bcd34..51028a404c2 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -197,10 +197,10 @@ class Issue < ActiveRecord::Base def suggested_branch_name return to_branch_name unless project.repository.branch_exists?(to_branch_name) - index = 2 - index += 1 while project.repository.branch_exists?("#{to_branch_name}-#{index}") - - "#{to_branch_name}-#{index}" + start_counting_from = 2 + Uniquify.new.string(-> (counter) { "#{to_branch_name}-#{counter}" }, start_counting_from) do |suggested_branch_name| + project.repository.branch_exists?(suggested_branch_name) + end end # Returns boolean if a related branch exists for the current issue diff --git a/spec/models/concerns/uniquify_spec.rb b/spec/models/concerns/uniquify_spec.rb index 914730718e7..00341213bbe 100644 --- a/spec/models/concerns/uniquify_spec.rb +++ b/spec/models/concerns/uniquify_spec.rb @@ -22,6 +22,13 @@ describe Uniquify do expect(result).to eq('test_string2') end + it 'allows to pass an initial value for the counter' do + start_counting_from = 2 + result = uniquify.string('test_string', start_counting_from) { |s| s == 'test_string' } + + expect(result).to eq('test_string2') + end + it 'allows passing in a base function that defines the location of the counter' do result = uniquify.string(-> (counter) { "test_#{counter}_string" }) do |s| s == 'test__string' From 9000626a60c889c76ff1dfc8c6247f15953ca993 Mon Sep 17 00:00:00 2001 From: Jacopo Date: Thu, 19 Apr 2018 11:31:01 +0200 Subject: [PATCH 19/37] Moves Uniquify counter in the initializer --- app/models/concerns/uniquify.rb | 27 ++++++++++++++++----------- app/models/issue.rb | 2 +- spec/models/concerns/uniquify_spec.rb | 4 +++- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/app/models/concerns/uniquify.rb b/app/models/concerns/uniquify.rb index db51ed2dbeb..549a76da20e 100644 --- a/app/models/concerns/uniquify.rb +++ b/app/models/concerns/uniquify.rb @@ -1,16 +1,21 @@ +# Uniquify +# +# Return a version of the given 'base' string that is unique +# by appending a counter to it. Uniqueness is determined by +# repeated calls to the passed block. +# +# You can pass an initial value for the counter, if not given +# counting starts from 1. +# +# If `base` is a function/proc, we expect that calling it with a +# candidate counter returns a string to test/return. class Uniquify - # Return a version of the given 'base' string that is unique - # by appending a counter to it. Uniqueness is determined by - # repeated calls to the passed block. - # - # You can pass an initial value for the counter, if not given - # counting starts from 1. - # - # If `base` is a function/proc, we expect that calling it with a - # candidate counter returns a string to test/return. - def string(base, counter = nil) - @base = base + def initialize(counter = nil) @counter = counter + end + + def string(base) + @base = base increment_counter! while yield(base_string) base_string diff --git a/app/models/issue.rb b/app/models/issue.rb index 51028a404c2..0332bfa9371 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -198,7 +198,7 @@ class Issue < ActiveRecord::Base return to_branch_name unless project.repository.branch_exists?(to_branch_name) start_counting_from = 2 - Uniquify.new.string(-> (counter) { "#{to_branch_name}-#{counter}" }, start_counting_from) do |suggested_branch_name| + Uniquify.new(start_counting_from).string(-> (counter) { "#{to_branch_name}-#{counter}" }) do |suggested_branch_name| project.repository.branch_exists?(suggested_branch_name) end end diff --git a/spec/models/concerns/uniquify_spec.rb b/spec/models/concerns/uniquify_spec.rb index 00341213bbe..6cd2de6dcce 100644 --- a/spec/models/concerns/uniquify_spec.rb +++ b/spec/models/concerns/uniquify_spec.rb @@ -24,7 +24,9 @@ describe Uniquify do it 'allows to pass an initial value for the counter' do start_counting_from = 2 - result = uniquify.string('test_string', start_counting_from) { |s| s == 'test_string' } + uniquify = described_class.new(start_counting_from) + + result = uniquify.string('test_string') { |s| s == 'test_string' } expect(result).to eq('test_string2') end From 3a7c148d0b353e81bb15a5e5ac1e5ee28f47673f Mon Sep 17 00:00:00 2001 From: Dennis Tang Date: Thu, 19 Apr 2018 12:04:15 +0000 Subject: [PATCH 20/37] Fix typo in vue.md --- doc/development/fe_guide/vue.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/development/fe_guide/vue.md b/doc/development/fe_guide/vue.md index 944b56a65f1..9c4b0e86351 100644 --- a/doc/development/fe_guide/vue.md +++ b/doc/development/fe_guide/vue.md @@ -524,7 +524,7 @@ export default new Vuex.Store({ _Note:_ If the state of the application is too complex, an individual file for the state may be better. ##### `actions.js` -An action commits a mutatation. In this file, we will write the actions that will call the respective mutation: +An action commits a mutation. In this file, we will write the actions that will commit the respective mutation: ```javascript import * as types from './mutation_types'; @@ -661,7 +661,7 @@ describe('component', () => { }; // populate the store - store.dipatch('addUser', user); + store.dispatch('addUser', user); vm = new Component({ store, From 9c5664e699213209bfbc0ef4d4a7efa6ebc75907 Mon Sep 17 00:00:00 2001 From: Gilbert Roulot Date: Thu, 19 Apr 2018 14:17:48 +0200 Subject: [PATCH 21/37] Update Container Scanning documentation --- doc/ci/examples/container_scanning.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/ci/examples/container_scanning.md b/doc/ci/examples/container_scanning.md index dc34f4acd75..eb76521cc02 100644 --- a/doc/ci/examples/container_scanning.md +++ b/doc/ci/examples/container_scanning.md @@ -31,6 +31,9 @@ sast:container: - chmod +x clair-scanner - touch clair-whitelist.yml - while( ! wget -q -O /dev/null http://docker:6060/v1/namespaces ) ; do sleep 1 ; done + - retries=0 + - echo "Waiting for clair daemon to start" + - while( ! wget -T 10 -q -O /dev/null http://docker:6060/v1/namespaces ) ; do sleep 1 ; echo -n "." ; if [ $retries -eq 10 ] ; then echo " Timeout, aborting." ; exit 1 ; fi ; retries=$(($retries+1)) ; done - ./clair-scanner -c http://docker:6060 --ip $(hostname -i) -r gl-sast-container-report.json -l clair.log -w clair-whitelist.yml ${CI_APPLICATION_REPOSITORY}:${CI_APPLICATION_TAG} || true artifacts: paths: [gl-sast-container-report.json] From 49b85262c5a1944d7fdb50e43900a4adb13aba6c Mon Sep 17 00:00:00 2001 From: Dennis Tang Date: Thu, 19 Apr 2018 14:43:20 +0000 Subject: [PATCH 22/37] Resolve "Improve tooltips of collapsed sidebars" --- app/assets/javascripts/due_date_select.js | 9 +- app/assets/javascripts/labels_select.js | 5 +- app/assets/javascripts/milestone_select.js | 14 ++- app/assets/javascripts/right_sidebar.js | 12 ++- .../components/assignees/assignees.vue | 20 +++- .../assignees/sidebar_assignees.vue | 12 ++- .../confidential_issue_sidebar.vue | 19 +++- .../components/lock/lock_issue_sidebar.vue | 21 ++++- .../components/participants/participants.vue | 18 +++- .../time_tracking/collapsed_state.vue | 32 ++++++- .../javascripts/sidebar/mount_sidebar.js | 1 + app/assets/javascripts/users_select.js | 7 +- .../components/sidebar/toggle_sidebar.vue | 39 +++++--- .../stylesheets/framework/variables.scss | 3 + app/assets/stylesheets/pages/issuable.scss | 28 +++++- app/assets/stylesheets/pages/milestone.scss | 14 ++- app/helpers/issuables_helper.rb | 38 +++++++- app/helpers/milestones_helper.rb | 91 +++++++++++++++---- app/models/concerns/milestoneish.rb | 4 +- app/serializers/entity_date_helper.rb | 27 ++++++ app/views/projects/issues/_issue.html.haml | 2 +- .../merge_requests/_merge_request.html.haml | 2 +- app/views/shared/issuable/_sidebar.html.haml | 11 +-- .../issuable/_sidebar_assignees.html.haml | 2 +- .../shared/issuable/_sidebar_todo.html.haml | 6 +- .../form/_merge_request_assignee.html.haml | 2 +- .../shared/milestones/_sidebar.html.haml | 35 ++++--- .../25010-collapsed-sidebar-tooltips.yml | 5 + .../project/issues/issues_functionalities.md | 2 +- doc/workflow/todos.md | 4 +- spec/features/issues/todo_spec.rb | 4 +- spec/helpers/issuables_helper_spec.rb | 8 +- spec/helpers/milestones_helper_spec.rb | 54 ----------- .../collapsed_sidebar_todo_spec.js | 10 +- spec/models/milestone_spec.rb | 12 ++- spec/serializers/entity_date_helper_spec.rb | 55 +++++++++++ 36 files changed, 458 insertions(+), 170 deletions(-) create mode 100644 changelogs/unreleased/25010-collapsed-sidebar-tooltips.yml diff --git a/app/assets/javascripts/due_date_select.js b/app/assets/javascripts/due_date_select.js index 842a4255f08..4164149dd06 100644 --- a/app/assets/javascripts/due_date_select.js +++ b/app/assets/javascripts/due_date_select.js @@ -2,7 +2,9 @@ import $ from 'jquery'; import Pikaday from 'pikaday'; +import { __ } from '~/locale'; import axios from './lib/utils/axios_utils'; +import { timeFor } from './lib/utils/datetime_utility'; import { parsePikadayDate, pikadayToString } from './lib/utils/datefix'; class DueDateSelect { @@ -14,6 +16,7 @@ class DueDateSelect { this.$dropdownParent = $dropdownParent; this.$datePicker = $dropdownParent.find('.js-due-date-calendar'); this.$block = $block; + this.$sidebarCollapsedValue = $block.find('.sidebar-collapsed-icon'); this.$selectbox = $dropdown.closest('.selectbox'); this.$value = $block.find('.value'); this.$valueContent = $block.find('.value-content'); @@ -128,7 +131,8 @@ class DueDateSelect { submitSelectedDate(isDropdown) { const selectedDateValue = this.datePayload[this.abilityName].due_date; - const displayedDateStyle = this.displayedDate !== 'No due date' ? 'bold' : 'no-value'; + const hasDueDate = this.displayedDate !== 'No due date'; + const displayedDateStyle = hasDueDate ? 'bold' : 'no-value'; this.$loading.removeClass('hidden').fadeIn(); @@ -145,10 +149,13 @@ class DueDateSelect { return axios.put(this.issueUpdateURL, this.datePayload) .then(() => { + const tooltipText = hasDueDate ? `${__('Due date')}
${selectedDateValue} (${timeFor(selectedDateValue)})` : __('Due date'); if (isDropdown) { this.$dropdown.trigger('loaded.gl.dropdown'); this.$dropdown.dropdown('toggle'); } + this.$sidebarCollapsedValue.attr('data-original-title', tooltipText); + return this.$loading.fadeOut(); }); } diff --git a/app/assets/javascripts/labels_select.js b/app/assets/javascripts/labels_select.js index d0050abb8e9..9b62cfb8206 100644 --- a/app/assets/javascripts/labels_select.js +++ b/app/assets/javascripts/labels_select.js @@ -83,7 +83,7 @@ export default class LabelsSelect { $dropdown.trigger('loading.gl.dropdown'); axios.put(issueUpdateURL, data) .then(({ data }) => { - var labelCount, template, labelTooltipTitle, labelTitles; + var labelCount, template, labelTooltipTitle, labelTitles, formattedLabels; $loading.fadeOut(); $dropdown.trigger('loaded.gl.dropdown'); $selectbox.hide(); @@ -115,8 +115,7 @@ export default class LabelsSelect { labelTooltipTitle = labelTitles.join(', '); } else { - labelTooltipTitle = ''; - $sidebarLabelTooltip.tooltip('destroy'); + labelTooltipTitle = __('Labels'); } $sidebarLabelTooltip diff --git a/app/assets/javascripts/milestone_select.js b/app/assets/javascripts/milestone_select.js index d0a2b27b0e6..7e9a50a885d 100644 --- a/app/assets/javascripts/milestone_select.js +++ b/app/assets/javascripts/milestone_select.js @@ -4,6 +4,7 @@ import $ from 'jquery'; import _ from 'underscore'; +import { __ } from '~/locale'; import axios from './lib/utils/axios_utils'; import { timeFor } from './lib/utils/datetime_utility'; import ModalStore from './boards/stores/modal_store'; @@ -25,7 +26,7 @@ export default class MilestoneSelect { } $els.each((i, dropdown) => { - let collapsedSidebarLabelTemplate, milestoneLinkNoneTemplate, milestoneLinkTemplate, selectedMilestone, selectedMilestoneDefault; + let milestoneLinkNoneTemplate, milestoneLinkTemplate, selectedMilestone, selectedMilestoneDefault; const $dropdown = $(dropdown); const projectId = $dropdown.data('projectId'); const milestonesUrl = $dropdown.data('milestones'); @@ -52,7 +53,6 @@ export default class MilestoneSelect { if (issueUpdateURL) { milestoneLinkTemplate = _.template('<%- title %>'); milestoneLinkNoneTemplate = 'None'; - collapsedSidebarLabelTemplate = _.template(' <%- title %> '); } return $dropdown.glDropdown({ showMenuAbove: showMenuAbove, @@ -214,10 +214,16 @@ export default class MilestoneSelect { data.milestone.remaining = timeFor(data.milestone.due_date); data.milestone.name = data.milestone.title; $value.html(milestoneLinkTemplate(data.milestone)); - return $sidebarCollapsedValue.find('span').html(collapsedSidebarLabelTemplate(data.milestone)); + return $sidebarCollapsedValue + .attr('data-original-title', `${data.milestone.name}
${data.milestone.remaining}`) + .find('span') + .text(data.milestone.title); } else { $value.html(milestoneLinkNoneTemplate); - return $sidebarCollapsedValue.find('span').text('No'); + return $sidebarCollapsedValue + .attr('data-original-title', __('Milestone')) + .find('span') + .text(__('None')); } }) .catch(() => { diff --git a/app/assets/javascripts/right_sidebar.js b/app/assets/javascripts/right_sidebar.js index 2088a49590a..6eb0b62fa1c 100644 --- a/app/assets/javascripts/right_sidebar.js +++ b/app/assets/javascripts/right_sidebar.js @@ -5,6 +5,7 @@ import _ from 'underscore'; import Cookies from 'js-cookie'; import flash from './flash'; import axios from './lib/utils/axios_utils'; +import { __ } from './locale'; function Sidebar(currentUser) { this.toggleTodo = this.toggleTodo.bind(this); @@ -41,12 +42,14 @@ Sidebar.prototype.addEventListeners = function() { }; Sidebar.prototype.sidebarToggleClicked = function (e, triggered) { - var $allGutterToggleIcons, $this, $thisIcon; + var $allGutterToggleIcons, $this, isExpanded, tooltipLabel; e.preventDefault(); $this = $(this); - $thisIcon = $this.find('i'); + isExpanded = $this.find('i').hasClass('fa-angle-double-right'); + tooltipLabel = isExpanded ? __('Expand sidebar') : __('Collapse sidebar'); $allGutterToggleIcons = $('.js-sidebar-toggle i'); - if ($thisIcon.hasClass('fa-angle-double-right')) { + + if (isExpanded) { $allGutterToggleIcons.removeClass('fa-angle-double-right').addClass('fa-angle-double-left'); $('aside.right-sidebar').removeClass('right-sidebar-expanded').addClass('right-sidebar-collapsed'); $('.layout-page').removeClass('right-sidebar-expanded').addClass('right-sidebar-collapsed'); @@ -57,6 +60,9 @@ Sidebar.prototype.sidebarToggleClicked = function (e, triggered) { if (gl.lazyLoader) gl.lazyLoader.loadCheck(); } + + $this.attr('data-original-title', tooltipLabel); + if (!triggered) { Cookies.set("collapsed_gutter", $('.right-sidebar').hasClass('right-sidebar-collapsed')); } diff --git a/app/assets/javascripts/sidebar/components/assignees/assignees.vue b/app/assets/javascripts/sidebar/components/assignees/assignees.vue index 1e7f46454bf..2d00e8ac7e0 100644 --- a/app/assets/javascripts/sidebar/components/assignees/assignees.vue +++ b/app/assets/javascripts/sidebar/components/assignees/assignees.vue @@ -1,6 +1,12 @@