diff options
93 files changed, 1326 insertions, 1728 deletions
diff --git a/app/assets/javascripts/contributors/components/contributors.vue b/app/assets/javascripts/contributors/components/contributors.vue new file mode 100644 index 00000000000..7dd6b051cb4 --- /dev/null +++ b/app/assets/javascripts/contributors/components/contributors.vue @@ -0,0 +1,227 @@ +<script> +import { __ } from '~/locale'; +import _ from 'underscore'; +import { mapActions, mapState, mapGetters } from 'vuex'; +import { GlLoadingIcon } from '@gitlab/ui'; +import { GlAreaChart } from '@gitlab/ui/dist/charts'; +import { getSvgIconPathContent } from '~/lib/utils/icon_utils'; +import { getDatesInRange } from '~/lib/utils/datetime_utility'; +import { xAxisLabelFormatter, dateFormatter } from '../utils'; + +export default { + components: { + GlAreaChart, + GlLoadingIcon, + }, + props: { + endpoint: { + type: String, + required: true, + }, + branch: { + type: String, + required: true, + }, + }, + data() { + return { + masterChart: null, + individualCharts: [], + svgs: {}, + masterChartHeight: 264, + individualChartHeight: 216, + }; + }, + computed: { + ...mapState(['chartData', 'loading']), + ...mapGetters(['showChart', 'parsedData']), + masterChartData() { + const data = {}; + this.xAxisRange.forEach(date => { + data[date] = this.parsedData.total[date] || 0; + }); + return [ + { + name: __('Commits'), + data: Object.entries(data), + }, + ]; + }, + masterChartOptions() { + return { + ...this.getCommonChartOptions(true), + yAxis: { + name: __('Number of commits'), + }, + grid: { + bottom: 64, + left: 64, + right: 20, + top: 20, + }, + }; + }, + individualChartsData() { + const maxNumberOfIndividualContributorsCharts = 100; + + return Object.keys(this.parsedData.byAuthor) + .map(name => { + const author = this.parsedData.byAuthor[name]; + return { + name, + email: author.email, + commits: author.commits, + dates: [ + { + name: __('Commits'), + data: this.xAxisRange.map(date => [date, author.dates[date] || 0]), + }, + ], + }; + }) + .sort((a, b) => b.commits - a.commits) + .slice(0, maxNumberOfIndividualContributorsCharts); + }, + individualChartOptions() { + return { + ...this.getCommonChartOptions(false), + yAxis: { + name: __('Commits'), + max: this.individualChartYAxisMax, + }, + grid: { + bottom: 27, + left: 64, + right: 20, + top: 8, + }, + }; + }, + individualChartYAxisMax() { + return this.individualChartsData.reduce((acc, item) => { + const values = item.dates[0].data.map(value => value[1]); + return Math.max(acc, ...values); + }, 0); + }, + xAxisRange() { + const dates = Object.keys(this.parsedData.total).sort((a, b) => new Date(a) - new Date(b)); + + const firstContributionDate = new Date(dates[0]); + const lastContributionDate = new Date(dates[dates.length - 1]); + + return getDatesInRange(firstContributionDate, lastContributionDate, dateFormatter); + }, + firstContributionDate() { + return this.xAxisRange[0]; + }, + lastContributionDate() { + return this.xAxisRange[this.xAxisRange.length - 1]; + }, + charts() { + return _.uniq(this.individualCharts); + }, + }, + mounted() { + this.fetchChartData(this.endpoint); + }, + methods: { + ...mapActions(['fetchChartData']), + getCommonChartOptions(isMasterChart) { + return { + xAxis: { + type: 'time', + name: '', + data: this.xAxisRange, + axisLabel: { + formatter: xAxisLabelFormatter, + showMaxLabel: false, + showMinLabel: false, + }, + boundaryGap: false, + splitNumber: isMasterChart ? 24 : 18, + // 28 days + minInterval: 28 * 86400 * 1000, + min: this.firstContributionDate, + max: this.lastContributionDate, + }, + }; + }, + setSvg(name) { + return getSvgIconPathContent(name) + .then(path => { + if (path) { + this.$set(this.svgs, name, `path://${path}`); + } + }) + .catch(() => {}); + }, + onMasterChartCreated(chart) { + this.masterChart = chart; + this.setSvg('scroll-handle') + .then(() => { + this.masterChart.setOption({ + dataZoom: [ + { + type: 'slider', + handleIcon: this.svgs['scroll-handle'], + }, + ], + }); + }) + .catch(() => {}); + this.masterChart.on('datazoom', _.debounce(this.setIndividualChartsZoom, 200)); + }, + onIndividualChartCreated(chart) { + this.individualCharts.push(chart); + }, + setIndividualChartsZoom(options) { + this.charts.forEach(chart => + chart.setOption( + { + dataZoom: { + start: options.start, + end: options.end, + show: false, + }, + }, + { lazyUpdate: true }, + ), + ); + }, + }, +}; +</script> + +<template> + <div> + <div v-if="loading" class="contributors-loader text-center"> + <gl-loading-icon :inline="true" :size="4" /> + </div> + + <div v-else-if="showChart" class="contributors-charts"> + <h4>{{ __('Commits to') }} {{ branch }}</h4> + <span>{{ __('Excluding merge commits. Limited to 6,000 commits.') }}</span> + <div> + <gl-area-chart + :data="masterChartData" + :option="masterChartOptions" + :height="masterChartHeight" + @created="onMasterChartCreated" + /> + </div> + + <div class="row"> + <div v-for="contributor in individualChartsData" :key="contributor.name" class="col-6"> + <h4>{{ contributor.name }}</h4> + <p>{{ n__('%d commit', '%d commits', contributor.commits) }} ({{ contributor.email }})</p> + <gl-area-chart + :data="contributor.dates" + :option="individualChartOptions" + :height="individualChartHeight" + @created="onIndividualChartCreated" + /> + </div> + </div> + </div> + </div> +</template> diff --git a/app/assets/javascripts/contributors/index.js b/app/assets/javascripts/contributors/index.js new file mode 100644 index 00000000000..b6063589734 --- /dev/null +++ b/app/assets/javascripts/contributors/index.js @@ -0,0 +1,23 @@ +import Vue from 'vue'; +import ContributorsGraphs from './components/contributors.vue'; +import store from './stores'; + +export default () => { + const el = document.querySelector('.js-contributors-graph'); + + if (!el) return null; + + return new Vue({ + el, + store, + + render(createElement) { + return createElement(ContributorsGraphs, { + props: { + endpoint: el.dataset.projectGraphPath, + branch: el.dataset.projectBranch, + }, + }); + }, + }); +}; diff --git a/app/assets/javascripts/contributors/services/contributors_service.js b/app/assets/javascripts/contributors/services/contributors_service.js new file mode 100644 index 00000000000..5a8bbb66511 --- /dev/null +++ b/app/assets/javascripts/contributors/services/contributors_service.js @@ -0,0 +1,7 @@ +import axios from '~/lib/utils/axios_utils'; + +export default { + fetchChartData(endpoint) { + return axios.get(endpoint); + }, +}; diff --git a/app/assets/javascripts/contributors/stores/actions.js b/app/assets/javascripts/contributors/stores/actions.js new file mode 100644 index 00000000000..4138ff24f1d --- /dev/null +++ b/app/assets/javascripts/contributors/stores/actions.js @@ -0,0 +1,20 @@ +import flash from '~/flash'; +import { __ } from '~/locale'; +import service from '../services/contributors_service'; +import * as types from './mutation_types'; + +export const fetchChartData = ({ commit }, endpoint) => { + commit(types.SET_LOADING_STATE, true); + + return service + .fetchChartData(endpoint) + .then(res => res.data) + .then(data => { + commit(types.SET_CHART_DATA, data); + commit(types.SET_LOADING_STATE, false); + }) + .catch(() => flash(__('An error occurred while loading chart data'))); +}; + +// prevent babel-plugin-rewire from generating an invalid default during karma tests +export default () => {}; diff --git a/app/assets/javascripts/contributors/stores/getters.js b/app/assets/javascripts/contributors/stores/getters.js new file mode 100644 index 00000000000..9e02e3ed9e7 --- /dev/null +++ b/app/assets/javascripts/contributors/stores/getters.js @@ -0,0 +1,33 @@ +export const showChart = state => Boolean(!state.loading && state.chartData); + +export const parsedData = state => { + const byAuthor = {}; + const total = {}; + + state.chartData.forEach(({ date, author_name, author_email }) => { + total[date] = total[date] ? total[date] + 1 : 1; + + const authorData = byAuthor[author_name]; + + if (!authorData) { + byAuthor[author_name] = { + email: author_email.toLowerCase(), + commits: 1, + dates: { + [date]: 1, + }, + }; + } else { + authorData.commits += 1; + authorData.dates[date] = authorData.dates[date] ? authorData.dates[date] + 1 : 1; + } + }); + + return { + total, + byAuthor, + }; +}; + +// prevent babel-plugin-rewire from generating an invalid default during karma tests +export default () => {}; diff --git a/app/assets/javascripts/contributors/stores/index.js b/app/assets/javascripts/contributors/stores/index.js new file mode 100644 index 00000000000..bc739851aa7 --- /dev/null +++ b/app/assets/javascripts/contributors/stores/index.js @@ -0,0 +1,18 @@ +import Vue from 'vue'; +import Vuex from 'vuex'; +import state from './state'; +import mutations from './mutations'; +import * as getters from './getters'; +import * as actions from './actions'; + +Vue.use(Vuex); + +export const createStore = () => + new Vuex.Store({ + actions, + mutations, + getters, + state: state(), + }); + +export default createStore(); diff --git a/app/assets/javascripts/contributors/stores/mutation_types.js b/app/assets/javascripts/contributors/stores/mutation_types.js new file mode 100644 index 00000000000..62e0a51d5f8 --- /dev/null +++ b/app/assets/javascripts/contributors/stores/mutation_types.js @@ -0,0 +1,3 @@ +export const SET_CHART_DATA = 'SET_CHART_DATA'; +export const SET_LOADING_STATE = 'SET_LOADING_STATE'; +export const SET_ACTIVE_BRANCH = 'SET_ACTIVE_BRANCH'; diff --git a/app/assets/javascripts/contributors/stores/mutations.js b/app/assets/javascripts/contributors/stores/mutations.js new file mode 100644 index 00000000000..f1f460d072d --- /dev/null +++ b/app/assets/javascripts/contributors/stores/mutations.js @@ -0,0 +1,17 @@ +import * as types from './mutation_types'; + +export default { + [types.SET_LOADING_STATE](state, value) { + state.loading = value; + }, + [types.SET_CHART_DATA](state, chartData) { + Object.assign(state, { + chartData, + }); + }, + [types.SET_ACTIVE_BRANCH](state, branch) { + Object.assign(state, { + branch, + }); + }, +}; diff --git a/app/assets/javascripts/contributors/stores/state.js b/app/assets/javascripts/contributors/stores/state.js new file mode 100644 index 00000000000..1dc1a3c7b75 --- /dev/null +++ b/app/assets/javascripts/contributors/stores/state.js @@ -0,0 +1,5 @@ +export default () => ({ + loading: false, + chartData: null, + branch: 'master', +}); diff --git a/app/assets/javascripts/contributors/utils.js b/app/assets/javascripts/contributors/utils.js new file mode 100644 index 00000000000..7d8932ce495 --- /dev/null +++ b/app/assets/javascripts/contributors/utils.js @@ -0,0 +1,30 @@ +import { getMonthNames } from '~/lib/utils/datetime_utility'; + +/** + * Converts provided string to date and returns formatted value as a year for date in January and month name for the rest + * @param {String} + * @returns {String} - formatted value + * + * xAxisLabelFormatter('01-12-2019') will return '2019' + * xAxisLabelFormatter('02-12-2019') will return 'Feb' + * xAxisLabelFormatter('07-12-2019') will return 'Jul' + */ +export const xAxisLabelFormatter = val => { + const date = new Date(val); + const month = date.getUTCMonth(); + const year = date.getUTCFullYear(); + return month === 0 ? `${year}` : getMonthNames(true)[month]; +}; + +/** + * Formats provided date to YYYY-MM-DD format + * @param {Date} + * @returns {String} - formatted value + */ +export const dateFormatter = date => { + const year = date.getUTCFullYear(); + const month = date.getUTCMonth(); + const day = date.getUTCDate(); + + return `${year}-${`0${month + 1}`.slice(-2)}-${`0${day}`.slice(-2)}`; +}; diff --git a/app/assets/javascripts/lib/utils/datetime_utility.js b/app/assets/javascripts/lib/utils/datetime_utility.js index 37b0215f6f9..e2d188103bc 100644 --- a/app/assets/javascripts/lib/utils/datetime_utility.js +++ b/app/assets/javascripts/lib/utils/datetime_utility.js @@ -564,3 +564,26 @@ export const getDateInPast = (date, daysInPast) => { export const beginOfDayTime = 'T00:00:00Z'; export const endOfDayTime = 'T23:59:59Z'; + +/** + * @param {Date} d1 + * @param {Date} d2 + * @param {Function} formatter + * @return {Any[]} an array of formatted dates between 2 given dates (including start&end date) + */ +export const getDatesInRange = (d1, d2, formatter = x => x) => { + if (!(d1 instanceof Date) || !(d2 instanceof Date)) { + return []; + } + let startDate = d1.getTime(); + const endDate = d2.getTime(); + const oneDay = 24 * 3600 * 1000; + const range = [d1]; + + while (startDate < endDate) { + startDate += oneDay; + range.push(new Date(startDate)); + } + + return range.map(formatter); +}; diff --git a/app/assets/javascripts/pages/projects/graphs/show/index.js b/app/assets/javascripts/pages/projects/graphs/show/index.js index f79c386b59e..09d9c78c446 100644 --- a/app/assets/javascripts/pages/projects/graphs/show/index.js +++ b/app/assets/javascripts/pages/projects/graphs/show/index.js @@ -1,25 +1,3 @@ -import $ from 'jquery'; -import flash from '~/flash'; -import { __ } from '~/locale'; -import axios from '~/lib/utils/axios_utils'; -import ContributorsStatGraph from './stat_graph_contributors'; +import initContributorsGraphs from '~/contributors'; -document.addEventListener('DOMContentLoaded', () => { - const url = document.querySelector('.js-graphs-show').dataset.projectGraphPath; - - axios - .get(url) - .then(({ data }) => { - const graph = new ContributorsStatGraph(); - graph.init(data); - - $('#brush_change').change(() => { - graph.change_date_header(); - graph.redraw_authors(); - }); - - $('.stat-graph').fadeIn(); - $('.loading-graph').hide(); - }) - .catch(() => flash(__('Error fetching contributors data.'))); -}); +document.addEventListener('DOMContentLoaded', initContributorsGraphs); diff --git a/app/assets/javascripts/pages/projects/graphs/show/stat_graph_contributors.js b/app/assets/javascripts/pages/projects/graphs/show/stat_graph_contributors.js deleted file mode 100644 index 5b873e6b909..00000000000 --- a/app/assets/javascripts/pages/projects/graphs/show/stat_graph_contributors.js +++ /dev/null @@ -1,140 +0,0 @@ -/* eslint-disable func-names, no-var, one-var, camelcase, no-param-reassign, no-return-assign */ - -import $ from 'jquery'; -import _ from 'underscore'; -import { n__, s__, createDateTimeFormat, sprintf } from '~/locale'; -import { - ContributorsGraph, - ContributorsAuthorGraph, - ContributorsMasterGraph, -} from './stat_graph_contributors_graph'; -import ContributorsStatGraphUtil from './stat_graph_contributors_util'; - -export default (function() { - function ContributorsStatGraph() { - this.dateFormat = createDateTimeFormat({ year: 'numeric', month: 'long', day: 'numeric' }); - } - - ContributorsStatGraph.prototype.init = function(log) { - var author_commits, total_commits; - this.parsed_log = ContributorsStatGraphUtil.parse_log(log); - this.set_current_field('commits'); - total_commits = ContributorsStatGraphUtil.get_total_data(this.parsed_log, this.field); - author_commits = ContributorsStatGraphUtil.get_author_data(this.parsed_log, this.field); - this.add_master_graph(total_commits); - this.add_authors_graph(author_commits); - return this.change_date_header(); - }; - - ContributorsStatGraph.prototype.add_master_graph = function(total_data) { - this.master_graph = new ContributorsMasterGraph(total_data); - return this.master_graph.draw(); - }; - - ContributorsStatGraph.prototype.add_authors_graph = function(author_data) { - var limited_author_data; - this.authors = []; - limited_author_data = author_data.slice(0, 100); - return _.each( - limited_author_data, - (function(_this) { - return function(d) { - var author_graph, author_header; - author_header = _this.create_author_header(d); - $('.contributors-list').append(author_header); - - author_graph = new ContributorsAuthorGraph(d.dates); - _this.authors[d.author_name] = author_graph; - return author_graph.draw(); - }; - })(this), - ); - }; - - ContributorsStatGraph.prototype.format_author_commit_info = function(author) { - var commits; - commits = $('<span/>', { - class: 'graph-author-commits-count', - }); - commits.text(n__('%d commit', '%d commits', author.commits)); - return $('<span/>').append(commits); - }; - - ContributorsStatGraph.prototype.create_author_header = function(author) { - var author_commit_info, author_commit_info_span, author_email, author_name, list_item; - list_item = $('<li/>', { - class: 'person', - style: 'display: block;', - }); - author_name = $(`<h4>${author.author_name}</h4>`); - author_email = $(`<p class="graph-author-email">${author.author_email}</p>`); - author_commit_info_span = $('<span/>', { - class: 'commits', - }); - author_commit_info = this.format_author_commit_info(author); - author_commit_info_span.html(author_commit_info); - list_item.append(author_name); - list_item.append(author_email); - list_item.append(author_commit_info_span); - return list_item; - }; - - ContributorsStatGraph.prototype.redraw_master = function() { - var total_data; - total_data = ContributorsStatGraphUtil.get_total_data(this.parsed_log, this.field); - this.master_graph.set_data(total_data); - return this.master_graph.redraw(); - }; - - ContributorsStatGraph.prototype.redraw_authors = function() { - $('ol').html(''); - - const { x_domain } = ContributorsGraph.prototype; - const author_commits = ContributorsStatGraphUtil.get_author_data( - this.parsed_log, - this.field, - x_domain, - ); - - return _.each( - author_commits, - (function(_this) { - return function(d) { - _this.redraw_author_commit_info(d); - if (_this.authors[d.author_name] != null) { - $(_this.authors[d.author_name].list_item).appendTo('ol'); - _this.authors[d.author_name].set_data(d.dates); - return _this.authors[d.author_name].redraw(); - } - return ''; - }; - })(this), - ); - }; - - ContributorsStatGraph.prototype.set_current_field = function(field) { - return (this.field = field); - }; - - ContributorsStatGraph.prototype.change_date_header = function() { - const { x_domain } = ContributorsGraph.prototype; - const formattedDateRange = sprintf(s__('ContributorsPage|%{startDate} – %{endDate}'), { - startDate: this.dateFormat.format(new Date(x_domain[0])), - endDate: this.dateFormat.format(new Date(x_domain[1])), - }); - return $('#date_header').text(formattedDateRange); - }; - - ContributorsStatGraph.prototype.redraw_author_commit_info = function(author) { - var author_commit_info, author_list_item, $author; - $author = this.authors[author.author_name]; - if ($author != null) { - author_list_item = $(this.authors[author.author_name].list_item); - author_commit_info = this.format_author_commit_info(author); - return author_list_item.find('span').html(author_commit_info); - } - return ''; - }; - - return ContributorsStatGraph; -})(); diff --git a/app/assets/javascripts/pages/projects/graphs/show/stat_graph_contributors_graph.js b/app/assets/javascripts/pages/projects/graphs/show/stat_graph_contributors_graph.js deleted file mode 100644 index 86794800f87..00000000000 --- a/app/assets/javascripts/pages/projects/graphs/show/stat_graph_contributors_graph.js +++ /dev/null @@ -1,379 +0,0 @@ -/* eslint-disable func-names, no-restricted-syntax, no-use-before-define, no-param-reassign, new-cap, no-underscore-dangle, no-return-assign, no-else-return, no-shadow */ - -import $ from 'jquery'; -import _ from 'underscore'; -import { extent, max } from 'd3-array'; -import { select, event as d3Event } from 'd3-selection'; -import { scaleTime, scaleLinear } from 'd3-scale'; -import { axisLeft, axisBottom } from 'd3-axis'; -import { area } from 'd3-shape'; -import { brushX } from 'd3-brush'; -import { timeParse } from 'd3-time-format'; -import { dateTickFormat } from '~/lib/utils/tick_formats'; - -const d3 = { - extent, - max, - select, - scaleTime, - scaleLinear, - axisLeft, - axisBottom, - area, - brushX, - timeParse, -}; - -const hasProp = {}.hasOwnProperty; -const extend = function(child, parent) { - for (const key in parent) { - if (hasProp.call(parent, key)) child[key] = parent[key]; - } - function ctor() { - this.constructor = child; - } - ctor.prototype = parent.prototype; - child.prototype = new ctor(); - child.__super__ = parent.prototype; - return child; -}; - -export const ContributorsGraph = (function() { - function ContributorsGraph() {} - - ContributorsGraph.prototype.MARGIN = { - top: 20, - right: 10, - bottom: 30, - left: 40, - }; - - ContributorsGraph.prototype.x_domain = null; - - ContributorsGraph.prototype.y_domain = null; - - ContributorsGraph.prototype.dates = []; - - ContributorsGraph.prototype.determine_width = function(baseWidth, $parentElement) { - const parentPaddingWidth = - parseFloat($parentElement.css('padding-left')) + - parseFloat($parentElement.css('padding-right')); - const marginWidth = this.MARGIN.left + this.MARGIN.right; - return baseWidth - parentPaddingWidth - marginWidth; - }; - - ContributorsGraph.set_x_domain = function(data) { - return (ContributorsGraph.prototype.x_domain = data); - }; - - ContributorsGraph.set_y_domain = function(data) { - return (ContributorsGraph.prototype.y_domain = [ - 0, - d3.max(data, d => (d.commits = d.commits || d.additions || d.deletions)), - ]); - }; - - ContributorsGraph.init_x_domain = function(data) { - return (ContributorsGraph.prototype.x_domain = d3.extent(data, d => d.date)); - }; - - ContributorsGraph.init_y_domain = function(data) { - return (ContributorsGraph.prototype.y_domain = [ - 0, - d3.max(data, d => (d.commits = d.commits || d.additions || d.deletions)), - ]); - }; - - ContributorsGraph.init_domain = function(data) { - ContributorsGraph.init_x_domain(data); - return ContributorsGraph.init_y_domain(data); - }; - - ContributorsGraph.set_dates = function(data) { - return (ContributorsGraph.prototype.dates = data); - }; - - ContributorsGraph.prototype.set_x_domain = function() { - return this.x.domain(this.x_domain); - }; - - ContributorsGraph.prototype.set_y_domain = function() { - return this.y.domain(this.y_domain); - }; - - ContributorsGraph.prototype.set_domain = function() { - this.set_x_domain(); - return this.set_y_domain(); - }; - - ContributorsGraph.prototype.create_scale = function(width, height) { - this.x = d3 - .scaleTime() - .range([0, width]) - .clamp(true); - return (this.y = d3 - .scaleLinear() - .range([height, 0]) - .nice()); - }; - - ContributorsGraph.prototype.draw_x_axis = function() { - return this.svg - .append('g') - .attr('class', 'x axis') - .attr('transform', `translate(0, ${this.height})`) - .call(this.x_axis); - }; - - ContributorsGraph.prototype.draw_y_axis = function() { - return this.svg - .append('g') - .attr('class', 'y axis') - .call(this.y_axis); - }; - - ContributorsGraph.prototype.set_data = function(data) { - return (this.data = data); - }; - - return ContributorsGraph; -})(); - -export const ContributorsMasterGraph = (function(superClass) { - extend(ContributorsMasterGraph, superClass); - - function ContributorsMasterGraph(data1) { - const $parentElement = $('#contributors-master'); - - this.data = data1; - this.update_content = this.update_content.bind(this); - this.width = this.determine_width($('.js-graphs-show').width(), $parentElement); - this.height = 200; - this.x = null; - this.y = null; - this.x_axis = null; - this.y_axis = null; - this.area = null; - this.svg = null; - this.brush = null; - this.x_max_domain = null; - } - - ContributorsMasterGraph.prototype.process_dates = function(data) { - const dates = this.get_dates(data); - this.parse_dates(data); - return ContributorsGraph.set_dates(dates); - }; - - ContributorsMasterGraph.prototype.get_dates = function(data) { - return _.pluck(data, 'date'); - }; - - ContributorsMasterGraph.prototype.parse_dates = function(data) { - const parseDate = d3.timeParse('%Y-%m-%d'); - return data.forEach(d => (d.date = parseDate(d.date))); - }; - - ContributorsMasterGraph.prototype.create_scale = function() { - return ContributorsMasterGraph.__super__.create_scale.call(this, this.width, this.height); - }; - - ContributorsMasterGraph.prototype.create_axes = function() { - this.x_axis = d3 - .axisBottom() - .scale(this.x) - .tickFormat(dateTickFormat); - return (this.y_axis = d3 - .axisLeft() - .scale(this.y) - .ticks(5)); - }; - - ContributorsMasterGraph.prototype.create_svg = function() { - this.svg = d3 - .select('#contributors-master') - .append('svg') - .attr('width', this.width + this.MARGIN.left + this.MARGIN.right) - .attr('height', this.height + this.MARGIN.top + this.MARGIN.bottom) - .attr('class', 'tint-box') - .append('g') - .attr('transform', `translate(${this.MARGIN.left},${this.MARGIN.top})`); - return this.svg; - }; - - ContributorsMasterGraph.prototype.create_area = function(x, y) { - return (this.area = d3 - .area() - .x(d => x(d.date)) - .y0(this.height) - .y1(d => { - d.commits = d.commits || d.additions || d.deletions; - return y(d.commits); - })); - }; - - ContributorsMasterGraph.prototype.create_brush = function() { - return (this.brush = d3 - .brushX(this.x) - .extent([[this.x.range()[0], 0], [this.x.range()[1], this.height]]) - .on('end', this.update_content)); - }; - - ContributorsMasterGraph.prototype.draw_path = function(data) { - return this.svg - .append('path') - .datum(data) - .attr('class', 'area') - .attr('d', this.area); - }; - - ContributorsMasterGraph.prototype.add_brush = function() { - return this.svg - .append('g') - .attr('class', 'selection') - .call(this.brush) - .selectAll('rect') - .attr('height', this.height); - }; - - ContributorsMasterGraph.prototype.update_content = function() { - // d3Event.selection replaces the function brush.empty() calls - if (d3Event.selection != null) { - ContributorsGraph.set_x_domain(d3Event.selection.map(this.x.invert)); - } else { - ContributorsGraph.set_x_domain(this.x_max_domain); - } - return $('#brush_change').trigger('change'); - }; - - ContributorsMasterGraph.prototype.draw = function() { - this.process_dates(this.data); - this.create_scale(); - this.create_axes(); - ContributorsGraph.init_domain(this.data); - this.x_max_domain = this.x_domain; - this.set_domain(); - this.create_area(this.x, this.y); - this.create_svg(); - this.create_brush(); - this.draw_path(this.data); - this.draw_x_axis(); - this.draw_y_axis(); - return this.add_brush(); - }; - - ContributorsMasterGraph.prototype.redraw = function() { - this.process_dates(this.data); - ContributorsGraph.set_y_domain(this.data); - this.set_y_domain(); - this.svg.select('path').datum(this.data); - this.svg.select('path').attr('d', this.area); - return this.svg.select('.y.axis').call(this.y_axis); - }; - - return ContributorsMasterGraph; -})(ContributorsGraph); - -export const ContributorsAuthorGraph = (function(superClass) { - extend(ContributorsAuthorGraph, superClass); - - function ContributorsAuthorGraph(data1) { - const $parentElements = $('.person'); - - this.data = data1; - // Don't split graph size in half for mobile devices. - if ($(window).width() < 790) { - this.width = this.determine_width($('.js-graphs-show').width(), $parentElements); - } else { - this.width = this.determine_width($('.js-graphs-show').width() / 2, $parentElements); - } - this.height = 200; - this.x = null; - this.y = null; - this.x_axis = null; - this.y_axis = null; - this.area = null; - this.svg = null; - this.list_item = null; - } - - ContributorsAuthorGraph.prototype.create_scale = function() { - return ContributorsAuthorGraph.__super__.create_scale.call(this, this.width, this.height); - }; - - ContributorsAuthorGraph.prototype.create_axes = function() { - this.x_axis = d3 - .axisBottom() - .scale(this.x) - .ticks(8) - .tickFormat(dateTickFormat); - return (this.y_axis = d3 - .axisLeft() - .scale(this.y) - .ticks(5)); - }; - - ContributorsAuthorGraph.prototype.create_area = function(x, y) { - return (this.area = d3 - .area() - .x(d => { - const parseDate = d3.timeParse('%Y-%m-%d'); - return x(parseDate(d)); - }) - .y0(this.height) - .y1( - (function(_this) { - return function(d) { - if (_this.data[d] != null) { - return y(_this.data[d]); - } else { - return y(0); - } - }; - })(this), - )); - }; - - ContributorsAuthorGraph.prototype.create_svg = function() { - const persons = document.querySelectorAll('.person'); - this.list_item = persons[persons.length - 1]; - this.svg = d3 - .select(this.list_item) - .append('svg') - .attr('width', this.width + this.MARGIN.left + this.MARGIN.right) - .attr('height', this.height + this.MARGIN.top + this.MARGIN.bottom) - .attr('class', 'spark') - .append('g') - .attr('transform', `translate(${this.MARGIN.left},${this.MARGIN.top})`); - return this.svg; - }; - - ContributorsAuthorGraph.prototype.draw_path = function(data) { - return this.svg - .append('path') - .datum(data) - .attr('class', 'area-contributor') - .attr('d', this.area); - }; - - ContributorsAuthorGraph.prototype.draw = function() { - this.create_scale(); - this.create_axes(); - this.set_domain(); - this.create_area(this.x, this.y); - this.create_svg(); - this.draw_path(this.dates); - this.draw_x_axis(); - return this.draw_y_axis(); - }; - - ContributorsAuthorGraph.prototype.redraw = function() { - this.set_domain(); - this.svg.select('path').datum(this.dates); - this.svg.select('path').attr('d', this.area); - this.svg.select('.x.axis').call(this.x_axis); - return this.svg.select('.y.axis').call(this.y_axis); - }; - - return ContributorsAuthorGraph; -})(ContributorsGraph); diff --git a/app/assets/javascripts/pages/projects/graphs/show/stat_graph_contributors_util.js b/app/assets/javascripts/pages/projects/graphs/show/stat_graph_contributors_util.js deleted file mode 100644 index a89a13fe37a..00000000000 --- a/app/assets/javascripts/pages/projects/graphs/show/stat_graph_contributors_util.js +++ /dev/null @@ -1,143 +0,0 @@ -/* eslint-disable func-names, no-var, one-var, camelcase, no-param-reassign, no-return-assign, consistent-return, no-cond-assign, no-else-return */ -import _ from 'underscore'; - -export default { - parse_log(log) { - var by_author, by_email, data, entry, i, len, total, normalized_email; - total = {}; - by_author = {}; - by_email = {}; - for (i = 0, len = log.length; i < len; i += 1) { - entry = log[i]; - if (total[entry.date] == null) { - this.add_date(entry.date, total); - } - normalized_email = entry.author_email.toLowerCase(); - data = by_author[entry.author_name] || by_email[normalized_email]; - if (data == null) { - data = this.add_author(entry, by_author, by_email); - } - if (!data[entry.date]) { - this.add_date(entry.date, data); - } - this.store_data(entry, total[entry.date], data[entry.date]); - } - total = _.toArray(total); - by_author = _.toArray(by_author); - return { - total, - by_author, - }; - }, - add_date(date, collection) { - collection[date] = {}; - return (collection[date].date = date); - }, - add_author(author, by_author, by_email) { - var data, normalized_email; - data = {}; - data.author_name = author.author_name; - data.author_email = author.author_email; - normalized_email = author.author_email.toLowerCase(); - by_author[author.author_name] = data; - by_email[normalized_email] = data; - return data; - }, - store_data(entry, total, by_author) { - this.store_commits(total, by_author); - this.store_additions(entry, total, by_author); - return this.store_deletions(entry, total, by_author); - }, - store_commits(total, by_author) { - this.add(total, 'commits', 1); - return this.add(by_author, 'commits', 1); - }, - add(collection, field, value) { - if (collection[field] == null) { - collection[field] = 0; - } - return (collection[field] += value); - }, - store_additions(entry, total, by_author) { - if (entry.additions == null) { - entry.additions = 0; - } - this.add(total, 'additions', entry.additions); - return this.add(by_author, 'additions', entry.additions); - }, - store_deletions(entry, total, by_author) { - if (entry.deletions == null) { - entry.deletions = 0; - } - this.add(total, 'deletions', entry.deletions); - return this.add(by_author, 'deletions', entry.deletions); - }, - get_total_data(parsed_log, field) { - var log, total_data; - log = parsed_log.total; - total_data = this.pick_field(log, field); - return _.sortBy(total_data, d => d.date); - }, - pick_field(log, field) { - var total_data; - total_data = []; - _.each(log, d => total_data.push(_.pick(d, [field, 'date']))); - return total_data; - }, - get_author_data(parsed_log, field, date_range) { - var author_data, log; - if (date_range == null) { - date_range = null; - } - log = parsed_log.by_author; - author_data = []; - _.each( - log, - (function(_this) { - return function(log_entry) { - var parsed_log_entry; - parsed_log_entry = _this.parse_log_entry(log_entry, field, date_range); - if (!_.isEmpty(parsed_log_entry.dates)) { - return author_data.push(parsed_log_entry); - } - }; - })(this), - ); - return _.sortBy(author_data, d => d[field]).reverse(); - }, - parse_log_entry(log_entry, field, date_range) { - var parsed_entry; - parsed_entry = {}; - - parsed_entry.author_name = log_entry.author_name; - parsed_entry.author_email = log_entry.author_email; - parsed_entry.dates = {}; - - parsed_entry.commits = 0; - parsed_entry.additions = 0; - parsed_entry.deletions = 0; - - _.each( - _.omit(log_entry, 'author_name', 'author_email'), - (function(_this) { - return function(value) { - if (_this.in_range(value.date, date_range)) { - parsed_entry.dates[value.date] = value[field]; - parsed_entry.commits += value.commits; - parsed_entry.additions += value.additions; - return (parsed_entry.deletions += value.deletions); - } - }; - })(this), - ); - return parsed_entry; - }, - in_range(date, date_range) { - var ref; - if (date_range === null || (date_range[0] <= (ref = new Date(date)) && ref <= date_range[1])) { - return true; - } else { - return false; - } - }, -}; diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue index 89cac42abae..fcc57da0649 100644 --- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue +++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue @@ -345,8 +345,8 @@ export default { <project-setting-row v-if="pagesAvailable && pagesAccessControlEnabled" :help-path="pagesHelpPath" - label="Pages access control" - help-text="Access control for the project's static website" + :label="s__('ProjectSettings|Pages')" + :help-text="__('With GitLab Pages you can host your static websites on GitLab')" > <project-feature-setting v-model="pagesAccessLevel" diff --git a/app/assets/stylesheets/pages/graph.scss b/app/assets/stylesheets/pages/graph.scss index 3febf4cf826..a8de8303a19 100644 --- a/app/assets/stylesheets/pages/graph.scss +++ b/app/assets/stylesheets/pages/graph.scss @@ -17,21 +17,6 @@ } } -.graphs { - .graph-author-email { - float: right; - color: $gl-gray-500; - } - - .graph-additions { - color: $green-600; - } - - .graph-deletions { - color: $red-500; - } -} - .svg-graph-container { width: 100%; diff --git a/app/assets/stylesheets/pages/stat_graph.scss b/app/assets/stylesheets/pages/stat_graph.scss deleted file mode 100644 index 31ccdacbc02..00000000000 --- a/app/assets/stylesheets/pages/stat_graph.scss +++ /dev/null @@ -1,62 +0,0 @@ -.tint-box { - background: $stat-graph-common-bg; - position: relative; - margin-bottom: 10px; -} - -.area { - fill: $green-500; - fill-opacity: 0.5; -} - -.axis { - font-size: 10px; -} - -#contributors-master { - @include media-breakpoint-up(md) { - @include make-col-ready(); - @include make-col(12); - } -} - -#contributors { - flex: 1; - - .contributors-list { - margin: 0 0 10px; - list-style: none; - padding: 0; - } - - .person { - @include media-breakpoint-up(md) { - @include make-col-ready(); - @include make-col(6); - } - - margin-top: 10px; - - @include media-breakpoint-down(xs) { - width: 100%; - } - - .spark { - display: block; - background: $stat-graph-common-bg; - width: 100%; - } - - .area-contributor { - fill: $orange-500; - } - } -} - -.selection rect { - fill-opacity: 0.1; - stroke-width: 1px; - stroke-opacity: 0.4; - shape-rendering: crispedges; - stroke-dasharray: 3 3; -} diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index df9d1933271..3c72f41a4c9 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -281,10 +281,7 @@ module IssuablesHelper } data[:hasClosingMergeRequest] = issuable.merge_requests_count(current_user) != 0 if issuable.is_a?(Issue) - - zoom_links = Gitlab::ZoomLinkExtractor.new(issuable.description).links - - data[:zoomMeetingUrl] = zoom_links.last if zoom_links.any? + data[:zoomMeetingUrl] = ZoomMeeting.canonical_meeting_url(issuable) if issuable.is_a?(Issue) if parent.is_a?(Group) data[:groupPath] = parent.path diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index 978d0a8c8fb..a59cc3282bb 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -273,7 +273,7 @@ module SearchHelper sanitize(html, tags: %w(a p ol ul li pre code)) end - def search_tabs?(tab) + def show_user_search_tab? return false if Feature.disabled?(:users_search, default_enabled: true) if @project diff --git a/app/models/issue.rb b/app/models/issue.rb index 91bef81227f..260593c38f4 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -40,6 +40,7 @@ class Issue < ApplicationRecord has_many :issue_assignees has_many :assignees, class_name: "User", through: :issue_assignees + has_many :zoom_meetings validates :project, presence: true diff --git a/app/models/zoom_meeting.rb b/app/models/zoom_meeting.rb index cb3b5c60e54..a7ecd1e6a2c 100644 --- a/app/models/zoom_meeting.rb +++ b/app/models/zoom_meeting.rb @@ -14,4 +14,13 @@ class ZoomMeeting < ApplicationRecord scope :added_to_issue, -> { where(issue_status: :added) } scope :removed_from_issue, -> { where(issue_status: :removed) } + scope :canonical, -> (issue) { where(issue: issue).added_to_issue } + + def self.canonical_meeting(issue) + canonical(issue)&.take + end + + def self.canonical_meeting_url(issue) + canonical_meeting(issue)&.url + end end diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index 528b1ea61b3..b98a4d2567f 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -61,8 +61,6 @@ module Issues if added_mentions.present? notification_service.async.new_mentions_in_issue(issue, added_mentions, current_user) end - - ZoomNotesService.new(issue, project, current_user, old_description: old_associations[:description]).execute end def handle_task_changes(issuable) diff --git a/app/services/issues/zoom_link_service.rb b/app/services/issues/zoom_link_service.rb index 561c86475e5..023d7080e88 100644 --- a/app/services/issues/zoom_link_service.rb +++ b/app/services/issues/zoom_link_service.rb @@ -6,32 +6,37 @@ module Issues super(issue.project, user) @issue = issue + @added_meeting = ZoomMeeting.canonical_meeting(@issue) end def add_link(link) if can_add_link? && (link = parse_link(link)) - track_meeting_added_event - success(_('Zoom meeting added'), append_to_description(link)) + begin + add_zoom_meeting(link) + success(_('Zoom meeting added')) + rescue ActiveRecord::RecordNotUnique + error(_('Failed to add a Zoom meeting')) + end else error(_('Failed to add a Zoom meeting')) end end - def can_add_link? - can? && !link_in_issue_description? - end - def remove_link if can_remove_link? - track_meeting_removed_event - success(_('Zoom meeting removed'), remove_from_description) + remove_zoom_meeting + success(_('Zoom meeting removed')) else error(_('Failed to remove a Zoom meeting')) end end + def can_add_link? + can_update_issue? && !@added_meeting + end + def can_remove_link? - can? && link_in_issue_description? + can_update_issue? && !!@added_meeting end def parse_link(link) @@ -42,10 +47,6 @@ module Issues attr_reader :issue - def issue_description - issue.description || '' - end - def track_meeting_added_event ::Gitlab::Tracking.event('IncidentManagement::ZoomIntegration', 'add_zoom_meeting', label: 'Issue ID', value: issue.id) end @@ -54,39 +55,33 @@ module Issues ::Gitlab::Tracking.event('IncidentManagement::ZoomIntegration', 'remove_zoom_meeting', label: 'Issue ID', value: issue.id) end - def success(message, description) - ServiceResponse - .success(message: message, payload: { description: description }) - end - - def error(message) - ServiceResponse.error(message: message) + def add_zoom_meeting(link) + ZoomMeeting.create( + issue: @issue, + project: @issue.project, + issue_status: :added, + url: link + ) + track_meeting_added_event + SystemNoteService.zoom_link_added(@issue, @project, current_user) end - def append_to_description(link) - "#{issue_description}\n\n#{link}" + def remove_zoom_meeting + @added_meeting.update(issue_status: :removed) + track_meeting_removed_event + SystemNoteService.zoom_link_removed(@issue, @project, current_user) end - def remove_from_description - link = parse_link(issue_description) - return issue_description unless link - - issue_description.delete_suffix(link).rstrip + def success(message) + ServiceResponse.success(message: message) end - def link_in_issue_description? - link = extract_link_from_issue_description - return unless link - - Gitlab::ZoomLinkExtractor.new(link).match? - end - - def extract_link_from_issue_description - issue_description[/(\S+)\z/, 1] + def error(message) + ServiceResponse.error(message: message) end - def can? - current_user.can?(:update_issue, project) + def can_update_issue? + can?(current_user, :update_issue, project) end end end diff --git a/app/services/zoom_notes_service.rb b/app/services/zoom_notes_service.rb deleted file mode 100644 index 983a7fcacd1..00000000000 --- a/app/services/zoom_notes_service.rb +++ /dev/null @@ -1,42 +0,0 @@ -# frozen_string_literal: true - -class ZoomNotesService - def initialize(issue, project, current_user, old_description: nil) - @issue = issue - @project = project - @current_user = current_user - @old_description = old_description - end - - def execute - return if @issue.description == @old_description - - if zoom_link_added? - zoom_link_added_notification - elsif zoom_link_removed? - zoom_link_removed_notification - end - end - - private - - def zoom_link_added? - has_zoom_link?(@issue.description) && !has_zoom_link?(@old_description) - end - - def zoom_link_removed? - !has_zoom_link?(@issue.description) && has_zoom_link?(@old_description) - end - - def has_zoom_link?(text) - Gitlab::ZoomLinkExtractor.new(text).match? - end - - def zoom_link_added_notification - SystemNoteService.zoom_link_added(@issue, @project, @current_user) - end - - def zoom_link_removed_notification - SystemNoteService.zoom_link_removed(@issue, @project, @current_user) - end -end diff --git a/app/views/clusters/clusters/index.html.haml b/app/views/clusters/clusters/index.html.haml index 9bab3bf56aa..049010cadf4 100644 --- a/app/views/clusters/clusters/index.html.haml +++ b/app/views/clusters/clusters/index.html.haml @@ -16,7 +16,7 @@ .bs-callout.bs-callout-info = s_('ClusterIntegration|Clusters are utilized by selecting the nearest ancestor with a matching environment scope. For example, project clusters will override group clusters.') %strong - = link_to _('More information'), help_page_path('user/group/clusters/', anchor: 'cluster-precedence') + = link_to _('More information'), help_page_path('user/group/clusters/index', anchor: 'cluster-precedence') .clusters-table.js-clusters-list .gl-responsive-table-row.table-row-header{ role: "row" } diff --git a/app/views/projects/graphs/show.html.haml b/app/views/projects/graphs/show.html.haml index 6e5e4607232..a952db0eea3 100644 --- a/app/views/projects/graphs/show.html.haml +++ b/app/views/projects/graphs/show.html.haml @@ -1,26 +1,8 @@ - page_title _('Contributors') -.js-graphs-show{ 'data-project-graph-path': project_graph_path(@project, current_ref, format: :json) } - .sub-header-block - .tree-ref-holder.inline.vertical-align-middle - = render 'shared/ref_switcher', destination: 'graphs' - = link_to s_('Commits|History'), project_commits_path(@project, current_ref), class: 'btn' +.sub-header-block.bg-gray-light.gl-p-3 + .tree-ref-holder.inline.vertical-align-middle + = render 'shared/ref_switcher', destination: 'graphs' + = link_to s_('Commits|History'), project_commits_path(@project, current_ref), class: 'btn' - .loading-graph - .center - %h3.page-title - %i.fa.fa-spinner.fa-spin - = s_('ContributorsPage|Building repository graph.') - %p.slead - = s_('ContributorsPage|Please wait a moment, this page will automatically refresh when ready.') - - .stat-graph.hide - .header.clearfix - %h3#date_header.page-title - %p.light - = s_('ContributorsPage|Commits to %{branch_name}, excluding merge commits. Limited to 6,000 commits.') % { branch_name: @ref } - %input#brush_change{ :type => "hidden" } - .graphs.row - #contributors-master.svg-w-100 - #contributors.clearfix - %ol.contributors-list.svg-w-100.row +.js-contributors-graph{ class: container_class, 'data-project-graph-path': project_graph_path(@project, current_ref, format: :json),'data-project-branch': current_ref } diff --git a/app/views/projects/services/edit.html.haml b/app/views/projects/services/edit.html.haml index fc20bc52d1c..1e7903535c6 100644 --- a/app/views/projects/services/edit.html.haml +++ b/app/views/projects/services/edit.html.haml @@ -1,6 +1,7 @@ -- breadcrumb_title s_("ProjectService|Integrations") +- breadcrumb_title @service.title - page_title @service.title, s_("ProjectService|Services") - add_to_breadcrumbs(s_("ProjectService|Settings"), edit_project_path(@project)) +- add_to_breadcrumbs(s_("ProjectService|Integrations"), namespace_project_settings_integrations_path) = render 'deprecated_message' if @service.deprecation_message diff --git a/app/views/search/_category.html.haml b/app/views/search/_category.html.haml index eae2a491ceb..84198489e41 100644 --- a/app/views/search/_category.html.haml +++ b/app/views/search/_category.html.haml @@ -1,5 +1,5 @@ - users = capture_haml do - - if search_tabs?(:members) + - if show_user_search_tab? = search_filter_link 'users', _("Users") .scrolling-tabs-container.inner-page-scroll-tabs.is-smaller diff --git a/changelogs/unreleased/30131-breadcrumb-back-to-integrations-not-correct.yml b/changelogs/unreleased/30131-breadcrumb-back-to-integrations-not-correct.yml new file mode 100644 index 00000000000..5b8a46b4a88 --- /dev/null +++ b/changelogs/unreleased/30131-breadcrumb-back-to-integrations-not-correct.yml @@ -0,0 +1,5 @@ +--- +title: Add missing breadcrumb in Project > Settings > Integrations +merge_request: 18990 +author: +type: fixed diff --git a/changelogs/unreleased/33042-persist-zoom-meetings-added-to-issues-in-the-database-2.yml b/changelogs/unreleased/33042-persist-zoom-meetings-added-to-issues-in-the-database-2.yml new file mode 100644 index 00000000000..dd7df055c71 --- /dev/null +++ b/changelogs/unreleased/33042-persist-zoom-meetings-added-to-issues-in-the-database-2.yml @@ -0,0 +1,5 @@ +--- +title: Store Zoom URLs in a table rather than in the issue description +merge_request: 18620 +author: +type: changed diff --git a/changelogs/unreleased/34944-fix-kubernetes-help-text-link.yml b/changelogs/unreleased/34944-fix-kubernetes-help-text-link.yml new file mode 100644 index 00000000000..5fd87c43bbc --- /dev/null +++ b/changelogs/unreleased/34944-fix-kubernetes-help-text-link.yml @@ -0,0 +1,5 @@ +--- +title: Fix Kubernetes help text link +merge_request: 19121 +author: +type: fixed diff --git a/changelogs/unreleased/add-inheritable-mixin.yml b/changelogs/unreleased/add-inheritable-mixin.yml new file mode 100644 index 00000000000..e0fd2326cc5 --- /dev/null +++ b/changelogs/unreleased/add-inheritable-mixin.yml @@ -0,0 +1,5 @@ +--- +title: Make `Job`, `Bridge` and `Default` inheritable +merge_request: 18867 +author: +type: added diff --git a/lib/gitlab/ci/config/entry/default.rb b/lib/gitlab/ci/config/entry/default.rb index 6200d7c7f87..78297ff5846 100644 --- a/lib/gitlab/ci/config/entry/default.rb +++ b/lib/gitlab/ci/config/entry/default.rb @@ -11,8 +11,7 @@ module Gitlab # class Default < ::Gitlab::Config::Entry::Node include ::Gitlab::Config::Entry::Configurable - - DuplicateError = Class.new(Gitlab::Config::Loader::FormatError) + include ::Gitlab::Config::Entry::Inheritable ALLOWED_KEYS = %i[before_script image services after_script cache].freeze @@ -43,29 +42,16 @@ module Gitlab helpers :before_script, :image, :services, :after_script, :cache - def compose!(deps = nil) - super(self) - - inherit!(deps) - end - private - def inherit!(deps) - return unless deps + def overwrite_entry(deps, key, current_entry) + inherited_entry = deps[key] - self.class.nodes.each do |key, factory| - next unless factory.inheritable? - - root_entry = deps[key] - next unless root_entry.specified? - - if self[key].specified? - raise DuplicateError, "#{key} is defined in top-level and `default:` entry" - end - - @entries[key] = root_entry + if inherited_entry.specified? && current_entry.specified? + raise InheritError, "#{key} is defined in top-level and `default:` entry" end + + inherited_entry unless current_entry.specified? end end end diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index 07d5be86b1e..1298e2d3462 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -10,6 +10,7 @@ module Gitlab class Job < ::Gitlab::Config::Entry::Node include ::Gitlab::Config::Entry::Configurable include ::Gitlab::Config::Entry::Attributable + include ::Gitlab::Config::Entry::Inheritable ALLOWED_WHEN = %w[on_success on_failure always manual delayed].freeze ALLOWED_KEYS = %i[tags script only except rules type image services @@ -73,13 +74,16 @@ module Gitlab inherit: true entry :script, Entry::Commands, - description: 'Commands that will be executed in this job.' + description: 'Commands that will be executed in this job.', + inherit: false entry :stage, Entry::Stage, - description: 'Pipeline stage this job will be executed into.' + description: 'Pipeline stage this job will be executed into.', + inherit: false entry :type, Entry::Stage, - description: 'Deprecated: stage this job will be executed into.' + description: 'Deprecated: stage this job will be executed into.', + inherit: false entry :after_script, Entry::Script, description: 'Commands that will be executed when finishing job.', @@ -99,28 +103,36 @@ module Gitlab entry :only, Entry::Policy, description: 'Refs policy this job will be executed for.', - default: Entry::Policy::DEFAULT_ONLY + default: Entry::Policy::DEFAULT_ONLY, + inherit: false entry :except, Entry::Policy, - description: 'Refs policy this job will be executed for.' + description: 'Refs policy this job will be executed for.', + inherit: false entry :rules, Entry::Rules, - description: 'List of evaluable Rules to determine job inclusion.' + description: 'List of evaluable Rules to determine job inclusion.', + inherit: false entry :variables, Entry::Variables, - description: 'Environment variables available for this job.' + description: 'Environment variables available for this job.', + inherit: false entry :artifacts, Entry::Artifacts, - description: 'Artifacts configuration for this job.' + description: 'Artifacts configuration for this job.', + inherit: false entry :environment, Entry::Environment, - description: 'Environment configuration for this job.' + description: 'Environment configuration for this job.', + inherit: false entry :coverage, Entry::Coverage, - description: 'Coverage configuration for this job.' + description: 'Coverage configuration for this job.', + inherit: false entry :retry, Entry::Retry, - description: 'Retry configuration for this job.' + description: 'Retry configuration for this job.', + inherit: false helpers :before_script, :script, :stage, :type, :after_script, :cache, :image, :services, :only, :except, :variables, @@ -155,8 +167,6 @@ module Gitlab @entries.delete(:except) end end - - inherit!(deps) end def name @@ -185,21 +195,8 @@ module Gitlab private - # We inherit config entries from `default:` - # if the entry has the `inherit: true` flag set - def inherit!(deps) - return unless deps - - self.class.nodes.each do |key, factory| - next unless factory.inheritable? - - default_entry = deps.default[key] - job_entry = self[key] - - if default_entry.specified? && !job_entry.specified? - @entries[key] = default_entry - end - end + def overwrite_entry(deps, key, current_entry) + deps.default[key] unless current_entry.specified? end def to_hash diff --git a/lib/gitlab/ci/templates/Android-Fastlane.gitlab-ci.yml b/lib/gitlab/ci/templates/Android-Fastlane.gitlab-ci.yml index be584814271..bf5b485f853 100644 --- a/lib/gitlab/ci/templates/Android-Fastlane.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Android-Fastlane.gitlab-ci.yml @@ -113,9 +113,9 @@ promoteBeta: promoteProduction: extends: .promote_job stage: production - # We only allow production promotion on `master` because + # We only allow production promotion on the default branch because # it has its own production scoped secret variables only: - - master + - $CI_DEFAULT_BRANCH script: - bundle exec fastlane promote_beta_to_production diff --git a/lib/gitlab/ci/templates/Docker.gitlab-ci.yml b/lib/gitlab/ci/templates/Docker.gitlab-ci.yml index 15cdbf63cb1..76453881950 100644 --- a/lib/gitlab/ci/templates/Docker.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Docker.gitlab-ci.yml @@ -10,7 +10,7 @@ docker-build-master: - docker build --pull -t "$CI_REGISTRY_IMAGE" . - docker push "$CI_REGISTRY_IMAGE" only: - - master + - $CI_DEFAULT_BRANCH docker-build: # Official docker image. @@ -24,4 +24,4 @@ docker-build: - docker build --pull -t "$CI_REGISTRY_IMAGE:$CI_COMMIT_REF_SLUG" . - docker push "$CI_REGISTRY_IMAGE:$CI_COMMIT_REF_SLUG" except: - - master + - $CI_DEFAULT_BRANCH diff --git a/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml index 6de7aace8db..6243adcd57c 100644 --- a/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml @@ -25,7 +25,7 @@ review: kubernetes: active except: refs: - - master + - $CI_DEFAULT_BRANCH variables: - $REVIEW_DISABLED @@ -49,7 +49,7 @@ stop_review: kubernetes: active except: refs: - - master + - $CI_DEFAULT_BRANCH variables: - $REVIEW_DISABLED @@ -74,7 +74,7 @@ staging: url: http://$CI_PROJECT_PATH_SLUG-staging.$KUBE_INGRESS_BASE_DOMAIN only: refs: - - master + - $CI_DEFAULT_BRANCH kubernetes: active variables: - $STAGING_ENABLED @@ -99,7 +99,7 @@ canary: when: manual only: refs: - - master + - $CI_DEFAULT_BRANCH kubernetes: active variables: - $CANARY_ENABLED @@ -127,7 +127,7 @@ production: <<: *production_template only: refs: - - master + - $CI_DEFAULT_BRANCH kubernetes: active except: variables: @@ -142,7 +142,7 @@ production_manual: allow_failure: false only: refs: - - master + - $CI_DEFAULT_BRANCH kubernetes: active variables: - $STAGING_ENABLED @@ -152,7 +152,7 @@ production_manual: - $INCREMENTAL_ROLLOUT_ENABLED - $INCREMENTAL_ROLLOUT_MODE -# This job implements incremental rollout on for every push to `master`. +# This job implements incremental rollout for every push to the default branch. .rollout: &rollout_template extends: .auto-deploy @@ -179,7 +179,7 @@ production_manual: # This selectors are backward compatible mode with $INCREMENTAL_ROLLOUT_ENABLED (before 11.4) only: refs: - - master + - $CI_DEFAULT_BRANCH kubernetes: active variables: - $INCREMENTAL_ROLLOUT_MODE == "manual" @@ -194,7 +194,7 @@ production_manual: start_in: 5 minutes only: refs: - - master + - $CI_DEFAULT_BRANCH kubernetes: active variables: - $INCREMENTAL_ROLLOUT_MODE == "timed" diff --git a/lib/gitlab/ci/templates/Julia.gitlab-ci.yml b/lib/gitlab/ci/templates/Julia.gitlab-ci.yml index 32d4e07d398..49f35409cf6 100644 --- a/lib/gitlab/ci/templates/Julia.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Julia.gitlab-ci.yml @@ -64,7 +64,7 @@ pages: paths: - public only: - - master + - $CI_DEFAULT_BRANCH # WARNING: This template is using the `julia` images from [Docker # Hub][3]. One can use custom Julia images and/or the official ones found diff --git a/lib/gitlab/ci/templates/Maven.gitlab-ci.yml b/lib/gitlab/ci/templates/Maven.gitlab-ci.yml index 84bb0ff3b33..b9a3c144d28 100644 --- a/lib/gitlab/ci/templates/Maven.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Maven.gitlab-ci.yml @@ -6,7 +6,7 @@ # This template will build and test your projects # * Caches downloaded dependencies and plugins between invocation. # * Verify but don't deploy merge requests. -# * Deploy built artifacts from master branch only. +# * Deploy built artifacts from the default branch only. variables: # This will suppress any download for dependencies and plugins or upload messages which would clutter the console log. @@ -33,7 +33,7 @@ cache: script: - 'mvn $MAVEN_CLI_OPTS verify' except: - - master + - $CI_DEFAULT_BRANCH # Verify merge requests using JDK8 verify:jdk8: @@ -42,7 +42,7 @@ verify:jdk8: # To deploy packages from CI, create a ci_settings.xml file # For deploying packages to GitLab's Maven Repository: See https://docs.gitlab.com/ee/user/project/packages/maven_repository.html#creating-maven-packages-with-gitlab-cicd for more details. # Please note: The GitLab Maven Repository is currently only available in GitLab Premium / Ultimate. -# For `master` branch run `mvn deploy` automatically. +# For the default branch run `mvn deploy` automatically. deploy:jdk8: stage: deploy script: @@ -51,4 +51,4 @@ deploy:jdk8: fi - 'mvn $MAVEN_CLI_OPTS deploy -s ci_settings.xml' only: - - master + - $CI_DEFAULT_BRANCH diff --git a/lib/gitlab/ci/templates/Mono.gitlab-ci.yml b/lib/gitlab/ci/templates/Mono.gitlab-ci.yml index 10fb6be6c39..456b2b1b030 100644 --- a/lib/gitlab/ci/templates/Mono.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Mono.gitlab-ci.yml @@ -25,7 +25,7 @@ before_script: release: stage: deploy only: - - master + - $CI_DEFAULT_BRANCH artifacts: paths: - build/release/MyProject.exe diff --git a/lib/gitlab/ci/templates/OpenShift.gitlab-ci.yml b/lib/gitlab/ci/templates/OpenShift.gitlab-ci.yml index 65abee1f5eb..e25cede0252 100644 --- a/lib/gitlab/ci/templates/OpenShift.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/OpenShift.gitlab-ci.yml @@ -49,7 +49,7 @@ review: only: - branches except: - - master + - $CI_DEFAULT_BRANCH stop-review: <<: *deploy @@ -66,7 +66,7 @@ stop-review: only: - branches except: - - master + - $CI_DEFAULT_BRANCH staging: <<: *deploy @@ -78,7 +78,7 @@ staging: name: staging url: http://$CI_PROJECT_NAME-staging.$OPENSHIFT_DOMAIN only: - - master + - $CI_DEFAULT_BRANCH production: <<: *deploy @@ -91,4 +91,4 @@ production: name: production url: http://$CI_PROJECT_NAME.$OPENSHIFT_DOMAIN only: - - master + - $CI_DEFAULT_BRANCH diff --git a/lib/gitlab/ci/templates/Packer.gitlab-ci.yml b/lib/gitlab/ci/templates/Packer.gitlab-ci.yml index 0a3cf3dcf77..b942b14d474 100644 --- a/lib/gitlab/ci/templates/Packer.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Packer.gitlab-ci.yml @@ -25,4 +25,4 @@ build: - find . -maxdepth 1 -name '*.json' -print0 | xargs -t0n1 packer build when: manual only: - - master + - $CI_DEFAULT_BRANCH diff --git a/lib/gitlab/ci/templates/Pages/Brunch.gitlab-ci.yml b/lib/gitlab/ci/templates/Pages/Brunch.gitlab-ci.yml index d2dd3fbfb75..3b79a35c320 100644 --- a/lib/gitlab/ci/templates/Pages/Brunch.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Pages/Brunch.gitlab-ci.yml @@ -12,4 +12,4 @@ pages: paths: - public only: - - master + - $CI_DEFAULT_BRANCH diff --git a/lib/gitlab/ci/templates/Pages/Doxygen.gitlab-ci.yml b/lib/gitlab/ci/templates/Pages/Doxygen.gitlab-ci.yml index ba422c08614..891cf7d7799 100644 --- a/lib/gitlab/ci/templates/Pages/Doxygen.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Pages/Doxygen.gitlab-ci.yml @@ -10,4 +10,4 @@ pages: paths: - public only: - - master + - $CI_DEFAULT_BRANCH diff --git a/lib/gitlab/ci/templates/Pages/Gatsby.gitlab-ci.yml b/lib/gitlab/ci/templates/Pages/Gatsby.gitlab-ci.yml index a683561a455..6d151ef853f 100644 --- a/lib/gitlab/ci/templates/Pages/Gatsby.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Pages/Gatsby.gitlab-ci.yml @@ -14,4 +14,4 @@ pages: paths: - public only: - - master + - $CI_DEFAULT_BRANCH diff --git a/lib/gitlab/ci/templates/Pages/HTML.gitlab-ci.yml b/lib/gitlab/ci/templates/Pages/HTML.gitlab-ci.yml index 92f25280c6e..9d4ee7e1602 100644 --- a/lib/gitlab/ci/templates/Pages/HTML.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Pages/HTML.gitlab-ci.yml @@ -9,4 +9,4 @@ pages: paths: - public only: - - master + - $CI_DEFAULT_BRANCH diff --git a/lib/gitlab/ci/templates/Pages/Harp.gitlab-ci.yml b/lib/gitlab/ci/templates/Pages/Harp.gitlab-ci.yml index 0e206423fa5..63ca721bd4a 100644 --- a/lib/gitlab/ci/templates/Pages/Harp.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Pages/Harp.gitlab-ci.yml @@ -12,4 +12,4 @@ pages: paths: - public only: - - master + - $CI_DEFAULT_BRANCH diff --git a/lib/gitlab/ci/templates/Pages/Hexo.gitlab-ci.yml b/lib/gitlab/ci/templates/Pages/Hexo.gitlab-ci.yml index d91a8d7421f..e955628f27e 100644 --- a/lib/gitlab/ci/templates/Pages/Hexo.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Pages/Hexo.gitlab-ci.yml @@ -14,4 +14,4 @@ pages: - node_modules key: project only: - - master + - $CI_DEFAULT_BRANCH diff --git a/lib/gitlab/ci/templates/Pages/Hugo.gitlab-ci.yml b/lib/gitlab/ci/templates/Pages/Hugo.gitlab-ci.yml index 9a3ecd1c34f..b7330263845 100644 --- a/lib/gitlab/ci/templates/Pages/Hugo.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Pages/Hugo.gitlab-ci.yml @@ -8,10 +8,10 @@ pages: paths: - public only: - - master + - $CI_DEFAULT_BRANCH test: script: - hugo except: - - master + - $CI_DEFAULT_BRANCH diff --git a/lib/gitlab/ci/templates/Pages/Hyde.gitlab-ci.yml b/lib/gitlab/ci/templates/Pages/Hyde.gitlab-ci.yml index 7a441a2f70f..f7d393670ad 100644 --- a/lib/gitlab/ci/templates/Pages/Hyde.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Pages/Hyde.gitlab-ci.yml @@ -11,7 +11,7 @@ test: - pip install hyde - hyde gen except: - - master + - $CI_DEFAULT_BRANCH pages: stage: deploy @@ -22,4 +22,4 @@ pages: paths: - public only: - - master + - $CI_DEFAULT_BRANCH diff --git a/lib/gitlab/ci/templates/Pages/Jekyll.gitlab-ci.yml b/lib/gitlab/ci/templates/Pages/Jekyll.gitlab-ci.yml index e7dacd3a1fc..c82b70eb9b0 100644 --- a/lib/gitlab/ci/templates/Pages/Jekyll.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Pages/Jekyll.gitlab-ci.yml @@ -17,7 +17,7 @@ test: paths: - test except: - - master + - $CI_DEFAULT_BRANCH pages: stage: deploy @@ -27,4 +27,4 @@ pages: paths: - public only: - - master + - $CI_DEFAULT_BRANCH diff --git a/lib/gitlab/ci/templates/Pages/Jigsaw.gitlab-ci.yml b/lib/gitlab/ci/templates/Pages/Jigsaw.gitlab-ci.yml index 2d26b86a328..dda0cc52612 100644 --- a/lib/gitlab/ci/templates/Pages/Jigsaw.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Pages/Jigsaw.gitlab-ci.yml @@ -34,4 +34,4 @@ pages: paths: - public only: - - master + - $CI_DEFAULT_BRANCH diff --git a/lib/gitlab/ci/templates/Pages/Lektor.gitlab-ci.yml b/lib/gitlab/ci/templates/Pages/Lektor.gitlab-ci.yml index 93ab8e0be0d..ea07a7c3145 100644 --- a/lib/gitlab/ci/templates/Pages/Lektor.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Pages/Lektor.gitlab-ci.yml @@ -9,4 +9,4 @@ pages: paths: - public only: - - master + - $CI_DEFAULT_BRANCH diff --git a/lib/gitlab/ci/templates/Pages/Metalsmith.gitlab-ci.yml b/lib/gitlab/ci/templates/Pages/Metalsmith.gitlab-ci.yml index 6524405133a..a8815c9885a 100644 --- a/lib/gitlab/ci/templates/Pages/Metalsmith.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Pages/Metalsmith.gitlab-ci.yml @@ -13,4 +13,4 @@ pages: paths: - public only: - - master + - $CI_DEFAULT_BRANCH diff --git a/lib/gitlab/ci/templates/Pages/Middleman.gitlab-ci.yml b/lib/gitlab/ci/templates/Pages/Middleman.gitlab-ci.yml index 57ac323dfdf..4a1f357e699 100644 --- a/lib/gitlab/ci/templates/Pages/Middleman.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Pages/Middleman.gitlab-ci.yml @@ -12,7 +12,7 @@ test: - bundle install --path vendor - bundle exec middleman build except: - - master + - $CI_DEFAULT_BRANCH pages: script: @@ -24,4 +24,4 @@ pages: paths: - public only: - - master + - $CI_DEFAULT_BRANCH diff --git a/lib/gitlab/ci/templates/Pages/Nanoc.gitlab-ci.yml b/lib/gitlab/ci/templates/Pages/Nanoc.gitlab-ci.yml index 7f037b5f5cf..1b59ee352f6 100644 --- a/lib/gitlab/ci/templates/Pages/Nanoc.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Pages/Nanoc.gitlab-ci.yml @@ -9,4 +9,4 @@ pages: paths: - public only: - - master + - $CI_DEFAULT_BRANCH diff --git a/lib/gitlab/ci/templates/Pages/Octopress.gitlab-ci.yml b/lib/gitlab/ci/templates/Pages/Octopress.gitlab-ci.yml index 6d912a89bc1..e76363494c1 100644 --- a/lib/gitlab/ci/templates/Pages/Octopress.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Pages/Octopress.gitlab-ci.yml @@ -12,4 +12,4 @@ pages: paths: - public only: - - master + - $CI_DEFAULT_BRANCH diff --git a/lib/gitlab/ci/templates/Pages/SwaggerUI.gitlab-ci.yml b/lib/gitlab/ci/templates/Pages/SwaggerUI.gitlab-ci.yml index 8fd08ea7995..9093c7e29e2 100644 --- a/lib/gitlab/ci/templates/Pages/SwaggerUI.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Pages/SwaggerUI.gitlab-ci.yml @@ -26,4 +26,4 @@ pages: paths: - public only: - - master + - $CI_DEFAULT_BRANCH diff --git a/lib/gitlab/ci/templates/Python.gitlab-ci.yml b/lib/gitlab/ci/templates/Python.gitlab-ci.yml index 00b8b94b574..72753e6e9c1 100644 --- a/lib/gitlab/ci/templates/Python.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Python.gitlab-ci.yml @@ -48,4 +48,4 @@ pages: paths: - public only: - - master + - $CI_DEFAULT_BRANCH diff --git a/lib/gitlab/ci/templates/Swift.gitlab-ci.yml b/lib/gitlab/ci/templates/Swift.gitlab-ci.yml index ffed7a0fec2..c53ead81c51 100644 --- a/lib/gitlab/ci/templates/Swift.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Swift.gitlab-ci.yml @@ -22,7 +22,7 @@ archive_project: - xcodebuild clean archive -archivePath build/ProjectName -scheme SchemeName - xcodebuild -exportArchive -exportFormat ipa -archivePath "build/ProjectName.xcarchive" -exportPath "build/ProjectName.ipa" -exportProvisioningProfile "ProvisioningProfileName" only: - - master + - $CI_DEFAULT_BRANCH artifacts: paths: - build/ProjectName.ipa diff --git a/lib/gitlab/ci/templates/Terraform.gitlab-ci.yml b/lib/gitlab/ci/templates/Terraform.gitlab-ci.yml index f374bc7e26a..9dae76489e2 100644 --- a/lib/gitlab/ci/templates/Terraform.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Terraform.gitlab-ci.yml @@ -53,4 +53,4 @@ apply: - plan when: manual only: - - master + - $CI_DEFAULT_BRANCH diff --git a/lib/gitlab/config/entry/factory.rb b/lib/gitlab/config/entry/factory.rb index 8f1f4a81bb5..7c5ffaa7621 100644 --- a/lib/gitlab/config/entry/factory.rb +++ b/lib/gitlab/config/entry/factory.rb @@ -9,10 +9,12 @@ module Gitlab class Factory InvalidFactory = Class.new(StandardError) - def initialize(entry) - @entry = entry + attr_reader :entry_class + + def initialize(entry_class) + @entry_class = entry_class @metadata = {} - @attributes = { default: entry.default } + @attributes = { default: entry_class.default } end def value(value) @@ -34,6 +36,10 @@ module Gitlab @attributes[:description] end + def inherit + @attributes[:inherit] + end + def inheritable? @attributes[:inherit] end @@ -52,7 +58,7 @@ module Gitlab if @value.nil? Entry::Unspecified.new(fabricate_unspecified) else - fabricate(@entry, @value) + fabricate(entry_class, @value) end end @@ -68,12 +74,12 @@ module Gitlab if default.nil? fabricate(Entry::Undefined) else - fabricate(@entry, default) + fabricate(entry_class, default) end end - def fabricate(entry, value = nil) - entry.new(value, @metadata) do |node| + def fabricate(entry_class, value = nil) + entry_class.new(value, @metadata) do |node| node.key = @attributes[:key] node.parent = @attributes[:parent] node.default = @attributes[:default] diff --git a/lib/gitlab/config/entry/inheritable.rb b/lib/gitlab/config/entry/inheritable.rb new file mode 100644 index 00000000000..91ca82e6338 --- /dev/null +++ b/lib/gitlab/config/entry/inheritable.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Gitlab + module Config + module Entry + ## + # Entry that represents an inheritable configs. + # + module Inheritable + InheritError = Class.new(Gitlab::Config::Loader::FormatError) + + def compose!(deps = nil, &blk) + super(deps, &blk) + + inherit!(deps) + end + + private + + # We inherit config entries from `default:` + # if the entry has the `inherit: true` flag set + def inherit!(deps) + return unless deps + + self.class.nodes.each do |key, factory| + next unless factory.inheritable? + + new_entry = overwrite_entry(deps, key, self[key]) + + entries[key] = new_entry if new_entry&.specified? + end + end + + def overwrite_entry(deps, key, current_entry) + raise NotImplementedError + end + end + end + end +end diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index 141e73e6a47..9c00e3c8910 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -28,6 +28,7 @@ tree: - label: - :priorities - :issue_assignees + - :zoom_meetings - snippets: - :award_emoji - notes: diff --git a/lib/gitlab/quick_actions/issue_actions.rb b/lib/gitlab/quick_actions/issue_actions.rb index 404e0c31871..838aefb59f0 100644 --- a/lib/gitlab/quick_actions/issue_actions.rb +++ b/lib/gitlab/quick_actions/issue_actions.rb @@ -174,18 +174,14 @@ module Gitlab params '<Zoom URL>' types Issue condition do - zoom_link_service.can_add_link? + @zoom_service = zoom_link_service + @zoom_service.can_add_link? end parse_params do |link| - zoom_link_service.parse_link(link) + @zoom_service.parse_link(link) end command :zoom do |link| - result = zoom_link_service.add_link(link) - - if result.success? - @updates[:description] = result.payload[:description] - end - + result = @zoom_service.add_link(link) @execution_message[:zoom] = result.message end @@ -194,15 +190,11 @@ module Gitlab execution_message _('Zoom meeting removed') types Issue condition do - zoom_link_service.can_remove_link? + @zoom_service = zoom_link_service + @zoom_service.can_remove_link? end command :remove_zoom do - result = zoom_link_service.remove_link - - if result.success? - @updates[:description] = result.payload[:description] - end - + result = @zoom_service.remove_link @execution_message[:remove_zoom] = result.message end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 1e47dd679ac..bcce8ab490d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4226,6 +4226,9 @@ msgstr "" msgid "Commits per weekday" msgstr "" +msgid "Commits to" +msgstr "" + msgid "Commits|An error occurred while fetching merge requests data." msgstr "" @@ -4510,18 +4513,6 @@ msgstr "" msgid "Contributors" msgstr "" -msgid "ContributorsPage|%{startDate} – %{endDate}" -msgstr "" - -msgid "ContributorsPage|Building repository graph." -msgstr "" - -msgid "ContributorsPage|Commits to %{branch_name}, excluding merge commits. Limited to 6,000 commits." -msgstr "" - -msgid "ContributorsPage|Please wait a moment, this page will automatically refresh when ready." -msgstr "" - msgid "Control emails linked to your account" msgstr "" @@ -6465,9 +6456,6 @@ msgstr "" msgid "Error deleting %{issuableType}" msgstr "" -msgid "Error fetching contributors data." -msgstr "" - msgid "Error fetching diverging counts for branches. Please try again." msgstr "" @@ -6705,6 +6693,9 @@ msgstr "" msgid "Except policy:" msgstr "" +msgid "Excluding merge commits. Limited to 6,000 commits." +msgstr "" + msgid "Existing" msgstr "" @@ -11322,6 +11313,9 @@ msgstr "" msgid "Number of changes (branches or tags) in a single push to determine whether webhooks and services will be fired or not. Webhooks and services won't be submitted if it surpasses that value." msgstr "" +msgid "Number of commits" +msgstr "" + msgid "Number of commits per MR" msgstr "" @@ -12917,6 +12911,9 @@ msgstr "" msgid "ProjectSettings|Only signed commits can be pushed to this repository." msgstr "" +msgid "ProjectSettings|Pages" +msgstr "" + msgid "ProjectSettings|Pipelines must succeed" msgstr "" @@ -18905,6 +18902,9 @@ msgstr "" msgid "Will deploy to" msgstr "" +msgid "With GitLab Pages you can host your static websites on GitLab" +msgstr "" + msgid "With contribution analytics you can have an overview for the activity of issues, merge requests and push events of your organization and its members." msgstr "" diff --git a/spec/features/issues/user_creates_issue_spec.rb b/spec/features/issues/user_creates_issue_spec.rb index a71395c0e47..39ce3415727 100644 --- a/spec/features/issues/user_creates_issue_spec.rb +++ b/spec/features/issues/user_creates_issue_spec.rb @@ -92,19 +92,6 @@ describe "User creates issue" do .and have_content(label_titles.first) end end - - context "with Zoom link" do - it "adds Zoom button" do - issue_title = "Issue containing Zoom meeting link" - zoom_url = "https://gitlab.zoom.us/j/123456789" - - fill_in("Title", with: issue_title) - fill_in("Description", with: zoom_url) - click_button("Submit issue") - - expect(page).to have_link('Join Zoom meeting', href: zoom_url) - end - end end context "when signed in as user with special characters in their name" do diff --git a/spec/features/projects/graph_spec.rb b/spec/features/projects/graph_spec.rb index 6082eb03374..5dabaf20952 100644 --- a/spec/features/projects/graph_spec.rb +++ b/spec/features/projects/graph_spec.rb @@ -29,12 +29,6 @@ describe 'Project Graph', :js do end end - it 'renders graphs' do - visit project_graph_path(project, 'master') - - expect(page).to have_selector('.stat-graph', visible: false) - end - context 'commits graph' do before do visit commits_project_graph_path(project, 'master') diff --git a/spec/fixtures/lib/gitlab/import_export/project.json b/spec/fixtures/lib/gitlab/import_export/project.json index fbd752b7403..864933ca1b4 100644 --- a/spec/fixtures/lib/gitlab/import_export/project.json +++ b/spec/fixtures/lib/gitlab/import_export/project.json @@ -80,6 +80,17 @@ "issue_id": 40 } ], + "zoom_meetings": [ + { + "id": 1, + "project_id": 5, + "issue_id": 40, + "url": "https://zoom.us/j/123456789", + "issue_status": 1, + "created_at": "2016-06-14T15:02:04.418Z", + "updated_at": "2016-06-14T15:02:04.418Z" + } + ], "milestone": { "id": 1, "title": "test milestone", diff --git a/spec/frontend/contributors/component/__snapshots__/contributors_spec.js.snap b/spec/frontend/contributors/component/__snapshots__/contributors_spec.js.snap new file mode 100644 index 00000000000..b87afdd7eb4 --- /dev/null +++ b/spec/frontend/contributors/component/__snapshots__/contributors_spec.js.snap @@ -0,0 +1,47 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Contributors charts should render charts when loading completed and there is chart data 1`] = ` +<div> + <div + class="contributors-charts" + > + <h4> + Commits to master + </h4> + + <span> + Excluding merge commits. Limited to 6,000 commits. + </span> + + <div> + <glareachart-stub + data="[object Object]" + height="264" + option="[object Object]" + /> + </div> + + <div + class="row" + > + <div + class="col-6" + > + <h4> + John + </h4> + + <p> + 2 commits (jawnnypoo@gmail.com) + </p> + + <glareachart-stub + data="[object Object]" + height="216" + option="[object Object]" + /> + </div> + </div> + </div> +</div> +`; diff --git a/spec/frontend/contributors/component/contributors_spec.js b/spec/frontend/contributors/component/contributors_spec.js new file mode 100644 index 00000000000..fdba09ed26c --- /dev/null +++ b/spec/frontend/contributors/component/contributors_spec.js @@ -0,0 +1,69 @@ +import Vue from 'vue'; +import { shallowMount, createLocalVue } from '@vue/test-utils'; +import { createStore } from '~/contributors/stores'; +import axios from '~/lib/utils/axios_utils'; +import MockAdapter from 'axios-mock-adapter'; +import ContributorsCharts from '~/contributors/components/contributors.vue'; + +const localVue = createLocalVue(); +let wrapper; +let mock; +let store; +const Component = Vue.extend(ContributorsCharts); +const endpoint = 'contributors'; +const branch = 'master'; +const chartData = [ + { author_name: 'John', author_email: 'jawnnypoo@gmail.com', date: '2019-05-05' }, + { author_name: 'John', author_email: 'jawnnypoo@gmail.com', date: '2019-03-03' }, +]; + +function factory() { + mock = new MockAdapter(axios); + jest.spyOn(axios, 'get'); + mock.onGet().reply(200, chartData); + store = createStore(); + + wrapper = shallowMount(Component, { + propsData: { + endpoint, + branch, + }, + stubs: { + GlLoadingIcon: true, + GlAreaChart: true, + }, + store, + }); +} + +describe('Contributors charts', () => { + beforeEach(() => { + factory(); + }); + + afterEach(() => { + mock.restore(); + wrapper.destroy(); + }); + + it('should fetch chart data when mounted', () => { + expect(axios.get).toHaveBeenCalledWith(endpoint); + }); + + it('should display loader whiled loading data', () => { + wrapper.vm.$store.state.loading = true; + return localVue.nextTick(() => { + expect(wrapper.find('.contributors-loader').exists()).toBe(true); + }); + }); + + it('should render charts when loading completed and there is chart data', () => { + wrapper.vm.$store.state.loading = false; + wrapper.vm.$store.state.chartData = chartData; + return localVue.nextTick(() => { + expect(wrapper.find('.contributors-loader').exists()).toBe(false); + expect(wrapper.find('.contributors-charts').exists()).toBe(true); + expect(wrapper.element).toMatchSnapshot(); + }); + }); +}); diff --git a/spec/frontend/contributors/store/actions_spec.js b/spec/frontend/contributors/store/actions_spec.js new file mode 100644 index 00000000000..bb017e0ac0f --- /dev/null +++ b/spec/frontend/contributors/store/actions_spec.js @@ -0,0 +1,60 @@ +import axios from '~/lib/utils/axios_utils'; +import MockAdapter from 'axios-mock-adapter'; +import testAction from 'helpers/vuex_action_helper'; +import flashError from '~/flash'; +import * as actions from '~/contributors/stores/actions'; +import * as types from '~/contributors/stores/mutation_types'; + +jest.mock('~/flash.js'); + +describe('Contributors store actions', () => { + describe('fetchChartData', () => { + let mock; + const endpoint = '/contributors'; + const chartData = { '2017-11': 0, '2017-12': 2 }; + + beforeEach(() => { + mock = new MockAdapter(axios); + }); + + it('should commit SET_CHART_DATA with received response', done => { + mock.onGet().reply(200, chartData); + + testAction( + actions.fetchChartData, + { endpoint }, + {}, + [ + { type: types.SET_LOADING_STATE, payload: true }, + { type: types.SET_CHART_DATA, payload: chartData }, + { type: types.SET_LOADING_STATE, payload: false }, + ], + [], + () => { + mock.restore(); + done(); + }, + ); + }); + + it('should show flash on API error', done => { + mock.onGet().reply(400, 'Not Found'); + + testAction( + actions.fetchChartData, + { endpoint }, + {}, + [{ type: types.SET_LOADING_STATE, payload: true }], + [], + () => { + expect(flashError).toHaveBeenCalledWith(expect.stringMatching('error')); + mock.restore(); + done(); + }, + ); + }); + }); +}); + +// prevent babel-plugin-rewire from generating an invalid default during karma tests +export default () => {}; diff --git a/spec/frontend/contributors/store/getters_spec.js b/spec/frontend/contributors/store/getters_spec.js new file mode 100644 index 00000000000..62ae9b36f87 --- /dev/null +++ b/spec/frontend/contributors/store/getters_spec.js @@ -0,0 +1,73 @@ +import * as getters from '~/contributors/stores/getters'; + +describe('Contributors Store Getters', () => { + const state = {}; + + describe('showChart', () => { + it('should NOT show chart if loading', () => { + state.loading = true; + + expect(getters.showChart(state)).toEqual(false); + }); + + it('should NOT show chart there is not data', () => { + state.loading = false; + state.chartData = null; + + expect(getters.showChart(state)).toEqual(false); + }); + + it('should show the chart in case loading complated and there is data', () => { + state.loading = false; + state.chartData = true; + + expect(getters.showChart(state)).toEqual(true); + }); + + describe('parsedData', () => { + let parsed; + + beforeAll(() => { + state.chartData = [ + { author_name: 'John', author_email: 'jawnnypoo@gmail.com', date: '2019-05-05' }, + { author_name: 'John', author_email: 'jawnnypoo@gmail.com', date: '2019-05-05' }, + { author_name: 'Carlson', author_email: 'jawnnypoo@gmail.com', date: '2019-03-03' }, + { author_name: 'Carlson', author_email: 'jawnnypoo@gmail.com', date: '2019-05-05' }, + { author_name: 'John', author_email: 'jawnnypoo@gmail.com', date: '2019-04-04' }, + { author_name: 'John', author_email: 'jawnnypoo@gmail.com', date: '2019-04-04' }, + { author_name: 'John', author_email: 'jawnnypoo@gmail.com', date: '2019-03-03' }, + ]; + parsed = getters.parsedData(state); + }); + + it('should group contributions by date ', () => { + expect(parsed.total).toMatchObject({ '2019-05-05': 3, '2019-03-03': 2, '2019-04-04': 2 }); + }); + + it('should group contributions by author ', () => { + expect(parsed.byAuthor).toMatchObject({ + Carlson: { + email: 'jawnnypoo@gmail.com', + commits: 2, + dates: { + '2019-03-03': 1, + '2019-05-05': 1, + }, + }, + John: { + email: 'jawnnypoo@gmail.com', + commits: 5, + dates: { + '2019-03-03': 1, + '2019-04-04': 2, + '2019-05-05': 2, + }, + }, + }); + }); + }); + }); +}); + +// prevent babel-plugin-rewire from generating an invalid default during karma tests +export default () => {}; diff --git a/spec/frontend/contributors/store/mutations_spec.js b/spec/frontend/contributors/store/mutations_spec.js new file mode 100644 index 00000000000..e9e756d4a65 --- /dev/null +++ b/spec/frontend/contributors/store/mutations_spec.js @@ -0,0 +1,40 @@ +import state from '~/contributors/stores/state'; +import mutations from '~/contributors/stores/mutations'; +import * as types from '~/contributors/stores/mutation_types'; + +describe('Contributors mutations', () => { + let stateCopy; + + beforeEach(() => { + stateCopy = state(); + }); + + describe('SET_LOADING_STATE', () => { + it('should set loading flag', () => { + const loading = true; + mutations[types.SET_LOADING_STATE](stateCopy, loading); + + expect(stateCopy.loading).toEqual(loading); + }); + }); + + describe('SET_CHART_DATA', () => { + const chartData = { '2017-11': 0, '2017-12': 2 }; + + it('should set chart data', () => { + mutations[types.SET_CHART_DATA](stateCopy, chartData); + + expect(stateCopy.chartData).toEqual(chartData); + }); + }); + + describe('SET_ACTIVE_BRANCH', () => { + it('should set search query', () => { + const branch = 'feature-branch'; + + mutations[types.SET_ACTIVE_BRANCH](stateCopy, branch); + + expect(stateCopy.branch).toEqual(branch); + }); + }); +}); diff --git a/spec/frontend/contributors/utils_spec.js b/spec/frontend/contributors/utils_spec.js new file mode 100644 index 00000000000..a2b9154329b --- /dev/null +++ b/spec/frontend/contributors/utils_spec.js @@ -0,0 +1,21 @@ +import * as utils from '~/contributors/utils'; + +describe('Contributors Util Functions', () => { + describe('xAxisLabelFormatter', () => { + it('should return year if the date is in January', () => { + expect(utils.xAxisLabelFormatter(new Date('01-12-2019'))).toEqual('2019'); + }); + + it('should return month name otherwise', () => { + expect(utils.xAxisLabelFormatter(new Date('12-02-2019'))).toEqual('Dec'); + expect(utils.xAxisLabelFormatter(new Date('07-12-2019'))).toEqual('Jul'); + }); + }); + + describe('dateFormatter', () => { + it('should format provided date to YYYY-MM-DD format', () => { + expect(utils.dateFormatter(new Date('December 17, 1995 03:24:00'))).toEqual('1995-12-17'); + expect(utils.dateFormatter(new Date(1565308800000))).toEqual('2019-08-09'); + }); + }); +}); diff --git a/spec/frontend/lib/utils/datetime_utility_spec.js b/spec/frontend/lib/utils/datetime_utility_spec.js index e2e71229320..149ce331ae5 100644 --- a/spec/frontend/lib/utils/datetime_utility_spec.js +++ b/spec/frontend/lib/utils/datetime_utility_spec.js @@ -441,3 +441,34 @@ describe('getDateInPast', () => { expect(date).toStrictEqual(new Date(1563235200000)); }); }); + +describe('getDatesInRange', () => { + it('returns an empty array if 1st or 2nd argument is not a Date object', () => { + const d1 = new Date('2019-01-01'); + const d2 = 90; + const range = datetimeUtility.getDatesInRange(d1, d2); + + expect(range).toEqual([]); + }); + + it('returns a range of dates between two given dates', () => { + const d1 = new Date('2019-01-01'); + const d2 = new Date('2019-01-31'); + + const range = datetimeUtility.getDatesInRange(d1, d2); + + expect(range.length).toEqual(31); + }); + + it('applies mapper function if provided fro each item in range', () => { + const d1 = new Date('2019-01-01'); + const d2 = new Date('2019-01-31'); + const formatter = date => date.getDate(); + + const range = datetimeUtility.getDatesInRange(d1, d2, formatter); + + range.forEach((formattedItem, index) => { + expect(formattedItem).toEqual(index + 1); + }); + }); +}); diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb index 2f67ea457a0..1af8b7390bb 100644 --- a/spec/helpers/issuables_helper_spec.rb +++ b/spec/helpers/issuables_helper_spec.rb @@ -203,42 +203,53 @@ describe IssuablesHelper do end describe '#zoomMeetingUrl in issue' do - let(:issue) { create(:issue, author: user, description: description) } + let(:issue) { create(:issue, author: user) } before do assign(:project, issue.project) end - context 'no zoom links in the issue description' do - let(:description) { 'issue text' } - - it 'does not set zoomMeetingUrl' do - expect(helper.issuable_initial_data(issue)) - .not_to include(:zoomMeetingUrl) + shared_examples 'sets zoomMeetingUrl to nil' do + specify do + expect(helper.issuable_initial_data(issue)[:zoomMeetingUrl]) + .to be_nil end end - context 'no zoom links in the issue description if it has link but not a zoom link' do - let(:description) { 'issue text https://stackoverflow.com/questions/22' } + context 'with no "added" zoom mettings' do + it_behaves_like 'sets zoomMeetingUrl to nil' + + context 'with multiple removed meetings' do + before do + create(:zoom_meeting, issue: issue, issue_status: :removed) + create(:zoom_meeting, issue: issue, issue_status: :removed) + end - it 'does not set zoomMeetingUrl' do - expect(helper.issuable_initial_data(issue)) - .not_to include(:zoomMeetingUrl) + it_behaves_like 'sets zoomMeetingUrl to nil' end end - context 'with two zoom links in description' do - let(:description) do - <<~TEXT - issue text and - zoom call on https://zoom.us/j/123456789 this url - and new zoom url https://zoom.us/s/lastone and some more text - TEXT + context 'with "added" zoom meeting' do + before do + create(:zoom_meeting, issue: issue) end - it 'sets zoomMeetingUrl value to the last url' do - expect(helper.issuable_initial_data(issue)) - .to include(zoomMeetingUrl: 'https://zoom.us/s/lastone') + shared_examples 'sets zoomMeetingUrl to canonical meeting url' do + specify do + expect(helper.issuable_initial_data(issue)) + .to include(zoomMeetingUrl: 'https://zoom.us/j/123456789') + end + end + + it_behaves_like 'sets zoomMeetingUrl to canonical meeting url' + + context 'with muliple "removed" zoom meetings' do + before do + create(:zoom_meeting, issue: issue, issue_status: :removed) + create(:zoom_meeting, issue: issue, issue_status: :removed) + end + + it_behaves_like 'sets zoomMeetingUrl to canonical meeting url' end end end diff --git a/spec/helpers/search_helper_spec.rb b/spec/helpers/search_helper_spec.rb index 9e9f87b3407..561cd085ad1 100644 --- a/spec/helpers/search_helper_spec.rb +++ b/spec/helpers/search_helper_spec.rb @@ -271,4 +271,50 @@ describe SearchHelper do expect(link).to have_css('li[data-foo="bar"]') end end + + describe '#show_user_search_tab?' do + subject { show_user_search_tab? } + + context 'when users_search feature is disabled' do + before do + stub_feature_flags(users_search: false) + end + + it { is_expected.to eq(false) } + end + + context 'when project search' do + before do + @project = :some_project + + expect(self).to receive(:project_search_tabs?) + .with(:members) + .and_return(:value) + end + + it 'delegates to project_search_tabs?' do + expect(subject).to eq(:value) + end + end + + context 'when not project search' do + context 'when current_user can read_users_list' do + before do + allow(self).to receive(:current_user).and_return(:the_current_user) + allow(self).to receive(:can?).with(:the_current_user, :read_users_list).and_return(true) + end + + it { is_expected.to eq(true) } + end + + context 'when current_user cannot read_users_list' do + before do + allow(self).to receive(:current_user).and_return(:the_current_user) + allow(self).to receive(:can?).with(:the_current_user, :read_users_list).and_return(false) + end + + it { is_expected.to eq(false) } + end + end + end end diff --git a/spec/javascripts/graphs/stat_graph_contributors_graph_spec.js b/spec/javascripts/graphs/stat_graph_contributors_graph_spec.js deleted file mode 100644 index 563d134ca81..00000000000 --- a/spec/javascripts/graphs/stat_graph_contributors_graph_spec.js +++ /dev/null @@ -1,152 +0,0 @@ -/* eslint-disable jasmine/no-suite-dupes, vars-on-top, no-var */ - -import { scaleLinear, scaleTime } from 'd3-scale'; -import { timeParse } from 'd3-time-format'; -import { - ContributorsGraph, - ContributorsMasterGraph, -} from '~/pages/projects/graphs/show/stat_graph_contributors_graph'; - -const d3 = { scaleLinear, scaleTime, timeParse }; - -describe('ContributorsGraph', function() { - describe('#set_x_domain', function() { - it('set the x_domain', function() { - ContributorsGraph.set_x_domain(20); - - expect(ContributorsGraph.prototype.x_domain).toEqual(20); - }); - }); - - describe('#set_y_domain', function() { - it('sets the y_domain', function() { - ContributorsGraph.set_y_domain([{ commits: 30 }]); - - expect(ContributorsGraph.prototype.y_domain).toEqual([0, 30]); - }); - }); - - describe('#init_x_domain', function() { - it('sets the initial x_domain', function() { - ContributorsGraph.init_x_domain([{ date: '2013-01-31' }, { date: '2012-01-31' }]); - - expect(ContributorsGraph.prototype.x_domain).toEqual(['2012-01-31', '2013-01-31']); - }); - }); - - describe('#init_y_domain', function() { - it('sets the initial y_domain', function() { - ContributorsGraph.init_y_domain([{ commits: 30 }]); - - expect(ContributorsGraph.prototype.y_domain).toEqual([0, 30]); - }); - }); - - describe('#init_domain', function() { - it('calls init_x_domain and init_y_domain', function() { - spyOn(ContributorsGraph, 'init_x_domain'); - spyOn(ContributorsGraph, 'init_y_domain'); - ContributorsGraph.init_domain(); - - expect(ContributorsGraph.init_x_domain).toHaveBeenCalled(); - expect(ContributorsGraph.init_y_domain).toHaveBeenCalled(); - }); - }); - - describe('#set_dates', function() { - it('sets the dates', function() { - ContributorsGraph.set_dates('2013-12-01'); - - expect(ContributorsGraph.prototype.dates).toEqual('2013-12-01'); - }); - }); - - describe('#set_x_domain', function() { - it("sets the instance's x domain using the prototype's x_domain", function() { - ContributorsGraph.prototype.x_domain = 20; - var instance = new ContributorsGraph(); - instance.x = d3 - .scaleTime() - .range([0, 100]) - .clamp(true); - spyOn(instance.x, 'domain'); - instance.set_x_domain(); - - expect(instance.x.domain).toHaveBeenCalledWith(20); - }); - }); - - describe('#set_y_domain', function() { - it("sets the instance's y domain using the prototype's y_domain", function() { - ContributorsGraph.prototype.y_domain = 30; - var instance = new ContributorsGraph(); - instance.y = d3 - .scaleLinear() - .range([100, 0]) - .nice(); - spyOn(instance.y, 'domain'); - instance.set_y_domain(); - - expect(instance.y.domain).toHaveBeenCalledWith(30); - }); - }); - - describe('#set_domain', function() { - it('calls set_x_domain and set_y_domain', function() { - var instance = new ContributorsGraph(); - spyOn(instance, 'set_x_domain'); - spyOn(instance, 'set_y_domain'); - instance.set_domain(); - - expect(instance.set_x_domain).toHaveBeenCalled(); - expect(instance.set_y_domain).toHaveBeenCalled(); - }); - }); - - describe('#set_data', function() { - it('sets the data', function() { - var instance = new ContributorsGraph(); - instance.set_data('20'); - - expect(instance.data).toEqual('20'); - }); - }); -}); - -describe('ContributorsMasterGraph', function() { - // TODO: fix or remove - // describe("#process_dates", function () { - // it("gets and parses dates", function () { - // var graph = new ContributorsMasterGraph(); - // var data = 'random data here'; - // spyOn(graph, 'parse_dates'); - // spyOn(graph, 'get_dates').andReturn("get"); - // spyOn(ContributorsGraph,'set_dates').andCallThrough(); - // graph.process_dates(data); - // expect(graph.parse_dates).toHaveBeenCalledWith(data); - // expect(graph.get_dates).toHaveBeenCalledWith(data); - // expect(ContributorsGraph.set_dates).toHaveBeenCalledWith("get"); - // }); - // }); - - describe('#get_dates', function() { - it('plucks the date field from data collection', function() { - var graph = new ContributorsMasterGraph(); - var data = [{ date: '2013-01-01' }, { date: '2012-12-15' }]; - - expect(graph.get_dates(data)).toEqual(['2013-01-01', '2012-12-15']); - }); - }); - - describe('#parse_dates', function() { - it('parses the dates', function() { - var graph = new ContributorsMasterGraph(); - var parseDate = d3.timeParse('%Y-%m-%d'); - var data = [{ date: '2013-01-01' }, { date: '2012-12-15' }]; - var correct = [{ date: parseDate(data[0].date) }, { date: parseDate(data[1].date) }]; - graph.parse_dates(data); - - expect(data).toEqual(correct); - }); - }); -}); diff --git a/spec/javascripts/graphs/stat_graph_contributors_spec.js b/spec/javascripts/graphs/stat_graph_contributors_spec.js deleted file mode 100644 index 2ebb6845a8b..00000000000 --- a/spec/javascripts/graphs/stat_graph_contributors_spec.js +++ /dev/null @@ -1,28 +0,0 @@ -import ContributorsStatGraph from '~/pages/projects/graphs/show/stat_graph_contributors'; -import { ContributorsGraph } from '~/pages/projects/graphs/show/stat_graph_contributors_graph'; - -import { setLanguage } from '../helpers/locale_helper'; - -describe('ContributorsStatGraph', () => { - describe('change_date_header', () => { - beforeAll(() => { - setLanguage('de'); - }); - - afterAll(() => { - setLanguage(null); - }); - - it('uses the locale to display date ranges', () => { - ContributorsGraph.init_x_domain([{ date: '2013-01-31' }, { date: '2012-01-31' }]); - setFixtures('<div id="date_header"></div>'); - const graph = new ContributorsStatGraph(); - - graph.change_date_header(); - - expect(document.getElementById('date_header').innerText).toBe( - '31. Januar 2012 – 31. Januar 2013', - ); - }); - }); -}); diff --git a/spec/javascripts/graphs/stat_graph_contributors_util_spec.js b/spec/javascripts/graphs/stat_graph_contributors_util_spec.js deleted file mode 100644 index 511b660c671..00000000000 --- a/spec/javascripts/graphs/stat_graph_contributors_util_spec.js +++ /dev/null @@ -1,298 +0,0 @@ -/* eslint-disable no-var, camelcase, vars-on-top */ - -import ContributorsStatGraphUtil from '~/pages/projects/graphs/show/stat_graph_contributors_util'; - -describe('ContributorsStatGraphUtil', function() { - describe('#parse_log', function() { - it('returns a correctly parsed log', function() { - var fake_log = [ - { - author_email: 'karlo@email.com', - author_name: 'Karlo Soriano', - date: '2013-05-09', - additions: 471, - }, - { - author_email: 'dzaporozhets@email.com', - author_name: 'Dmitriy Zaporozhets', - date: '2013-05-08', - additions: 6, - deletions: 1, - }, - { - author_email: 'dzaporozhets@email.com', - author_name: 'Dmitriy Zaporozhets', - date: '2013-05-08', - additions: 19, - deletions: 3, - }, - { - author_email: 'dzaporozhets@email.com', - author_name: 'Dmitriy Zaporozhets', - date: '2013-05-08', - additions: 29, - deletions: 3, - }, - ]; - - var correct_parsed_log = { - total: [ - { date: '2013-05-09', additions: 471, deletions: 0, commits: 1 }, - { date: '2013-05-08', additions: 54, deletions: 7, commits: 3 }, - ], - by_author: [ - { - author_name: 'Karlo Soriano', - author_email: 'karlo@email.com', - '2013-05-09': { date: '2013-05-09', additions: 471, deletions: 0, commits: 1 }, - }, - { - author_name: 'Dmitriy Zaporozhets', - author_email: 'dzaporozhets@email.com', - '2013-05-08': { date: '2013-05-08', additions: 54, deletions: 7, commits: 3 }, - }, - ], - }; - - expect(ContributorsStatGraphUtil.parse_log(fake_log)).toEqual(correct_parsed_log); - }); - }); - - describe('#store_data', function() { - var fake_entry = { author: 'Karlo Soriano', date: '2013-05-09', additions: 471 }; - var fake_total = {}; - var fake_by_author = {}; - - it('calls #store_commits', function() { - spyOn(ContributorsStatGraphUtil, 'store_commits'); - ContributorsStatGraphUtil.store_data(fake_entry, fake_total, fake_by_author); - - expect(ContributorsStatGraphUtil.store_commits).toHaveBeenCalled(); - }); - - it('calls #store_additions', function() { - spyOn(ContributorsStatGraphUtil, 'store_additions'); - ContributorsStatGraphUtil.store_data(fake_entry, fake_total, fake_by_author); - - expect(ContributorsStatGraphUtil.store_additions).toHaveBeenCalled(); - }); - - it('calls #store_deletions', function() { - spyOn(ContributorsStatGraphUtil, 'store_deletions'); - ContributorsStatGraphUtil.store_data(fake_entry, fake_total, fake_by_author); - - expect(ContributorsStatGraphUtil.store_deletions).toHaveBeenCalled(); - }); - }); - - // TODO: fix or remove - // describe("#store_commits", function () { - // var fake_total = "fake_total"; - // var fake_by_author = "fake_by_author"; - // - // it("calls #add twice with arguments fake_total and fake_by_author respectively", function () { - // spyOn(ContributorsStatGraphUtil, 'add'); - // ContributorsStatGraphUtil.store_commits(fake_total, fake_by_author); - // expect(ContributorsStatGraphUtil.add.argsForCall).toEqual([["fake_total", "commits", 1], ["fake_by_author", "commits", 1]]); - // }); - // }); - - describe('#add', function() { - it('adds 1 to current test_field in collection', function() { - var fake_collection = { test_field: 10 }; - ContributorsStatGraphUtil.add(fake_collection, 'test_field', 1); - - expect(fake_collection.test_field).toEqual(11); - }); - - it('inits and adds 1 if test_field in collection is not defined', function() { - var fake_collection = {}; - ContributorsStatGraphUtil.add(fake_collection, 'test_field', 1); - - expect(fake_collection.test_field).toEqual(1); - }); - }); - - // TODO: fix or remove - // describe("#store_additions", function () { - // var fake_entry = {additions: 10}; - // var fake_total= "fake_total"; - // var fake_by_author = "fake_by_author"; - // it("calls #add twice with arguments fake_total and fake_by_author respectively", function () { - // spyOn(ContributorsStatGraphUtil, 'add'); - // ContributorsStatGraphUtil.store_additions(fake_entry, fake_total, fake_by_author); - // expect(ContributorsStatGraphUtil.add.argsForCall).toEqual([["fake_total", "additions", 10], ["fake_by_author", "additions", 10]]); - // }); - // }); - - // TODO: fix or remove - // describe("#store_deletions", function () { - // var fake_entry = {deletions: 10}; - // var fake_total= "fake_total"; - // var fake_by_author = "fake_by_author"; - // it("calls #add twice with arguments fake_total and fake_by_author respectively", function () { - // spyOn(ContributorsStatGraphUtil, 'add'); - // ContributorsStatGraphUtil.store_deletions(fake_entry, fake_total, fake_by_author); - // expect(ContributorsStatGraphUtil.add.argsForCall).toEqual([["fake_total", "deletions", 10], ["fake_by_author", "deletions", 10]]); - // }); - // }); - - describe('#add_date', function() { - it('adds a date field to the collection', function() { - var fake_date = '2013-10-02'; - var fake_collection = {}; - ContributorsStatGraphUtil.add_date(fake_date, fake_collection); - - expect(fake_collection[fake_date].date).toEqual('2013-10-02'); - }); - }); - - describe('#add_author', function() { - it('adds an author field to the collection', function() { - var fake_author = { author_name: 'Author', author_email: 'fake@email.com' }; - var fake_author_collection = {}; - var fake_email_collection = {}; - ContributorsStatGraphUtil.add_author( - fake_author, - fake_author_collection, - fake_email_collection, - ); - - expect(fake_author_collection[fake_author.author_name].author_name).toEqual('Author'); - expect(fake_email_collection[fake_author.author_email].author_name).toEqual('Author'); - }); - }); - - describe('#get_total_data', function() { - it('returns the collection sorted via specified field', function() { - var fake_parsed_log = { - total: [ - { date: '2013-05-09', additions: 471, deletions: 0, commits: 1 }, - { date: '2013-05-08', additions: 54, deletions: 7, commits: 3 }, - ], - by_author: [ - { - author: 'Karlo Soriano', - '2013-05-09': { date: '2013-05-09', additions: 471, deletions: 0, commits: 1 }, - }, - { - author: 'Dmitriy Zaporozhets', - '2013-05-08': { date: '2013-05-08', additions: 54, deletions: 7, commits: 3 }, - }, - ], - }; - var correct_total_data = [ - { date: '2013-05-08', commits: 3 }, - { date: '2013-05-09', commits: 1 }, - ]; - - expect(ContributorsStatGraphUtil.get_total_data(fake_parsed_log, 'commits')).toEqual( - correct_total_data, - ); - }); - }); - - describe('#pick_field', function() { - it('returns the collection with only the specified field and date', function() { - var fake_parsed_log_total = [ - { date: '2013-05-09', additions: 471, deletions: 0, commits: 1 }, - { date: '2013-05-08', additions: 54, deletions: 7, commits: 3 }, - ]; - ContributorsStatGraphUtil.pick_field(fake_parsed_log_total, 'commits'); - var correct_pick_field_data = [ - { date: '2013-05-09', commits: 1 }, - { date: '2013-05-08', commits: 3 }, - ]; - - expect(ContributorsStatGraphUtil.pick_field(fake_parsed_log_total, 'commits')).toEqual( - correct_pick_field_data, - ); - }); - }); - - describe('#get_author_data', function() { - it('returns the log by author sorted by specified field', function() { - var fake_parsed_log = { - total: [ - { date: '2013-05-09', additions: 471, deletions: 0, commits: 1 }, - { date: '2013-05-08', additions: 54, deletions: 7, commits: 3 }, - ], - by_author: [ - { - author_name: 'Karlo Soriano', - author_email: 'karlo@email.com', - '2013-05-09': { date: '2013-05-09', additions: 471, deletions: 0, commits: 1 }, - }, - { - author_name: 'Dmitriy Zaporozhets', - author_email: 'dzaporozhets@email.com', - '2013-05-08': { date: '2013-05-08', additions: 54, deletions: 7, commits: 3 }, - }, - ], - }; - var correct_author_data = [ - { - author_name: 'Dmitriy Zaporozhets', - author_email: 'dzaporozhets@email.com', - dates: { '2013-05-08': 3 }, - deletions: 7, - additions: 54, - commits: 3, - }, - { - author_name: 'Karlo Soriano', - author_email: 'karlo@email.com', - dates: { '2013-05-09': 1 }, - deletions: 0, - additions: 471, - commits: 1, - }, - ]; - - expect(ContributorsStatGraphUtil.get_author_data(fake_parsed_log, 'commits')).toEqual( - correct_author_data, - ); - }); - }); - - describe('#parse_log_entry', function() { - it('adds the corresponding info from the log entry to the author', function() { - var fake_log_entry = { - author_name: 'Karlo Soriano', - author_email: 'karlo@email.com', - '2013-05-09': { date: '2013-05-09', additions: 471, deletions: 0, commits: 1 }, - }; - var correct_parsed_log = { - author_name: 'Karlo Soriano', - author_email: 'karlo@email.com', - dates: { '2013-05-09': 1 }, - deletions: 0, - additions: 471, - commits: 1, - }; - - expect(ContributorsStatGraphUtil.parse_log_entry(fake_log_entry, 'commits', null)).toEqual( - correct_parsed_log, - ); - }); - }); - - describe('#in_range', function() { - var date = '2013-05-09'; - it('returns true if date_range is null', function() { - expect(ContributorsStatGraphUtil.in_range(date, null)).toEqual(true); - }); - - it('returns true if date is in range', function() { - var date_range = [new Date('2013-01-01'), new Date('2013-12-12')]; - - expect(ContributorsStatGraphUtil.in_range(date, date_range)).toEqual(true); - }); - - it('returns false if date is not in range', function() { - var date_range = [new Date('1999-12-01'), new Date('2000-12-01')]; - - expect(ContributorsStatGraphUtil.in_range(date, date_range)).toEqual(false); - }); - }); -}); diff --git a/spec/lib/gitlab/ci/config/entry/default_spec.rb b/spec/lib/gitlab/ci/config/entry/default_spec.rb index 27d63dbd407..f09df698f68 100644 --- a/spec/lib/gitlab/ci/config/entry/default_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/default_spec.rb @@ -5,6 +5,18 @@ require 'spec_helper' describe Gitlab::Ci::Config::Entry::Default do let(:entry) { described_class.new(config) } + it_behaves_like 'with inheritable CI config' do + let(:inheritable_key) { nil } + let(:inheritable_class) { Gitlab::Ci::Config::Entry::Root } + + # These are entries defined in Root + # that we know that we don't want to inherit + # as they do not have sense in context of Default + let(:ignored_inheritable_columns) do + %i[default include variables stages types] + end + end + describe '.nodes' do it 'returns a hash' do expect(described_class.nodes).to be_a(Hash) @@ -87,7 +99,7 @@ describe Gitlab::Ci::Config::Entry::Default do it 'raises error' do expect { entry.compose!(deps) }.to raise_error( - Gitlab::Ci::Config::Entry::Default::DuplicateError) + Gitlab::Ci::Config::Entry::Default::InheritError) end end diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index 1c4887e87c4..d3eb5a9663f 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -5,6 +5,18 @@ require 'spec_helper' describe Gitlab::Ci::Config::Entry::Job do let(:entry) { described_class.new(config, name: :rspec) } + it_behaves_like 'with inheritable CI config' do + let(:inheritable_key) { 'default' } + let(:inheritable_class) { Gitlab::Ci::Config::Entry::Default } + + # These are entries defined in Default + # that we know that we don't want to inherit + # as they do not have sense in context of Job + let(:ignored_inheritable_columns) do + %i[] + end + end + describe '.nodes' do context 'when filtering all the entry/node names' do subject { described_class.nodes.keys } diff --git a/spec/lib/gitlab/ci/config/entry/root_spec.rb b/spec/lib/gitlab/ci/config/entry/root_spec.rb index 7e1a80414d4..8243fcfd162 100644 --- a/spec/lib/gitlab/ci/config/entry/root_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/root_spec.rb @@ -12,6 +12,11 @@ describe Gitlab::Ci::Config::Entry::Root do context 'when filtering all the entry/node names' do it 'contains the expected node names' do + # No inheritable fields should be added to the `Root` + # + # Inheritable configuration can only be added to `default:` + # + # The purpose of `Root` is have only globally defined configuration. expect(described_class.nodes.keys) .to match_array(%i[before_script image services after_script variables cache diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 2e3bc4606b9..3b665d95004 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -29,6 +29,7 @@ issues: - prometheus_alerts - prometheus_alert_events - self_managed_prometheus_alert_events +- zoom_meetings events: - author - project @@ -529,4 +530,6 @@ versions: &version - issue - designs - actions +zoom_meetings: +- issue design_versions: *version diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index ebd2c6089ce..88a7fbea1d1 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -211,6 +211,13 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do expect(CustomIssueTrackerService.first).not_to be_nil end + it 'restores zoom meetings' do + meetings = @project.issues.first.zoom_meetings + + expect(meetings.count).to eq(1) + expect(meetings.first.url).to eq('https://zoom.us/j/123456789') + end + context 'Merge requests' do it 'always has the new project as a target' do expect(MergeRequest.find_by_title('MR1').target_project).to eq(@project) diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index ede8eb4b2bd..3835b23123f 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -753,3 +753,11 @@ DesignManagement::Version: - sha - issue_id - author_id +ZoomMeeting: +- id +- issue_id +- project_id +- issue_status +- url +- created_at +- updated_at diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 1eec3b97ea1..21e308e6636 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -187,7 +187,6 @@ describe Issues::UpdateService, :mailer do it 'creates system note about issue reassign' do note = find_note('assigned to') - expect(note).not_to be_nil expect(note.note).to include "assigned to #{user2.to_reference}" end @@ -202,14 +201,12 @@ describe Issues::UpdateService, :mailer do it 'creates system note about title change' do note = find_note('changed title') - expect(note).not_to be_nil expect(note.note).to eq 'changed title from **{-Old-} title** to **{+New+} title**' end it 'creates system note about discussion lock' do note = find_note('locked this issue') - expect(note).not_to be_nil expect(note.note).to eq 'locked this issue' end end @@ -221,20 +218,10 @@ describe Issues::UpdateService, :mailer do note = find_note('changed the description') - expect(note).not_to be_nil expect(note.note).to eq('changed the description') end end - it 'creates zoom_link_added system note when a zoom link is added to the description' do - update_issue(description: 'Changed description https://zoom.us/j/5873603787') - - note = find_note('added a Zoom call') - - expect(note).not_to be_nil - expect(note.note).to eq('added a Zoom call to this issue') - end - context 'when issue turns confidential' do let(:opts) do { @@ -252,7 +239,6 @@ describe Issues::UpdateService, :mailer do note = find_note('made the issue confidential') - expect(note).not_to be_nil expect(note.note).to eq 'made the issue confidential' end diff --git a/spec/services/issues/zoom_link_service_spec.rb b/spec/services/issues/zoom_link_service_spec.rb index ba3f007c917..ecca9467965 100644 --- a/spec/services/issues/zoom_link_service_spec.rb +++ b/spec/services/issues/zoom_link_service_spec.rb @@ -14,27 +14,16 @@ describe Issues::ZoomLinkService do project.add_reporter(user) end - shared_context 'with Zoom link' do + shared_context '"added" Zoom meeting' do before do - issue.update!(description: "Description\n\n#{zoom_link}") + create(:zoom_meeting, issue: issue) end end - shared_context 'with Zoom link not at the end' do + shared_context '"removed" zoom meetings' do before do - issue.update!(description: "Description with #{zoom_link} some where") - end - end - - shared_context 'without Zoom link' do - before do - issue.update!(description: "Description\n\nhttp://example.com") - end - end - - shared_context 'without issue description' do - before do - issue.update!(description: nil) + create(:zoom_meeting, issue: issue, issue_status: :removed) + create(:zoom_meeting, issue: issue, issue_status: :removed) end end @@ -45,11 +34,10 @@ describe Issues::ZoomLinkService do end describe '#add_link' do - shared_examples 'can add link' do - it 'appends the link to issue description' do + shared_examples 'can add meeting' do + it 'appends the new meeting to zoom_meetings' do expect(result).to be_success - expect(result.payload[:description]) - .to eq("#{issue.description}\n\n#{zoom_link}") + expect(ZoomMeeting.canonical_meeting_url(issue)).to eq(zoom_link) end it 'tracks the add event' do @@ -57,55 +45,63 @@ describe Issues::ZoomLinkService do .with('IncidentManagement::ZoomIntegration', 'add_zoom_meeting', label: 'Issue ID', value: issue.id) result end + + it 'creates a zoom_link_added notification' do + expect(SystemNoteService).to receive(:zoom_link_added).with(issue, project, user) + expect(SystemNoteService).not_to receive(:zoom_link_removed) + result + end end - shared_examples 'cannot add link' do - it 'cannot add the link' do + shared_examples 'cannot add meeting' do + it 'cannot add the meeting' do expect(result).to be_error expect(result.message).to eq('Failed to add a Zoom meeting') end + + it 'creates no notification' do + expect(SystemNoteService).not_to receive(:zoom_link_added) + expect(SystemNoteService).not_to receive(:zoom_link_removed) + result + end end subject(:result) { service.add_link(zoom_link) } - context 'without Zoom link in the issue description' do - include_context 'without Zoom link' - include_examples 'can add link' + context 'without existing Zoom meeting' do + include_examples 'can add meeting' - context 'with invalid Zoom link' do + context 'with invalid Zoom url' do let(:zoom_link) { 'https://not-zoom.link' } - include_examples 'cannot add link' + include_examples 'cannot add meeting' end context 'with insufficient permissions' do include_context 'insufficient permissions' - include_examples 'cannot add link' + include_examples 'cannot add meeting' end end - context 'with Zoom link in the issue description' do - include_context 'with Zoom link' - include_examples 'cannot add link' + context 'with "added" Zoom meeting' do + include_context '"added" Zoom meeting' + include_examples 'cannot add meeting' + end - context 'but not at the end' do - include_context 'with Zoom link not at the end' - include_examples 'can add link' + context 'with "added" Zoom meeting and race condition' do + include_context '"added" Zoom meeting' + before do + allow(service).to receive(:can_add_link?).and_return(true) end - end - context 'without issue description' do - include_context 'without issue description' - include_examples 'can add link' + include_examples 'cannot add meeting' end end describe '#can_add_link?' do subject { service.can_add_link? } - context 'without Zoom link in the issue description' do - include_context 'without Zoom link' - + context 'without "added" zoom meeting' do it { is_expected.to eq(true) } context 'with insufficient permissions' do @@ -115,81 +111,93 @@ describe Issues::ZoomLinkService do end end - context 'with Zoom link in the issue description' do - include_context 'with Zoom link' + context 'with Zoom meeting in the issue description' do + include_context '"added" Zoom meeting' it { is_expected.to eq(false) } end end describe '#remove_link' do - shared_examples 'cannot remove link' do - it 'cannot remove the link' do + shared_examples 'cannot remove meeting' do + it 'cannot remove the meeting' do expect(result).to be_error expect(result.message).to eq('Failed to remove a Zoom meeting') end - end - subject(:result) { service.remove_link } + it 'creates no notification' do + expect(SystemNoteService).not_to receive(:zoom_link_added) + expect(SystemNoteService).not_to receive(:zoom_link_removed) + result + end + end - context 'with Zoom link in the issue description' do - include_context 'with Zoom link' + shared_examples 'can remove meeting' do + it 'creates no notification' do + expect(SystemNoteService).not_to receive(:zoom_link_added).with(issue, project, user) + expect(SystemNoteService).to receive(:zoom_link_removed) + result + end - it 'removes the link from the issue description' do + it 'can remove the meeting' do expect(result).to be_success - expect(result.payload[:description]) - .to eq(issue.description.delete_suffix("\n\n#{zoom_link}")) + expect(ZoomMeeting.canonical_meeting_url(issue)).to eq(nil) end it 'tracks the remove event' do expect(Gitlab::Tracking).to receive(:event) - .with('IncidentManagement::ZoomIntegration', 'remove_zoom_meeting', label: 'Issue ID', value: issue.id) - + .with('IncidentManagement::ZoomIntegration', 'remove_zoom_meeting', label: 'Issue ID', value: issue.id) result end + end - context 'with insufficient permissions' do - include_context 'insufficient permissions' - include_examples 'cannot remove link' - end + subject(:result) { service.remove_link } - context 'but not at the end' do - include_context 'with Zoom link not at the end' - include_examples 'cannot remove link' + context 'with Zoom meeting' do + include_context '"added" Zoom meeting' + + context 'removes the link' do + include_examples 'can remove meeting' end - end - context 'without Zoom link in the issue description' do - include_context 'without Zoom link' - include_examples 'cannot remove link' + context 'with insufficient permissions' do + include_context 'insufficient permissions' + include_examples 'cannot remove meeting' + end end - context 'without issue description' do - include_context 'without issue description' - include_examples 'cannot remove link' + context 'without "added" Zoom meeting' do + include_context '"removed" zoom meetings' + include_examples 'cannot remove meeting' end end describe '#can_remove_link?' do subject { service.can_remove_link? } - context 'with Zoom link in the issue description' do - include_context 'with Zoom link' + context 'without Zoom meeting' do + it { is_expected.to eq(false) } + end + + context 'with only "removed" zoom meetings' do + include_context '"removed" zoom meetings' + it { is_expected.to eq(false) } + end + context 'with "added" Zoom meeting' do + include_context '"added" Zoom meeting' it { is_expected.to eq(true) } + context 'with "removed" zoom meetings' do + include_context '"removed" zoom meetings' + it { is_expected.to eq(true) } + end + context 'with insufficient permissions' do include_context 'insufficient permissions' - it { is_expected.to eq(false) } end end - - context 'without Zoom link in the issue description' do - include_context 'without Zoom link' - - it { is_expected.to eq(false) } - end end describe '#parse_link' do diff --git a/spec/services/zoom_notes_service_spec.rb b/spec/services/zoom_notes_service_spec.rb deleted file mode 100644 index 419ecf3f374..00000000000 --- a/spec/services/zoom_notes_service_spec.rb +++ /dev/null @@ -1,81 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe ZoomNotesService do - describe '#execute' do - let(:issue) { OpenStruct.new(description: description) } - let(:project) { Object.new } - let(:user) { Object.new } - let(:description) { 'an issue description' } - let(:old_description) { nil } - - subject { described_class.new(issue, project, user, old_description: old_description) } - - shared_examples 'no notifications' do - it "doesn't create notifications" do - expect(SystemNoteService).not_to receive(:zoom_link_added) - expect(SystemNoteService).not_to receive(:zoom_link_removed) - - subject.execute - end - end - - it_behaves_like 'no notifications' - - context 'when the zoom link exists in both description and old_description' do - let(:description) { 'a changed issue description https://zoom.us/j/123' } - let(:old_description) { 'an issue description https://zoom.us/j/123' } - - it_behaves_like 'no notifications' - end - - context "when the zoom link doesn't exist in both description and old_description" do - let(:description) { 'a changed issue description' } - let(:old_description) { 'an issue description' } - - it_behaves_like 'no notifications' - end - - context 'when description == old_description' do - let(:old_description) { 'an issue description' } - - it_behaves_like 'no notifications' - end - - context 'when the description contains a zoom link and old_description is nil' do - let(:description) { 'a changed issue description https://zoom.us/j/123' } - - it 'creates a zoom_link_added notification' do - expect(SystemNoteService).to receive(:zoom_link_added).with(issue, project, user) - expect(SystemNoteService).not_to receive(:zoom_link_removed) - - subject.execute - end - end - - context 'when the zoom link has been added to the description' do - let(:description) { 'a changed issue description https://zoom.us/j/123' } - let(:old_description) { 'an issue description' } - - it 'creates a zoom_link_added notification' do - expect(SystemNoteService).to receive(:zoom_link_added).with(issue, project, user) - expect(SystemNoteService).not_to receive(:zoom_link_removed) - - subject.execute - end - end - - context 'when the zoom link has been removed from the description' do - let(:description) { 'a changed issue description' } - let(:old_description) { 'an issue description https://zoom.us/j/123' } - - it 'creates a zoom_link_removed notification' do - expect(SystemNoteService).not_to receive(:zoom_link_added).with(issue, project, user) - expect(SystemNoteService).to receive(:zoom_link_removed) - - subject.execute - end - end - end -end diff --git a/spec/support/shared_examples/lib/gitlab/config/inheritable_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/config/inheritable_shared_examples.rb new file mode 100644 index 00000000000..556d81133bc --- /dev/null +++ b/spec/support/shared_examples/lib/gitlab/config/inheritable_shared_examples.rb @@ -0,0 +1,105 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'with inheritable CI config' do + using RSpec::Parameterized::TableSyntax + + let(:ignored_inheritable_columns) { [] } + + it 'does prepend an Inheritable mixin' do + expect(described_class).to include_module(Gitlab::Config::Entry::Inheritable) + end + + it 'all inheritable entries are covered' do + inheritable_entries = inheritable_class.nodes.keys + entries = described_class.nodes.keys + + expect(entries + ignored_inheritable_columns).to include( + *inheritable_entries) + end + + it 'all entries do have inherit flag' do + without_inherit_flag = described_class.nodes.map do |key, factory| + key if factory.inherit.nil? + end.compact + + expect(without_inherit_flag).to be_empty + end + + context 'for non-inheritable entries' do + where(:entry_key) do + described_class.nodes.map do |key, factory| + [key] unless factory.inherit + end.compact + end + + with_them do + it 'inheritable_class does not define entry' do + expect(inheritable_class.nodes).not_to include(entry_key) + end + end + end + + context 'for inheritable entries' do + where(:entry_key, :entry_class) do + described_class.nodes.map do |key, factory| + [key, factory.entry_class] if factory.inherit + end.compact + end + + with_them do + let(:specified) { double('deps_specified', 'specified?' => true, value: 'specified') } + let(:unspecified) { double('unspecified', 'specified?' => false) } + let(:inheritable) { double(inheritable_key, '[]' => unspecified) } + + let(:deps) do + if inheritable_key + double('deps', inheritable_key => inheritable, '[]' => unspecified) + else + inheritable + end + end + + it 'inheritable_class does define entry' do + expect(inheritable_class.nodes).to include(entry_key) + expect(inheritable_class.nodes[entry_key].entry_class).to eq(entry_class) + end + + context 'when is specified' do + it 'does inherit value' do + expect(inheritable).to receive('[]').with(entry_key).and_return(specified) + + entry.compose!(deps) + + expect(entry[entry_key]).to eq(specified) + end + + context 'when entry is specified' do + let(:entry_specified) do + double('entry_specified', 'specified?' => true, value: 'specified', errors: []) + end + + it 'does not inherit value' do + entry.send(:entries)[entry_key] = entry_specified + + allow(inheritable).to receive('[]').with(entry_key).and_return(specified) + + expect do + # we ignore exceptions as `#overwrite_entry` + # can raise exception on duplicates + entry.compose!(deps) rescue described_class::InheritError + end.not_to change { entry[entry_key] } + end + end + end + + context 'when inheritable does not specify' do + it 'does not inherit value' do + entry.compose!(deps) + + expect(entry[entry_key]).to be_a( + Gitlab::Config::Entry::Undefined) + end + end + end + end +end diff --git a/spec/support/shared_examples/quick_actions/issue/zoom_quick_actions_shared_examples.rb b/spec/support/shared_examples/quick_actions/issue/zoom_quick_actions_shared_examples.rb index b4a8e3fca4d..92bbc4abe77 100644 --- a/spec/support/shared_examples/quick_actions/issue/zoom_quick_actions_shared_examples.rb +++ b/spec/support/shared_examples/quick_actions/issue/zoom_quick_actions_shared_examples.rb @@ -2,22 +2,19 @@ shared_examples 'zoom quick actions' do let(:zoom_link) { 'https://zoom.us/j/123456789' } + let(:existing_zoom_link) { 'https://zoom.us/j/123456780' } let(:invalid_zoom_link) { 'https://invalid-zoom' } - before do - issue.update!(description: description) - end - describe '/zoom' do shared_examples 'skip silently' do - it 'skip addition silently' do + it 'skips addition silently' do add_note("/zoom #{zoom_link}") wait_for_requests expect(page).not_to have_content('Zoom meeting added') expect(page).not_to have_content('Failed to add a Zoom meeting') - expect(issue.reload.description).to eq(description) + expect(ZoomMeeting.canonical_meeting_url(issue.reload)).not_to eq(zoom_link) end end @@ -28,13 +25,11 @@ shared_examples 'zoom quick actions' do wait_for_requests expect(page).to have_content('Zoom meeting added') - expect(issue.reload.description).to end_with(zoom_link) + expect(ZoomMeeting.canonical_meeting_url(issue.reload)).to eq(zoom_link) end end - context 'without issue description' do - let(:description) { nil } - + context 'without zoom_meetings' do include_examples 'success' it 'cannot add invalid zoom link' do @@ -47,14 +42,18 @@ shared_examples 'zoom quick actions' do end end - context 'with Zoom link not at the end of the issue description' do - let(:description) { "A link #{zoom_link} not at the end" } + context 'with "removed" zoom meeting' do + before do + create(:zoom_meeting, issue_status: :removed, url: existing_zoom_link, issue: issue) + end include_examples 'success' end - context 'with Zoom link at end of the issue description' do - let(:description) { "Text\n#{zoom_link}" } + context 'with "added" zoom meeting' do + before do + create(:zoom_meeting, issue_status: :added, url: existing_zoom_link, issue: issue) + end include_examples 'skip silently' end @@ -62,19 +61,19 @@ shared_examples 'zoom quick actions' do describe '/remove_zoom' do shared_examples 'skip silently' do - it 'skip removal silently' do + it 'skips removal silently' do add_note('/remove_zoom') wait_for_requests expect(page).not_to have_content('Zoom meeting removed') expect(page).not_to have_content('Failed to remove a Zoom meeting') - expect(issue.reload.description).to eq(description) + expect(ZoomMeeting.canonical_meeting_url(issue.reload)).to be_nil end end - context 'with Zoom link in the description' do - let(:description) { "Text with #{zoom_link}\n\n\n#{zoom_link}" } + context 'with added zoom meeting' do + let!(:added_zoom_meeting) { create(:zoom_meeting, url: zoom_link, issue: issue, issue_status: :added) } it 'removes last Zoom link' do add_note('/remove_zoom') @@ -82,14 +81,8 @@ shared_examples 'zoom quick actions' do wait_for_requests expect(page).to have_content('Zoom meeting removed') - expect(issue.reload.description).to eq("Text with #{zoom_link}") + expect(ZoomMeeting.canonical_meeting_url(issue.reload)).to be_nil end end - - context 'with a Zoom link not at the end of the description' do - let(:description) { "A link #{zoom_link} not at the end" } - - include_examples 'skip silently' - end end end |