diff options
107 files changed, 2177 insertions, 340 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 01d4a546b97..2b79f0825e2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -553,7 +553,7 @@ the feature you contribute through all of these steps. 1. Description explaining the relevancy (see following item) 1. Working and clean code that is commented where needed -1. [Unit and system tests][testing] that pass on the CI server +1. [Unit, integration, and system tests][testing] that pass on the CI server 1. Performance/scalability implications have been considered, addressed, and tested 1. [Documented][doc-styleguide] in the `/doc` directory 1. [Changelog entry added][changelog], if necessary diff --git a/app/assets/javascripts/groups/components/group_item.vue b/app/assets/javascripts/groups/components/group_item.vue index 6421547bbde..02129d39846 100644 --- a/app/assets/javascripts/groups/components/group_item.vue +++ b/app/assets/javascripts/groups/components/group_item.vue @@ -77,7 +77,8 @@ export default { class="group-row" > <div - class="group-row-contents"> + class="group-row-contents" + :class="{ 'project-row-contents': !isGroup }"> <item-actions v-if="isGroup" :group="group" @@ -97,7 +98,7 @@ export default { /> </div> <div - class="avatar-container s40 hidden-xs" + class="avatar-container prepend-top-8 prepend-left-5 s24 hidden-xs" :class="{ 'content-loading': group.isChildrenLoading }" > <a @@ -106,11 +107,12 @@ export default { > <img v-if="hasAvatar" - class="avatar s40" + class="avatar s24" :src="group.avatarUrl" /> <identicon v-else + size-class="s24" :entity-id=group.id :entity-name="group.name" /> @@ -123,7 +125,7 @@ export default { :href="group.relativePath" :title="group.fullName" class="no-expand" - data-placement="top" + data-placement="bottom" >{{ // ending bracket must be by closing tag to prevent // link hover text-decoration from over-extending diff --git a/app/assets/javascripts/groups/components/item_actions.vue b/app/assets/javascripts/groups/components/item_actions.vue index 58ba5aff7cf..a685960d862 100644 --- a/app/assets/javascripts/groups/components/item_actions.vue +++ b/app/assets/javascripts/groups/components/item_actions.vue @@ -1,14 +1,14 @@ <script> -import { s__ } from '../../locale'; -import tooltip from '../../vue_shared/directives/tooltip'; -import modal from '../../vue_shared/components/modal.vue'; +import { s__ } from '~/locale'; +import tooltip from '~/vue_shared/directives/tooltip'; +import icon from '~/vue_shared/components/icon.vue'; +import modal from '~/vue_shared/components/modal.vue'; import eventHub from '../event_hub'; import { COMMON_STR } from '../constants'; -import Icon from '../../vue_shared/components/icon.vue'; export default { components: { - Icon, + icon, modal, }, directives: { @@ -64,10 +64,9 @@ export default { :title="editBtnTitle" :aria-label="editBtnTitle" data-container="body" + data-placement="bottom" class="edit-group btn no-expand"> - <icon - name="settings"> - </icon> + <icon name="settings"/> </a> <a v-tooltip @@ -77,10 +76,9 @@ export default { :title="leaveBtnTitle" :aria-label="leaveBtnTitle" data-container="body" + data-placement="bottom" class="leave-group btn no-expand"> - <i - class="fa fa-sign-out" - aria-hidden="true"/> + <icon name="leave"/> </a> <modal v-show="modalStatus" diff --git a/app/assets/javascripts/groups/components/item_caret.vue b/app/assets/javascripts/groups/components/item_caret.vue index 959b984816f..9e90fe2b701 100644 --- a/app/assets/javascripts/groups/components/item_caret.vue +++ b/app/assets/javascripts/groups/components/item_caret.vue @@ -1,4 +1,6 @@ <script> +import icon from '~/vue_shared/components/icon.vue'; + export default { props: { isGroupOpen: { @@ -7,9 +9,12 @@ export default { default: false, }, }, + components: { + icon, + }, computed: { iconClass() { - return this.isGroupOpen ? 'fa-caret-down' : 'fa-caret-right'; + return this.isGroupOpen ? 'angle-down' : 'angle-right'; }, }, }; @@ -17,9 +22,9 @@ export default { <template> <span class="folder-caret"> - <i - :class="iconClass" - class="fa" - aria-hidden="true"/> + <icon + :size="12" + :name="iconClass" + /> </span> </template> diff --git a/app/assets/javascripts/groups/components/item_stats.vue b/app/assets/javascripts/groups/components/item_stats.vue index d9bc0588908..bd9a434e255 100644 --- a/app/assets/javascripts/groups/components/item_stats.vue +++ b/app/assets/javascripts/groups/components/item_stats.vue @@ -1,15 +1,19 @@ <script> -import tooltip from '../../vue_shared/directives/tooltip'; +import icon from '~/vue_shared/components/icon.vue'; +import timeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; import { ITEM_TYPE, VISIBILITY_TYPE_ICON, GROUP_VISIBILITY_TYPE, - PROJECT_VISIBILITY_TYPE, + PROJECT_VISIBILITY_TYPE } from '../constants'; +import itemStatsValue from './item_stats_value.vue'; export default { - directives: { - tooltip, + components: { + icon, + timeAgoTooltip, + itemStatsValue, }, props: { item: { @@ -39,65 +43,47 @@ export default { <template> <div class="stats"> - <span - v-tooltip + <item-stats-value v-if="isGroup" + css-class="number-subgroups" + icon-name="folder" :title="s__('Subgroups')" - class="number-subgroups" - data-placement="top" - data-container="body"> - <i - class="fa fa-folder" - aria-hidden="true" - /> - {{item.subgroupCount}} - </span> - <span - v-tooltip + :value=item.subgroupCount + /> + <item-stats-value v-if="isGroup" + css-class="number-projects" + icon-name="bookmark" :title="s__('Projects')" - class="number-projects" - data-placement="top" - data-container="body"> - <i - class="fa fa-bookmark" - aria-hidden="true" - /> - {{item.projectCount}} - </span> - <span - v-tooltip + :value=item.projectCount + /> + <item-stats-value v-if="isGroup" + css-class="number-users" + icon-name="users" :title="s__('Members')" - class="number-users" - data-placement="top" - data-container="body"> - <i - class="fa fa-users" - aria-hidden="true" - /> - {{item.memberCount}} - </span> - <span + :value=item.memberCount + /> + <item-stats-value v-if="isProject" - class="project-stars"> - <i - class="fa fa-star" - aria-hidden="true" - /> - {{item.starCount}} - </span> - <span - v-tooltip + css-class="project-stars" + icon-name="star" + :value=item.starCount + /> + <item-stats-value + css-class="item-visibility" + tooltip-placement="left" + :icon-name="visibilityIcon" :title="visibilityTooltip" - data-placement="left" - data-container="body" - class="item-visibility"> - <i - :class="visibilityIcon" - class="fa" - aria-hidden="true" + /> + <div + class="last-updated" + v-if="isProject" + > + <time-ago-tooltip + tooltip-placement="bottom" + :time="item.updatedAt" /> - </span> + </div> </div> </template> diff --git a/app/assets/javascripts/groups/components/item_stats_value.vue b/app/assets/javascripts/groups/components/item_stats_value.vue new file mode 100644 index 00000000000..f441cabf6d2 --- /dev/null +++ b/app/assets/javascripts/groups/components/item_stats_value.vue @@ -0,0 +1,68 @@ +<script> +import tooltip from '~/vue_shared/directives/tooltip'; +import icon from '~/vue_shared/components/icon.vue'; + +export default { + props: { + title: { + type: String, + required: false, + default: '', + }, + cssClass: { + type: String, + required: false, + default: '', + }, + iconName: { + type: String, + required: true, + }, + tooltipPlacement: { + type: String, + required: false, + default: 'bottom', + }, + /** + * value could either be number or string + * as `memberCount` is always passed as string + * while `subgroupCount` & `projectCount` + * are always number + */ + value: { + type: [Number, String], + required: false, + default: '', + }, + }, + directives: { + tooltip, + }, + components: { + icon, + }, + computed: { + isValuePresent() { + return this.value !== ''; + }, + }, +}; +</script> + +<template> + <span + v-tooltip + data-container="body" + :data-placement="tooltipPlacement" + :class="cssClass" + :title="title" + > + <icon :name="iconName"/> + <span + v-if="isValuePresent" + class="stat-value" + > + {{value}} + </span> + </span> +</template> diff --git a/app/assets/javascripts/groups/components/item_type_icon.vue b/app/assets/javascripts/groups/components/item_type_icon.vue index c02a8ad6d8c..118d94d4937 100644 --- a/app/assets/javascripts/groups/components/item_type_icon.vue +++ b/app/assets/javascripts/groups/components/item_type_icon.vue @@ -1,7 +1,11 @@ <script> +import icon from '~/vue_shared/components/icon.vue'; import { ITEM_TYPE } from '../constants'; export default { + components: { + icon, + }, props: { itemType: { type: String, @@ -16,9 +20,9 @@ export default { computed: { iconClass() { if (this.itemType === ITEM_TYPE.GROUP) { - return this.isGroupOpen ? 'fa-folder-open' : 'fa-folder'; + return this.isGroupOpen ? 'folder-open' : 'folder'; } - return 'fa-bookmark'; + return 'bookmark'; }, }, }; @@ -26,9 +30,6 @@ export default { <template> <span class="item-type-icon"> - <i - :class="iconClass" - class="fa" - aria-hidden="true"/> + <icon :name="iconClass"/> </span> </template> diff --git a/app/assets/javascripts/groups/constants.js b/app/assets/javascripts/groups/constants.js index 6fde41414b3..b8baed682f5 100644 --- a/app/assets/javascripts/groups/constants.js +++ b/app/assets/javascripts/groups/constants.js @@ -29,7 +29,7 @@ export const PROJECT_VISIBILITY_TYPE = { }; export const VISIBILITY_TYPE_ICON = { - public: 'fa-globe', - internal: 'fa-shield', - private: 'fa-lock', + public: 'earth', + internal: 'shield', + private: 'lock', }; diff --git a/app/assets/javascripts/groups/store/groups_store.js b/app/assets/javascripts/groups/store/groups_store.js index a1689f4c5cc..ffc86175548 100644 --- a/app/assets/javascripts/groups/store/groups_store.js +++ b/app/assets/javascripts/groups/store/groups_store.js @@ -91,6 +91,7 @@ export default class GroupsStore { subgroupCount: rawGroupItem.subgroup_count, memberCount: rawGroupItem.number_users_with_delimiter, starCount: rawGroupItem.star_count, + updatedAt: rawGroupItem.updated_at, }; } diff --git a/app/assets/javascripts/ide/components/ide_context_bar.vue b/app/assets/javascripts/ide/components/ide_context_bar.vue index 5a08718e386..78c01272af6 100644 --- a/app/assets/javascripts/ide/components/ide_context_bar.vue +++ b/app/assets/javascripts/ide/components/ide_context_bar.vue @@ -2,11 +2,18 @@ import { mapGetters, mapState, mapActions } from 'vuex'; import repoCommitSection from './repo_commit_section.vue'; import icon from '../../vue_shared/components/icon.vue'; +import panelResizer from '../../vue_shared/components/panel_resizer.vue'; export default { + data() { + return { + width: 290, + }; + }, components: { repoCommitSection, icon, + panelResizer, }, computed: { ...mapState([ @@ -18,10 +25,20 @@ export default { currentIcon() { return this.rightPanelCollapsed ? 'angle-double-left' : 'angle-double-right'; }, + maxSize() { + return window.innerWidth / 2; + }, + panelStyle() { + if (!this.rightPanelCollapsed) { + return { width: `${this.width}px` }; + } + return {}; + }, }, methods: { ...mapActions([ 'setPanelCollapsedStatus', + 'setResizingStatus', ]), toggleCollapsed() { this.setPanelCollapsedStatus({ @@ -29,6 +46,12 @@ export default { collapsed: !this.rightPanelCollapsed, }); }, + resizingStarted() { + this.setResizingStatus(true); + }, + resizingEnded() { + this.setResizingStatus(false); + }, }, }; </script> @@ -39,6 +62,7 @@ export default { :class="{ 'is-collapsed': rightPanelCollapsed, }" + :style="panelStyle" > <div class="multi-file-commit-panel-section"> @@ -71,5 +95,14 @@ export default { <repo-commit-section class=""/> </div> + <panel-resizer + :size.sync="width" + :enabled="!rightPanelCollapsed" + :start-size="290" + :min-size="200" + :max-size="maxSize" + @resize-start="resizingStarted" + @resize-end="resizingEnded" + side="left"/> </div> </template> diff --git a/app/assets/javascripts/ide/components/ide_side_bar.vue b/app/assets/javascripts/ide/components/ide_side_bar.vue index 535398d98c2..269f300a04d 100644 --- a/app/assets/javascripts/ide/components/ide_side_bar.vue +++ b/app/assets/javascripts/ide/components/ide_side_bar.vue @@ -2,11 +2,18 @@ import { mapState, mapActions } from 'vuex'; import projectTree from './ide_project_tree.vue'; import icon from '../../vue_shared/components/icon.vue'; +import panelResizer from '../../vue_shared/components/panel_resizer.vue'; export default { + data() { + return { + width: 290, + }; + }, components: { projectTree, icon, + panelResizer, }, computed: { ...mapState([ @@ -16,10 +23,20 @@ export default { currentIcon() { return this.leftPanelCollapsed ? 'angle-double-right' : 'angle-double-left'; }, + maxSize() { + return window.innerWidth / 2; + }, + panelStyle() { + if (!this.leftPanelCollapsed) { + return { width: `${this.width}px` }; + } + return {}; + }, }, methods: { ...mapActions([ 'setPanelCollapsedStatus', + 'setResizingStatus', ]), toggleCollapsed() { this.setPanelCollapsedStatus({ @@ -27,6 +44,12 @@ export default { collapsed: !this.leftPanelCollapsed, }); }, + resizingStarted() { + this.setResizingStatus(true); + }, + resizingEnded() { + this.setResizingStatus(false); + }, }, }; </script> @@ -37,6 +60,7 @@ export default { :class="{ 'is-collapsed': leftPanelCollapsed, }" + :style="panelStyle" > <div class="multi-file-commit-panel-inner"> <project-tree @@ -58,5 +82,14 @@ export default { class="collapse-text" >Collapse sidebar</span> </button> + <panel-resizer + :size.sync="width" + :enabled="!leftPanelCollapsed" + :start-size="290" + :min-size="200" + :max-size="maxSize" + @resize-start="resizingStarted" + @resize-end="resizingEnded" + side="right"/> </div> </template> diff --git a/app/assets/javascripts/ide/components/repo_editor.vue b/app/assets/javascripts/ide/components/repo_editor.vue index 221be4b9074..343fd0a5300 100644 --- a/app/assets/javascripts/ide/components/repo_editor.vue +++ b/app/assets/javascripts/ide/components/repo_editor.vue @@ -90,6 +90,11 @@ export default { rightPanelCollapsed() { this.editor.updateDimensions(); }, + panelResizing(isResizing) { + if (isResizing === false) { + this.editor.updateDimensions(); + } + }, }, computed: { ...mapGetters([ @@ -99,6 +104,7 @@ export default { ...mapState([ 'leftPanelCollapsed', 'rightPanelCollapsed', + 'panelResizing', ]), shouldHideEditor() { return this.activeFile.binary && !this.activeFile.raw; diff --git a/app/assets/javascripts/ide/stores/actions.js b/app/assets/javascripts/ide/stores/actions.js index c01046c8c76..335882bb6d7 100644 --- a/app/assets/javascripts/ide/stores/actions.js +++ b/app/assets/javascripts/ide/stores/actions.js @@ -63,6 +63,10 @@ export const setPanelCollapsedStatus = ({ commit }, { side, collapsed }) => { } }; +export const setResizingStatus = ({ commit }, resizing) => { + commit(types.SET_RESIZING_STATUS, resizing); +}; + export const checkCommitStatus = ({ state }) => service .getBranchData(state.currentProjectId, state.currentBranchId) diff --git a/app/assets/javascripts/ide/stores/mutation_types.js b/app/assets/javascripts/ide/stores/mutation_types.js index 4e3c10972ba..69b218a5e7d 100644 --- a/app/assets/javascripts/ide/stores/mutation_types.js +++ b/app/assets/javascripts/ide/stores/mutation_types.js @@ -5,6 +5,7 @@ export const SET_ROOT = 'SET_ROOT'; export const SET_LAST_COMMIT_DATA = 'SET_LAST_COMMIT_DATA'; export const SET_LEFT_PANEL_COLLAPSED = 'SET_LEFT_PANEL_COLLAPSED'; export const SET_RIGHT_PANEL_COLLAPSED = 'SET_RIGHT_PANEL_COLLAPSED'; +export const SET_RESIZING_STATUS = 'SET_RESIZING_STATUS'; // Project Mutation Types export const SET_PROJECT = 'SET_PROJECT'; diff --git a/app/assets/javascripts/ide/stores/mutations.js b/app/assets/javascripts/ide/stores/mutations.js index 2fed9019cb6..03d81be10a1 100644 --- a/app/assets/javascripts/ide/stores/mutations.js +++ b/app/assets/javascripts/ide/stores/mutations.js @@ -49,6 +49,11 @@ export default { rightPanelCollapsed: collapsed, }); }, + [types.SET_RESIZING_STATUS](state, resizing) { + Object.assign(state, { + panelResizing: resizing, + }); + }, [types.SET_LAST_COMMIT_DATA](state, { entry, lastCommit }) { Object.assign(entry.lastCommit, { id: lastCommit.commit.id, diff --git a/app/assets/javascripts/ide/stores/state.js b/app/assets/javascripts/ide/stores/state.js index 539e382830f..61d12096946 100644 --- a/app/assets/javascripts/ide/stores/state.js +++ b/app/assets/javascripts/ide/stores/state.js @@ -19,4 +19,5 @@ export default () => ({ projects: {}, leftPanelCollapsed: false, rightPanelCollapsed: true, + panelResizing: false, }); diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.js index b25cc3443ef..dd8b2665b1d 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_closed.js @@ -16,9 +16,9 @@ export default { <div class="media-body"> <mr-widget-author-and-time actionText="Closed by" - :author="mr.closedEvent.author" - :dateTitle="mr.closedEvent.updatedAt" - :dateReadable="mr.closedEvent.formattedUpdatedAt" + :author="mr.metrics.closedBy" + :dateTitle="mr.metrics.closedAt" + :dateReadable="mr.metrics.readableClosedAt" /> <section class="mr-info-list"> <p> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.js index 7c73ebf667d..ba9681680ef 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.js @@ -68,9 +68,9 @@ export default { <div class="space-children"> <mr-widget-author-and-time actionText="Merged by" - :author="mr.mergedEvent.author" - :date-title="mr.mergedEvent.updatedAt" - :date-readable="mr.mergedEvent.formattedUpdatedAt" /> + :author="mr.metrics.mergedBy" + :date-title="mr.metrics.mergedAt" + :date-readable="mr.metrics.readableMergedAt" /> <a v-if="mr.canRevertInCurrentMR" v-tooltip diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js index 93d31a2a684..474c17ec133 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js @@ -39,9 +39,8 @@ export default class MergeRequestStore { } this.updatedAt = data.updated_at; - this.mergedEvent = MergeRequestStore.getEventObject(data.merge_event); - this.closedEvent = MergeRequestStore.getEventObject(data.closed_event); - this.setToMWPSBy = MergeRequestStore.getAuthorObject({ author: data.merge_user || {} }); + this.metrics = MergeRequestStore.buildMetrics(data.metrics); + this.setToMWPSBy = MergeRequestStore.formatUserObject(data.merge_user || {}); this.mergeUserId = data.merge_user_id; this.currentUserId = gon.current_user_id; this.sourceBranchPath = data.source_branch_path; @@ -125,43 +124,42 @@ export default class MergeRequestStore { return this.state === stateKey.nothingToMerge; } - static getEventObject(event) { + static buildMetrics(metrics) { + if (!metrics) { + return {}; + } + return { - author: MergeRequestStore.getAuthorObject(event), - updatedAt: formatDate(MergeRequestStore.getEventUpdatedAtDate(event)), - formattedUpdatedAt: MergeRequestStore.getEventDate(event), + mergedBy: MergeRequestStore.formatUserObject(metrics.merged_by), + closedBy: MergeRequestStore.formatUserObject(metrics.closed_by), + mergedAt: formatDate(metrics.merged_at), + closedAt: formatDate(metrics.closed_at), + readableMergedAt: MergeRequestStore.getReadableDate(metrics.merged_at), + readableClosedAt: MergeRequestStore.getReadableDate(metrics.closed_at), }; } - static getAuthorObject(event) { - if (!event) { + static formatUserObject(user) { + if (!user) { return {}; } return { - name: event.author.name || '', - username: event.author.username || '', - webUrl: event.author.web_url || '', - avatarUrl: event.author.avatar_url || '', + name: user.name || '', + username: user.username || '', + webUrl: user.web_url || '', + avatarUrl: user.avatar_url || '', }; } - static getEventUpdatedAtDate(event) { - if (!event) { + static getReadableDate(date) { + if (!date) { return ''; } - return event.updated_at; - } - - static getEventDate(event) { const timeagoInstance = new Timeago(); - if (!event) { - return ''; - } - - return timeagoInstance.format(MergeRequestStore.getEventUpdatedAtDate(event)); + return timeagoInstance.format(date); } } diff --git a/app/assets/javascripts/vue_shared/components/panel_resizer.vue b/app/assets/javascripts/vue_shared/components/panel_resizer.vue new file mode 100644 index 00000000000..4371534d345 --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/panel_resizer.vue @@ -0,0 +1,91 @@ +<script> +export default { + props: { + startSize: { + type: Number, + required: true, + }, + side: { + type: String, + required: true, + }, + minSize: { + type: Number, + required: false, + default: 0, + }, + maxSize: { + type: Number, + required: false, + default: Number.MAX_VALUE, + }, + enabled: { + type: Boolean, + required: false, + default: true, + }, + }, + data() { + return { + size: this.startSize, + }; + }, + computed: { + className() { + return `drag${this.side}`; + }, + cursorStyle() { + if (this.enabled) { + return { cursor: 'ew-resize' }; + } + return {}; + }, + }, + methods: { + resetSize(e) { + e.preventDefault(); + this.size = this.startSize; + this.$emit('update:size', this.size); + }, + startDrag(e) { + if (this.enabled) { + e.preventDefault(); + this.startPos = e.clientX; + this.currentStartSize = this.size; + document.addEventListener('mousemove', this.drag); + document.addEventListener('mouseup', this.endDrag, { once: true }); + this.$emit('resize-start', this.size); + } + }, + drag(e) { + e.preventDefault(); + let moved = e.clientX - this.startPos; + if (this.side === 'left') moved = -moved; + let newSize = this.currentStartSize + moved; + if (newSize < this.minSize) { + newSize = this.minSize; + } else if (newSize > this.maxSize) { + newSize = this.maxSize; + } + this.size = newSize; + + this.$emit('update:size', newSize); + }, + endDrag(e) { + e.preventDefault(); + document.removeEventListener('mousemove', this.drag); + this.$emit('resize-end', this.size); + }, + }, +}; +</script> + +<template> + <div + class="dragHandle" + :class="className" + :style="cursorStyle" + @mousedown="startDrag" + @dblclick="resetSize" + ></div> +</template> diff --git a/app/assets/stylesheets/framework/avatar.scss b/app/assets/stylesheets/framework/avatar.scss index 26db2386879..077d0424093 100644 --- a/app/assets/stylesheets/framework/avatar.scss +++ b/app/assets/stylesheets/framework/avatar.scss @@ -71,7 +71,7 @@ vertical-align: top; &.s16 { font-size: 12px; line-height: 1.33; } - &.s24 { font-size: 14px; line-height: 1.8; } + &.s24 { font-size: 13px; line-height: 1.8; } &.s26 { font-size: 20px; line-height: 1.33; } &.s32 { font-size: 20px; line-height: 30px; } &.s40 { font-size: 16px; line-height: 38px; } diff --git a/app/assets/stylesheets/framework/lists.scss b/app/assets/stylesheets/framework/lists.scss index dba482bfb23..dcd98cb522f 100644 --- a/app/assets/stylesheets/framework/lists.scss +++ b/app/assets/stylesheets/framework/lists.scss @@ -126,10 +126,8 @@ ul.content-list { } .description { - p { - @include str-truncated; - margin-bottom: 0; - } + @include str-truncated; + color: $gl-text-color-secondary; } .controls { @@ -315,7 +313,7 @@ ul.indent-list { border: 2px solid $white-normal; &.identicon { - line-height: 30px; + line-height: 15px; } } } @@ -349,14 +347,19 @@ ul.indent-list { .folder-caret { width: 15px; + + svg { + margin-bottom: 2px; + } } .item-type-icon { + margin-top: 2px; width: 20px; } > .group-row:not(.has-children) { - .folder-caret .fa { + .folder-caret { opacity: 0; } } @@ -439,12 +442,61 @@ ul.indent-list { .avatar-container > a { width: 100%; + text-decoration: none; } &.has-more-items { display: block; padding: 20px 10px; } + + .stats { + position: relative; + line-height: 46px; + + > span { + display: inline-flex; + align-items: center; + height: 16px; + min-width: 30px; + } + + > span:last-child { + margin-right: 0; + } + + .stat-value { + margin: 2px 0 0 5px; + } + } + + .controls { + margin-left: 5px; + + > .btn { + margin-right: $btn-xs-side-margin; + } + } + } + + .project-row-contents .stats { + line-height: inherit; + + > span:first-child { + margin-left: 25px; + } + + .item-visibility { + margin-right: 0; + } + + .last-updated { + position: absolute; + right: 12px; + min-width: 250px; + text-align: right; + color: $gl-text-color-secondary; + } } } @@ -456,12 +508,12 @@ ul.indent-list { ul.group-list-tree { li.group-row { - &.has-description .title { - line-height: inherit; + > .group-row-contents .title { + line-height: $list-text-height; } - &:not(.has-description) .title { - line-height: $list-text-height; + &.has-description > .group-row-contents .title { + line-height: inherit; } } } diff --git a/app/assets/stylesheets/pages/repo.scss b/app/assets/stylesheets/pages/repo.scss index 51cc1729d9a..d01cbadebcc 100644 --- a/app/assets/stylesheets/pages/repo.scss +++ b/app/assets/stylesheets/pages/repo.scss @@ -36,10 +36,6 @@ } } -.with-performance-bar .ide-view { - height: calc(100vh - #{$header-height}); -} - .ide-file-list { flex: 1; @@ -242,12 +238,13 @@ table.table tr td.multi-file-table-name { .multi-file-commit-panel { display: flex; + position: relative; flex-direction: column; height: 100%; width: 290px; padding: 0; background-color: $gray-light; - border-left: 1px solid $white-dark; + padding-right: 3px; .projects-sidebar { display: flex; @@ -496,3 +493,30 @@ table.table tr td.multi-file-table-name { margin-top: $header-height; margin-bottom: 0; } + +.with-performance-bar { + .ide-flash-container.flash-container { + margin-top: $header-height + $performance-bar-height; + } + + .ide-view { + height: calc(100vh - #{$header-height + $performance-bar-height}); + } +} + + +.dragHandle { + position: absolute; + top: 0; + bottom: 0; + width: 3px; + background-color: $white-dark; + + &.dragright { + right: 0; + } + + &.dragleft { + left: 0; + } +} diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss index 5d630c7d61e..6353482ede7 100644 --- a/app/assets/stylesheets/pages/settings.scss +++ b/app/assets/stylesheets/pages/settings.scss @@ -268,3 +268,7 @@ margin: 0 0 5px 17px; } } + +.deprecated-service { + cursor: default; +} diff --git a/app/helpers/services_helper.rb b/app/helpers/services_helper.rb index 3707bb5ba36..240783bc7fd 100644 --- a/app/helpers/services_helper.rb +++ b/app/helpers/services_helper.rb @@ -27,5 +27,16 @@ module ServicesHelper "#{event}_events" end + def service_save_button(service) + button_tag(class: 'btn btn-save', type: 'submit', disabled: service.deprecated?) do + icon('spinner spin', class: 'hidden js-btn-spinner') + + content_tag(:span, 'Save changes', class: 'js-btn-label') + end + end + + def disable_fields_service?(service) + !current_controller?("admin/services") && service.deprecated? + end + extend self end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 5ca4a7086cb..4251561a0a0 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -96,7 +96,7 @@ module Issuable strip_attributes :title - after_save :record_metrics, unless: :imported? + after_save :ensure_metrics, unless: :imported? # We want to use optimistic lock for cases when only title or description are involved # http://api.rubyonrails.org/classes/ActiveRecord/Locking/Optimistic.html @@ -335,11 +335,6 @@ module Issuable false end - def record_metrics - metrics = self.metrics || create_metrics - metrics.record! - end - ## # Override in issuable specialization # @@ -347,6 +342,10 @@ module Issuable false end + def ensure_metrics + self.metrics || create_metrics + end + ## # Overriden in MergeRequest # diff --git a/app/models/issue.rb b/app/models/issue.rb index dc64888b6fc..4eafc1316d6 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -276,6 +276,11 @@ class Issue < ActiveRecord::Base private + def ensure_metrics + super + metrics.record! + end + # Returns `true` if the given User can read the current Issue. # # This method duplicates the same check of issue_policy.rb diff --git a/app/models/merge_request/metrics.rb b/app/models/merge_request/metrics.rb index cdc408738be..9e660eccd86 100644 --- a/app/models/merge_request/metrics.rb +++ b/app/models/merge_request/metrics.rb @@ -1,12 +1,6 @@ class MergeRequest::Metrics < ActiveRecord::Base belongs_to :merge_request belongs_to :pipeline, class_name: 'Ci::Pipeline', foreign_key: :pipeline_id - - def record! - if merge_request.merged? && self.merged_at.blank? - self.merged_at = Time.now - end - - self.save - end + belongs_to :latest_closed_by, class_name: 'User' + belongs_to :merged_by, class_name: 'User' end diff --git a/app/models/project_services/kubernetes_service.rb b/app/models/project_services/kubernetes_service.rb index b82567ce2b3..c72b01b64af 100644 --- a/app/models/project_services/kubernetes_service.rb +++ b/app/models/project_services/kubernetes_service.rb @@ -31,6 +31,7 @@ class KubernetesService < DeploymentService before_validation :enforce_namespace_to_lower_case + validate :deprecation_validation, unless: :template? validates :namespace, allow_blank: true, length: 1..63, @@ -145,6 +146,17 @@ class KubernetesService < DeploymentService @kubeclient ||= build_kubeclient! end + def deprecated? + !active + end + + def deprecation_message + content = <<-MESSAGE.strip_heredoc + Kubernetes service integration has been deprecated. #{deprecated_message_content} your clusters using the new <a href=\'#{Gitlab::Routing.url_helpers.project_clusters_path(project)}'/>Clusters</a> page + MESSAGE + content.html_safe + end + TEMPLATE_PLACEHOLDER = 'Kubernetes namespace'.freeze private @@ -226,4 +238,20 @@ class KubernetesService < DeploymentService def enforce_namespace_to_lower_case self.namespace = self.namespace&.downcase end + + def deprecation_validation + return if active_changed?(from: true, to: false) + + if deprecated? + errors[:base] << deprecation_message + end + end + + def deprecated_message_content + if active? + "Your cluster information on this page is still editable, but you are advised to disable and reconfigure" + else + "Fields on this page are now uneditable, you can configure" + end + end end diff --git a/app/models/service.rb b/app/models/service.rb index 3c4f1885dd0..176b472e724 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -263,6 +263,14 @@ class Service < ActiveRecord::Base service end + def deprecated? + false + end + + def deprecation_message + nil + end + private def cache_project_has_external_issue_tracker diff --git a/app/serializers/event_entity.rb b/app/serializers/event_entity.rb deleted file mode 100644 index 935d67a4f37..00000000000 --- a/app/serializers/event_entity.rb +++ /dev/null @@ -1,4 +0,0 @@ -class EventEntity < Grape::Entity - expose :author, using: UserEntity - expose :updated_at -end diff --git a/app/serializers/merge_request_metrics_entity.rb b/app/serializers/merge_request_metrics_entity.rb new file mode 100644 index 00000000000..3548107ac16 --- /dev/null +++ b/app/serializers/merge_request_metrics_entity.rb @@ -0,0 +1,6 @@ +class MergeRequestMetricsEntity < Grape::Entity + expose :latest_closed_at, as: :closed_at + expose :merged_at + expose :latest_closed_by, as: :closed_by, using: UserEntity + expose :merged_by, using: UserEntity +end diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb index f8e59b2ffd7..e905e6876c2 100644 --- a/app/serializers/merge_request_widget_entity.rb +++ b/app/serializers/merge_request_widget_entity.rb @@ -17,9 +17,11 @@ class MergeRequestWidgetEntity < IssuableEntity merge_request.project.merge_requests_ff_only_enabled end - # Events - expose :merge_event, using: EventEntity - expose :closed_event, using: EventEntity + expose :metrics do |merge_request| + metrics = build_metrics(merge_request) + + MergeRequestMetricsEntity.new(metrics).as_json + end # User entities expose :merge_user, using: UserEntity @@ -178,4 +180,27 @@ class MergeRequestWidgetEntity < IssuableEntity @presenters ||= {} @presenters[merge_request] ||= MergeRequestPresenter.new(merge_request, current_user: current_user) end + + # Once SchedulePopulateMergeRequestMetricsWithEventsData fully runs, + # we can remove this method and just serialize MergeRequest#metrics + # instead. See https://gitlab.com/gitlab-org/gitlab-ce/issues/41587 + def build_metrics(merge_request) + # There's no need to query and serialize metrics data for merge requests that are not + # merged or closed. + return unless merge_request.merged? || merge_request.closed? + return merge_request.metrics if merge_request.merged? && merge_request.metrics&.merged_by_id + return merge_request.metrics if merge_request.closed? && merge_request.metrics&.latest_closed_by_id + + build_metrics_from_events(merge_request) + end + + def build_metrics_from_events(merge_request) + closed_event = merge_request.closed_event + merge_event = merge_request.merge_event + + MergeRequest::Metrics.new(latest_closed_at: closed_event&.updated_at, + latest_closed_by: closed_event&.author, + merged_at: merge_event&.updated_at, + merged_by: merge_event&.author) + end end diff --git a/app/services/event_create_service.rb b/app/services/event_create_service.rb index 6328d567a07..44dc90b3462 100644 --- a/app/services/event_create_service.rb +++ b/app/services/event_create_service.rb @@ -103,6 +103,6 @@ class EventCreateService author_id: current_user.id ) - Event.create(attributes) + Event.create!(attributes) end end diff --git a/app/services/merge_request_metrics_service.rb b/app/services/merge_request_metrics_service.rb new file mode 100644 index 00000000000..9248de14a53 --- /dev/null +++ b/app/services/merge_request_metrics_service.rb @@ -0,0 +1,19 @@ +class MergeRequestMetricsService + delegate :update!, to: :@merge_request_metrics + + def initialize(merge_request_metrics) + @merge_request_metrics = merge_request_metrics + end + + def merge(event) + update!(merged_by_id: event.author_id, merged_at: event.created_at) + end + + def close(event) + update!(latest_closed_by_id: event.author_id, latest_closed_at: event.created_at) + end + + def reopen + update!(latest_closed_by_id: nil, latest_closed_at: nil) + end +end diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 6b32d65a74b..20a2b50d3de 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -24,6 +24,10 @@ module MergeRequests private + def merge_request_metrics_service(merge_request) + MergeRequestMetricsService.new(merge_request.metrics) + end + def create_assignee_note(merge_request) SystemNoteService.change_assignee( merge_request, merge_request.project, current_user, merge_request.assignee) diff --git a/app/services/merge_requests/close_service.rb b/app/services/merge_requests/close_service.rb index 40213c99014..f727ec002e7 100644 --- a/app/services/merge_requests/close_service.rb +++ b/app/services/merge_requests/close_service.rb @@ -8,7 +8,7 @@ module MergeRequests merge_request.allow_broken = true if merge_request.close - event_service.close_mr(merge_request, current_user) + create_event(merge_request) create_note(merge_request) notification_service.close_mr(merge_request, current_user) todo_service.close_merge_request(merge_request, current_user) @@ -19,5 +19,16 @@ module MergeRequests merge_request end + + private + + def create_event(merge_request) + # Making sure MergeRequest::Metrics updates are in sync with + # Event creation. + Event.transaction do + close_event = event_service.close_mr(merge_request, current_user) + merge_request_metrics_service(merge_request).close(close_event) + end + end end end diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb index b1d6bac4d4a..c78e78afcd1 100644 --- a/app/services/merge_requests/post_merge_service.rb +++ b/app/services/merge_requests/post_merge_service.rb @@ -9,7 +9,7 @@ module MergeRequests close_issues(merge_request) todo_service.merge_merge_request(merge_request, current_user) merge_request.mark_as_merged - create_merge_event(merge_request, current_user) + create_event(merge_request) create_note(merge_request) notification_service.merge_mr(merge_request, current_user) execute_hooks(merge_request, 'merge') @@ -34,5 +34,14 @@ module MergeRequests def create_merge_event(merge_request, current_user) EventCreateService.new.merge_mr(merge_request, current_user) end + + def create_event(merge_request) + # Making sure MergeRequest::Metrics updates are in sync with + # Event creation. + Event.transaction do + merge_event = create_merge_event(merge_request, current_user) + merge_request_metrics_service(merge_request).merge(merge_event) + end + end end end diff --git a/app/services/merge_requests/reopen_service.rb b/app/services/merge_requests/reopen_service.rb index c599a90f9fe..120677a7149 100644 --- a/app/services/merge_requests/reopen_service.rb +++ b/app/services/merge_requests/reopen_service.rb @@ -4,7 +4,7 @@ module MergeRequests return merge_request unless can?(current_user, :update_merge_request, merge_request) if merge_request.reopen - event_service.reopen_mr(merge_request, current_user) + create_event(merge_request) create_note(merge_request, 'reopened') notification_service.reopen_mr(merge_request, current_user) execute_hooks(merge_request, 'reopen') @@ -16,5 +16,16 @@ module MergeRequests merge_request end + + private + + def create_event(merge_request) + # Making sure MergeRequest::Metrics updates are in sync with + # Event creation. + Event.transaction do + event_service.reopen_mr(merge_request, current_user) + merge_request_metrics_service(merge_request).reopen + end + end end end diff --git a/app/views/projects/services/_deprecated_message.html.haml b/app/views/projects/services/_deprecated_message.html.haml new file mode 100644 index 00000000000..fea9506a4bb --- /dev/null +++ b/app/views/projects/services/_deprecated_message.html.haml @@ -0,0 +1,3 @@ +.flash-container.flash-container-page + .flash-alert.deprecated-service + %span= @service.deprecation_message diff --git a/app/views/projects/services/_form.html.haml b/app/views/projects/services/_form.html.haml index c0b1c62e8ef..21acd857ce7 100644 --- a/app/views/projects/services/_form.html.haml +++ b/app/views/projects/services/_form.html.haml @@ -13,10 +13,7 @@ = render 'shared/service_settings', form: form, subject: @service - if @service.editable? .footer-block.row-content-block - %button.btn.btn-save{ type: 'submit' } - = icon('spinner spin', class: 'hidden js-btn-spinner') - %span.js-btn-label - Save changes + = service_save_button(@service) - if @service.valid? && @service.activated? - unless @service.can_test? diff --git a/app/views/projects/services/edit.html.haml b/app/views/projects/services/edit.html.haml index 25770df1c90..df1fd583670 100644 --- a/app/views/projects/services/edit.html.haml +++ b/app/views/projects/services/edit.html.haml @@ -2,4 +2,6 @@ - page_title @service.title, "Services" - add_to_breadcrumbs("Settings", edit_project_path(@project)) += render 'deprecated_message' if @service.deprecation_message + = render 'form' diff --git a/app/views/shared/_field.html.haml b/app/views/shared/_field.html.haml index 795447a9ca6..aea0a8fd8e0 100644 --- a/app/views/shared/_field.html.haml +++ b/app/views/shared/_field.html.haml @@ -7,6 +7,7 @@ - choices = field[:choices] - default_choice = field[:default_choice] - help = field[:help] +- disabled = disable_fields_service?(@service) .form-group - if type == "password" && value.present? @@ -15,14 +16,14 @@ = form.label name, title, class: "control-label" .col-sm-10 - if type == 'text' - = form.text_field name, class: "form-control", placeholder: placeholder, required: required + = form.text_field name, class: "form-control", placeholder: placeholder, required: required, disabled: disabled - elsif type == 'textarea' - = form.text_area name, rows: 5, class: "form-control", placeholder: placeholder, required: required + = form.text_area name, rows: 5, class: "form-control", placeholder: placeholder, required: required, disabled: disabled - elsif type == 'checkbox' - = form.check_box name + = form.check_box name, disabled: disabled - elsif type == 'select' - = form.select name, options_for_select(choices, value ? value : default_choice), {}, { class: "form-control" } + = form.select name, options_for_select(choices, value ? value : default_choice), {}, { class: "form-control", disabled: disabled} - elsif type == 'password' - = form.password_field name, autocomplete: "new-password", class: "form-control", required: value.blank? && :required + = form.password_field name, autocomplete: "new-password", class: "form-control", required: value.blank? && required, disabled: disabled - if help %span.help-block= help diff --git a/app/views/shared/_service_settings.html.haml b/app/views/shared/_service_settings.html.haml index 7ca14ac93cc..61b39afb5d4 100644 --- a/app/views/shared/_service_settings.html.haml +++ b/app/views/shared/_service_settings.html.haml @@ -11,7 +11,7 @@ .form-group = form.label :active, "Active", class: "control-label" .col-sm-10 - = form.check_box :active + = form.check_box :active, disabled: disable_fields_service?(@service) - if @service.supported_events.present? .form-group diff --git a/changelogs/unreleased/40453-fix-api-endpoints-to-edit-wiki-pages-where-project-belongs-to-a-group.yml b/changelogs/unreleased/40453-fix-api-endpoints-to-edit-wiki-pages-where-project-belongs-to-a-group.yml new file mode 100644 index 00000000000..30917098a95 --- /dev/null +++ b/changelogs/unreleased/40453-fix-api-endpoints-to-edit-wiki-pages-where-project-belongs-to-a-group.yml @@ -0,0 +1,5 @@ +--- +title: Fix API endpoints to edit wiki pages where project belongs to a group +merge_request: 16170 +author: +type: fixed diff --git a/changelogs/unreleased/40533-groups-tree-updates.yml b/changelogs/unreleased/40533-groups-tree-updates.yml new file mode 100644 index 00000000000..1bc0aa90f9e --- /dev/null +++ b/changelogs/unreleased/40533-groups-tree-updates.yml @@ -0,0 +1,6 @@ +--- +title: Update groups tree to use GitLab SVG icons, add last updated at information + for projects +merge_request: 15980 +author: +type: changed diff --git a/changelogs/unreleased/41054-disable-creation-of-new-kubernetes-integrations.yml b/changelogs/unreleased/41054-disable-creation-of-new-kubernetes-integrations.yml new file mode 100644 index 00000000000..b960b14624c --- /dev/null +++ b/changelogs/unreleased/41054-disable-creation-of-new-kubernetes-integrations.yml @@ -0,0 +1,6 @@ +--- +title: Disable creation of new Kubernetes Integrations unless they're active or created + from template +merge_request: 41054 +author: +type: added diff --git a/changelogs/unreleased/change-issues-closed-at-background-migration.yml b/changelogs/unreleased/change-issues-closed-at-background-migration.yml new file mode 100644 index 00000000000..1c81c6a889e --- /dev/null +++ b/changelogs/unreleased/change-issues-closed-at-background-migration.yml @@ -0,0 +1,5 @@ +--- +title: Use a background migration for issues.closed_at +merge_request: +author: +type: other diff --git a/changelogs/unreleased/osw-introduce-merge-request-statistics.yml b/changelogs/unreleased/osw-introduce-merge-request-statistics.yml new file mode 100644 index 00000000000..fed7c2141fb --- /dev/null +++ b/changelogs/unreleased/osw-introduce-merge-request-statistics.yml @@ -0,0 +1,5 @@ +--- +title: Cache merged and closed events data in merge_request_metrics table +merge_request: +author: +type: performance diff --git a/changelogs/unreleased/sh-validate-path-project-import.yml b/changelogs/unreleased/sh-validate-path-project-import.yml new file mode 100644 index 00000000000..acad66c0ab2 --- /dev/null +++ b/changelogs/unreleased/sh-validate-path-project-import.yml @@ -0,0 +1,5 @@ +--- +title: Avoid leaving a push event empty if payload cannot be created +merge_request: +author: +type: fixed diff --git a/db/migrate/20171127151038_add_events_related_columns_to_merge_request_metrics.rb b/db/migrate/20171127151038_add_events_related_columns_to_merge_request_metrics.rb new file mode 100644 index 00000000000..18af697cf88 --- /dev/null +++ b/db/migrate/20171127151038_add_events_related_columns_to_merge_request_metrics.rb @@ -0,0 +1,37 @@ +class AddEventsRelatedColumnsToMergeRequestMetrics < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + change_table :merge_request_metrics do |t| + t.references :merged_by, references: :users + t.references :latest_closed_by, references: :users + end + + add_column :merge_request_metrics, :latest_closed_at, :datetime_with_timezone + + add_concurrent_foreign_key :merge_request_metrics, :users, + column: :merged_by_id, + on_delete: :nullify + + add_concurrent_foreign_key :merge_request_metrics, :users, + column: :latest_closed_by_id, + on_delete: :nullify + end + + def down + if foreign_keys_for(:merge_request_metrics, :merged_by_id).any? + remove_foreign_key :merge_request_metrics, column: :merged_by_id + end + + if foreign_keys_for(:merge_request_metrics, :latest_closed_by_id).any? + remove_foreign_key :merge_request_metrics, column: :latest_closed_by_id + end + + remove_columns :merge_request_metrics, + :merged_by_id, :latest_closed_by_id, :latest_closed_at + end +end diff --git a/db/post_migrate/20171128214150_schedule_populate_merge_request_metrics_with_events_data.rb b/db/post_migrate/20171128214150_schedule_populate_merge_request_metrics_with_events_data.rb new file mode 100644 index 00000000000..547cc68e10e --- /dev/null +++ b/db/post_migrate/20171128214150_schedule_populate_merge_request_metrics_with_events_data.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true +# rubocop:disable GitlabSecurity/SqlInjection + +class SchedulePopulateMergeRequestMetricsWithEventsData < ActiveRecord::Migration + DOWNTIME = false + BATCH_SIZE = 10_000 + MIGRATION = 'PopulateMergeRequestMetricsWithEventsData' + + disable_ddl_transaction! + + class MergeRequest < ActiveRecord::Base + self.table_name = 'merge_requests' + + include ::EachBatch + end + + def up + merge_requests = MergeRequest.where("id IN (#{updatable_merge_requests_union_sql})").reorder(:id) + + say 'Scheduling `PopulateMergeRequestMetricsWithEventsData` jobs' + # It will update around 4_000_000 records in batches of 10_000 merge + # requests (running between 10 minutes) and should take around 66 hours to complete. + # Apparently, production PostgreSQL is able to vacuum 10k-20k dead_tuples by + # minute, and at maximum, each of these jobs should UPDATE 20k records. + # + # More information about the updates in `PopulateMergeRequestMetricsWithEventsData` class. + # + merge_requests.each_batch(of: BATCH_SIZE) do |relation, index| + range = relation.pluck('MIN(id)', 'MAX(id)').first + + BackgroundMigrationWorker.perform_in(index * 10.minutes, MIGRATION, range) + end + end + + def down + execute "update merge_request_metrics set latest_closed_at = null" + execute "update merge_request_metrics set latest_closed_by_id = null" + execute "update merge_request_metrics set merged_by_id = null" + end + + private + + # On staging: + # Planning time: 0.682 ms + # Execution time: 22033.158 ms + # + def updatable_merge_requests_union_sql + metrics_not_exists_clause = + 'NOT EXISTS (SELECT 1 FROM merge_request_metrics WHERE merge_request_metrics.merge_request_id = merge_requests.id)' + + without_metrics_data = <<-SQL.strip_heredoc + merge_request_metrics.merged_by_id IS NULL OR + merge_request_metrics.latest_closed_by_id IS NULL OR + merge_request_metrics.latest_closed_at IS NULL + SQL + + mrs_without_metrics_record = MergeRequest + .where(metrics_not_exists_clause) + .select(:id) + + mrs_without_events_data = MergeRequest + .joins('INNER JOIN merge_request_metrics ON merge_requests.id = merge_request_metrics.merge_request_id') + .where(without_metrics_data) + .select(:id) + + Gitlab::SQL::Union.new([mrs_without_metrics_record, mrs_without_events_data]).to_sql + end +end diff --git a/db/post_migrate/20171221140220_schedule_issues_closed_at_type_change.rb b/db/post_migrate/20171221140220_schedule_issues_closed_at_type_change.rb new file mode 100644 index 00000000000..be18c5866ae --- /dev/null +++ b/db/post_migrate/20171221140220_schedule_issues_closed_at_type_change.rb @@ -0,0 +1,45 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class ScheduleIssuesClosedAtTypeChange < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + class Issue < ActiveRecord::Base + self.table_name = 'issues' + include EachBatch + + def self.to_migrate + where('closed_at IS NOT NULL') + end + end + + def up + return unless migrate_column_type? + + change_column_type_using_background_migration( + Issue.to_migrate, + :closed_at, + :datetime_with_timezone + ) + end + + def down + return if migrate_column_type? + + change_column_type_using_background_migration( + Issue.to_migrate, + :closed_at, + :datetime + ) + end + + def migrate_column_type? + # Some environments may have already executed the previous version of this + # migration, thus we don't need to migrate those environments again. + column_for('issues', 'closed_at').type == :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index 42715d5e1e8..925629f28fb 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: 20171220191323) do +ActiveRecord::Schema.define(version: 20171221140220) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1056,6 +1056,9 @@ ActiveRecord::Schema.define(version: 20171220191323) do t.datetime "created_at", null: false t.datetime "updated_at", null: false t.integer "pipeline_id" + t.integer "merged_by_id" + t.integer "latest_closed_by_id" + t.datetime_with_timezone "latest_closed_at" end add_index "merge_request_metrics", ["first_deployed_to_production_at"], name: "index_merge_request_metrics_on_first_deployed_to_production_at", using: :btree @@ -1995,6 +1998,8 @@ ActiveRecord::Schema.define(version: 20171220191323) do add_foreign_key "merge_request_diffs", "merge_requests", name: "fk_8483f3258f", on_delete: :cascade add_foreign_key "merge_request_metrics", "ci_pipelines", column: "pipeline_id", on_delete: :cascade add_foreign_key "merge_request_metrics", "merge_requests", on_delete: :cascade + add_foreign_key "merge_request_metrics", "users", column: "latest_closed_by_id", name: "fk_ae440388cc", on_delete: :nullify + add_foreign_key "merge_request_metrics", "users", column: "merged_by_id", name: "fk_7f28d925f3", on_delete: :nullify add_foreign_key "merge_requests", "ci_pipelines", column: "head_pipeline_id", name: "fk_fd82eae0b9", on_delete: :nullify add_foreign_key "merge_requests", "merge_request_diffs", column: "latest_merge_request_diff_id", name: "fk_06067f5644", on_delete: :nullify add_foreign_key "merge_requests", "milestones", name: "fk_6a5165a692", on_delete: :nullify diff --git a/doc/development/what_requires_downtime.md b/doc/development/what_requires_downtime.md index 05e0a64af18..9d0c62ecc35 100644 --- a/doc/development/what_requires_downtime.md +++ b/doc/development/what_requires_downtime.md @@ -195,6 +195,63 @@ end And that's it, we're done! +## Changing Column Types For Large Tables + +While `change_column_type_concurrently` can be used for changing the type of a +column without downtime it doesn't work very well for large tables. Because all +of the work happens in sequence the migration can take a very long time to +complete, preventing a deployment from proceeding. +`change_column_type_concurrently` can also produce a lot of pressure on the +database due to it rapidly updating many rows in sequence. + +To reduce database pressure you should instead use +`change_column_type_using_background_migration` when migrating a column in a +large table (e.g. `issues`). This method works similar to +`change_column_type_concurrently` but uses background migration to spread the +work / load over a longer time period, without slowing down deployments. + +Usage of this method is fairly simple: + +```ruby +class ExampleMigration < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + disable_ddl_transaction! + + class Issue < ActiveRecord::Base + self.table_name = 'issues' + + include EachBatch + + def self.to_migrate + where('closed_at IS NOT NULL') + end + end + + def up + change_column_type_using_background_migration( + Issue.to_migrate, + :closed_at, + :datetime_with_timezone + ) + end + + def down + change_column_type_using_background_migration( + Issue.to_migrate, + :closed_at, + :datetime + ) + end +end +``` + +This would change the type of `issues.closed_at` to `timestamp with time zone`. + +Keep in mind that the relation passed to +`change_column_type_using_background_migration` _must_ include `EachBatch`, +otherwise it will raise a `TypeError`. + ## Adding Indexes Adding indexes is an expensive process that blocks INSERT and UPDATE queries for diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 9ba15893f55..8ad4b2ecbf3 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -69,7 +69,7 @@ module API end def wiki_page - page = user_project.wiki.find_page(params[:slug]) + page = ProjectWiki.new(user_project, current_user).find_page(params[:slug]) page || not_found!('Wiki Page') end diff --git a/lib/gitlab/background_migration/cleanup_concurrent_type_change.rb b/lib/gitlab/background_migration/cleanup_concurrent_type_change.rb new file mode 100644 index 00000000000..de622f657b2 --- /dev/null +++ b/lib/gitlab/background_migration/cleanup_concurrent_type_change.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # Background migration for cleaning up a concurrent column rename. + class CleanupConcurrentTypeChange + include Database::MigrationHelpers + + RESCHEDULE_DELAY = 10.minutes + + # table - The name of the table the migration is performed for. + # old_column - The name of the old (to drop) column. + # new_column - The name of the new column. + def perform(table, old_column, new_column) + return unless column_exists?(:issues, new_column) + + rows_to_migrate = define_model_for(table) + .where(new_column => nil) + .where + .not(old_column => nil) + + if rows_to_migrate.any? + BackgroundMigrationWorker.perform_in( + RESCHEDULE_DELAY, + 'CleanupConcurrentTypeChange', + [table, old_column, new_column] + ) + else + cleanup_concurrent_column_type_change(table, old_column) + end + end + + # These methods are necessary so we can re-use the migration helpers in + # this class. + def connection + ActiveRecord::Base.connection + end + + def method_missing(name, *args, &block) + connection.__send__(name, *args, &block) # rubocop: disable GitlabSecurity/PublicSend + end + + def respond_to_missing?(*args) + connection.respond_to?(*args) || super + end + + def define_model_for(table) + Class.new(ActiveRecord::Base) do + self.table_name = table + end + end + end + end +end diff --git a/lib/gitlab/background_migration/copy_column.rb b/lib/gitlab/background_migration/copy_column.rb new file mode 100644 index 00000000000..a2cb215c230 --- /dev/null +++ b/lib/gitlab/background_migration/copy_column.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # CopyColumn is a simple (reusable) background migration that can be used to + # update the value of a column based on the value of another column in the + # same table. + # + # For this background migration to work the table that is migrated _has_ to + # have an `id` column as the primary key. + class CopyColumn + # table - The name of the table that contains the columns. + # copy_from - The column containing the data to copy. + # copy_to - The column to copy the data to. + # start_id - The start ID of the range of rows to update. + # end_id - The end ID of the range of rows to update. + def perform(table, copy_from, copy_to, start_id, end_id) + return unless connection.column_exists?(table, copy_to) + + quoted_table = connection.quote_table_name(table) + quoted_copy_from = connection.quote_column_name(copy_from) + quoted_copy_to = connection.quote_column_name(copy_to) + + # We're using raw SQL here since this job may be frequently executed. As + # a result dynamically defining models would lead to many unnecessary + # schema information queries. + connection.execute <<-SQL.strip_heredoc + UPDATE #{quoted_table} + SET #{quoted_copy_to} = #{quoted_copy_from} + WHERE id BETWEEN #{start_id} AND #{end_id} + SQL + end + + def connection + ActiveRecord::Base.connection + end + end + end +end diff --git a/lib/gitlab/background_migration/migrate_events_to_push_event_payloads.rb b/lib/gitlab/background_migration/migrate_events_to_push_event_payloads.rb index 84ac00f1a5c..7088aa0860a 100644 --- a/lib/gitlab/background_migration/migrate_events_to_push_event_payloads.rb +++ b/lib/gitlab/background_migration/migrate_events_to_push_event_payloads.rb @@ -128,8 +128,14 @@ module Gitlab end def process_event(event) - replicate_event(event) - create_push_event_payload(event) if event.push_event? + ActiveRecord::Base.transaction do + replicate_event(event) + create_push_event_payload(event) if event.push_event? + end + rescue ActiveRecord::InvalidForeignKey => e + # A foreign key error means the associated event was removed. In this + # case we'll just skip migrating the event. + Rails.logger.error("Unable to migrate event #{event.id}: #{e}") end def replicate_event(event) @@ -137,9 +143,6 @@ module Gitlab .with_indifferent_access.except(:title, :data) EventForMigration.create!(new_attributes) - rescue ActiveRecord::InvalidForeignKey - # A foreign key error means the associated event was removed. In this - # case we'll just skip migrating the event. end def create_push_event_payload(event) @@ -156,9 +159,6 @@ module Gitlab ref: event.trimmed_ref_name, commit_title: event.commit_title ) - rescue ActiveRecord::InvalidForeignKey - # A foreign key error means the associated event was removed. In this - # case we'll just skip migrating the event. end def find_events(start_id, end_id) diff --git a/lib/gitlab/background_migration/populate_merge_request_metrics_with_events_data.rb b/lib/gitlab/background_migration/populate_merge_request_metrics_with_events_data.rb new file mode 100644 index 00000000000..8a901a9bf39 --- /dev/null +++ b/lib/gitlab/background_migration/populate_merge_request_metrics_with_events_data.rb @@ -0,0 +1,135 @@ +# frozen_string_literal: true +# rubocop:disable Metrics/LineLength +# rubocop:disable Metrics/MethodLength +# rubocop:disable Metrics/ClassLength +# rubocop:disable Style/Documentation + +module Gitlab + module BackgroundMigration + class PopulateMergeRequestMetricsWithEventsData + def perform(min_merge_request_id, max_merge_request_id) + insert_metrics_for_range(min_merge_request_id, max_merge_request_id) + update_metrics_with_events_data(min_merge_request_id, max_merge_request_id) + end + + # Inserts merge_request_metrics records for merge_requests without it for + # a given merge request batch. + def insert_metrics_for_range(min, max) + metrics_not_exists_clause = + <<-SQL.strip_heredoc + NOT EXISTS (SELECT 1 FROM merge_request_metrics + WHERE merge_request_metrics.merge_request_id = merge_requests.id) + SQL + + MergeRequest.where(metrics_not_exists_clause).where(id: min..max).each_batch do |batch| + select_sql = batch.select(:id, :created_at, :updated_at).to_sql + + execute("INSERT INTO merge_request_metrics (merge_request_id, created_at, updated_at) #{select_sql}") + end + end + + def update_metrics_with_events_data(min, max) + if Gitlab::Database.postgresql? + # Uses WITH syntax in order to update merged and closed events with a single UPDATE. + # WITH is not supported by MySQL. + update_events_for_range(min, max) + else + update_merged_events_for_range(min, max) + update_closed_events_for_range(min, max) + end + end + + private + + # Updates merge_request_metrics latest_closed_at, latest_closed_by_id and merged_by_id + # based on the latest event records on events table for a given merge request batch. + def update_events_for_range(min, max) + sql = <<-SQL.strip_heredoc + WITH events_for_update AS ( + SELECT DISTINCT ON (target_id, action) target_id, action, author_id, updated_at + FROM events + WHERE target_id BETWEEN #{min} AND #{max} + AND target_type = 'MergeRequest' + AND action IN (#{Event::CLOSED},#{Event::MERGED}) + ORDER BY target_id, action, id DESC + ) + UPDATE merge_request_metrics met + SET latest_closed_at = latest_closed.updated_at, + latest_closed_by_id = latest_closed.author_id, + merged_by_id = latest_merged.author_id + FROM (SELECT * FROM events_for_update WHERE action = #{Event::CLOSED}) AS latest_closed + FULL OUTER JOIN + (SELECT * FROM events_for_update WHERE action = #{Event::MERGED}) AS latest_merged + USING (target_id) + WHERE target_id = merge_request_id; + SQL + + execute(sql) + end + + # Updates merge_request_metrics latest_closed_at, latest_closed_by_id based on the latest closed + # records on events table for a given merge request batch. + def update_closed_events_for_range(min, max) + sql = + <<-SQL.strip_heredoc + UPDATE merge_request_metrics metrics, + (#{select_events(min, max, Event::CLOSED)}) closed_events + SET metrics.latest_closed_by_id = closed_events.author_id, + metrics.latest_closed_at = closed_events.updated_at #{where_matches_closed_events}; + SQL + + execute(sql) + end + + # Updates merge_request_metrics merged_by_id based on the latest merged + # records on events table for a given merge request batch. + def update_merged_events_for_range(min, max) + sql = + <<-SQL.strip_heredoc + UPDATE merge_request_metrics metrics, + (#{select_events(min, max, Event::MERGED)}) merged_events + SET metrics.merged_by_id = merged_events.author_id #{where_matches_merged_events}; + SQL + + execute(sql) + end + + def execute(sql) + @connection ||= ActiveRecord::Base.connection + @connection.execute(sql) + end + + def select_events(min, max, action) + select_max_event_id = <<-SQL.strip_heredoc + SELECT max(id) + FROM events + WHERE action = #{action} + AND target_type = 'MergeRequest' + AND target_id BETWEEN #{min} AND #{max} + GROUP BY target_id + SQL + + <<-SQL.strip_heredoc + SELECT author_id, updated_at, target_id + FROM events + WHERE id IN(#{select_max_event_id}) + SQL + end + + def where_matches_closed_events + <<-SQL.strip_heredoc + WHERE metrics.merge_request_id = closed_events.target_id + AND metrics.latest_closed_at IS NULL + AND metrics.latest_closed_by_id IS NULL + SQL + end + + def where_matches_merged_events + <<-SQL.strip_heredoc + WHERE metrics.merge_request_id = merged_events.target_id + AND metrics.merged_by_id IS NULL + SQL + end + end + end +end diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index 3f65bc912de..33171f83692 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -385,10 +385,27 @@ module Gitlab # necessary since we copy over old values further down. change_column_default(table, new, old_col.default) if old_col.default - trigger_name = rename_trigger_name(table, old, new) + install_rename_triggers(table, old, new) + + update_column_in_batches(table, new, Arel::Table.new(table)[old]) + + change_column_null(table, new, false) unless old_col.null + + copy_indexes(table, old, new) + copy_foreign_keys(table, old, new) + end + + # Installs triggers in a table that keep a new column in sync with an old + # one. + # + # table - The name of the table to install the trigger in. + # old_column - The name of the old column. + # new_column - The name of the new column. + def install_rename_triggers(table, old_column, new_column) + trigger_name = rename_trigger_name(table, old_column, new_column) quoted_table = quote_table_name(table) - quoted_old = quote_column_name(old) - quoted_new = quote_column_name(new) + quoted_old = quote_column_name(old_column) + quoted_new = quote_column_name(new_column) if Database.postgresql? install_rename_triggers_for_postgresql(trigger_name, quoted_table, @@ -397,13 +414,6 @@ module Gitlab install_rename_triggers_for_mysql(trigger_name, quoted_table, quoted_old, quoted_new) end - - update_column_in_batches(table, new, Arel::Table.new(table)[old]) - - change_column_null(table, new, false) unless old_col.null - - copy_indexes(table, old, new) - copy_foreign_keys(table, old, new) end # Changes the type of a column concurrently. @@ -455,6 +465,97 @@ module Gitlab remove_column(table, old) end + # Changes the column type of a table using a background migration. + # + # Because this method uses a background migration it's more suitable for + # large tables. For small tables it's better to use + # `change_column_type_concurrently` since it can complete its work in a + # much shorter amount of time and doesn't rely on Sidekiq. + # + # Example usage: + # + # class Issue < ActiveRecord::Base + # self.table_name = 'issues' + # + # include EachBatch + # + # def self.to_migrate + # where('closed_at IS NOT NULL') + # end + # end + # + # change_column_type_using_background_migration( + # Issue.to_migrate, + # :closed_at, + # :datetime_with_timezone + # ) + # + # Reverting a migration like this is done exactly the same way, just with + # a different type to migrate to (e.g. `:datetime` in the above example). + # + # relation - An ActiveRecord relation to use for scheduling jobs and + # figuring out what table we're modifying. This relation _must_ + # have the EachBatch module included. + # + # column - The name of the column for which the type will be changed. + # + # new_type - The new type of the column. + # + # batch_size - The number of rows to schedule in a single background + # migration. + # + # interval - The time interval between every background migration. + def change_column_type_using_background_migration( + relation, + column, + new_type, + batch_size: 10_000, + interval: 10.minutes + ) + unless relation.model < EachBatch + raise TypeError, 'The relation must include the EachBatch module' + end + + temp_column = "#{column}_for_type_change" + table = relation.table_name + max_index = 0 + + add_column(table, temp_column, new_type) + install_rename_triggers(table, column, temp_column) + + # Schedule the jobs that will copy the data from the old column to the + # new one. + relation.each_batch(of: batch_size) do |batch, index| + start_id, end_id = batch.pluck('MIN(id), MAX(id)').first + max_index = index + + BackgroundMigrationWorker.perform_in( + index * interval, + 'CopyColumn', + [table, column, temp_column, start_id, end_id] + ) + end + + # Schedule the renaming of the column to happen (initially) 1 hour after + # the last batch finished. + BackgroundMigrationWorker.perform_in( + (max_index * interval) + 1.hour, + 'CleanupConcurrentTypeChange', + [table, column, temp_column] + ) + + if perform_background_migration_inline? + # To ensure the schema is up to date immediately we perform the + # migration inline in dev / test environments. + Gitlab::BackgroundMigration.steal('CopyColumn') + Gitlab::BackgroundMigration.steal('CleanupConcurrentTypeChange') + end + end + + def perform_background_migration_inline? + Rails.env.test? || Rails.env.development? + end + # Performs a concurrent column rename when using PostgreSQL. def install_rename_triggers_for_postgresql(trigger, table, old, new) execute <<-EOF.strip_heredoc diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 37d67b6052c..176bd953ca1 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -1665,6 +1665,7 @@ module Gitlab cmd = %W[#{Gitlab.config.git.bin_path} --git-dir=#{path} rev-list] cmd << "--after=#{options[:after].iso8601}" if options[:after] cmd << "--before=#{options[:before].iso8601}" if options[:before] + cmd << "--max-count=#{options[:max_count]}" if options[:max_count] cmd += %W[--count #{options[:ref]}] cmd += %W[-- #{options[:path]}] if options[:path].present? diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index 8a29e8ec5b6..fed05bb6c64 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -130,6 +130,7 @@ module Gitlab request.after = Google::Protobuf::Timestamp.new(seconds: options[:after].to_i) if options[:after].present? request.before = Google::Protobuf::Timestamp.new(seconds: options[:before].to_i) if options[:before].present? request.path = options[:path] if options[:path].present? + request.max_count = options[:max_count] if options[:max_count].present? GitalyClient.call(@repository.storage, :commit_service, :count_commits, request, timeout: GitalyClient.medium_timeout).count end diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index c7732764880..ae1753ff0ae 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -101,6 +101,7 @@ module Gitlab request_enum.push(Gitaly::UserMergeBranchRequest.new(apply: true)) branch_update = response_enum.next.branch_update + return if branch_update.nil? raise Gitlab::Git::CommitError.new('failed to apply merge to branch') unless branch_update.commit_id.present? Gitlab::Git::OperationService::BranchUpdate.from_gitaly(branch_update) diff --git a/qa/qa/runtime/user.rb b/qa/qa/runtime/user.rb index 7bd50023561..2832439d9e0 100644 --- a/qa/qa/runtime/user.rb +++ b/qa/qa/runtime/user.rb @@ -12,13 +12,13 @@ module QA end def ssh_key - <<~KEY.tr("\n", '') - ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDFf6RYK3qu/RKF/3ndJmL5xgMLp3O9 - 6x8lTay+QGZ0+9FnnAXMdUqBq/ZU6d/gyMB4IaW3nHzM1w049++yAB6UPCzMB8Uo27K5 - /jyZCtj7Vm9PFNjF/8am1kp46c/SeYicQgQaSBdzIW3UDEa1Ef68qroOlvpi9PYZ/tA7 - M0YP0K5PXX+E36zaIRnJVMPT3f2k+GnrxtjafZrwFdpOP/Fol5BQLBgcsyiU+LM1SuaC - rzd8c9vyaTA1CxrkxaZh+buAi0PmdDtaDrHd42gqZkXCKavyvgM5o2CkQ5LJHCgzpXy0 - 5qNFzmThBSkb+XtoxbyagBiGbVZtSVow6Xa7qewz= dummy@gitlab.com + <<~KEY.delete("\n") + ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDFf6RYK3qu/RKF/3ndJmL5xgMLp3O9 + 6x8lTay+QGZ0+9FnnAXMdUqBq/ZU6d/gyMB4IaW3nHzM1w049++yAB6UPCzMB8Uo27K5 + /jyZCtj7Vm9PFNjF/8am1kp46c/SeYicQgQaSBdzIW3UDEa1Ef68qroOlvpi9PYZ/tA7 + M0YP0K5PXX+E36zaIRnJVMPT3f2k+GnrxtjafZrwFdpOP/Fol5BQLBgcsyiU+LM1SuaC + rzd8c9vyaTA1CxrkxaZh+buAi0PmdDtaDrHd42gqZkXCKavyvgM5o2CkQ5LJHCgzpXy0 + 5qNFzmThBSkb+XtoxbyagBiGbVZtSVow6Xa7qewz= dummy@gitlab.com KEY end end diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb index 2c6ad00515e..847ac6f2be0 100644 --- a/spec/controllers/projects/services_controller_spec.rb +++ b/spec/controllers/projects/services_controller_spec.rb @@ -114,5 +114,41 @@ describe Projects::ServicesController do expect(flash[:notice]).to eq 'HipChat settings saved, but not activated.' end end + + context 'with a deprecated service' do + let(:service) { create(:kubernetes_service, project: project) } + + before do + put :update, + namespace_id: project.namespace, project_id: project, id: service.to_param, service: { namespace: 'updated_namespace' } + end + + it 'should not update the service' do + service.reload + expect(service.namespace).not_to eq('updated_namespace') + end + end + end + + describe "GET #edit" do + before do + get :edit, namespace_id: project.namespace, project_id: project, id: service_id + end + + context 'with approved services' do + let(:service_id) { 'jira' } + + it 'should render edit page' do + expect(response).to be_success + end + end + + context 'with a deprecated service' do + let(:service_id) { 'kubernetes' } + + it 'should render edit page' do + expect(response).to be_success + end + end end end diff --git a/spec/factories/keys.rb b/spec/factories/keys.rb index e6eb76f71d3..552b4b7e06e 100644 --- a/spec/factories/keys.rb +++ b/spec/factories/keys.rb @@ -21,12 +21,14 @@ FactoryBot.define do factory :rsa_key_2048 do key do - 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDFf6RYK3qu/RKF/3ndJmL5xgMLp3O9' \ - '6x8lTay+QGZ0+9FnnAXMdUqBq/ZU6d/gyMB4IaW3nHzM1w049++yAB6UPCzMB8Uo27K5' \ - '/jyZCtj7Vm9PFNjF/8am1kp46c/SeYicQgQaSBdzIW3UDEa1Ef68qroOlvpi9PYZ/tA7' \ - 'M0YP0K5PXX+E36zaIRnJVMPT3f2k+GnrxtjafZrwFdpOP/Fol5BQLBgcsyiU+LM1SuaC' \ - 'rzd8c9vyaTA1CxrkxaZh+buAi0PmdDtaDrHd42gqZkXCKavyvgM5o2CkQ5LJHCgzpXy0' \ - '5qNFzmThBSkb+XtoxbyagBiGbVZtSVow6Xa7qewz= dummy@gitlab.com' + <<~KEY.delete("\n") + ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDFf6RYK3qu/RKF/3ndJmL5xgMLp3O9 + 6x8lTay+QGZ0+9FnnAXMdUqBq/ZU6d/gyMB4IaW3nHzM1w049++yAB6UPCzMB8Uo27K5 + /jyZCtj7Vm9PFNjF/8am1kp46c/SeYicQgQaSBdzIW3UDEa1Ef68qroOlvpi9PYZ/tA7 + M0YP0K5PXX+E36zaIRnJVMPT3f2k+GnrxtjafZrwFdpOP/Fol5BQLBgcsyiU+LM1SuaC + rzd8c9vyaTA1CxrkxaZh+buAi0PmdDtaDrHd42gqZkXCKavyvgM5o2CkQ5LJHCgzpXy0 + 5qNFzmThBSkb+XtoxbyagBiGbVZtSVow6Xa7qewz= dummy@gitlab.com + KEY end factory :rsa_deploy_key_2048, class: 'DeployKey' @@ -34,37 +36,44 @@ FactoryBot.define do factory :dsa_key_2048 do key do - 'ssh-dss AAAAB3NzaC1kc3MAAAEBAO/3/NPLA/zSFkMOCaTtGo+uos1flfQ5f038Uk+G' \ - 'Y9AeLGzX+Srhw59GdVXmOQLYBrOt5HdGwqYcmLnE2VurUGmhtfeO5H+3p5pGJbkS0Gxp' \ - 'YH1HRO9lWsncF3Hh1w4lYsDjkclDiSTdfTuN8F4Kb3DXNnVSCieeonp+B25F/CXagyTQ' \ - '/pvNmHFeYgGCVdnBtFdi+xfxaZ8NKdPrGggzokbKHElDZQ4Xo5EpdcyLajgM7nB2r2Rz' \ - 'OrmeaevKi5lV68ehRa9Yyrb7vxvwiwBwOgqR/mnN7Gnaq1jUdmJY+ct04Qwx37f5jvhv' \ - '5gA4U40SGMoiHM8RFIN7Ksz0jsyX73MAAAAVALRWOfjfzHpK7KLz4iqDvvTUAevJAAAB' \ - 'AEa9NZ+6y9iQ5erGsdfLTXFrhSefTG0NhghoO/5IFkSGfd8V7kzTvCHaFrcfpEA5kP8t' \ - 'poeOG0TASB6tgGOxm1Bq4Wncry5RORBPJlAVpDGRcvZ931ddH7IgltEInS6za2uH6F/1' \ - 'M1QfKePSLr6xJ1ZLYfP0Og5KTp1x6yMQvfwV0a+XdA+EPgaJWLWp/pWwKWa0oLUgjsIH' \ - 'MYzuOGh5c708uZrmkzqvgtW2NgXhcIroRgynT3IfI2lP2rqqb3uuuE/qH5UCUFO+Dc3H' \ - 'nAFNeQDT/M25AERdPYBAY5a+iPjIgO+jT7BfmfByT+AZTqZySrCyc7nNZL3YgGLK0l6A' \ - '1GgAAAEBAN9FpFOdIXE+YEZhKl1vPmbcn+b1y5zOl6N4x1B7Q8pD/pLMziWROIS8uLzb' \ - 'aZ0sMIWezHIkxuo1iROMeT+jtCubn7ragaN6AX7nMpxYUH9+mYZZs/fyElt6wCviVhTI' \ - 'zM+u7VdQsnZttOOlQfogHdL+SpeAft0DsfJjlcgQnsLlHQKv6aPqCPYUST2nE7RyW/Ex' \ - 'PrMxLtOWt0/j8RYHbwwqvyeZqBz3ESBgrS9c5tBdBfauwYUV/E7gPLOU3OZFw9ue7o+z' \ - 'wzoTZqW6Xouy5wtWvSLQSLT5XwOslmQz8QMBxD0AQyDfEFGsBCWzmbTgKv9uqrBjubsS' \ - 'Taja+Cf9kMo== dummy@gitlab.com' + <<~KEY.delete("\n") + ssh-dss AAAAB3NzaC1kc3MAAAEBAO/3/NPLA/zSFkMOCaTtGo+uos1flfQ5f038Uk+G + Y9AeLGzX+Srhw59GdVXmOQLYBrOt5HdGwqYcmLnE2VurUGmhtfeO5H+3p5pGJbkS0Gxp + YH1HRO9lWsncF3Hh1w4lYsDjkclDiSTdfTuN8F4Kb3DXNnVSCieeonp+B25F/CXagyTQ + /pvNmHFeYgGCVdnBtFdi+xfxaZ8NKdPrGggzokbKHElDZQ4Xo5EpdcyLajgM7nB2r2Rz + OrmeaevKi5lV68ehRa9Yyrb7vxvwiwBwOgqR/mnN7Gnaq1jUdmJY+ct04Qwx37f5jvhv + 5gA4U40SGMoiHM8RFIN7Ksz0jsyX73MAAAAVALRWOfjfzHpK7KLz4iqDvvTUAevJAAAB + AEa9NZ+6y9iQ5erGsdfLTXFrhSefTG0NhghoO/5IFkSGfd8V7kzTvCHaFrcfpEA5kP8t + poeOG0TASB6tgGOxm1Bq4Wncry5RORBPJlAVpDGRcvZ931ddH7IgltEInS6za2uH6F/1 + M1QfKePSLr6xJ1ZLYfP0Og5KTp1x6yMQvfwV0a+XdA+EPgaJWLWp/pWwKWa0oLUgjsIH + MYzuOGh5c708uZrmkzqvgtW2NgXhcIroRgynT3IfI2lP2rqqb3uuuE/qH5UCUFO+Dc3H + nAFNeQDT/M25AERdPYBAY5a+iPjIgO+jT7BfmfByT+AZTqZySrCyc7nNZL3YgGLK0l6A + 1GgAAAEBAN9FpFOdIXE+YEZhKl1vPmbcn+b1y5zOl6N4x1B7Q8pD/pLMziWROIS8uLzb + aZ0sMIWezHIkxuo1iROMeT+jtCubn7ragaN6AX7nMpxYUH9+mYZZs/fyElt6wCviVhTI + zM+u7VdQsnZttOOlQfogHdL+SpeAft0DsfJjlcgQnsLlHQKv6aPqCPYUST2nE7RyW/Ex + PrMxLtOWt0/j8RYHbwwqvyeZqBz3ESBgrS9c5tBdBfauwYUV/E7gPLOU3OZFw9ue7o+z + wzoTZqW6Xouy5wtWvSLQSLT5XwOslmQz8QMBxD0AQyDfEFGsBCWzmbTgKv9uqrBjubsS + Taja+Cf9kMo== dummy@gitlab.com + KEY end end factory :ecdsa_key_256 do key do - 'ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYA' \ - 'AABBBJZmkzTgY0fiCQ+DVReyH/fFwTFz0XoR3RUO0u+199H19KFw7mNPxRSMOVS7tEtO' \ - 'Nj3Q7FcZXfqthHvgAzDiHsc= dummy@gitlab.com' + <<~KEY.delete("\n") + ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYA + AABBBJZmkzTgY0fiCQ+DVReyH/fFwTFz0XoR3RUO0u+199H19KFw7mNPxRSMOVS7tEtO + Nj3Q7FcZXfqthHvgAzDiHsc= dummy@gitlab.com + KEY end end factory :ed25519_key_256 do key do - 'ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIETnVTgzqC1gatgSlC4zH6aYt2CAQzgJOhDRvf59ohL6 dummy@gitlab.com' + <<~KEY.delete("\n") + ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIETnVTgzqC1gatgSlC4zH6aYt2CAQzgJ + OhDRvf59ohL6 dummy@gitlab.com + KEY end end end diff --git a/spec/factories/services.rb b/spec/factories/services.rb index 4b0377967c7..110ef33c6f7 100644 --- a/spec/factories/services.rb +++ b/spec/factories/services.rb @@ -18,6 +18,7 @@ FactoryBot.define do factory :kubernetes_service do project + type 'KubernetesService' active true properties({ api_url: 'https://kubernetes.example.com', diff --git a/spec/features/dashboard/groups_list_spec.rb b/spec/features/dashboard/groups_list_spec.rb index d92c002b4e7..a71020002dc 100644 --- a/spec/features/dashboard/groups_list_spec.rb +++ b/spec/features/dashboard/groups_list_spec.rb @@ -94,22 +94,14 @@ feature 'Dashboard Groups page', :js do end it 'can toggle parent group' do - # Collapsed by default - expect(page).not_to have_selector("#group-#{group.id} .fa-caret-down", count: 1) - expect(page).to have_selector("#group-#{group.id} .fa-caret-right") - # expand click_group_caret(group) - expect(page).to have_selector("#group-#{group.id} .fa-caret-down") - expect(page).not_to have_selector("#group-#{group.id} .fa-caret-right", count: 1) expect(page).to have_selector("#group-#{group.id} #group-#{subgroup.id}") # collapse click_group_caret(group) - expect(page).not_to have_selector("#group-#{group.id} .fa-caret-down", count: 1) - expect(page).to have_selector("#group-#{group.id} .fa-caret-right") expect(page).not_to have_selector("#group-#{group.id} #group-#{subgroup.id}") end end diff --git a/spec/features/projects/clusters/interchangeability_spec.rb b/spec/features/projects/clusters/interchangeability_spec.rb index 01f9526608f..3ddb35c755c 100644 --- a/spec/features/projects/clusters/interchangeability_spec.rb +++ b/spec/features/projects/clusters/interchangeability_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' feature 'Interchangeability between KubernetesService and Platform::Kubernetes' do - EXCEPT_METHODS = %i[test title description help fields initialize_properties namespace namespace= api_url api_url=].freeze + EXCEPT_METHODS = %i[test title description help fields initialize_properties namespace namespace= api_url api_url= deprecated? deprecation_message].freeze EXCEPT_METHODS_GREP_V = %w[_touched? _changed? _was].freeze it 'Clusters::Platform::Kubernetes covers core interfaces in KubernetesService' do diff --git a/spec/fixtures/api/schemas/entities/merge_request_metrics.json b/spec/fixtures/api/schemas/entities/merge_request_metrics.json new file mode 100644 index 00000000000..3fa767f85df --- /dev/null +++ b/spec/fixtures/api/schemas/entities/merge_request_metrics.json @@ -0,0 +1,21 @@ +{ + "type": "object", + "required": ["closed_at", "merged_at", "closed_by", "merged_by"], + "properties" : { + "closed_at": { "type": ["datetime", "null"] }, + "merged_at": { "type": ["datetime", "null"] }, + "closed_by": { + "oneOf": [ + { "type": "null" }, + { "$ref": "user.json" } + ] + }, + "merged_by": { + "oneOf": [ + { "type": "null" }, + { "$ref": "user.json" } + ] + } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/api/schemas/entities/merge_request_widget.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json index 342890c3dee..9de27bee751 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_widget.json +++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json @@ -31,8 +31,12 @@ "source_project_id": { "type": "integer" }, "target_branch": { "type": "string" }, "target_project_id": { "type": "integer" }, - "merge_event": { "type": ["object", "null"] }, - "closed_event": { "type": ["object", "null"] }, + "metrics": { + "oneOf": [ + { "type": "null" }, + { "$ref": "merge_request_metrics.json" } + ] + }, "author": { "type": ["object", "null"] }, "merge_user": { "type": ["object", "null"] }, "diff_head_sha": { "type": ["string", "null"] }, diff --git a/spec/fixtures/api/schemas/entities/user.json b/spec/fixtures/api/schemas/entities/user.json new file mode 100644 index 00000000000..6482e0eedd2 --- /dev/null +++ b/spec/fixtures/api/schemas/entities/user.json @@ -0,0 +1,17 @@ +{ + "type": "object", + "required": [ + "id", + "state", + "avatar_url", + "web_url", + "path" + ], + "properties": { + "id": { "type": "integer" }, + "state": { "type": "string" }, + "avatar_url": { "type": "string" }, + "web_url": { "type": "string" }, + "path": { "type": "string" } + } +} diff --git a/spec/javascripts/groups/components/item_caret_spec.js b/spec/javascripts/groups/components/item_caret_spec.js index 4310a07e6e6..8faad455825 100644 --- a/spec/javascripts/groups/components/item_caret_spec.js +++ b/spec/javascripts/groups/components/item_caret_spec.js @@ -16,24 +16,20 @@ describe('ItemCaretComponent', () => { describe('template', () => { it('should render component template correctly', () => { const vm = createComponent(); - vm.$mount(); expect(vm.$el.classList.contains('folder-caret')).toBeTruthy(); + expect(vm.$el.querySelectorAll('svg').length).toBe(1); vm.$destroy(); }); it('should render caret down icon if `isGroupOpen` prop is `true`', () => { const vm = createComponent(true); - vm.$mount(); - expect(vm.$el.querySelectorAll('i.fa.fa-caret-down').length).toBe(1); - expect(vm.$el.querySelectorAll('i.fa.fa-caret-right').length).toBe(0); + expect(vm.$el.querySelector('svg use').getAttribute('xlink:href')).toContain('angle-down'); vm.$destroy(); }); it('should render caret right icon if `isGroupOpen` prop is `false`', () => { const vm = createComponent(); - vm.$mount(); - expect(vm.$el.querySelectorAll('i.fa.fa-caret-down').length).toBe(0); - expect(vm.$el.querySelectorAll('i.fa.fa-caret-right').length).toBe(1); + expect(vm.$el.querySelector('svg use').getAttribute('xlink:href')).toContain('angle-right'); vm.$destroy(); }); }); diff --git a/spec/javascripts/groups/components/item_stats_spec.js b/spec/javascripts/groups/components/item_stats_spec.js index e200f9f08bd..55a7a713ca6 100644 --- a/spec/javascripts/groups/components/item_stats_spec.js +++ b/spec/javascripts/groups/components/item_stats_spec.js @@ -26,7 +26,6 @@ describe('ItemStatsComponent', () => { Object.keys(VISIBILITY_TYPE_ICON).forEach((visibility) => { const item = Object.assign({}, mockParentGroupItem, { visibility }); const vm = createComponent(item); - vm.$mount(); expect(vm.visibilityIcon).toBe(VISIBILITY_TYPE_ICON[visibility]); vm.$destroy(); }); @@ -41,7 +40,6 @@ describe('ItemStatsComponent', () => { type: ITEM_TYPE.GROUP, }); const vm = createComponent(item); - vm.$mount(); expect(vm.visibilityTooltip).toBe(GROUP_VISIBILITY_TYPE[visibility]); vm.$destroy(); }); @@ -54,7 +52,6 @@ describe('ItemStatsComponent', () => { type: ITEM_TYPE.PROJECT, }); const vm = createComponent(item); - vm.$mount(); expect(vm.visibilityTooltip).toBe(PROJECT_VISIBILITY_TYPE[visibility]); vm.$destroy(); }); @@ -68,13 +65,11 @@ describe('ItemStatsComponent', () => { item = Object.assign({}, mockParentGroupItem, { type: ITEM_TYPE.PROJECT }); vm = createComponent(item); - vm.$mount(); expect(vm.isProject).toBeTruthy(); vm.$destroy(); item = Object.assign({}, mockParentGroupItem, { type: ITEM_TYPE.GROUP }); vm = createComponent(item); - vm.$mount(); expect(vm.isProject).toBeFalsy(); vm.$destroy(); }); @@ -87,13 +82,11 @@ describe('ItemStatsComponent', () => { item = Object.assign({}, mockParentGroupItem, { type: ITEM_TYPE.GROUP }); vm = createComponent(item); - vm.$mount(); expect(vm.isGroup).toBeTruthy(); vm.$destroy(); item = Object.assign({}, mockParentGroupItem, { type: ITEM_TYPE.PROJECT }); vm = createComponent(item); - vm.$mount(); expect(vm.isGroup).toBeFalsy(); vm.$destroy(); }); @@ -101,57 +94,37 @@ describe('ItemStatsComponent', () => { }); describe('template', () => { - it('should render component template correctly', () => { + it('renders component container element correctly', () => { const vm = createComponent(); - vm.$mount(); - const visibilityIconEl = vm.$el.querySelector('.item-visibility'); - expect(vm.$el.classList.contains('.stats')).toBeDefined(); - expect(visibilityIconEl).toBeDefined(); - expect(visibilityIconEl.dataset.originalTitle).toBe(vm.visibilityTooltip); - expect(visibilityIconEl.querySelector('i.fa')).toBeDefined(); + expect(vm.$el.classList.contains('stats')).toBeTruthy(); vm.$destroy(); }); - it('should render stat icons if `item.type` is Group', () => { - const item = Object.assign({}, mockParentGroupItem, { type: ITEM_TYPE.GROUP }); - const vm = createComponent(item); - vm.$mount(); - - const subgroupIconEl = vm.$el.querySelector('span.number-subgroups'); - expect(subgroupIconEl).toBeDefined(); - expect(subgroupIconEl.dataset.originalTitle).toBe('Subgroups'); - expect(subgroupIconEl.querySelector('i.fa.fa-folder')).toBeDefined(); - expect(subgroupIconEl.innerText.trim()).toBe(`${vm.item.subgroupCount}`); - - const projectsIconEl = vm.$el.querySelector('span.number-projects'); - expect(projectsIconEl).toBeDefined(); - expect(projectsIconEl.dataset.originalTitle).toBe('Projects'); - expect(projectsIconEl.querySelector('i.fa.fa-bookmark')).toBeDefined(); - expect(projectsIconEl.innerText.trim()).toBe(`${vm.item.projectCount}`); - - const membersIconEl = vm.$el.querySelector('span.number-users'); - expect(membersIconEl).toBeDefined(); - expect(membersIconEl.dataset.originalTitle).toBe('Members'); - expect(membersIconEl.querySelector('i.fa.fa-users')).toBeDefined(); - expect(membersIconEl.innerText.trim()).toBe(`${vm.item.memberCount}`); + it('renders item visibility icon and tooltip correctly', () => { + const vm = createComponent(); + + const visibilityIconEl = vm.$el.querySelector('.item-visibility'); + expect(visibilityIconEl).not.toBe(null); + expect(visibilityIconEl.dataset.originalTitle).toBe(vm.visibilityTooltip); + expect(visibilityIconEl.querySelectorAll('svg').length > 0).toBeTruthy(); vm.$destroy(); }); - it('should render stat icons if `item.type` is Project', () => { + it('renders start count and last updated information for project item correctly', () => { const item = Object.assign({}, mockParentGroupItem, { type: ITEM_TYPE.PROJECT, starCount: 4, }); const vm = createComponent(item); - vm.$mount(); const projectStarIconEl = vm.$el.querySelector('.project-stars'); - expect(projectStarIconEl).toBeDefined(); - expect(projectStarIconEl.querySelector('i.fa.fa-star')).toBeDefined(); - expect(projectStarIconEl.innerText.trim()).toBe(`${vm.item.starCount}`); + expect(projectStarIconEl).not.toBe(null); + expect(projectStarIconEl.querySelectorAll('svg').length > 0).toBeTruthy(); + expect(projectStarIconEl.querySelectorAll('.stat-value').length > 0).toBeTruthy(); + expect(vm.$el.querySelectorAll('.last-updated').length > 0).toBeTruthy(); vm.$destroy(); }); diff --git a/spec/javascripts/groups/components/item_stats_value_spec.js b/spec/javascripts/groups/components/item_stats_value_spec.js new file mode 100644 index 00000000000..e990870aaa6 --- /dev/null +++ b/spec/javascripts/groups/components/item_stats_value_spec.js @@ -0,0 +1,81 @@ +import Vue from 'vue'; + +import itemStatsValueComponent from '~/groups/components/item_stats_value.vue'; + +import mountComponent from '../../helpers/vue_mount_component_helper'; + +const createComponent = ({ title, cssClass, iconName, tooltipPlacement, value }) => { + const Component = Vue.extend(itemStatsValueComponent); + + return mountComponent(Component, { + title, + cssClass, + iconName, + tooltipPlacement, + value, + }); +}; + +describe('ItemStatsValueComponent', () => { + describe('computed', () => { + let vm; + const itemConfig = { + title: 'Subgroups', + cssClass: 'number-subgroups', + iconName: 'folder', + tooltipPlacement: 'left', + }; + + describe('isValuePresent', () => { + it('returns true if non-empty `value` is present', () => { + vm = createComponent(Object.assign({}, itemConfig, { value: 10 })); + expect(vm.isValuePresent).toBeTruthy(); + }); + + it('returns false if empty `value` is present', () => { + vm = createComponent(itemConfig); + expect(vm.isValuePresent).toBeFalsy(); + }); + + afterEach(() => { + vm.$destroy(); + }); + }); + }); + + describe('template', () => { + let vm; + beforeEach(() => { + vm = createComponent({ + title: 'Subgroups', + cssClass: 'number-subgroups', + iconName: 'folder', + tooltipPlacement: 'left', + value: 10, + }); + }); + + it('renders component element correctly', () => { + expect(vm.$el.classList.contains('number-subgroups')).toBeTruthy(); + expect(vm.$el.querySelectorAll('svg').length > 0).toBeTruthy(); + expect(vm.$el.querySelectorAll('.stat-value').length > 0).toBeTruthy(); + }); + + it('renders element tooltip correctly', () => { + expect(vm.$el.dataset.originalTitle).toBe('Subgroups'); + expect(vm.$el.dataset.placement).toBe('left'); + }); + + it('renders element icon correctly', () => { + expect(vm.$el.querySelector('svg use').getAttribute('xlink:href')).toContain('folder'); + }); + + it('renders value count correctly', () => { + expect(vm.$el.querySelector('.stat-value').innerText.trim()).toContain('10'); + }); + + afterEach(() => { + vm.$destroy(); + }); + }); +}); diff --git a/spec/javascripts/groups/components/item_type_icon_spec.js b/spec/javascripts/groups/components/item_type_icon_spec.js index 528e6ed1b4c..495cc97b475 100644 --- a/spec/javascripts/groups/components/item_type_icon_spec.js +++ b/spec/javascripts/groups/components/item_type_icon_spec.js @@ -28,12 +28,12 @@ describe('ItemTypeIconComponent', () => { vm = createComponent(ITEM_TYPE.GROUP, true); vm.$mount(); - expect(vm.$el.querySelector('i.fa.fa-folder-open')).toBeDefined(); + expect(vm.$el.querySelector('use').getAttribute('xlink:href')).toContain('folder-open'); vm.$destroy(); vm = createComponent(ITEM_TYPE.GROUP); vm.$mount(); - expect(vm.$el.querySelector('i.fa.fa-folder')).toBeDefined(); + expect(vm.$el.querySelector('use').getAttribute('xlink:href')).toContain('folder'); vm.$destroy(); }); @@ -42,12 +42,12 @@ describe('ItemTypeIconComponent', () => { vm = createComponent(ITEM_TYPE.PROJECT); vm.$mount(); - expect(vm.$el.querySelectorAll('i.fa.fa-bookmark').length).toBe(1); + expect(vm.$el.querySelector('use').getAttribute('xlink:href')).toContain('bookmark'); vm.$destroy(); vm = createComponent(ITEM_TYPE.GROUP); vm.$mount(); - expect(vm.$el.querySelectorAll('i.fa.fa-bookmark').length).toBe(0); + expect(vm.$el.querySelector('use').getAttribute('xlink:href')).not.toContain('bookmark'); vm.$destroy(); }); }); diff --git a/spec/javascripts/groups/mock_data.js b/spec/javascripts/groups/mock_data.js index 6184d671790..8bf6417487d 100644 --- a/spec/javascripts/groups/mock_data.js +++ b/spec/javascripts/groups/mock_data.js @@ -18,9 +18,9 @@ export const PROJECT_VISIBILITY_TYPE = { }; export const VISIBILITY_TYPE_ICON = { - public: 'fa-globe', - internal: 'fa-shield', - private: 'fa-lock', + public: 'earth', + internal: 'shield', + private: 'lock', }; export const mockParentGroupItem = { @@ -46,6 +46,7 @@ export const mockParentGroupItem = { isOpen: true, isChildrenLoading: false, isBeingRemoved: false, + updatedAt: '2017-04-09T18:40:39.101Z', }; export const mockRawChildren = [ @@ -69,6 +70,7 @@ export const mockRawChildren = [ subgroup_count: 2, can_leave: false, children: [], + updated_at: '2017-04-09T18:40:39.101Z', }, ]; @@ -96,6 +98,7 @@ export const mockChildren = [ isOpen: true, isChildrenLoading: false, isBeingRemoved: false, + updatedAt: '2017-04-09T18:40:39.101Z', }, ]; @@ -119,6 +122,7 @@ export const mockGroups = [ project_count: 2, subgroup_count: 0, can_leave: false, + updated_at: '2017-04-09T18:40:39.101Z', }, { id: 67, @@ -139,6 +143,7 @@ export const mockGroups = [ project_count: 0, subgroup_count: 0, can_leave: false, + updated_at: '2017-04-09T18:40:39.101Z', }, { id: 54, @@ -159,6 +164,7 @@ export const mockGroups = [ project_count: 0, subgroup_count: 1, can_leave: false, + updated_at: '2017-04-09T18:40:39.101Z', }, { id: 5, @@ -179,6 +185,7 @@ export const mockGroups = [ project_count: 1, subgroup_count: 0, can_leave: false, + updated_at: '2017-04-09T18:40:39.101Z', }, { id: 4, @@ -199,6 +206,7 @@ export const mockGroups = [ project_count: 2, subgroup_count: 0, can_leave: false, + updated_at: '2017-04-09T18:40:39.101Z', }, { id: 3, @@ -219,6 +227,7 @@ export const mockGroups = [ project_count: 1, subgroup_count: 0, can_leave: false, + updated_at: '2017-04-09T18:40:39.101Z', }, { id: 2, @@ -239,6 +248,7 @@ export const mockGroups = [ project_count: 4, subgroup_count: 0, can_leave: false, + updated_at: '2017-04-09T18:40:39.101Z', }, ]; @@ -262,6 +272,7 @@ export const mockSearchedGroups = [ project_count: 1, subgroup_count: 2, can_leave: false, + updated_at: '2017-04-09T18:40:39.101Z', children: [ { id: 57, @@ -282,6 +293,7 @@ export const mockSearchedGroups = [ project_count: 4, subgroup_count: 2, can_leave: false, + updated_at: '2017-04-09T18:40:39.101Z', children: [ { id: 60, @@ -302,6 +314,7 @@ export const mockSearchedGroups = [ project_count: 0, subgroup_count: 1, can_leave: false, + updated_at: '2017-04-09T18:40:39.101Z', children: [ { id: 61, @@ -322,6 +335,7 @@ export const mockSearchedGroups = [ project_count: 2, subgroup_count: 0, can_leave: false, + updated_at: '2017-04-09T18:40:39.101Z', children: [ { id: 17, @@ -336,6 +350,7 @@ export const mockSearchedGroups = [ permission: null, edit_path: '/platform/hardware/bsp/kernel/common/v4.4/edit', star_count: 0, + updated_at: '2017-09-12T06:37:04.925Z', }, { id: 16, @@ -350,6 +365,7 @@ export const mockSearchedGroups = [ permission: null, edit_path: '/platform/hardware/bsp/kernel/common/v4.1/edit', star_count: 0, + updated_at: '2017-04-09T18:41:03.112Z', }, ], }, diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_closed_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_closed_spec.js index d23b558f4ea..1bf97bbf093 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_closed_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_closed_spec.js @@ -4,13 +4,16 @@ import closedComponent from '~/vue_merge_request_widget/components/states/mr_wid const mr = { targetBranch: 'good-branch', targetBranchPath: '/good-branch', - closedEvent: { - author: { + metrics: { + mergedBy: {}, + mergedAt: 'mergedUpdatedAt', + closedBy: { name: 'Fatih Acet', username: 'fatihacet', }, - updatedAt: 'closedEventUpdatedAt', - formattedUpdatedAt: '', + closedAt: 'closedEventUpdatedAt', + readableMergedAt: '', + readableClosedAt: '', }, updatedAt: 'mrUpdatedAt', closedAt: '1 day ago', @@ -56,7 +59,7 @@ describe('MRWidgetClosed', () => { it('should have correct elements', () => { expect(el.querySelector('h4').textContent).toContain('Closed by'); - expect(el.querySelector('h4').textContent).toContain(mr.closedEvent.author.name); + expect(el.querySelector('h4').textContent).toContain(mr.metrics.closedBy.name); expect(el.textContent).toContain('The changes were not merged into'); expect(el.querySelector('.label-branch').getAttribute('href')).toEqual(mr.targetBranchPath); expect(el.querySelector('.label-branch').textContent).toContain(mr.targetBranch); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js index 9d3ae4b448f..2dc3b72ea40 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js @@ -14,10 +14,13 @@ const createComponent = () => { canRevertInCurrentMR: true, canRemoveSourceBranch: true, sourceBranchRemoved: true, - mergedEvent: { - author: {}, - updatedAt: 'mergedUpdatedAt', - formattedUpdatedAt: '', + metrics: { + mergedBy: {}, + mergedAt: 'mergedUpdatedAt', + readableMergedAt: '', + closedBy: {}, + closedAt: 'mergedUpdatedAt', + readableClosedAt: '', }, updatedAt: 'mrUpdatedAt', targetBranch, diff --git a/spec/javascripts/vue_mr_widget/mock_data.js b/spec/javascripts/vue_mr_widget/mock_data.js index 1ad7c2d8efa..ca29c9fee32 100644 --- a/spec/javascripts/vue_mr_widget/mock_data.js +++ b/spec/javascripts/vue_mr_widget/mock_data.js @@ -33,8 +33,8 @@ export default { "source_project_id": 19, "target_branch": "master", "target_project_id": 19, - "merge_event": { - "author": { + "metrics": { + "merged_by": { "name": "Administrator", "username": "root", "id": 1, @@ -42,9 +42,10 @@ export default { "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", "web_url": "http://localhost:3000/root" }, - "updated_at": "2017-04-07T15:39:25.696Z" + "merged_at": "2017-04-07T15:39:25.696Z", + "closed_by": null, + "closed_at": null }, - "closed_event": null, "author": { "name": "Administrator", "username": "root", diff --git a/spec/javascripts/vue_shared/components/panel_resizer_spec.js b/spec/javascripts/vue_shared/components/panel_resizer_spec.js new file mode 100644 index 00000000000..70ce3dffaba --- /dev/null +++ b/spec/javascripts/vue_shared/components/panel_resizer_spec.js @@ -0,0 +1,59 @@ +import Vue from 'vue'; +import panelResizer from '~/vue_shared/components/panel_resizer.vue'; +import mountComponent from '../../helpers/vue_mount_component_helper'; + +describe('Panel Resizer component', () => { + let vm; + let PanelResizer; + + const triggerEvent = (eventName, el = vm.$el, clientX = 0) => { + const event = document.createEvent('MouseEvents'); + event.initMouseEvent(eventName, true, true, window, 1, clientX, 0, clientX, 0, false, false, + false, false, 0, null); + + el.dispatchEvent(event); + }; + + beforeEach(() => { + PanelResizer = Vue.extend(panelResizer); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('should render a div element with the correct classes and styles', () => { + vm = mountComponent(PanelResizer, { + startSize: 100, + side: 'left', + }); + + expect(vm.$el.tagName).toEqual('DIV'); + expect(vm.$el.getAttribute('class')).toBe('dragHandle dragleft'); + expect(vm.$el.getAttribute('style')).toBe('cursor: ew-resize;'); + }); + + it('should render a div element with the correct classes for a right side panel', () => { + vm = mountComponent(PanelResizer, { + startSize: 100, + side: 'right', + }); + + expect(vm.$el.tagName).toEqual('DIV'); + expect(vm.$el.getAttribute('class')).toBe('dragHandle dragright'); + }); + + it('drag the resizer', () => { + vm = mountComponent(PanelResizer, { + startSize: 100, + side: 'left', + }); + + spyOn(vm, '$emit'); + triggerEvent('mousedown', vm.$el); + triggerEvent('mousemove', document); + triggerEvent('mouseup', document); + expect(vm.$emit.calls.allArgs()).toEqual([['resize-start', 100], ['update:size', 100], ['resize-end', 100]]); + expect(vm.size).toBe(100); + }); +}); diff --git a/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb b/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb index 7351d45336a..5432d270555 100644 --- a/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb +++ b/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb @@ -281,6 +281,17 @@ describe Gitlab::BackgroundMigration::MigrateEventsToPushEventPayloads, :migrati migration.process_event(event) end + + it 'handles an error gracefully' do + event1 = create_push_event(project, author, { commits: [] }) + + expect(migration).to receive(:replicate_event).and_call_original + expect(migration).to receive(:create_push_event_payload).and_raise(ActiveRecord::InvalidForeignKey, 'invalid foreign key') + + migration.process_event(event1) + + expect(described_class::EventForMigration.all.count).to eq(0) + end end describe '#replicate_event' do @@ -335,9 +346,8 @@ describe Gitlab::BackgroundMigration::MigrateEventsToPushEventPayloads, :migrati it 'does not create push event payloads for removed events' do allow(event).to receive(:id).and_return(-1) - payload = migration.create_push_event_payload(event) + expect { migration.create_push_event_payload(event) }.to raise_error(ActiveRecord::InvalidForeignKey) - expect(payload).to be_nil expect(PushEventPayload.count).to eq(0) end diff --git a/spec/lib/gitlab/background_migration/populate_merge_request_metrics_with_events_data_spec.rb b/spec/lib/gitlab/background_migration/populate_merge_request_metrics_with_events_data_spec.rb new file mode 100644 index 00000000000..dfe3b31f1c0 --- /dev/null +++ b/spec/lib/gitlab/background_migration/populate_merge_request_metrics_with_events_data_spec.rb @@ -0,0 +1,124 @@ +require 'rails_helper' + +describe Gitlab::BackgroundMigration::PopulateMergeRequestMetricsWithEventsData, :migration, schema: 20171128214150 do + describe '#perform' do + let(:mr_with_event) { create(:merge_request) } + let!(:merged_event) { create(:event, :merged, target: mr_with_event) } + let!(:closed_event) { create(:event, :closed, target: mr_with_event) } + + before do + # Make sure no metrics are created and kept through after_* callbacks. + mr_with_event.metrics.destroy! + end + + it 'inserts metrics and updates closed and merged events' do + subject.perform(mr_with_event.id, mr_with_event.id) + + mr_with_event.reload + + expect(mr_with_event.metrics).to have_attributes(latest_closed_by_id: closed_event.author_id, + merged_by_id: merged_event.author_id) + expect(mr_with_event.metrics.latest_closed_at.to_s).to eq(closed_event.updated_at.to_s) + end + end + + describe '#insert_metrics_for_range' do + let!(:mrs_without_metrics) { create_list(:merge_request, 3) } + let!(:mrs_with_metrics) { create_list(:merge_request, 2) } + + before do + # Make sure no metrics are created and kept through after_* callbacks. + mrs_without_metrics.each { |m| m.metrics.destroy! } + end + + it 'inserts merge_request_metrics for merge_requests without one' do + expect { subject.insert_metrics_for_range(MergeRequest.first.id, MergeRequest.last.id) } + .to change(MergeRequest::Metrics, :count).from(2).to(5) + + mrs_without_metrics.each do |mr_without_metrics| + expect(mr_without_metrics.reload.metrics).to be_present + end + end + + it 'does not inserts merge_request_metrics for MRs out of given range' do + expect { subject.insert_metrics_for_range(mrs_with_metrics.first.id, mrs_with_metrics.last.id) } + .not_to change(MergeRequest::Metrics, :count).from(2) + end + end + + describe '#update_metrics_with_events_data' do + context 'closed events data update' do + let(:users) { create_list(:user, 3) } + let(:mrs_with_event) { create_list(:merge_request, 3) } + + before do + create_list(:event, 2, :closed, author: users.first, target: mrs_with_event.first) + create_list(:event, 3, :closed, author: users.second, target: mrs_with_event.second) + create(:event, :closed, author: users.third, target: mrs_with_event.third) + end + + it 'migrates multiple MR metrics with closed event data' do + mr_without_event = create(:merge_request) + create(:event, :merged) + + subject.update_metrics_with_events_data(mrs_with_event.first.id, mrs_with_event.last.id) + + mrs_with_event.each do |mr_with_event| + latest_event = Event.where(action: 3, target: mr_with_event).last + + mr_with_event.metrics.reload + + expect(mr_with_event.metrics.latest_closed_by).to eq(latest_event.author) + expect(mr_with_event.metrics.latest_closed_at.to_s).to eq(latest_event.updated_at.to_s) + end + + expect(mr_without_event.metrics.reload).to have_attributes(latest_closed_by_id: nil, + latest_closed_at: nil) + end + + it 'does not updates metrics out of given range' do + out_of_range_mr = create(:merge_request) + create(:event, :closed, author: users.last, target: out_of_range_mr) + + expect { subject.perform(mrs_with_event.first.id, mrs_with_event.second.id) } + .not_to change { out_of_range_mr.metrics.reload.merged_by } + .from(nil) + end + end + + context 'merged events data update' do + let(:users) { create_list(:user, 3) } + let(:mrs_with_event) { create_list(:merge_request, 3) } + + before do + create_list(:event, 2, :merged, author: users.first, target: mrs_with_event.first) + create_list(:event, 3, :merged, author: users.second, target: mrs_with_event.second) + create(:event, :merged, author: users.third, target: mrs_with_event.third) + end + + it 'migrates multiple MR metrics with merged event data' do + mr_without_event = create(:merge_request) + create(:event, :merged) + + subject.update_metrics_with_events_data(mrs_with_event.first.id, mrs_with_event.last.id) + + mrs_with_event.each do |mr_with_event| + latest_event = Event.where(action: Event::MERGED, target: mr_with_event).last + + expect(mr_with_event.metrics.reload.merged_by).to eq(latest_event.author) + end + + expect(mr_without_event.metrics.reload).to have_attributes(merged_by_id: nil) + end + + it 'does not updates metrics out of given range' do + out_of_range_mr = create(:merge_request) + create(:event, :merged, author: users.last, target: out_of_range_mr) + + expect { subject.perform(mrs_with_event.first.id, mrs_with_event.second.id) } + .not_to change { out_of_range_mr.metrics.reload.merged_by } + .from(nil) + end + end + end +end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 664ba0f7234..7727a1d81b1 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -902,7 +902,7 @@ describe Gitlab::Database::MigrationHelpers do describe '#check_trigger_permissions!' do it 'does nothing when the user has the correct permissions' do expect { model.check_trigger_permissions!('users') } - .not_to raise_error(RuntimeError) + .not_to raise_error end it 'raises RuntimeError when the user does not have the correct permissions' do @@ -1036,4 +1036,93 @@ describe Gitlab::Database::MigrationHelpers do end end end + + describe '#change_column_type_using_background_migration' do + let!(:issue) { create(:issue) } + + let(:issue_model) do + Class.new(ActiveRecord::Base) do + self.table_name = 'issues' + include EachBatch + end + end + + it 'changes the type of a column using a background migration' do + expect(model) + .to receive(:add_column) + .with('issues', 'closed_at_for_type_change', :datetime_with_timezone) + + expect(model) + .to receive(:install_rename_triggers) + .with('issues', :closed_at, 'closed_at_for_type_change') + + expect(BackgroundMigrationWorker) + .to receive(:perform_in) + .ordered + .with( + 10.minutes, + 'CopyColumn', + ['issues', :closed_at, 'closed_at_for_type_change', issue.id, issue.id] + ) + + expect(BackgroundMigrationWorker) + .to receive(:perform_in) + .ordered + .with( + 1.hour + 10.minutes, + 'CleanupConcurrentTypeChange', + ['issues', :closed_at, 'closed_at_for_type_change'] + ) + + expect(Gitlab::BackgroundMigration) + .to receive(:steal) + .ordered + .with('CopyColumn') + + expect(Gitlab::BackgroundMigration) + .to receive(:steal) + .ordered + .with('CleanupConcurrentTypeChange') + + model.change_column_type_using_background_migration( + issue_model.all, + :closed_at, + :datetime_with_timezone + ) + end + end + + describe '#perform_background_migration_inline?' do + it 'returns true in a test environment' do + allow(Rails.env) + .to receive(:test?) + .and_return(true) + + expect(model.perform_background_migration_inline?).to eq(true) + end + + it 'returns true in a development environment' do + allow(Rails.env) + .to receive(:test?) + .and_return(false) + + allow(Rails.env) + .to receive(:development?) + .and_return(true) + + expect(model.perform_background_migration_inline?).to eq(true) + end + + it 'returns false in a production environment' do + allow(Rails.env) + .to receive(:test?) + .and_return(false) + + allow(Rails.env) + .to receive(:development?) + .and_return(false) + + expect(model.perform_background_migration_inline?).to eq(false) + end + end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 0e4292026df..2531e1e7507 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1015,7 +1015,7 @@ describe Gitlab::Git::Repository, seed_helper: true do shared_examples 'extended commit counting' do context 'with after timestamp' do it 'returns the number of commits after timestamp' do - options = { ref: 'master', limit: nil, after: Time.iso8601('2013-03-03T20:15:01+00:00') } + options = { ref: 'master', after: Time.iso8601('2013-03-03T20:15:01+00:00') } expect(repository.count_commits(options)).to eq(25) end @@ -1023,7 +1023,7 @@ describe Gitlab::Git::Repository, seed_helper: true do context 'with before timestamp' do it 'returns the number of commits before timestamp' do - options = { ref: 'feature', limit: nil, before: Time.iso8601('2015-03-03T20:15:01+00:00') } + options = { ref: 'feature', before: Time.iso8601('2015-03-03T20:15:01+00:00') } expect(repository.count_commits(options)).to eq(9) end @@ -1031,11 +1031,19 @@ describe Gitlab::Git::Repository, seed_helper: true do context 'with path' do it 'returns the number of commits with path ' do - options = { ref: 'master', limit: nil, path: "encoding" } + options = { ref: 'master', path: "encoding" } expect(repository.count_commits(options)).to eq(2) end end + + context 'with max_count' do + it 'returns the number of commits up to the passed limit' do + options = { ref: 'master', max_count: 10, after: Time.iso8601('2013-03-03T20:15:01+00:00') } + + expect(repository.count_commits(options)).to eq(10) + end + end end context 'when Gitaly count_commits feature is enabled' do @@ -1719,6 +1727,20 @@ describe Gitlab::Git::Repository, seed_helper: true do expect(result.repo_created).to eq(false) expect(result.branch_created).to eq(false) end + + it 'returns nil if there was a concurrent branch update' do + concurrent_update_id = '33f3729a45c02fc67d00adb1b8bca394b0e761d9' + result = repository.merge(user, source_sha, target_branch, 'Test merge') do + # This ref update should make the merge fail + repository.write_ref(Gitlab::Git::BRANCH_REF_PREFIX + target_branch, concurrent_update_id) + end + + # This 'nil' signals that the merge was not applied + expect(result).to be_nil + + # Our concurrent ref update should not have been undone + expect(repository.find_branch(target_branch).target).to eq(concurrent_update_id) + end end context 'with gitaly' do diff --git a/spec/migrations/schedule_populate_merge_request_metrics_with_events_data_spec.rb b/spec/migrations/schedule_populate_merge_request_metrics_with_events_data_spec.rb new file mode 100644 index 00000000000..97e089c5cb8 --- /dev/null +++ b/spec/migrations/schedule_populate_merge_request_metrics_with_events_data_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20171128214150_schedule_populate_merge_request_metrics_with_events_data.rb') + +describe SchedulePopulateMergeRequestMetricsWithEventsData, :migration, :sidekiq do + let!(:mrs) { create_list(:merge_request, 3) } + + it 'correctly schedules background migrations' do + stub_const("#{described_class.name}::BATCH_SIZE", 2) + + Sidekiq::Testing.fake! do + Timecop.freeze do + migrate! + + expect(described_class::MIGRATION) + .to be_scheduled_migration(10.minutes, mrs.first.id, mrs.second.id) + + expect(described_class::MIGRATION) + .to be_scheduled_migration(20.minutes, mrs.third.id, mrs.third.id) + + expect(BackgroundMigrationWorker.jobs.size).to eq(2) + end + end + end +end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index c9deeb45c1a..5ced000cdb6 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -23,6 +23,32 @@ describe Issue do it { is_expected.to have_db_index(:deleted_at) } end + describe 'callbacks' do + describe '#ensure_metrics' do + it 'creates metrics after saving' do + issue = create(:issue) + + expect(issue.metrics).to be_persisted + expect(Issue::Metrics.count).to eq(1) + end + + it 'does not create duplicate metrics for an issue' do + issue = create(:issue) + + issue.close! + + expect(issue.metrics).to be_persisted + expect(Issue::Metrics.count).to eq(1) + end + + it 'records current metrics' do + expect_any_instance_of(Issue::Metrics).to receive(:record!) + + create(:issue) + end + end + end + describe '#order_by_position_and_priority' do let(:project) { create :project } let(:p1) { create(:label, title: 'P1', project: project, priority: 1) } diff --git a/spec/models/merge_request/metrics_spec.rb b/spec/models/merge_request/metrics_spec.rb index 9353d5c3c8a..02ff7839739 100644 --- a/spec/models/merge_request/metrics_spec.rb +++ b/spec/models/merge_request/metrics_spec.rb @@ -1,16 +1,11 @@ require 'spec_helper' describe MergeRequest::Metrics do - subject { create(:merge_request) } + subject { described_class.new } - describe "when recording the default set of metrics on merge request save" do - it "records the merge time" do - time = Time.now - Timecop.freeze(time) { subject.mark_as_merged } - metrics = subject.metrics - - expect(metrics).to be_present - expect(metrics.merged_at).to be_like_time(time) - end + describe 'associations' do + it { is_expected.to belong_to(:merge_request) } + it { is_expected.to belong_to(:latest_closed_by).class_name('User') } + it { is_expected.to belong_to(:merged_by).class_name('User') } end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index df94617a19e..d8ebd46faab 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -65,6 +65,25 @@ describe MergeRequest do end end + describe 'callbacks' do + describe '#ensure_merge_request_metrics' do + it 'creates metrics after saving' do + merge_request = create(:merge_request) + + expect(merge_request.metrics).to be_persisted + expect(MergeRequest::Metrics.count).to eq(1) + end + + it 'does not duplicate metrics for a merge request' do + merge_request = create(:merge_request) + + merge_request.mark_as_merged! + + expect(MergeRequest::Metrics.count).to eq(1) + end + end + end + describe 'respond to' do it { is_expected.to respond_to(:unchecked?) } it { is_expected.to respond_to(:can_be_merged?) } diff --git a/spec/models/project_services/kubernetes_service_spec.rb b/spec/models/project_services/kubernetes_service_spec.rb index f037ee77a94..6980ba335b8 100644 --- a/spec/models/project_services/kubernetes_service_spec.rb +++ b/spec/models/project_services/kubernetes_service_spec.rb @@ -52,12 +52,75 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do context 'when service is inactive' do before do + subject.project = project subject.active = false end it { is_expected.not_to validate_presence_of(:api_url) } it { is_expected.not_to validate_presence_of(:token) } end + + context 'with a deprecated service' do + let(:kubernetes_service) { create(:kubernetes_service) } + + before do + kubernetes_service.update_attribute(:active, false) + kubernetes_service.properties[:namespace] = "foo" + end + + it 'should not update attributes' do + expect(kubernetes_service.save).to be_falsy + end + + it 'should include an error with a deprecation message' do + kubernetes_service.valid? + expect(kubernetes_service.errors[:base].first).to match(/Kubernetes service integration has been deprecated/) + end + end + + context 'with a non-deprecated service' do + let(:kubernetes_service) { create(:kubernetes_service) } + + it 'should update attributes' do + kubernetes_service.properties[:namespace] = 'foo' + expect(kubernetes_service.save).to be_truthy + end + end + + context 'with an active and deprecated service' do + let(:kubernetes_service) { create(:kubernetes_service) } + + before do + kubernetes_service.active = false + kubernetes_service.properties[:namespace] = 'foo' + kubernetes_service.save + end + + it 'should deactive the service' do + expect(kubernetes_service.active?).to be_falsy + end + + it 'should not include a deprecation message as error' do + expect(kubernetes_service.errors.messages.count).to eq(0) + end + + it 'should update attributes' do + expect(kubernetes_service.properties[:namespace]).to eq("foo") + end + end + + context 'with a template service' do + let(:kubernetes_service) { create(:kubernetes_service, template: true, active: false) } + + before do + kubernetes_service.properties[:namespace] = 'foo' + end + + it 'should update attributes' do + expect(kubernetes_service.save).to be_truthy + expect(kubernetes_service.properties[:namespace]).to eq('foo') + end + end end describe '#initialize_properties' do @@ -318,4 +381,42 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do it { is_expected.to eq(pods: []) } end end + + describe "#deprecated?" do + let(:kubernetes_service) { create(:kubernetes_service) } + + context 'with an active kubernetes service' do + it 'should return false' do + expect(kubernetes_service.deprecated?).to be_falsy + end + end + + context 'with a inactive kubernetes service' do + it 'should return true' do + kubernetes_service.update_attribute(:active, false) + expect(kubernetes_service.deprecated?).to be_truthy + end + end + end + + describe "#deprecation_message" do + let(:kubernetes_service) { create(:kubernetes_service) } + + it 'should indicate the service is deprecated' do + expect(kubernetes_service.deprecation_message).to match(/Kubernetes service integration has been deprecated/) + end + + context 'if the services is active' do + it 'should return a message' do + expect(kubernetes_service.deprecation_message).to match(/Your cluster information on this page is still editable/) + end + end + + context 'if the service is not active' do + it 'should return a message' do + kubernetes_service.update_attribute(:active, false) + expect(kubernetes_service.deprecation_message).to match(/Fields on this page are now uneditable/) + end + end + end end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 0f2f906c667..540615de117 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -254,4 +254,22 @@ describe Service do end end end + + describe "#deprecated?" do + let(:project) { create(:project, :repository) } + + it 'should return false by default' do + service = create(:service, project: project) + expect(service.deprecated?).to be_falsy + end + end + + describe "#deprecation_message" do + let(:project) { create(:project, :repository) } + + it 'should be empty by default' do + service = create(:service, project: project) + expect(service.deprecation_message).to be_nil + end + end end diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index be8d9c19125..981c9c27325 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -464,7 +464,7 @@ describe API::Notes do describe "POST /projects/:id/noteable/:noteable_id/notes to test observer on create" do it "creates an activity event when an issue note is created" do - expect(Event).to receive(:create) + expect(Event).to receive(:create!) post api("/projects/#{project.id}/issues/#{issue.iid}/notes", user), body: 'hi!' end diff --git a/spec/requests/api/project_milestones_spec.rb b/spec/requests/api/project_milestones_spec.rb index 6fe8ab5a3f6..08ea7314bb3 100644 --- a/spec/requests/api/project_milestones_spec.rb +++ b/spec/requests/api/project_milestones_spec.rb @@ -16,7 +16,7 @@ describe API::ProjectMilestones do describe 'PUT /projects/:id/milestones/:milestone_id to test observer on close' do it 'creates an activity event when an milestone is closed' do - expect(Event).to receive(:create) + expect(Event).to receive(:create!) put api("/projects/#{project.id}/milestones/#{milestone.id}", user), state_event: 'close' diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb index ceafa0e2058..26d56c04862 100644 --- a/spec/requests/api/services_spec.rb +++ b/spec/requests/api/services_spec.rb @@ -53,6 +53,10 @@ describe API::Services do describe "DELETE /projects/:id/services/#{service.dasherize}" do include_context service + before do + initialize_service(service) + end + it "deletes #{service}" do delete api("/projects/#{project.id}/services/#{dashed_service}", user) @@ -67,9 +71,7 @@ describe API::Services do # inject some properties into the service before do - service_object = project.find_or_initialize_service(service) - service_object.properties = service_attrs - service_object.save + initialize_service(service) end it 'returns authentication error when unauthenticated' do diff --git a/spec/requests/api/v3/milestones_spec.rb b/spec/requests/api/v3/milestones_spec.rb index 9ee71ea9c5d..6021600e09c 100644 --- a/spec/requests/api/v3/milestones_spec.rb +++ b/spec/requests/api/v3/milestones_spec.rb @@ -161,7 +161,7 @@ describe API::V3::Milestones do describe 'PUT /projects/:id/milestones/:milestone_id to test observer on close' do it 'creates an activity event when an milestone is closed' do - expect(Event).to receive(:create) + expect(Event).to receive(:create!) put v3_api("/projects/#{project.id}/milestones/#{milestone.id}", user), state_event: 'close' diff --git a/spec/requests/api/v3/notes_spec.rb b/spec/requests/api/v3/notes_spec.rb index 6428d9daaba..5532795ab02 100644 --- a/spec/requests/api/v3/notes_spec.rb +++ b/spec/requests/api/v3/notes_spec.rb @@ -302,7 +302,7 @@ describe API::V3::Notes do describe "POST /projects/:id/noteable/:noteable_id/notes to test observer on create" do it "creates an activity event when an issue note is created" do - expect(Event).to receive(:create) + expect(Event).to receive(:create!) post v3_api("/projects/#{project.id}/issues/#{issue.id}/notes", user), body: 'hi!' end diff --git a/spec/requests/api/v3/services_spec.rb b/spec/requests/api/v3/services_spec.rb index 8f212ab6be6..c69a7d58ca6 100644 --- a/spec/requests/api/v3/services_spec.rb +++ b/spec/requests/api/v3/services_spec.rb @@ -10,6 +10,10 @@ describe API::V3::Services do describe "DELETE /projects/:id/services/#{service.dasherize}" do include_context service + before do + initialize_service(service) + end + it "deletes #{service}" do delete v3_api("/projects/#{project.id}/services/#{dashed_service}", user) diff --git a/spec/requests/api/wikis_spec.rb b/spec/requests/api/wikis_spec.rb index 65bd001e491..fb0806ff9f1 100644 --- a/spec/requests/api/wikis_spec.rb +++ b/spec/requests/api/wikis_spec.rb @@ -12,6 +12,8 @@ require 'spec_helper' describe API::Wikis do let(:user) { create(:user) } + let(:group) { create(:group).tap { |g| g.add_owner(user) } } + let(:project_wiki) { create(:project_wiki, project: project, user: user) } let(:payload) { { content: 'content', format: 'rdoc', title: 'title' } } let(:expected_keys_with_content) { %w(content format slug title) } let(:expected_keys_without_content) { %w(format slug title) } @@ -19,8 +21,8 @@ describe API::Wikis do shared_examples_for 'returns list of wiki pages' do context 'when wiki has pages' do let!(:pages) do - [create(:wiki_page, wiki: project.wiki, attrs: { title: 'page1', content: 'content of page1' }), - create(:wiki_page, wiki: project.wiki, attrs: { title: 'page2', content: 'content of page2' })] + [create(:wiki_page, wiki: project_wiki, attrs: { title: 'page1', content: 'content of page1' }), + create(:wiki_page, wiki: project_wiki, attrs: { title: 'page2', content: 'content of page2' })] end it 'returns the list of wiki pages without content' do @@ -445,7 +447,7 @@ describe API::Wikis do end describe 'PUT /projects/:id/wikis/:slug' do - let(:page) { create(:wiki_page, wiki: project.wiki) } + let(:page) { create(:wiki_page, wiki: project_wiki) } let(:payload) { { title: 'new title', content: 'new content' } } let(:url) { "/projects/#{project.id}/wikis/#{page.slug}" } @@ -568,10 +570,20 @@ describe API::Wikis do end end end + + context 'when wiki belongs to a group project' do + let(:project) { create(:project, namespace: group) } + + before do + put(api(url, user), payload) + end + + include_examples 'updates wiki page' + end end describe 'DELETE /projects/:id/wikis/:slug' do - let(:page) { create(:wiki_page, wiki: project.wiki) } + let(:page) { create(:wiki_page, wiki: project_wiki) } let(:url) { "/projects/#{project.id}/wikis/#{page.slug}" } context 'when wiki is disabled' do @@ -675,5 +687,15 @@ describe API::Wikis do end end end + + context 'when wiki belongs to a group project' do + let(:project) { create(:project, namespace: group) } + + before do + delete(api(url, user)) + end + + include_examples '204 No Content' + end end end diff --git a/spec/serializers/event_entity_spec.rb b/spec/serializers/event_entity_spec.rb deleted file mode 100644 index bb54597c967..00000000000 --- a/spec/serializers/event_entity_spec.rb +++ /dev/null @@ -1,13 +0,0 @@ -require 'spec_helper' - -describe EventEntity do - subject { described_class.represent(create(:event)).as_json } - - it 'exposes author' do - expect(subject).to include(:author) - end - - it 'exposes core elements of event' do - expect(subject).to include(:updated_at) - end -end diff --git a/spec/serializers/merge_request_widget_entity_spec.rb b/spec/serializers/merge_request_widget_entity_spec.rb index a5924a8589c..e25552eb0d8 100644 --- a/spec/serializers/merge_request_widget_entity_spec.rb +++ b/spec/serializers/merge_request_widget_entity_spec.rb @@ -35,6 +35,81 @@ describe MergeRequestWidgetEntity do end end + describe 'metrics' do + context 'when metrics record exists with merged data' do + before do + resource.mark_as_merged! + resource.metrics.update!(merged_by: user) + end + + it 'matches merge request metrics schema' do + expect(subject[:metrics].with_indifferent_access) + .to match_schema('entities/merge_request_metrics') + end + + it 'returns values from metrics record' do + expect(subject.dig(:metrics, :merged_by, :id)) + .to eq(resource.metrics.merged_by_id) + end + end + + context 'when metrics record exists with closed data' do + before do + resource.close! + resource.metrics.update!(latest_closed_by: user) + end + + it 'matches merge request metrics schema' do + expect(subject[:metrics].with_indifferent_access) + .to match_schema('entities/merge_request_metrics') + end + + it 'returns values from metrics record' do + expect(subject.dig(:metrics, :closed_by, :id)) + .to eq(resource.metrics.latest_closed_by_id) + end + end + + context 'when metrics does not exists' do + before do + resource.mark_as_merged! + resource.metrics.destroy! + resource.reload + end + + context 'when events exists' do + let!(:closed_event) { create(:event, :closed, project: project, target: resource) } + let!(:merge_event) { create(:event, :merged, project: project, target: resource) } + + it 'matches merge request metrics schema' do + expect(subject[:metrics].with_indifferent_access) + .to match_schema('entities/merge_request_metrics') + end + + it 'returns values from events record' do + expect(subject.dig(:metrics, :merged_by, :id)) + .to eq(merge_event.author_id) + + expect(subject.dig(:metrics, :closed_by, :id)) + .to eq(closed_event.author_id) + + expect(subject.dig(:metrics, :merged_at).to_s) + .to eq(merge_event.updated_at.to_s) + + expect(subject.dig(:metrics, :closed_at).to_s) + .to eq(closed_event.updated_at.to_s) + end + end + + context 'when events does not exists' do + it 'matches merge request metrics schema' do + expect(subject[:metrics].with_indifferent_access) + .to match_schema('entities/merge_request_metrics') + end + end + end + end + it 'has email_patches_path' do expect(subject[:email_patches_path]) .to eq("/#{resource.project.full_path}/merge_requests/#{resource.iid}.patch") diff --git a/spec/services/create_deployment_service_spec.rb b/spec/services/create_deployment_service_spec.rb index 08267d6e6a0..b9bfbb11511 100644 --- a/spec/services/create_deployment_service_spec.rb +++ b/spec/services/create_deployment_service_spec.rb @@ -266,7 +266,7 @@ describe CreateDeploymentService do context "while updating the 'first_deployed_to_production_at' time" do before do - merge_request.mark_as_merged + merge_request.metrics.update!(merged_at: Time.now) end context "for merge requests merged before the current deploy" do diff --git a/spec/services/merge_requests/close_service_spec.rb b/spec/services/merge_requests/close_service_spec.rb index 2a59bc4594a..4d12de3ecce 100644 --- a/spec/services/merge_requests/close_service_spec.rb +++ b/spec/services/merge_requests/close_service_spec.rb @@ -52,6 +52,19 @@ describe MergeRequests::CloseService do end end + it 'updates metrics' do + metrics = merge_request.metrics + metrics_service = double(MergeRequestMetricsService) + allow(MergeRequestMetricsService) + .to receive(:new) + .with(metrics) + .and_return(metrics_service) + + expect(metrics_service).to receive(:close) + + described_class.new(project, user, {}).execute(merge_request) + end + it 'refreshes the number of open merge requests for a valid MR', :use_clean_rails_memory_store_caching do service = described_class.new(project, user, {}) diff --git a/spec/services/merge_requests/post_merge_service_spec.rb b/spec/services/merge_requests/post_merge_service_spec.rb index 8f2c5df5907..70957431942 100644 --- a/spec/services/merge_requests/post_merge_service_spec.rb +++ b/spec/services/merge_requests/post_merge_service_spec.rb @@ -22,5 +22,18 @@ describe MergeRequests::PostMergeService do expect { service.execute(merge_request) } .to change { project.open_merge_requests_count }.from(1).to(0) end + + it 'updates metrics' do + metrics = merge_request.metrics + metrics_service = double(MergeRequestMetricsService) + allow(MergeRequestMetricsService) + .to receive(:new) + .with(metrics) + .and_return(metrics_service) + + expect(metrics_service).to receive(:merge) + + described_class.new(project, user, {}).execute(merge_request) + end end end diff --git a/spec/services/merge_requests/reopen_service_spec.rb b/spec/services/merge_requests/reopen_service_spec.rb index 94f31ff139c..a44d63e5f9f 100644 --- a/spec/services/merge_requests/reopen_service_spec.rb +++ b/spec/services/merge_requests/reopen_service_spec.rb @@ -47,6 +47,19 @@ describe MergeRequests::ReopenService do end end + it 'updates metrics' do + metrics = merge_request.metrics + service = double(MergeRequestMetricsService) + allow(MergeRequestMetricsService) + .to receive(:new) + .with(metrics) + .and_return(service) + + expect(service).to receive(:reopen) + + described_class.new(project, user, {}).execute(merge_request) + end + it 'refreshes the number of open merge requests for a valid MR' do service = described_class.new(project, user, {}) diff --git a/spec/services/update_merge_request_metrics_service_spec.rb b/spec/services/update_merge_request_metrics_service_spec.rb new file mode 100644 index 00000000000..b5fb999381d --- /dev/null +++ b/spec/services/update_merge_request_metrics_service_spec.rb @@ -0,0 +1,42 @@ +require 'rails_helper' + +describe MergeRequestMetricsService do + let(:metrics) { create(:merge_request).metrics } + + describe '#merge' do + it 'updates metrics' do + user = create(:user) + service = described_class.new(metrics) + event = double(Event, author_id: user.id, created_at: Time.now) + + service.merge(event) + + expect(metrics.merged_by).to eq(user) + expect(metrics.merged_at).to eq(event.created_at) + end + end + + describe '#close' do + it 'updates metrics' do + user = create(:user) + service = described_class.new(metrics) + event = double(Event, author_id: user.id, created_at: Time.now) + + service.close(event) + + expect(metrics.latest_closed_by).to eq(user) + expect(metrics.latest_closed_at).to eq(event.created_at) + end + end + + describe '#reopen' do + it 'updates metrics' do + service = described_class.new(metrics) + + service.reopen + + expect(metrics.latest_closed_by).to be_nil + expect(metrics.latest_closed_at).to be_nil + end + end +end diff --git a/spec/support/services_shared_context.rb b/spec/support/services_shared_context.rb index 7457484a932..3f1fd169b72 100644 --- a/spec/support/services_shared_context.rb +++ b/spec/support/services_shared_context.rb @@ -29,5 +29,13 @@ Service.available_services_names.each do |service| end end end + + def initialize_service(service) + service_item = project.find_or_initialize_service(service) + service_item.properties = service_attrs + service_item.active = true if service == "kubernetes" + service_item.save + service_item + end end end |