From 74b87f02db2ebda0b2b16a60dd6759fe6e8de95a Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Fri, 10 Nov 2017 17:41:04 -0600 Subject: Backport of add-epic-sidebar --- .../javascripts/lib/utils/datetime_utility.js | 15 +- app/assets/javascripts/lib/utils/text_utility.js | 4 + .../javascripts/vue_shared/components/pikaday.vue | 79 ++++++++++ .../components/sidebar/collapsed_calendar_icon.vue | 46 ++++++ .../sidebar/collapsed_grouped_date_picker.vue | 109 ++++++++++++++ .../vue_shared/components/sidebar/date_picker.vue | 163 +++++++++++++++++++++ .../components/sidebar/toggle_sidebar.vue | 30 ++++ app/assets/stylesheets/framework/buttons.scss | 23 +++ app/assets/stylesheets/framework/sidebar.scss | 25 +++- app/assets/stylesheets/pages/issuable.scss | 29 +++- spec/javascripts/datetime_utility_spec.js | 22 ++- spec/javascripts/lib/utils/text_utility_spec.js | 14 +- .../vue_shared/components/pikaday_spec.js | 29 ++++ .../sidebar/collapsed_calendar_icon_spec.js | 35 +++++ .../sidebar/collapsed_grouped_date_picker_spec.js | 91 ++++++++++++ .../components/sidebar/date_picker_spec.js | 117 +++++++++++++++ .../components/sidebar/toggle_sidebar_spec.js | 32 ++++ 17 files changed, 849 insertions(+), 14 deletions(-) create mode 100644 app/assets/javascripts/vue_shared/components/pikaday.vue create mode 100644 app/assets/javascripts/vue_shared/components/sidebar/collapsed_calendar_icon.vue create mode 100644 app/assets/javascripts/vue_shared/components/sidebar/collapsed_grouped_date_picker.vue create mode 100644 app/assets/javascripts/vue_shared/components/sidebar/date_picker.vue create mode 100644 app/assets/javascripts/vue_shared/components/sidebar/toggle_sidebar.vue create mode 100644 spec/javascripts/vue_shared/components/pikaday_spec.js create mode 100644 spec/javascripts/vue_shared/components/sidebar/collapsed_calendar_icon_spec.js create mode 100644 spec/javascripts/vue_shared/components/sidebar/collapsed_grouped_date_picker_spec.js create mode 100644 spec/javascripts/vue_shared/components/sidebar/date_picker_spec.js create mode 100644 spec/javascripts/vue_shared/components/sidebar/toggle_sidebar_spec.js diff --git a/app/assets/javascripts/lib/utils/datetime_utility.js b/app/assets/javascripts/lib/utils/datetime_utility.js index 29fc91733b3..8d0bab55843 100644 --- a/app/assets/javascripts/lib/utils/datetime_utility.js +++ b/app/assets/javascripts/lib/utils/datetime_utility.js @@ -135,7 +135,6 @@ window.dateFormat = dateFormat; * @param {Number} seconds * @return {String} */ -// eslint-disable-next-line import/prefer-default-export export function timeIntervalInWords(intervalInSeconds) { const secondsInteger = parseInt(intervalInSeconds, 10); const minutes = Math.floor(secondsInteger / 60); @@ -149,3 +148,17 @@ export function timeIntervalInWords(intervalInSeconds) { } return text; } + +export function dateInWords(date, abbreviated = false) { + if (!date) return date; + + const month = date.getMonth(); + const year = date.getFullYear(); + + const monthNames = [s__('January'), s__('February'), s__('March'), s__('April'), s__('May'), s__('June'), s__('July'), s__('August'), s__('September'), s__('October'), s__('November'), s__('December')]; + const monthNamesAbbr = [s__('Jan'), s__('Feb'), s__('Mar'), s__('Apr'), s__('May'), s__('Jun'), s__('Jul'), s__('Aug'), s__('Sep'), s__('Oct'), s__('Nov'), s__('Dec')]; + + const monthName = abbreviated ? monthNamesAbbr[month] : monthNames[month]; + + return `${monthName} ${date.getDate()}, ${year}`; +} diff --git a/app/assets/javascripts/lib/utils/text_utility.js b/app/assets/javascripts/lib/utils/text_utility.js index f776829f69c..0decfff6921 100644 --- a/app/assets/javascripts/lib/utils/text_utility.js +++ b/app/assets/javascripts/lib/utils/text_utility.js @@ -24,6 +24,10 @@ export function highCountTrim(count) { return count > 99 ? '99+' : count; } +export function capitalizeFirstCharacter(text) { + return `${text[0].toUpperCase()}${text.slice(1)}`; +} + gl.text.randomString = function() { return Math.random().toString(36).substring(7); }; diff --git a/app/assets/javascripts/vue_shared/components/pikaday.vue b/app/assets/javascripts/vue_shared/components/pikaday.vue new file mode 100644 index 00000000000..d8d974a2ff7 --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/pikaday.vue @@ -0,0 +1,79 @@ + + + diff --git a/app/assets/javascripts/vue_shared/components/sidebar/collapsed_calendar_icon.vue b/app/assets/javascripts/vue_shared/components/sidebar/collapsed_calendar_icon.vue new file mode 100644 index 00000000000..a88e1310131 --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/sidebar/collapsed_calendar_icon.vue @@ -0,0 +1,46 @@ + + + diff --git a/app/assets/javascripts/vue_shared/components/sidebar/collapsed_grouped_date_picker.vue b/app/assets/javascripts/vue_shared/components/sidebar/collapsed_grouped_date_picker.vue new file mode 100644 index 00000000000..9ede5553bc5 --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/sidebar/collapsed_grouped_date_picker.vue @@ -0,0 +1,109 @@ + + + diff --git a/app/assets/javascripts/vue_shared/components/sidebar/date_picker.vue b/app/assets/javascripts/vue_shared/components/sidebar/date_picker.vue new file mode 100644 index 00000000000..9c3413377a3 --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/sidebar/date_picker.vue @@ -0,0 +1,163 @@ + + + diff --git a/app/assets/javascripts/vue_shared/components/sidebar/toggle_sidebar.vue b/app/assets/javascripts/vue_shared/components/sidebar/toggle_sidebar.vue new file mode 100644 index 00000000000..5ae76adad71 --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/sidebar/toggle_sidebar.vue @@ -0,0 +1,30 @@ + + + diff --git a/app/assets/stylesheets/framework/buttons.scss b/app/assets/stylesheets/framework/buttons.scss index b2f26cf7159..dfa3d4c6fb9 100644 --- a/app/assets/stylesheets/framework/buttons.scss +++ b/app/assets/stylesheets/framework/buttons.scss @@ -408,6 +408,7 @@ padding: 0; background: transparent; border: 0; + border-radius: 0; &:hover, &:active, @@ -417,3 +418,25 @@ box-shadow: none; } } + +.btn-link.btn-secondary-hover-link { + color: $gl-text-color-secondary; + + &:hover, + &:active, + &:focus { + color: $gl-link-color; + text-decoration: none; + } +} + +.btn-link.btn-primary-hover-link { + color: inherit; + + &:hover, + &:active, + &:focus { + color: $gl-link-color; + text-decoration: none; + } +} diff --git a/app/assets/stylesheets/framework/sidebar.scss b/app/assets/stylesheets/framework/sidebar.scss index 1a19b7320a0..792981fdc48 100644 --- a/app/assets/stylesheets/framework/sidebar.scss +++ b/app/assets/stylesheets/framework/sidebar.scss @@ -43,11 +43,13 @@ } .sidebar-collapsed-icon { - cursor: pointer; - .btn { background-color: $gray-light; } + + &:not(.disabled) { + cursor: pointer; + } } } @@ -55,6 +57,10 @@ padding-right: 0; z-index: 300; + .btn-sidebar-action { + display: inline-flex; + } + @media (min-width: $screen-sm-min) and (max-width: $screen-sm-max) { &:not(.wiki-sidebar):not(.build-sidebar):not(.issuable-bulk-update-sidebar) .content-wrapper { padding-right: $gutter_collapsed_width; @@ -136,3 +142,18 @@ .issuable-sidebar { @include new-style-dropdown; } + +.pikaday-container { + .pika-single { + margin-top: 2px; + width: 250px; + } + + .dropdown-menu-toggle { + line-height: 20px; + } +} + +.sidebar-collapsed-icon .sidebar-collapsed-value { + font-size: 12px; +} diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index 760c7c80aff..d9d00197dc0 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -284,10 +284,15 @@ font-weight: $gl-font-weight-normal; } - .no-value { + .no-value, + .btn-secondary-hover-link { color: $gl-text-color-secondary; } + .btn-secondary-hover-link:hover { + color: $gl-link-color; + } + .sidebar-collapsed-icon { display: none; } @@ -295,6 +300,8 @@ .gutter-toggle { margin-top: 7px; border-left: 1px solid $border-gray-normal; + padding-left: 0; + text-align: center; } .title .gutter-toggle { @@ -367,7 +374,7 @@ fill: $issuable-sidebar-color; } - &:hover, + &:hover:not(.disabled), &:hover .todo-undone { color: $gl-text-color; @@ -908,3 +915,21 @@ margin: 0 3px; } } + +.right-sidebar-collapsed { + .sidebar-grouped-item { + .sidebar-collapsed-icon { + margin-bottom: 0; + } + + .sidebar-collapsed-divider { + line-height: 5px; + font-size: 12px; + color: $theme-gray-700; + + + .sidebar-collapsed-icon { + padding-top: 0; + } + } + } +} diff --git a/spec/javascripts/datetime_utility_spec.js b/spec/javascripts/datetime_utility_spec.js index 3391cade541..0f7bf9ec712 100644 --- a/spec/javascripts/datetime_utility_spec.js +++ b/spec/javascripts/datetime_utility_spec.js @@ -1,4 +1,4 @@ -import { timeIntervalInWords } from '~/lib/utils/datetime_utility'; +import * as datetimeUtility from '~/lib/utils/datetime_utility'; (() => { describe('Date time utils', () => { @@ -89,10 +89,22 @@ import { timeIntervalInWords } from '~/lib/utils/datetime_utility'; describe('timeIntervalInWords', () => { it('should return string with number of minutes and seconds', () => { - expect(timeIntervalInWords(9.54)).toEqual('9 seconds'); - expect(timeIntervalInWords(1)).toEqual('1 second'); - expect(timeIntervalInWords(200)).toEqual('3 minutes 20 seconds'); - expect(timeIntervalInWords(6008)).toEqual('100 minutes 8 seconds'); + expect(datetimeUtility.timeIntervalInWords(9.54)).toEqual('9 seconds'); + expect(datetimeUtility.timeIntervalInWords(1)).toEqual('1 second'); + expect(datetimeUtility.timeIntervalInWords(200)).toEqual('3 minutes 20 seconds'); + expect(datetimeUtility.timeIntervalInWords(6008)).toEqual('100 minutes 8 seconds'); + }); + }); + + describe('dateInWords', () => { + const date = new Date('07/01/2016'); + + it('should return date in words', () => { + expect(datetimeUtility.dateInWords(date)).toEqual('July 1, 2016'); + }); + + it('should return abbreviated month name', () => { + expect(datetimeUtility.dateInWords(date, true)).toEqual('Jul 1, 2016'); }); }); })(); diff --git a/spec/javascripts/lib/utils/text_utility_spec.js b/spec/javascripts/lib/utils/text_utility_spec.js index 829b3ef5735..f2e2ce79d27 100644 --- a/spec/javascripts/lib/utils/text_utility_spec.js +++ b/spec/javascripts/lib/utils/text_utility_spec.js @@ -1,4 +1,4 @@ -import { highCountTrim } from '~/lib/utils/text_utility'; +import * as textUtility from '~/lib/utils/text_utility'; describe('text_utility', () => { describe('gl.text.getTextWidth', () => { @@ -37,12 +37,18 @@ describe('text_utility', () => { describe('highCountTrim', () => { it('returns 99+ for count >= 100', () => { - expect(highCountTrim(105)).toBe('99+'); - expect(highCountTrim(100)).toBe('99+'); + expect(textUtility.highCountTrim(105)).toBe('99+'); + expect(textUtility.highCountTrim(100)).toBe('99+'); }); it('returns exact number for count < 100', () => { - expect(highCountTrim(45)).toBe(45); + expect(textUtility.highCountTrim(45)).toBe(45); + }); + }); + + describe('capitalizeFirstCharacter', () => { + it('returns string with first letter capitalized', () => { + expect(textUtility.capitalizeFirstCharacter('gitlab')).toEqual('Gitlab'); }); }); diff --git a/spec/javascripts/vue_shared/components/pikaday_spec.js b/spec/javascripts/vue_shared/components/pikaday_spec.js new file mode 100644 index 00000000000..47af9534737 --- /dev/null +++ b/spec/javascripts/vue_shared/components/pikaday_spec.js @@ -0,0 +1,29 @@ +import Vue from 'vue'; +import datePicker from '~/vue_shared/components/pikaday.vue'; +import mountComponent from '../../helpers/vue_mount_component_helper'; + +describe('datePicker', () => { + let vm; + beforeEach(() => { + const DatePicker = Vue.extend(datePicker); + vm = mountComponent(DatePicker, { + label: 'label', + }); + }); + + it('should render label text', () => { + expect(vm.$el.querySelector('.dropdown-toggle-text').innerText.trim()).toEqual('label'); + }); + + it('should show calendar', () => { + expect(vm.$el.querySelector('.pika-single')).toBeDefined(); + }); + + it('should toggle when dropdown is clicked', () => { + const hidePicker = jasmine.createSpy(); + vm.$on('hidePicker', hidePicker); + + vm.$el.querySelector('.dropdown-menu-toggle').click(); + expect(hidePicker).toHaveBeenCalled(); + }); +}); diff --git a/spec/javascripts/vue_shared/components/sidebar/collapsed_calendar_icon_spec.js b/spec/javascripts/vue_shared/components/sidebar/collapsed_calendar_icon_spec.js new file mode 100644 index 00000000000..cce53193870 --- /dev/null +++ b/spec/javascripts/vue_shared/components/sidebar/collapsed_calendar_icon_spec.js @@ -0,0 +1,35 @@ +import Vue from 'vue'; +import collapsedCalendarIcon from '~/vue_shared/components/sidebar/collapsed_calendar_icon.vue'; +import mountComponent from '../../../helpers/vue_mount_component_helper'; + +describe('collapsedCalendarIcon', () => { + let vm; + beforeEach(() => { + const CollapsedCalendarIcon = Vue.extend(collapsedCalendarIcon); + vm = mountComponent(CollapsedCalendarIcon, { + containerClass: 'test-class', + text: 'text', + showIcon: false, + }); + }); + + it('should add class to container', () => { + expect(vm.$el.classList.contains('test-class')).toEqual(true); + }); + + it('should hide calendar icon if showIcon', () => { + expect(vm.$el.querySelector('.fa-calendar')).toBeNull(); + }); + + it('should render text', () => { + expect(vm.$el.querySelector('span').innerText.trim()).toEqual('text'); + }); + + it('should emit click event when container is clicked', () => { + const click = jasmine.createSpy(); + vm.$on('click', click); + + vm.$el.click(); + expect(click).toHaveBeenCalled(); + }); +}); diff --git a/spec/javascripts/vue_shared/components/sidebar/collapsed_grouped_date_picker_spec.js b/spec/javascripts/vue_shared/components/sidebar/collapsed_grouped_date_picker_spec.js new file mode 100644 index 00000000000..20363e78094 --- /dev/null +++ b/spec/javascripts/vue_shared/components/sidebar/collapsed_grouped_date_picker_spec.js @@ -0,0 +1,91 @@ +import Vue from 'vue'; +import collapsedGroupedDatePicker from '~/vue_shared/components/sidebar/collapsed_grouped_date_picker.vue'; +import mountComponent from '../../../helpers/vue_mount_component_helper'; + +describe('collapsedGroupedDatePicker', () => { + let vm; + beforeEach(() => { + const CollapsedGroupedDatePicker = Vue.extend(collapsedGroupedDatePicker); + vm = mountComponent(CollapsedGroupedDatePicker, { + showToggleSidebar: true, + }); + }); + + it('should render toggle sidebar if showToggleSidebar', (done) => { + expect(vm.$el.querySelector('.issuable-sidebar-header')).toBeDefined(); + + vm.showToggleSidebar = false; + Vue.nextTick(() => { + expect(vm.$el.querySelector('.issuable-sidebar-header')).toBeNull(); + done(); + }); + }); + + it('toggleCollapse events', () => { + const toggleCollapse = jasmine.createSpy(); + + beforeEach((done) => { + vm.minDate = new Date('07/17/2016'); + Vue.nextTick(done); + }); + + it('should emit when sidebar is toggled', () => { + vm.$el.querySelector('.gutter-toggle').click(); + expect(toggleCollapse).toHaveBeenCalled(); + }); + + it('should emit when collapsed-calendar-icon is clicked', () => { + vm.$el.querySelector('.sidebar-collapsed-icon').click(); + expect(toggleCollapse).toHaveBeenCalled(); + }); + }); + + describe('minDate and maxDate', () => { + beforeEach((done) => { + vm.minDate = new Date('07/17/2016'); + vm.maxDate = new Date('07/17/2017'); + Vue.nextTick(done); + }); + + it('should render both collapsed-calendar-icon', () => { + const icons = vm.$el.querySelectorAll('.sidebar-collapsed-icon'); + expect(icons.length).toEqual(2); + expect(icons[0].innerText.trim()).toEqual('Jul 17 2016'); + expect(icons[1].innerText.trim()).toEqual('Jul 17 2017'); + }); + }); + + describe('minDate', () => { + beforeEach((done) => { + vm.minDate = new Date('07/17/2016'); + Vue.nextTick(done); + }); + + it('should render minDate in collapsed-calendar-icon', () => { + const icons = vm.$el.querySelectorAll('.sidebar-collapsed-icon'); + expect(icons.length).toEqual(1); + expect(icons[0].innerText.trim()).toEqual('From Jul 17 2016'); + }); + }); + + describe('maxDate', () => { + beforeEach((done) => { + vm.maxDate = new Date('07/17/2017'); + Vue.nextTick(done); + }); + + it('should render maxDate in collapsed-calendar-icon', () => { + const icons = vm.$el.querySelectorAll('.sidebar-collapsed-icon'); + expect(icons.length).toEqual(1); + expect(icons[0].innerText.trim()).toEqual('Until Jul 17 2017'); + }); + }); + + describe('no dates', () => { + it('should render None', () => { + const icons = vm.$el.querySelectorAll('.sidebar-collapsed-icon'); + expect(icons.length).toEqual(1); + expect(icons[0].innerText.trim()).toEqual('None'); + }); + }); +}); diff --git a/spec/javascripts/vue_shared/components/sidebar/date_picker_spec.js b/spec/javascripts/vue_shared/components/sidebar/date_picker_spec.js new file mode 100644 index 00000000000..926e11b4d30 --- /dev/null +++ b/spec/javascripts/vue_shared/components/sidebar/date_picker_spec.js @@ -0,0 +1,117 @@ +import Vue from 'vue'; +import sidebarDatePicker from '~/vue_shared/components/sidebar/date_picker.vue'; +import mountComponent from '../../../helpers/vue_mount_component_helper'; + +describe('sidebarDatePicker', () => { + let vm; + beforeEach(() => { + const SidebarDatePicker = Vue.extend(sidebarDatePicker); + vm = mountComponent(SidebarDatePicker, { + label: 'label', + isLoading: true, + }); + }); + + it('should emit toggleCollapse when collapsed toggle sidebar is clicked', () => { + const toggleCollapse = jasmine.createSpy(); + vm.$on('toggleCollapse', toggleCollapse); + + vm.$el.querySelector('.issuable-sidebar-header .gutter-toggle').click(); + expect(toggleCollapse).toHaveBeenCalled(); + }); + + it('should render collapsed-calendar-icon', () => { + expect(vm.$el.querySelector('.sidebar-collapsed-icon')).toBeDefined(); + }); + + it('should render label', () => { + expect(vm.$el.querySelector('.title').innerText.trim()).toEqual('label'); + }); + + it('should render loading-icon when isLoading', () => { + expect(vm.$el.querySelector('.fa-spin')).toBeDefined(); + }); + + it('should render value when not editing', () => { + expect(vm.$el.querySelector('.value-content')).toBeDefined(); + }); + + it('should render None if there is no selectedDate', () => { + expect(vm.$el.querySelector('.value-content span').innerText.trim()).toEqual('None'); + }); + + it('should render date-picker when editing', (done) => { + vm.editing = true; + Vue.nextTick(() => { + expect(vm.$el.querySelector('.pika-label')).toBeDefined(); + done(); + }); + }); + + describe('editable', () => { + beforeEach((done) => { + vm.editable = true; + Vue.nextTick(done); + }); + + it('should render edit button', () => { + expect(vm.$el.querySelector('.title .btn-blank').innerText.trim()).toEqual('Edit'); + }); + + it('should enable editing when edit button is clicked', (done) => { + vm.isLoading = false; + Vue.nextTick(() => { + vm.$el.querySelector('.title .btn-blank').click(); + expect(vm.editing).toEqual(true); + done(); + }); + }); + }); + + it('should render date if selectedDate', (done) => { + vm.selectedDate = new Date('07/07/2017'); + Vue.nextTick(() => { + expect(vm.$el.querySelector('.value-content strong').innerText.trim()).toEqual('Jul 7, 2017'); + done(); + }); + }); + + describe('selectedDate and editable', () => { + beforeEach((done) => { + vm.selectedDate = new Date('07/07/2017'); + vm.editable = true; + Vue.nextTick(done); + }); + + it('should render remove button if selectedDate and editable', () => { + expect(vm.$el.querySelector('.value-content .btn-blank').innerText.trim()).toEqual('remove'); + }); + + it('should emit saveDate when remove button is clicked', () => { + const saveDate = jasmine.createSpy(); + vm.$on('saveDate', saveDate); + + vm.$el.querySelector('.value-content .btn-blank').click(); + expect(saveDate).toHaveBeenCalled(); + }); + }); + + describe('showToggleSidebar', () => { + beforeEach((done) => { + vm.showToggleSidebar = true; + Vue.nextTick(done); + }); + + it('should render toggle-sidebar when showToggleSidebar', () => { + expect(vm.$el.querySelector('.title .gutter-toggle')).toBeDefined(); + }); + + it('should emit toggleCollapse when toggle sidebar is clicked', () => { + const toggleCollapse = jasmine.createSpy(); + vm.$on('toggleCollapse', toggleCollapse); + + vm.$el.querySelector('.title .gutter-toggle').click(); + expect(toggleCollapse).toHaveBeenCalled(); + }); + }); +}); diff --git a/spec/javascripts/vue_shared/components/sidebar/toggle_sidebar_spec.js b/spec/javascripts/vue_shared/components/sidebar/toggle_sidebar_spec.js new file mode 100644 index 00000000000..752a9e89d50 --- /dev/null +++ b/spec/javascripts/vue_shared/components/sidebar/toggle_sidebar_spec.js @@ -0,0 +1,32 @@ +import Vue from 'vue'; +import toggleSidebar from '~/vue_shared/components/sidebar/toggle_sidebar.vue'; +import mountComponent from '../../../helpers/vue_mount_component_helper'; + +describe('toggleSidebar', () => { + let vm; + beforeEach(() => { + const ToggleSidebar = Vue.extend(toggleSidebar); + vm = mountComponent(ToggleSidebar, { + collapsed: true, + }); + }); + + it('should render << when collapsed', () => { + expect(vm.$el.querySelector('.fa').classList.contains('fa-angle-double-left')).toEqual(true); + }); + + it('should render >> when collapsed', () => { + vm.collapsed = false; + Vue.nextTick(() => { + expect(vm.$el.querySelector('.fa').classList.contains('fa-angle-double-right')).toEqual(true); + }); + }); + + it('should emit toggle event when button clicked', () => { + const toggle = jasmine.createSpy(); + vm.$on('toggle', toggle); + vm.$el.click(); + + expect(toggle).toHaveBeenCalled(); + }); +}); -- cgit v1.2.1 From 56a04158464048d7a8eff6f09044085cc62a0656 Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Thu, 16 Nov 2017 13:33:35 -0600 Subject: Fix potential EE conflict --- app/assets/javascripts/lib/utils/datetime_utility.js | 1 + app/assets/javascripts/lib/utils/text_utility.js | 19 +++++++++---------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/lib/utils/datetime_utility.js b/app/assets/javascripts/lib/utils/datetime_utility.js index 52bdc05a58c..426a81a976d 100644 --- a/app/assets/javascripts/lib/utils/datetime_utility.js +++ b/app/assets/javascripts/lib/utils/datetime_utility.js @@ -136,6 +136,7 @@ window.dateFormat = dateFormat; * @param {Number} seconds * @return {String} */ +// eslint-disable-next-line import/prefer-default-export export function timeIntervalInWords(intervalInSeconds) { const secondsInteger = parseInt(intervalInSeconds, 10); const minutes = Math.floor(secondsInteger / 60); diff --git a/app/assets/javascripts/lib/utils/text_utility.js b/app/assets/javascripts/lib/utils/text_utility.js index 5f027769d4f..dc0073be5c0 100644 --- a/app/assets/javascripts/lib/utils/text_utility.js +++ b/app/assets/javascripts/lib/utils/text_utility.js @@ -17,16 +17,6 @@ export const addDelimiter = text => (text ? text.toString().replace(/\B(?=(\d{3} */ export const highCountTrim = count => (count > 99 ? '99+' : count); -/** - * Capitalizes first character - * - * @param {String} text - * @return {String} - */ -export function capitalizeFirstCharacter(text) { - return `${text[0].toUpperCase()}${text.slice(1)}`; -} - /** * Converst first char to uppercase and replaces undercores with spaces * @param {String} string @@ -65,3 +55,12 @@ export const slugify = str => str.trim().toLowerCase(); */ export const truncate = (string, maxLength) => `${string.substr(0, (maxLength - 3))}...`; +/** + * Capitalizes first character + * + * @param {String} text + * @return {String} + */ +export function capitalizeFirstCharacter(text) { + return `${text[0].toUpperCase()}${text.slice(1)}`; +} -- cgit v1.2.1 From fe93f9827537e4d761b1874218b009668a914ae4 Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Fri, 17 Nov 2017 16:53:18 +0000 Subject: Update text_utility.js --- app/assets/javascripts/lib/utils/text_utility.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/lib/utils/text_utility.js b/app/assets/javascripts/lib/utils/text_utility.js index dc0073be5c0..9280b7f150c 100644 --- a/app/assets/javascripts/lib/utils/text_utility.js +++ b/app/assets/javascripts/lib/utils/text_utility.js @@ -18,7 +18,7 @@ export const addDelimiter = text => (text ? text.toString().replace(/\B(?=(\d{3} export const highCountTrim = count => (count > 99 ? '99+' : count); /** - * Converst first char to uppercase and replaces undercores with spaces + * Converts first char to uppercase and replaces undercores with spaces * @param {String} string * @requires {String} */ -- cgit v1.2.1 From 880c9a2c7860001f22a181ed36f559b62341e148 Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Wed, 22 Nov 2017 17:20:23 -0600 Subject: Backport fix epic fullscreen --- app/assets/javascripts/dropzone_input.js | 5 +- app/assets/javascripts/zen_mode.js | 8 +- spec/javascripts/zen_mode_spec.js | 146 +++++++++++++++++-------------- 3 files changed, 91 insertions(+), 68 deletions(-) diff --git a/app/assets/javascripts/dropzone_input.js b/app/assets/javascripts/dropzone_input.js index b7747ee3f83..c84be42649a 100644 --- a/app/assets/javascripts/dropzone_input.js +++ b/app/assets/javascripts/dropzone_input.js @@ -36,7 +36,10 @@ export default function dropzoneInput(form) { $formDropzone.append(divHover); $formDropzone.find('.div-dropzone-hover').append(iconPaperclip); - if (!uploadsPath) return; + if (!uploadsPath) { + $formDropzone.addClass('js-invalid-dropzone'); + return; + } const dropzone = $formDropzone.dropzone({ url: uploadsPath, diff --git a/app/assets/javascripts/zen_mode.js b/app/assets/javascripts/zen_mode.js index cba7b9227cd..06a86f3b94a 100644 --- a/app/assets/javascripts/zen_mode.js +++ b/app/assets/javascripts/zen_mode.js @@ -71,7 +71,7 @@ export default class ZenMode { this.active_textarea = this.active_backdrop.find('textarea'); // Prevent a user-resized textarea from persisting to fullscreen this.active_textarea.removeAttr('style'); - return this.active_textarea.focus(); + this.active_textarea.focus(); } exit() { @@ -81,7 +81,11 @@ export default class ZenMode { this.scrollTo(this.active_textarea); this.active_textarea = null; this.active_backdrop = null; - return Dropzone.forElement('.div-dropzone').enable(); + + const $dropzone = $('.div-dropzone'); + if ($dropzone && !$dropzone.hasClass('js-invalid-dropzone')) { + Dropzone.forElement('.div-dropzone').enable(); + } } } diff --git a/spec/javascripts/zen_mode_spec.js b/spec/javascripts/zen_mode_spec.js index 7047053d131..45a0bb0650f 100644 --- a/spec/javascripts/zen_mode_spec.js +++ b/spec/javascripts/zen_mode_spec.js @@ -1,77 +1,93 @@ -/* eslint-disable space-before-function-paren, no-var, one-var, one-var-declaration-per-line, object-shorthand, comma-dangle, no-return-assign, new-cap, max-len */ /* global Mousetrap */ import Dropzone from 'dropzone'; import ZenMode from '~/zen_mode'; -(function() { - var enterZen, escapeKeydown, exitZen; - - describe('ZenMode', function() { - var fixtureName = 'merge_requests/merge_request_with_comment.html.raw'; - preloadFixtures(fixtureName); - beforeEach(function() { - loadFixtures(fixtureName); - spyOn(Dropzone, 'forElement').and.callFake(function() { - return { - enable: function() { - return true; - } - }; - // Stub Dropzone.forElement(...).enable() - }); - this.zen = new ZenMode(); - // Set this manually because we can't actually scroll the window - return this.zen.scroll_position = 456; +describe('ZenMode', () => { + let zen; + const fixtureName = 'merge_requests/merge_request_with_comment.html.raw'; + + preloadFixtures(fixtureName); + + function enterZen() { + $('.notes-form .js-zen-enter').click(); + } + + function exitZen() { + $('.notes-form .js-zen-leave').click(); + } + + function escapeKeydown() { + $('.notes-form textarea').trigger($.Event('keydown', { + keyCode: 27, + })); + } + + beforeEach(() => { + loadFixtures(fixtureName); + + spyOn(Dropzone, 'forElement').and.callFake(() => ({ + enable: () => true, + })); + zen = new ZenMode(); + + // Set this manually because we can't actually scroll the window + zen.scroll_position = 456; + }); + + describe('on enter', () => { + it('pauses Mousetrap', () => { + spyOn(Mousetrap, 'pause'); + enterZen(); + expect(Mousetrap.pause).toHaveBeenCalled(); }); - describe('on enter', function() { - it('pauses Mousetrap', function() { - spyOn(Mousetrap, 'pause'); - enterZen(); - return expect(Mousetrap.pause).toHaveBeenCalled(); - }); - return it('removes textarea styling', function() { - $('.notes-form textarea').attr('style', 'height: 400px'); - enterZen(); - return expect($('.notes-form textarea')).not.toHaveAttr('style'); - }); + + it('removes textarea styling', () => { + $('.notes-form textarea').attr('style', 'height: 400px'); + enterZen(); + expect($('.notes-form textarea')).not.toHaveAttr('style'); }); - describe('in use', function() { - beforeEach(function() { - return enterZen(); - }); - return it('exits on Escape', function() { - escapeKeydown(); - return expect($('.notes-form .zen-backdrop')).not.toHaveClass('fullscreen'); - }); + }); + + describe('in use', () => { + beforeEach(enterZen); + + it('exits on Escape', () => { + escapeKeydown(); + expect($('.notes-form .zen-backdrop')).not.toHaveClass('fullscreen'); + }); + }); + + describe('on exit', () => { + beforeEach(enterZen); + + it('unpauses Mousetrap', () => { + spyOn(Mousetrap, 'unpause'); + exitZen(); + expect(Mousetrap.unpause).toHaveBeenCalled(); }); - return describe('on exit', function() { - beforeEach(function() { - return enterZen(); - }); - it('unpauses Mousetrap', function() { - spyOn(Mousetrap, 'unpause'); - exitZen(); - return expect(Mousetrap.unpause).toHaveBeenCalled(); - }); - return it('restores the scroll position', function() { - spyOn(this.zen, 'scrollTo'); - exitZen(); - return expect(this.zen.scrollTo).toHaveBeenCalled(); - }); + + it('restores the scroll position', () => { + spyOn(zen, 'scrollTo'); + exitZen(); + expect(zen.scrollTo).toHaveBeenCalled(); }); }); - enterZen = function() { - return $('.notes-form .js-zen-enter').click(); - }; + describe('enabling dropzone', () => { + beforeEach(() => { + enterZen(); + }); - exitZen = function() { - return $('.notes-form .js-zen-leave').click(); - }; + it('should not call dropzone if element is not dropzone valid', () => { + $('.div-dropzone').addClass('js-invalid-dropzone'); + exitZen(); + expect(Dropzone.forElement).not.toHaveBeenCalled(); + }); - escapeKeydown = function() { - return $('.notes-form textarea').trigger($.Event('keydown', { - keyCode: 27 - })); - }; -}).call(window); + it('should call dropzone if element is dropzone valid', () => { + $('.div-dropzone').removeClass('js-invalid-dropzone'); + exitZen(); + expect(Dropzone.forElement).toHaveBeenCalled(); + }); + }); +}); -- cgit v1.2.1 From fab9244c26b3344d9c6d17e69689c9a4a159fb18 Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Wed, 22 Nov 2017 17:25:11 -0600 Subject: backport disable autocomplete --- app/assets/javascripts/issue_show/components/app.vue | 6 ++++++ .../javascripts/issue_show/components/fields/description.vue | 9 ++++++++- app/assets/javascripts/issue_show/components/form.vue | 9 ++++++++- app/assets/javascripts/vue_shared/components/markdown/field.vue | 7 ++++++- 4 files changed, 28 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue index 4e39d483b31..1f9571ff413 100644 --- a/app/assets/javascripts/issue_show/components/app.vue +++ b/app/assets/javascripts/issue_show/components/app.vue @@ -34,6 +34,11 @@ export default { required: false, default: true, }, + enableAutocomplete: { + type: Boolean, + required: false, + default: true, + }, issuableRef: { type: String, required: true, @@ -240,6 +245,7 @@ export default { :project-namespace="projectNamespace" :show-delete-button="showDeleteButton" :can-attach-file="canAttachFile" + :enable-autocomplete="enableAutocomplete" />
+ :can-attach-file="canAttachFile" + :enable-autocomplete="enableAutocomplete" + > - - import timeAgoTooltip from '../../vue_shared/components/time_ago_tooltip.vue'; - - export default { - name: 'editedNoteText', - props: { - actionText: { - type: String, - required: true, - }, - editedAt: { - type: String, - required: true, - }, - editedBy: { - type: Object, - required: false, - }, - className: { - type: String, - required: false, - default: 'edited-text', - }, - }, - components: { - timeAgoTooltip, - }, - }; - - - diff --git a/app/assets/javascripts/notes/components/note_edited_text.vue b/app/assets/javascripts/notes/components/note_edited_text.vue new file mode 100644 index 00000000000..49e09f0ecc5 --- /dev/null +++ b/app/assets/javascripts/notes/components/note_edited_text.vue @@ -0,0 +1,47 @@ + + + diff --git a/spec/javascripts/notes/components/issue_note_edited_text_spec.js b/spec/javascripts/notes/components/issue_note_edited_text_spec.js deleted file mode 100644 index 6603241eb64..00000000000 --- a/spec/javascripts/notes/components/issue_note_edited_text_spec.js +++ /dev/null @@ -1,47 +0,0 @@ -import Vue from 'vue'; -import issueNoteEditedText from '~/notes/components/issue_note_edited_text.vue'; - -describe('issue_note_edited_text', () => { - let vm; - let props; - - beforeEach(() => { - const Component = Vue.extend(issueNoteEditedText); - props = { - actionText: 'Edited', - className: 'foo-bar', - editedAt: '2017-08-04T09:52:31.062Z', - editedBy: { - avatar_url: 'path', - id: 1, - name: 'Root', - path: '/root', - state: 'active', - username: 'root', - }, - }; - - vm = new Component({ - propsData: props, - }).$mount(); - }); - - afterEach(() => { - vm.$destroy(); - }); - - it('should render block with provided className', () => { - expect(vm.$el.className).toEqual(props.className); - }); - - it('should render provided actionText', () => { - expect(vm.$el.textContent).toContain(props.actionText); - }); - - it('should render provided user information', () => { - const authorLink = vm.$el.querySelector('.js-vue-author'); - - expect(authorLink.getAttribute('href')).toEqual(props.editedBy.path); - expect(authorLink.textContent.trim()).toEqual(props.editedBy.name); - }); -}); diff --git a/spec/javascripts/notes/components/note_edited_text_spec.js b/spec/javascripts/notes/components/note_edited_text_spec.js new file mode 100644 index 00000000000..e0b991c32ec --- /dev/null +++ b/spec/javascripts/notes/components/note_edited_text_spec.js @@ -0,0 +1,47 @@ +import Vue from 'vue'; +import noteEditedText from '~/notes/components/note_edited_text.vue'; + +describe('note_edited_text', () => { + let vm; + let props; + + beforeEach(() => { + const Component = Vue.extend(noteEditedText); + props = { + actionText: 'Edited', + className: 'foo-bar', + editedAt: '2017-08-04T09:52:31.062Z', + editedBy: { + avatar_url: 'path', + id: 1, + name: 'Root', + path: '/root', + state: 'active', + username: 'root', + }, + }; + + vm = new Component({ + propsData: props, + }).$mount(); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('should render block with provided className', () => { + expect(vm.$el.className).toEqual(props.className); + }); + + it('should render provided actionText', () => { + expect(vm.$el.textContent).toContain(props.actionText); + }); + + it('should render provided user information', () => { + const authorLink = vm.$el.querySelector('.js-vue-author'); + + expect(authorLink.getAttribute('href')).toEqual(props.editedBy.path); + expect(authorLink.textContent.trim()).toEqual(props.editedBy.name); + }); +}); -- cgit v1.2.1 From 864fa5a94fd065423828d52af97e822954a06b90 Mon Sep 17 00:00:00 2001 From: Simon Knox Date: Fri, 24 Nov 2017 19:37:02 +1100 Subject: rename issue_note_attachment --- .../notes/components/issue_note_attachment.vue | 37 ---------------------- .../notes/components/issue_note_body.vue | 6 ++-- .../notes/components/note_attachment.vue | 37 ++++++++++++++++++++++ .../notes/components/issue_note_attachment_spec.js | 23 -------------- .../notes/components/note_attachment_spec.js | 23 ++++++++++++++ 5 files changed, 63 insertions(+), 63 deletions(-) delete mode 100644 app/assets/javascripts/notes/components/issue_note_attachment.vue create mode 100644 app/assets/javascripts/notes/components/note_attachment.vue delete mode 100644 spec/javascripts/notes/components/issue_note_attachment_spec.js create mode 100644 spec/javascripts/notes/components/note_attachment_spec.js diff --git a/app/assets/javascripts/notes/components/issue_note_attachment.vue b/app/assets/javascripts/notes/components/issue_note_attachment.vue deleted file mode 100644 index 7134a3eb47e..00000000000 --- a/app/assets/javascripts/notes/components/issue_note_attachment.vue +++ /dev/null @@ -1,37 +0,0 @@ - - - diff --git a/app/assets/javascripts/notes/components/issue_note_body.vue b/app/assets/javascripts/notes/components/issue_note_body.vue index 419c8f81e11..4b94d0a7896 100644 --- a/app/assets/javascripts/notes/components/issue_note_body.vue +++ b/app/assets/javascripts/notes/components/issue_note_body.vue @@ -1,7 +1,7 @@ + + diff --git a/spec/javascripts/notes/components/issue_note_attachment_spec.js b/spec/javascripts/notes/components/issue_note_attachment_spec.js deleted file mode 100644 index 8f33b874ad6..00000000000 --- a/spec/javascripts/notes/components/issue_note_attachment_spec.js +++ /dev/null @@ -1,23 +0,0 @@ -import Vue from 'vue'; -import issueNoteAttachment from '~/notes/components/issue_note_attachment.vue'; - -describe('issue note attachment', () => { - it('should render properly', () => { - const props = { - attachment: { - filename: 'dk.png', - image: true, - url: '/dk.png', - }, - }; - - const Component = Vue.extend(issueNoteAttachment); - const vm = new Component({ - propsData: props, - }).$mount(); - - expect(vm.$el.classList.contains('note-attachment')).toBeTruthy(); - expect(vm.$el.querySelector('img').src).toContain(props.attachment.url); - expect(vm.$el.querySelector('a').href).toContain(props.attachment.url); - }); -}); diff --git a/spec/javascripts/notes/components/note_attachment_spec.js b/spec/javascripts/notes/components/note_attachment_spec.js new file mode 100644 index 00000000000..b14a518b622 --- /dev/null +++ b/spec/javascripts/notes/components/note_attachment_spec.js @@ -0,0 +1,23 @@ +import Vue from 'vue'; +import noteAttachment from '~/notes/components/note_attachment.vue'; + +describe('issue note attachment', () => { + it('should render properly', () => { + const props = { + attachment: { + filename: 'dk.png', + image: true, + url: '/dk.png', + }, + }; + + const Component = Vue.extend(noteAttachment); + const vm = new Component({ + propsData: props, + }).$mount(); + + expect(vm.$el.classList.contains('note-attachment')).toBeTruthy(); + expect(vm.$el.querySelector('img').src).toContain(props.attachment.url); + expect(vm.$el.querySelector('a').href).toContain(props.attachment.url); + }); +}); -- cgit v1.2.1 From 1aae91c9571166344b5228362c800cb0a23c0d3e Mon Sep 17 00:00:00 2001 From: Simon Knox Date: Fri, 24 Nov 2017 19:39:26 +1100 Subject: rename issue_note_awards_list --- .../notes/components/issue_note_awards_list.vue | 227 --------------------- .../notes/components/issue_note_body.vue | 6 +- .../notes/components/note_awards_list.vue | 227 +++++++++++++++++++++ .../components/issue_note_awards_list_spec.js | 56 ----- .../notes/components/note_awards_list_spec.js | 56 +++++ 5 files changed, 286 insertions(+), 286 deletions(-) delete mode 100644 app/assets/javascripts/notes/components/issue_note_awards_list.vue create mode 100644 app/assets/javascripts/notes/components/note_awards_list.vue delete mode 100644 spec/javascripts/notes/components/issue_note_awards_list_spec.js create mode 100644 spec/javascripts/notes/components/note_awards_list_spec.js diff --git a/app/assets/javascripts/notes/components/issue_note_awards_list.vue b/app/assets/javascripts/notes/components/issue_note_awards_list.vue deleted file mode 100644 index c3a340139e7..00000000000 --- a/app/assets/javascripts/notes/components/issue_note_awards_list.vue +++ /dev/null @@ -1,227 +0,0 @@ - - - diff --git a/app/assets/javascripts/notes/components/issue_note_body.vue b/app/assets/javascripts/notes/components/issue_note_body.vue index 4b94d0a7896..a16c5f6a785 100644 --- a/app/assets/javascripts/notes/components/issue_note_body.vue +++ b/app/assets/javascripts/notes/components/issue_note_body.vue @@ -1,6 +1,6 @@ + + diff --git a/spec/javascripts/notes/components/issue_note_awards_list_spec.js b/spec/javascripts/notes/components/issue_note_awards_list_spec.js deleted file mode 100644 index 3b6c34f1494..00000000000 --- a/spec/javascripts/notes/components/issue_note_awards_list_spec.js +++ /dev/null @@ -1,56 +0,0 @@ -import Vue from 'vue'; -import store from '~/notes/stores'; -import awardsNote from '~/notes/components/issue_note_awards_list.vue'; -import { issueDataMock, notesDataMock } from '../mock_data'; - -describe('issue_note_awards_list component', () => { - let vm; - let awardsMock; - - beforeEach(() => { - const Component = Vue.extend(awardsNote); - - store.dispatch('setIssueData', issueDataMock); - store.dispatch('setNotesData', notesDataMock); - awardsMock = [ - { - name: 'flag_tz', - user: { id: 1, name: 'Administrator', username: 'root' }, - }, - { - name: 'cartwheel_tone3', - user: { id: 12, name: 'Bobbie Stehr', username: 'erin' }, - }, - ]; - - vm = new Component({ - store, - propsData: { - awards: awardsMock, - noteAuthorId: 2, - noteId: 545, - toggleAwardPath: '/gitlab-org/gitlab-ce/notes/545/toggle_award_emoji', - }, - }).$mount(); - }); - - afterEach(() => { - vm.$destroy(); - }); - - it('should render awarded emojis', () => { - expect(vm.$el.querySelector('.js-awards-block button [data-name="flag_tz"]')).toBeDefined(); - expect(vm.$el.querySelector('.js-awards-block button [data-name="cartwheel_tone3"]')).toBeDefined(); - }); - - it('should be possible to remove awareded emoji', () => { - spyOn(vm, 'handleAward').and.callThrough(); - vm.$el.querySelector('.js-awards-block button').click(); - - expect(vm.handleAward).toHaveBeenCalledWith('flag_tz'); - }); - - it('should be possible to add new emoji', () => { - expect(vm.$el.querySelector('.js-add-award')).toBeDefined(); - }); -}); diff --git a/spec/javascripts/notes/components/note_awards_list_spec.js b/spec/javascripts/notes/components/note_awards_list_spec.js new file mode 100644 index 00000000000..7292e5c3a8d --- /dev/null +++ b/spec/javascripts/notes/components/note_awards_list_spec.js @@ -0,0 +1,56 @@ +import Vue from 'vue'; +import store from '~/notes/stores'; +import awardsNote from '~/notes/components/note_awards_list.vue'; +import { issueDataMock, notesDataMock } from '../mock_data'; + +describe('note_awards_list component', () => { + let vm; + let awardsMock; + + beforeEach(() => { + const Component = Vue.extend(awardsNote); + + store.dispatch('setIssueData', issueDataMock); + store.dispatch('setNotesData', notesDataMock); + awardsMock = [ + { + name: 'flag_tz', + user: { id: 1, name: 'Administrator', username: 'root' }, + }, + { + name: 'cartwheel_tone3', + user: { id: 12, name: 'Bobbie Stehr', username: 'erin' }, + }, + ]; + + vm = new Component({ + store, + propsData: { + awards: awardsMock, + noteAuthorId: 2, + noteId: 545, + toggleAwardPath: '/gitlab-org/gitlab-ce/notes/545/toggle_award_emoji', + }, + }).$mount(); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('should render awarded emojis', () => { + expect(vm.$el.querySelector('.js-awards-block button [data-name="flag_tz"]')).toBeDefined(); + expect(vm.$el.querySelector('.js-awards-block button [data-name="cartwheel_tone3"]')).toBeDefined(); + }); + + it('should be possible to remove awareded emoji', () => { + spyOn(vm, 'handleAward').and.callThrough(); + vm.$el.querySelector('.js-awards-block button').click(); + + expect(vm.handleAward).toHaveBeenCalledWith('flag_tz'); + }); + + it('should be possible to add new emoji', () => { + expect(vm.$el.querySelector('.js-add-award')).toBeDefined(); + }); +}); -- cgit v1.2.1 From ba62143ac34de6cf96da4a19b498b220f7e5154b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 21 Nov 2017 18:13:07 +0100 Subject: Refactor the way we pass `old associations` to issuable's update services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- app/models/concerns/issuable.rb | 12 ++++++-- app/services/issuable_base_service.rb | 33 ++++++++++++---------- app/services/issues/base_service.rb | 8 +++--- app/services/issues/update_service.rb | 7 +++-- app/services/merge_requests/base_service.rb | 8 +++--- app/services/merge_requests/update_service.rb | 5 ++-- spec/models/concerns/issuable_spec.rb | 8 +++--- .../services/merge_requests/update_service_spec.rb | 16 ++++++++--- 8 files changed, 58 insertions(+), 39 deletions(-) diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index e607707475f..27cd3118f81 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -255,8 +255,10 @@ module Issuable participants(user).include?(user) end - def to_hook_data(user, old_labels: [], old_assignees: [], old_total_time_spent: nil) + def to_hook_data(user, old_associations: {}) changes = previous_changes + old_labels = old_associations.fetch(:labels, []) + old_assignees = old_associations.fetch(:assignees, []) if old_labels != labels changes[:labels] = [old_labels.map(&:hook_attrs), labels.map(&:hook_attrs)] @@ -270,8 +272,12 @@ module Issuable end end - if old_total_time_spent != total_time_spent - changes[:total_time_spent] = [old_total_time_spent, total_time_spent] + if self.respond_to?(:total_time_spent) + old_total_time_spent = old_associations.fetch(:total_time_spent, nil) + + if old_total_time_spent != total_time_spent + changes[:total_time_spent] = [old_total_time_spent, total_time_spent] + end end Gitlab::HookData::IssuableBuilder.new(self).build(user: user, changes: changes) diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 39a7299ff60..2c51ac13815 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -169,10 +169,7 @@ class IssuableBaseService < BaseService change_todo(issuable) toggle_award(issuable) filter_params(issuable) - old_labels = issuable.labels.to_a - old_mentioned_users = issuable.mentioned_users.to_a - old_assignees = issuable.assignees.to_a - old_total_time_spent = issuable.total_time_spent + old_associations = associations_before_update(issuable) label_ids = process_label_ids(params, existing_label_ids: issuable.label_ids) params[:label_ids] = label_ids if labels_changing?(issuable.label_ids, label_ids) @@ -193,18 +190,13 @@ class IssuableBaseService < BaseService if issuable.with_transaction_returning_status { issuable.save } # We do not touch as it will affect a update on updated_at field ActiveRecord::Base.no_touching do - Issuable::CommonSystemNotesService.new(project, current_user).execute(issuable, old_labels) + Issuable::CommonSystemNotesService.new(project, current_user).execute(issuable, old_associations[:labels]) end - handle_changes( - issuable, - old_labels: old_labels, - old_mentioned_users: old_mentioned_users, - old_assignees: old_assignees - ) + handle_changes(issuable, old_associations: old_associations) new_assignees = issuable.assignees.to_a - affected_assignees = (old_assignees + new_assignees) - (old_assignees & new_assignees) + affected_assignees = (old_associations[:assignees] + new_assignees) - (old_associations[:assignees] & new_assignees) invalidate_cache_counts(issuable, users: affected_assignees.compact) after_update(issuable) @@ -212,9 +204,8 @@ class IssuableBaseService < BaseService execute_hooks( issuable, 'update', - old_labels: old_labels, - old_assignees: old_assignees, - old_total_time_spent: old_total_time_spent) + old_associations: old_associations + ) issuable.update_project_counter_caches if update_project_counters end @@ -267,6 +258,18 @@ class IssuableBaseService < BaseService end end + def associations_before_update(issuable) + associations = + { + labels: issuable.labels.to_a, + mentioned_users: issuable.mentioned_users.to_a, + assignees: issuable.assignees.to_a + } + associations[:total_time_spent] = issuable.total_time_spent if issuable.respond_to?(:total_time_spent) + + associations + end + def has_changes?(issuable, old_labels: [], old_assignees: []) valid_attrs = [:title, :description, :assignee_id, :milestone_id, :target_branch] diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index 0f711bcc3cf..9f6cfc0f6d3 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -1,7 +1,7 @@ module Issues class BaseService < ::IssuableBaseService - def hook_data(issue, action, old_labels: [], old_assignees: [], old_total_time_spent: nil) - hook_data = issue.to_hook_data(current_user, old_labels: old_labels, old_assignees: old_assignees, old_total_time_spent: old_total_time_spent) + def hook_data(issue, action, old_associations: {}) + hook_data = issue.to_hook_data(current_user, old_associations: old_associations) hook_data[:object_attributes][:action] = action hook_data @@ -22,8 +22,8 @@ module Issues issue, issue.project, current_user, old_assignees) end - def execute_hooks(issue, action = 'open', old_labels: [], old_assignees: [], old_total_time_spent: nil) - issue_data = hook_data(issue, action, old_labels: old_labels, old_assignees: old_assignees, old_total_time_spent: old_total_time_spent) + def execute_hooks(issue, action = 'open', old_associations: {}) + issue_data = hook_data(issue, action, old_associations: old_associations) hooks_scope = issue.confidential? ? :confidential_issue_hooks : :issue_hooks issue.project.execute_hooks(issue_data, hooks_scope) issue.project.execute_services(issue_data, hooks_scope) diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index 1b7b5927c5a..d7aa7e2347e 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -14,9 +14,10 @@ module Issues end def handle_changes(issue, options) - old_labels = options[:old_labels] || [] - old_mentioned_users = options[:old_mentioned_users] || [] - old_assignees = options[:old_assignees] || [] + old_associations = options.fetch(:old_associations, {}) + old_labels = old_associations.fetch(:labels, []) + old_mentioned_users = old_associations.fetch(:mentioned_users, []) + old_assignees = old_associations.fetch(:assignees, []) if has_changes?(issue, old_labels: old_labels, old_assignees: old_assignees) todo_service.mark_pending_todos_as_done(issue, current_user) diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index f6ffd48deae..6b32d65a74b 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -4,8 +4,8 @@ module MergeRequests SystemNoteService.change_status(merge_request, merge_request.target_project, current_user, state, nil) end - def hook_data(merge_request, action, old_rev: nil, old_labels: [], old_assignees: [], old_total_time_spent: nil) - hook_data = merge_request.to_hook_data(current_user, old_labels: old_labels, old_assignees: old_assignees, old_total_time_spent: old_total_time_spent) + def hook_data(merge_request, action, old_rev: nil, old_associations: {}) + hook_data = merge_request.to_hook_data(current_user, old_associations: old_associations) hook_data[:object_attributes][:action] = action if old_rev && !Gitlab::Git.blank_ref?(old_rev) hook_data[:object_attributes][:oldrev] = old_rev @@ -14,9 +14,9 @@ module MergeRequests hook_data end - def execute_hooks(merge_request, action = 'open', old_rev: nil, old_labels: [], old_assignees: [], old_total_time_spent: nil) + def execute_hooks(merge_request, action = 'open', old_rev: nil, old_associations: {}) if merge_request.project - merge_data = hook_data(merge_request, action, old_rev: old_rev, old_labels: old_labels, old_assignees: old_assignees, old_total_time_spent: old_total_time_spent) + merge_data = hook_data(merge_request, action, old_rev: old_rev, old_associations: old_associations) merge_request.project.execute_hooks(merge_data, :merge_request_hooks) merge_request.project.execute_services(merge_data, :merge_request_hooks) end diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 1f394cacc64..c153872c874 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -22,8 +22,9 @@ module MergeRequests end def handle_changes(merge_request, options) - old_labels = options[:old_labels] || [] - old_mentioned_users = options[:old_mentioned_users] || [] + old_associations = options.fetch(:old_associations, {}) + old_labels = old_associations.fetch(:labels, []) + old_mentioned_users = old_associations.fetch(:mentioned_users, []) if has_changes?(merge_request, old_labels: old_labels) todo_service.mark_pending_todos_as_done(merge_request, current_user) diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 4dfbb14952e..765b2729918 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -283,7 +283,7 @@ describe Issuable do 'labels' => [[labels[0].hook_attrs], [labels[1].hook_attrs]] )) - issue.to_hook_data(user, old_labels: [labels[0]]) + issue.to_hook_data(user, old_associations: { labels: [labels[0]] }) end end @@ -302,7 +302,7 @@ describe Issuable do 'total_time_spent' => [1, 2] )) - issue.to_hook_data(user, old_total_time_spent: 1) + issue.to_hook_data(user, old_associations: { total_time_spent: 1 }) end end @@ -322,7 +322,7 @@ describe Issuable do 'assignees' => [[user.hook_attrs], [user.hook_attrs, user2.hook_attrs]] )) - issue.to_hook_data(user, old_assignees: [user]) + issue.to_hook_data(user, old_associations: { assignees: [user] }) end end @@ -345,7 +345,7 @@ describe Issuable do 'assignee' => [user.hook_attrs, user2.hook_attrs] )) - merge_request.to_hook_data(user, old_assignees: [user]) + merge_request.to_hook_data(user, old_associations: { assignees: [user] }) end end end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 5ce6ca70c83..7a66b809550 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -65,7 +65,7 @@ describe MergeRequests::UpdateService, :mailer do end end - it 'mathces base expectations' do + it 'matches base expectations' do expect(@merge_request).to be_valid expect(@merge_request.title).to eq('New title') expect(@merge_request.assignee).to eq(user2) @@ -78,9 +78,17 @@ describe MergeRequests::UpdateService, :mailer do end it 'executes hooks with update action' do - expect(service) - .to have_received(:execute_hooks) - .with(@merge_request, 'update', old_labels: [], old_assignees: [user3], old_total_time_spent: 0) + expect(service).to have_received(:execute_hooks) + .with( + @merge_request, + 'update', + old_associations: { + labels: [], + mentioned_users: [user2], + assignees: [user3], + total_time_spent: 0 + } + ) end it 'sends email to user2 about assign of new merge request and email to user3 about merge request unassignment' do -- cgit v1.2.1 From 4beacdb2ca6ea884473cb63aee603951df7f2da1 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Fri, 24 Nov 2017 14:27:38 +0100 Subject: Fix defaults for MR states and merge statuses This ensures that merge_requests.state and merge_requests.merge_status both have a proper default value and NOT NULL constraint on database level. We also make sure to update any bogus rows first, without blowing up the database. Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/40534 --- .../unreleased/default-values-for-mr-states.yml | 5 +++ ...2_add_default_values_to_merge_request_states.rb | 19 ++++++++ ...5748_populate_missing_merge_request_statuses.rb | 50 ++++++++++++++++++++++ ...4132536_make_merge_request_statuses_not_null.rb | 14 ++++++ db/schema.rb | 6 +-- 5 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/default-values-for-mr-states.yml create mode 100644 db/migrate/20171124125042_add_default_values_to_merge_request_states.rb create mode 100644 db/migrate/20171124125748_populate_missing_merge_request_statuses.rb create mode 100644 db/migrate/20171124132536_make_merge_request_statuses_not_null.rb diff --git a/changelogs/unreleased/default-values-for-mr-states.yml b/changelogs/unreleased/default-values-for-mr-states.yml new file mode 100644 index 00000000000..f873a5335d0 --- /dev/null +++ b/changelogs/unreleased/default-values-for-mr-states.yml @@ -0,0 +1,5 @@ +--- +title: Fix defaults for MR states and merge statuses +merge_request: +author: +type: fixed diff --git a/db/migrate/20171124125042_add_default_values_to_merge_request_states.rb b/db/migrate/20171124125042_add_default_values_to_merge_request_states.rb new file mode 100644 index 00000000000..d08863c3b78 --- /dev/null +++ b/db/migrate/20171124125042_add_default_values_to_merge_request_states.rb @@ -0,0 +1,19 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddDefaultValuesToMergeRequestStates < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + def up + change_column_default :merge_requests, :state, :opened + change_column_default :merge_requests, :merge_status, :unchecked + end + + def down + change_column_default :merge_requests, :state, nil + change_column_default :merge_requests, :merge_status, nil + end +end diff --git a/db/migrate/20171124125748_populate_missing_merge_request_statuses.rb b/db/migrate/20171124125748_populate_missing_merge_request_statuses.rb new file mode 100644 index 00000000000..72fbab59f4c --- /dev/null +++ b/db/migrate/20171124125748_populate_missing_merge_request_statuses.rb @@ -0,0 +1,50 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class PopulateMissingMergeRequestStatuses < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + disable_ddl_transaction! + + class MergeRequest < ActiveRecord::Base + include EachBatch + + self.table_name = 'merge_requests' + end + + def up + say 'Populating missing merge_requests.state values' + + # GitLab.com has no rows where "state" is NULL, and technically this should + # never happen. However it doesn't hurt to be 100% certain. + MergeRequest.where(state: nil).each_batch do |batch| + batch.update_all(state: 'opened') + end + + say 'Populating missing merge_requests.merge_status values. ' \ + 'This will take a few minutes...' + + # GitLab.com has 66 880 rows where "merge_status" is NULL, dating back all + # the way to 2011. + MergeRequest.where(merge_status: nil).each_batch(of: 10_000) do |batch| + batch.update_all(merge_status: 'unchecked') + + # We want to give PostgreSQL some time to vacuum any dead tuples. In + # production we see it takes roughly 1 minute for a vacuuming run to clear + # out 10-20k dead tuples, so we'll wait for 90 seconds between every + # batch. + sleep(90) if sleep? + end + end + + def down + # Reverting this makes no sense. + end + + def sleep? + Rails.env.staging? || Rails.env.production? + end +end diff --git a/db/migrate/20171124132536_make_merge_request_statuses_not_null.rb b/db/migrate/20171124132536_make_merge_request_statuses_not_null.rb new file mode 100644 index 00000000000..4bb09126036 --- /dev/null +++ b/db/migrate/20171124132536_make_merge_request_statuses_not_null.rb @@ -0,0 +1,14 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class MakeMergeRequestStatusesNotNull < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + def change + change_column_null :merge_requests, :state, false + change_column_null :merge_requests, :merge_status, false + end +end diff --git a/db/schema.rb b/db/schema.rb index d10561099b7..804bc8d6e37 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: 20171121144800) do +ActiveRecord::Schema.define(version: 20171124132536) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1049,8 +1049,8 @@ ActiveRecord::Schema.define(version: 20171121144800) do t.datetime "created_at" t.datetime "updated_at" t.integer "milestone_id" - t.string "state" - t.string "merge_status" + t.string "state", default: "opened", null: false + t.string "merge_status", default: "unchecked", null: false t.integer "target_project_id", null: false t.integer "iid" t.text "description" -- cgit v1.2.1 From aedd2cfa5b82c01f82ec26b64880fce2a07fe942 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 24 Nov 2017 11:45:19 +0100 Subject: Use Gitlab::SQL::Pattern where appropriate --- app/finders/notes_finder.rb | 3 +-- app/models/ci/runner.rb | 3 ++- app/models/group.rb | 14 -------------- app/models/milestone.rb | 3 ++- app/models/namespace.rb | 3 ++- app/models/note.rb | 5 +++++ app/models/snippet.rb | 8 +++----- app/models/user.rb | 2 +- 8 files changed, 16 insertions(+), 25 deletions(-) diff --git a/app/finders/notes_finder.rb b/app/finders/notes_finder.rb index 02eb983bf55..12157818bcd 100644 --- a/app/finders/notes_finder.rb +++ b/app/finders/notes_finder.rb @@ -104,8 +104,7 @@ class NotesFinder query = @params[:search] return notes unless query - pattern = "%#{query}%" - notes.where(Note.arel_table[:note].matches(pattern)) + notes.search(query) end # Notes changed since last fetch diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index c6509f89117..d91a66ab5c2 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -1,6 +1,7 @@ module Ci class Runner < ActiveRecord::Base extend Gitlab::Ci::Model + include Gitlab::SQL::Pattern RUNNER_QUEUE_EXPIRY_TIME = 60.minutes ONLINE_CONTACT_TIMEOUT = 1.hour @@ -60,7 +61,7 @@ module Ci # Returns an ActiveRecord::Relation. def self.search(query) t = arel_table - pattern = "%#{query}%" + pattern = to_pattern(query) where(t[:token].matches(pattern).or(t[:description].matches(pattern))) end diff --git a/app/models/group.rb b/app/models/group.rb index dc4500360b9..76262acf50c 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -50,20 +50,6 @@ class Group < Namespace Gitlab::Database.postgresql? end - # Searches for groups matching the given query. - # - # This method uses ILIKE on PostgreSQL and LIKE on MySQL. - # - # query - The search query as a String - # - # Returns an ActiveRecord::Relation. - def search(query) - table = Namespace.arel_table - pattern = "%#{query}%" - - where(table[:name].matches(pattern).or(table[:path].matches(pattern))) - end - def sort(method) if method == 'storage_size_desc' # storage_size is a virtual column so we need to diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 01458120cda..e25d72cf947 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -13,6 +13,7 @@ class Milestone < ActiveRecord::Base include Referable include StripAttribute include Milestoneish + include Gitlab::SQL::Pattern cache_markdown_field :title, pipeline: :single_line cache_markdown_field :description @@ -74,7 +75,7 @@ class Milestone < ActiveRecord::Base # Returns an ActiveRecord::Relation. def search(query) t = arel_table - pattern = "%#{query}%" + pattern = to_pattern(query) where(t[:title].matches(pattern).or(t[:description].matches(pattern))) end diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 4d401e7ba18..15bc7032a43 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -9,6 +9,7 @@ class Namespace < ActiveRecord::Base include Routable include AfterCommitQueue include Storage::LegacyNamespace + include Gitlab::SQL::Pattern # Prevent users from creating unreasonably deep level of nesting. # The number 20 was taken based on maximum nesting level of @@ -87,7 +88,7 @@ class Namespace < ActiveRecord::Base # Returns an ActiveRecord::Relation def search(query) t = arel_table - pattern = "%#{query}%" + pattern = to_pattern(query) where(t[:name].matches(pattern).or(t[:path].matches(pattern))) end diff --git a/app/models/note.rb b/app/models/note.rb index 50c9caf8529..d2aa8392229 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -14,6 +14,7 @@ class Note < ActiveRecord::Base include ResolvableNote include IgnorableColumn include Editable + include Gitlab::SQL::Pattern module SpecialRole FIRST_TIME_CONTRIBUTOR = :first_time_contributor @@ -167,6 +168,10 @@ class Note < ActiveRecord::Base def has_special_role?(role, note) note.special_role == role end + + def search(query) + where(arel_table[:note].matches(to_pattern(query))) + end end def cross_reference? diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 2a5f07a15c4..e621404f3ae 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -9,6 +9,7 @@ class Snippet < ActiveRecord::Base include Mentionable include Spammable include Editable + include Gitlab::SQL::Pattern extend Gitlab::CurrentSettings @@ -136,7 +137,7 @@ class Snippet < ActiveRecord::Base # Returns an ActiveRecord::Relation. def search(query) t = arel_table - pattern = "%#{query}%" + pattern = to_pattern(query) where(t[:title].matches(pattern).or(t[:file_name].matches(pattern))) end @@ -149,10 +150,7 @@ class Snippet < ActiveRecord::Base # # Returns an ActiveRecord::Relation. def search_code(query) - table = Snippet.arel_table - pattern = "%#{query}%" - - where(table[:content].matches(pattern)) + where(arel_table[:content].matches(to_pattern(query))) end end end diff --git a/app/models/user.rb b/app/models/user.rb index cf6b36559a8..9a35336c574 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -339,7 +339,7 @@ class User < ActiveRecord::Base def search_with_secondary_emails(query) table = arel_table email_table = Email.arel_table - pattern = "%#{query}%" + pattern = to_pattern(query) matched_by_emails_user_ids = email_table.project(email_table[:user_id]).where(email_table[:email].matches(pattern)) where( -- cgit v1.2.1 From b2c5363da1bdfb4df8693de38f9d83fe203e6e99 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 24 Nov 2017 12:08:16 +0100 Subject: Rename to_fuzzy_arel to fuzzy_arel_match --- app/models/concerns/issuable.rb | 6 +++--- lib/gitlab/sql/pattern.rb | 2 +- spec/lib/gitlab/sql/pattern_spec.rb | 12 ++++++------ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index e607707475f..176ce1152f1 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -122,7 +122,7 @@ module Issuable # # Returns an ActiveRecord::Relation. def search(query) - title = to_fuzzy_arel(:title, query) + title = fuzzy_arel_match(:title, query) where(title) end @@ -135,8 +135,8 @@ module Issuable # # Returns an ActiveRecord::Relation. def full_search(query) - title = to_fuzzy_arel(:title, query) - description = to_fuzzy_arel(:description, query) + title = fuzzy_arel_match(:title, query) + description = fuzzy_arel_match(:description, query) where(title&.or(description)) end diff --git a/lib/gitlab/sql/pattern.rb b/lib/gitlab/sql/pattern.rb index 7c2d1d8f887..8741aa0f1c4 100644 --- a/lib/gitlab/sql/pattern.rb +++ b/lib/gitlab/sql/pattern.rb @@ -19,7 +19,7 @@ module Gitlab query.length >= MIN_CHARS_FOR_PARTIAL_MATCHING end - def to_fuzzy_arel(column, query) + def fuzzy_arel_match(column, query) words = select_fuzzy_words(query) matches = words.map { |word| arel_table[column].matches(to_pattern(word)) } diff --git a/spec/lib/gitlab/sql/pattern_spec.rb b/spec/lib/gitlab/sql/pattern_spec.rb index 48d56628ed5..d2989489e49 100644 --- a/spec/lib/gitlab/sql/pattern_spec.rb +++ b/spec/lib/gitlab/sql/pattern_spec.rb @@ -137,14 +137,14 @@ describe Gitlab::SQL::Pattern do end end - describe '.to_fuzzy_arel' do - subject(:to_fuzzy_arel) { Issue.to_fuzzy_arel(:title, query) } + describe '.fuzzy_arel_match' do + subject(:fuzzy_arel_match) { Issue.fuzzy_arel_match(:title, query) } context 'with a word equal to 3 chars' do let(:query) { 'foo' } it 'returns a single ILIKE condition' do - expect(to_fuzzy_arel.to_sql).to match(/title.*I?LIKE '\%foo\%'/) + expect(fuzzy_arel_match.to_sql).to match(/title.*I?LIKE '\%foo\%'/) end end @@ -152,7 +152,7 @@ describe Gitlab::SQL::Pattern do let(:query) { 'fo' } it 'returns nil' do - expect(to_fuzzy_arel).to be_nil + expect(fuzzy_arel_match).to be_nil end end @@ -160,7 +160,7 @@ describe Gitlab::SQL::Pattern do let(:query) { 'foo baz' } it 'returns a joining LIKE condition using a AND' do - expect(to_fuzzy_arel.to_sql).to match(/title.+I?LIKE '\%foo\%' AND .*title.*I?LIKE '\%baz\%'/) + expect(fuzzy_arel_match.to_sql).to match(/title.+I?LIKE '\%foo\%' AND .*title.*I?LIKE '\%baz\%'/) end end @@ -168,7 +168,7 @@ describe Gitlab::SQL::Pattern do let(:query) { 'foo "really bar" baz' } it 'returns a joining LIKE condition using a AND' do - expect(to_fuzzy_arel.to_sql).to match(/title.+I?LIKE '\%foo\%' AND .*title.*I?LIKE '\%baz\%' AND .*title.*I?LIKE '\%really bar\%'/) + expect(fuzzy_arel_match.to_sql).to match(/title.+I?LIKE '\%foo\%' AND .*title.*I?LIKE '\%baz\%' AND .*title.*I?LIKE '\%really bar\%'/) end end end -- cgit v1.2.1 From 308f2ecd1d9355959298828c83fdabf5208c59a1 Mon Sep 17 00:00:00 2001 From: Christiaan Van den Poel Date: Sat, 25 Nov 2017 00:03:48 +0100 Subject: removed 'only once' from the docs --- doc/user/project/pipelines/schedules.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/user/project/pipelines/schedules.md b/doc/user/project/pipelines/schedules.md index eac706be3a7..2101e3b1d58 100644 --- a/doc/user/project/pipelines/schedules.md +++ b/doc/user/project/pipelines/schedules.md @@ -5,7 +5,7 @@ - In 9.2, the feature was [renamed to Pipeline Schedule][ce-10853]. - Cron notation is parsed by [Rufus-Scheduler](https://github.com/jmettraux/rufus-scheduler). -Pipeline schedules can be used to run pipelines only once, or for example every +Pipeline schedules can be used to run a pipeline at specific intervals, for example every month on the 22nd for a certain branch. ## Using Pipeline schedules -- cgit v1.2.1 From 73397a3e6c87065de2e0031c6ecc997b31b9a804 Mon Sep 17 00:00:00 2001 From: David Muckle Date: Sat, 25 Nov 2017 02:18:27 +0000 Subject: Add in the Oxford comma --- doc/user/project/clusters/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/user/project/clusters/index.md b/doc/user/project/clusters/index.md index cf0c7c109a8..e2924c66e70 100644 --- a/doc/user/project/clusters/index.md +++ b/doc/user/project/clusters/index.md @@ -82,7 +82,7 @@ added directly to your configured cluster. Those applications are needed for | Application | GitLab version | Description | | ----------- | :------------: | ----------- | | [Helm Tiller](https://docs.helm.sh/) | 10.2+ | Helm is a package manager for Kubernetes and is required to install all the other applications. It will be automatically installed as a dependency when you try to install a different app. It is installed in its own pod inside the cluster which can run the `helm` CLI in a safe environment. | -| [Ingress](https://kubernetes.io/docs/concepts/services-networking/ingress/) | 10.2+ | Ingress can provide load balancing, SSL termination and name-based virtual hosting. It acts as a web proxy for your applications and is useful if you want to use [Auto DevOps](../../../topics/autodevops/index.md) or deploy your own web apps. | +| [Ingress](https://kubernetes.io/docs/concepts/services-networking/ingress/) | 10.2+ | Ingress can provide load balancing, SSL termination, and name-based virtual hosting. It acts as a web proxy for your applications and is useful if you want to use [Auto DevOps](../../../topics/autodevops/index.md) or deploy your own web apps. | ## Enabling or disabling the Cluster integration -- cgit v1.2.1 From 7fb1bb01bd669cc46514ed17b1b8822a1d962970 Mon Sep 17 00:00:00 2001 From: George Andrinopoulos Date: Sat, 25 Nov 2017 17:04:45 +0200 Subject: Create issue and merge request destroy services --- app/controllers/concerns/issuable_actions.rb | 2 +- app/models/issue.rb | 1 - app/models/merge_request.rb | 1 - app/services/issuable/destroy_service.rb | 9 +++++ .../39601-create-issuable-destroy-service.yml | 5 +++ lib/api/issues.rb | 4 ++- lib/api/merge_requests.rb | 4 ++- spec/services/issuable/destroy_service_spec.rb | 38 ++++++++++++++++++++++ 8 files changed, 59 insertions(+), 5 deletions(-) create mode 100644 app/services/issuable/destroy_service.rb create mode 100644 changelogs/unreleased/39601-create-issuable-destroy-service.yml create mode 100644 spec/services/issuable/destroy_service_spec.rb diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index 072dffaff7a..f6d9f88032f 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/app/controllers/concerns/issuable_actions.rb @@ -54,7 +54,7 @@ module IssuableActions end def destroy - issuable.destroy + Issuable::DestroyService.new(project, current_user).execute(issuable) TodoService.new.destroy_issuable(issuable, current_user) name = issuable.human_class_name diff --git a/app/models/issue.rb b/app/models/issue.rb index a9863a50d84..d6ef58d150b 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -49,7 +49,6 @@ class Issue < ActiveRecord::Base scope :public_only, -> { where(confidential: false) } after_save :expire_etag_cache - after_commit :update_project_counter_caches, on: :destroy attr_spammable :title, spam_title: true attr_spammable :description, spam_description: true diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 2753e4b16e5..e4d8f486c77 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -52,7 +52,6 @@ class MergeRequest < ActiveRecord::Base after_create :ensure_merge_request_diff, unless: :importing? after_update :reload_diff_if_branch_changed - after_commit :update_project_counter_caches, on: :destroy # When this attribute is true some MR validation is ignored # It allows us to close or modify broken merge requests diff --git a/app/services/issuable/destroy_service.rb b/app/services/issuable/destroy_service.rb new file mode 100644 index 00000000000..0610b401213 --- /dev/null +++ b/app/services/issuable/destroy_service.rb @@ -0,0 +1,9 @@ +module Issuable + class DestroyService < IssuableBaseService + def execute(issuable) + if issuable.destroy + issuable.update_project_counter_caches + end + end + end +end diff --git a/changelogs/unreleased/39601-create-issuable-destroy-service.yml b/changelogs/unreleased/39601-create-issuable-destroy-service.yml new file mode 100644 index 00000000000..b0463f02eba --- /dev/null +++ b/changelogs/unreleased/39601-create-issuable-destroy-service.yml @@ -0,0 +1,5 @@ +--- +title: Create issuable destroy service +merge_request: 15604 +author: George Andrinopoulos +type: other diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 74dfd9f96de..e60e00d7956 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -255,7 +255,9 @@ module API authorize!(:destroy_issue, issue) - destroy_conditionally!(issue) + destroy_conditionally!(issue) do |issue| + Issuable::DestroyService.new(user_project, current_user).execute(issue) + end end desc 'List merge requests closing issue' do diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 5b4642a2f57..d34886fca2e 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -167,7 +167,9 @@ module API authorize!(:destroy_merge_request, merge_request) - destroy_conditionally!(merge_request) + destroy_conditionally!(merge_request) do |merge_request| + Issuable::DestroyService.new(user_project, current_user).execute(merge_request) + end end params do diff --git a/spec/services/issuable/destroy_service_spec.rb b/spec/services/issuable/destroy_service_spec.rb new file mode 100644 index 00000000000..d74d98c6079 --- /dev/null +++ b/spec/services/issuable/destroy_service_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe Issuable::DestroyService do + let(:user) { create(:user) } + let(:project) { create(:project) } + + subject(:service) { described_class.new(project, user) } + + describe '#execute' do + context 'when issuable is an issue' do + let!(:issue) { create(:issue, project: project, author: user) } + + it 'destroys the issue' do + expect { service.execute(issue) }.to change { project.issues.count }.by(-1) + end + + it 'updates open issues count cache' do + expect_any_instance_of(Projects::OpenIssuesCountService).to receive(:refresh_cache) + + service.execute(issue) + end + end + + context 'when issuable is a merge request' do + let!(:merge_request) { create(:merge_request, target_project: project, source_project: project, author: user) } + + it 'destroys the merge request' do + expect { service.execute(merge_request) }.to change { project.merge_requests.count }.by(-1) + end + + it 'updates open merge requests count cache' do + expect_any_instance_of(Projects::OpenMergeRequestsCountService).to receive(:refresh_cache) + + service.execute(merge_request) + end + end + end +end -- cgit v1.2.1 From c3444c9a2078c350d8da6e2d377ad18ba32287a1 Mon Sep 17 00:00:00 2001 From: Takuya Noguchi Date: Sun, 26 Nov 2017 18:45:11 +0900 Subject: Upgrade seed-fu to 2.3.7 --- Gemfile | 2 +- Gemfile.lock | 2 +- changelogs/unreleased/40568-bump-seed-fu-to-2-3-7.yml | 5 +++++ 3 files changed, 7 insertions(+), 2 deletions(-) create mode 100644 changelogs/unreleased/40568-bump-seed-fu-to-2-3-7.yml diff --git a/Gemfile b/Gemfile index a618b1116f5..b3e0d270ad8 100644 --- a/Gemfile +++ b/Gemfile @@ -111,7 +111,7 @@ gem 'google-api-client', '~> 0.13.6' gem 'unf', '~> 0.1.4' # Seed data -gem 'seed-fu', '~> 2.3.5' +gem 'seed-fu', '~> 2.3.7' # Markdown and HTML processing gem 'html-pipeline', '~> 1.11.0' diff --git a/Gemfile.lock b/Gemfile.lock index 3fcb223dd4e..cb4d0ea8faa 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -815,7 +815,7 @@ GEM rake (>= 0.9, < 13) sass (~> 3.4.20) securecompare (1.0.0) - seed-fu (2.3.6) + seed-fu (2.3.7) activerecord (>= 3.1) activesupport (>= 3.1) select2-rails (3.5.9.3) diff --git a/changelogs/unreleased/40568-bump-seed-fu-to-2-3-7.yml b/changelogs/unreleased/40568-bump-seed-fu-to-2-3-7.yml new file mode 100644 index 00000000000..708269d5c83 --- /dev/null +++ b/changelogs/unreleased/40568-bump-seed-fu-to-2-3-7.yml @@ -0,0 +1,5 @@ +--- +title: Upgrade seed-fu to 2.3.7 +merge_request: 15607 +author: Takuya Noguchi +type: other -- cgit v1.2.1 From f385d64710071b7339f84b70048f98c6ee04dd27 Mon Sep 17 00:00:00 2001 From: Simon Knox Date: Mon, 27 Nov 2017 13:13:15 +1100 Subject: simplify usage of issuable mixin --- .../javascripts/sidebar/components/lock/edit_form.vue | 9 ++------- .../sidebar/components/lock/lock_issue_sidebar.vue | 9 ++------- app/assets/javascripts/vue_shared/mixins/issuable.js | 13 +++++++++---- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/app/assets/javascripts/sidebar/components/lock/edit_form.vue b/app/assets/javascripts/sidebar/components/lock/edit_form.vue index c7a6edc7c70..242e826d471 100644 --- a/app/assets/javascripts/sidebar/components/lock/edit_form.vue +++ b/app/assets/javascripts/sidebar/components/lock/edit_form.vue @@ -18,11 +18,6 @@ export default { required: true, type: Function, }, - - issuableType: { - required: true, - type: String, - }, }, mixins: [ @@ -39,13 +34,13 @@ export default {

- Unlock this {{ issuableDisplayName(issuableType) }}? + Unlock this {{ issuableDisplayName }}? Everyone will be able to comment.

- Lock this {{ issuableDisplayName(issuableType) }}? + Lock this {{ issuableDisplayName }}? Only project members will be able to comment. diff --git a/app/assets/javascripts/sidebar/components/lock/lock_issue_sidebar.vue b/app/assets/javascripts/sidebar/components/lock/lock_issue_sidebar.vue index 9aff53cf8af..04c3a96bf74 100644 --- a/app/assets/javascripts/sidebar/components/lock/lock_issue_sidebar.vue +++ b/app/assets/javascripts/sidebar/components/lock/lock_issue_sidebar.vue @@ -23,11 +23,6 @@ export default { return mediatorObject.service && mediatorObject.service.update && mediatorObject.store; }, }, - - issuableType: { - required: true, - type: String, - }, }, mixins: [ @@ -59,7 +54,7 @@ export default { discussion_locked: locked, }) .then(() => location.reload()) - .catch(() => Flash(this.__(`Something went wrong trying to change the locked state of this ${this.issuableDisplayName(this.issuableType)}`))); + .catch(() => Flash(this.__(`Something went wrong trying to change the locked state of this ${this.issuableDisplayName}`))); }, }, }; @@ -77,7 +72,7 @@ export default {

- Lock {{issuableDisplayName(issuableType) }} + Lock {{ issuableDisplayName }}