diff options
177 files changed, 4539 insertions, 1225 deletions
diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index 32fbeaaa905..69ec6ab8600 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -130,7 +130,7 @@ export default { return `\`${this.diffFile.file_path}\``; }, isFileRenamed() { - return this.diffFile.viewer.name === diffViewerModes.renamed; + return this.diffFile.renamed_file; }, isModeChanged() { return this.diffFile.viewer.name === diffViewerModes.mode_changed; diff --git a/app/assets/javascripts/event_tracking/issue_sidebar.js b/app/assets/javascripts/event_tracking/issue_sidebar.js new file mode 100644 index 00000000000..6909f82c66f --- /dev/null +++ b/app/assets/javascripts/event_tracking/issue_sidebar.js @@ -0,0 +1,2 @@ +export const initSidebarTracking = () => {}; +export const trackEvent = () => {}; diff --git a/app/assets/javascripts/issue_show/index.js b/app/assets/javascripts/issue_show/index.js index 529b6386221..5a9dd91817e 100644 --- a/app/assets/javascripts/issue_show/index.js +++ b/app/assets/javascripts/issue_show/index.js @@ -1,4 +1,5 @@ import Vue from 'vue'; +import { initSidebarTracking } from 'ee_else_ce/event_tracking/issue_sidebar'; import issuableApp from './components/app.vue'; import { parseIssuableData } from './utils/parse_data'; import '../vue_shared/vue_resource_interceptor'; @@ -9,6 +10,9 @@ export default function initIssueableApp() { components: { issuableApp, }, + mounted() { + initSidebarTracking(); + }, render(createElement) { return createElement('issuable-app', { props: parseIssuableData(), diff --git a/app/assets/javascripts/monitoring/components/charts/area.vue b/app/assets/javascripts/monitoring/components/charts/area.vue index 90c764587a3..78d97a3c122 100644 --- a/app/assets/javascripts/monitoring/components/charts/area.vue +++ b/app/assets/javascripts/monitoring/components/charts/area.vue @@ -12,6 +12,9 @@ import { graphDataValidatorForValues } from '../../utils'; let debouncedResize; +// TODO: Remove this component in favor of the more general time_series.vue +// Please port all changes here to time_series.vue as well. + export default { components: { GlAreaChart, diff --git a/app/assets/javascripts/monitoring/components/charts/time_series.vue b/app/assets/javascripts/monitoring/components/charts/time_series.vue new file mode 100644 index 00000000000..2fdc75f63ca --- /dev/null +++ b/app/assets/javascripts/monitoring/components/charts/time_series.vue @@ -0,0 +1,334 @@ +<script> +import { __ } from '~/locale'; +import { mapState } from 'vuex'; +import { GlLink, GlButton } from '@gitlab/ui'; +import { GlAreaChart, GlLineChart, GlChartSeriesLabel } from '@gitlab/ui/dist/charts'; +import dateFormat from 'dateformat'; +import { debounceByAnimationFrame, roundOffFloat } from '~/lib/utils/common_utils'; +import { getSvgIconPathContent } from '~/lib/utils/icon_utils'; +import Icon from '~/vue_shared/components/icon.vue'; +import { chartHeight, graphTypes, lineTypes, symbolSizes, dateFormats } from '../../constants'; +import { makeDataSeries } from '~/helpers/monitor_helper'; +import { graphDataValidatorForValues } from '../../utils'; + +let debouncedResize; + +export default { + components: { + GlAreaChart, + GlLineChart, + GlButton, + GlChartSeriesLabel, + GlLink, + Icon, + }, + inheritAttrs: false, + props: { + graphData: { + type: Object, + required: true, + validator: graphDataValidatorForValues.bind(null, false), + }, + containerWidth: { + type: Number, + required: true, + }, + deploymentData: { + type: Array, + required: false, + default: () => [], + }, + projectPath: { + type: String, + required: false, + default: '', + }, + showBorder: { + type: Boolean, + required: false, + default: false, + }, + thresholds: { + type: Array, + required: false, + default: () => [], + }, + }, + data() { + return { + tooltip: { + title: '', + content: [], + commitUrl: '', + isDeployment: false, + sha: '', + }, + width: 0, + height: chartHeight, + svgs: {}, + primaryColor: null, + }; + }, + computed: { + ...mapState('monitoringDashboard', ['exportMetricsToCsvEnabled']), + chartData() { + // Transforms & supplements query data to render appropriate labels & styles + // Input: [{ queryAttributes1 }, { queryAttributes2 }] + // Output: [{ seriesAttributes1 }, { seriesAttributes2 }] + return this.graphData.queries.reduce((acc, query) => { + const { appearance } = query; + const lineType = + appearance && appearance.line && appearance.line.type + ? appearance.line.type + : lineTypes.default; + const lineWidth = + appearance && appearance.line && appearance.line.width + ? appearance.line.width + : undefined; + const areaStyle = { + opacity: + appearance && appearance.area && typeof appearance.area.opacity === 'number' + ? appearance.area.opacity + : undefined, + }; + + const series = makeDataSeries(query.result, { + name: this.formatLegendLabel(query), + lineStyle: { + type: lineType, + width: lineWidth, + }, + showSymbol: false, + areaStyle: this.graphData.type === 'area-chart' ? areaStyle : undefined, + }); + + return acc.concat(series); + }, []); + }, + chartOptions() { + return { + xAxis: { + name: __('Time'), + type: 'time', + axisLabel: { + formatter: date => dateFormat(date, dateFormats.timeOfDay), + }, + axisPointer: { + snap: true, + }, + }, + yAxis: { + name: this.yAxisLabel, + axisLabel: { + formatter: num => roundOffFloat(num, 3).toString(), + }, + }, + series: this.scatterSeries, + dataZoom: this.dataZoomConfig, + }; + }, + dataZoomConfig() { + const handleIcon = this.svgs['scroll-handle']; + + return handleIcon ? { handleIcon } : {}; + }, + earliestDatapoint() { + return this.chartData.reduce((acc, series) => { + const { data } = series; + const { length } = data; + if (!length) { + return acc; + } + + const [first] = data[0]; + const [last] = data[length - 1]; + const seriesEarliest = first < last ? first : last; + + return seriesEarliest < acc || acc === null ? seriesEarliest : acc; + }, null); + }, + glChartComponent() { + const chartTypes = { + 'area-chart': GlAreaChart, + 'line-chart': GlLineChart, + }; + return chartTypes[this.graphData.type] || GlAreaChart; + }, + isMultiSeries() { + return this.tooltip.content.length > 1; + }, + recentDeployments() { + return this.deploymentData.reduce((acc, deployment) => { + if (deployment.created_at >= this.earliestDatapoint) { + const { id, created_at, sha, ref, tag } = deployment; + acc.push({ + id, + createdAt: created_at, + sha, + commitUrl: `${this.projectPath}/commit/${sha}`, + tag, + tagUrl: tag ? `${this.tagsPath}/${ref.name}` : null, + ref: ref.name, + showDeploymentFlag: false, + }); + } + + return acc; + }, []); + }, + scatterSeries() { + return { + type: graphTypes.deploymentData, + data: this.recentDeployments.map(deployment => [deployment.createdAt, 0]), + symbol: this.svgs.rocket, + symbolSize: symbolSizes.default, + itemStyle: { + color: this.primaryColor, + }, + }; + }, + yAxisLabel() { + return `${this.graphData.y_label}`; + }, + csvText() { + const chartData = this.chartData[0].data; + const header = `timestamp,${this.graphData.y_label}\r\n`; // eslint-disable-line @gitlab/i18n/no-non-i18n-strings + return chartData.reduce((csv, data) => { + const row = data.join(','); + return `${csv}${row}\r\n`; + }, header); + }, + downloadLink() { + const data = new Blob([this.csvText], { type: 'text/plain' }); + return window.URL.createObjectURL(data); + }, + }, + watch: { + containerWidth: 'onResize', + }, + beforeDestroy() { + window.removeEventListener('resize', debouncedResize); + }, + created() { + debouncedResize = debounceByAnimationFrame(this.onResize); + window.addEventListener('resize', debouncedResize); + this.setSvg('rocket'); + this.setSvg('scroll-handle'); + }, + methods: { + formatLegendLabel(query) { + return `${query.label}`; + }, + formatTooltipText(params) { + this.tooltip.title = dateFormat(params.value, dateFormats.default); + this.tooltip.content = []; + params.seriesData.forEach(dataPoint => { + const [xVal, yVal] = dataPoint.value; + this.tooltip.isDeployment = dataPoint.componentSubType === graphTypes.deploymentData; + if (this.tooltip.isDeployment) { + const [deploy] = this.recentDeployments.filter( + deployment => deployment.createdAt === xVal, + ); + this.tooltip.sha = deploy.sha.substring(0, 8); + this.tooltip.commitUrl = deploy.commitUrl; + } else { + const { seriesName, color } = dataPoint; + const value = yVal.toFixed(3); + this.tooltip.content.push({ + name: seriesName, + value, + color, + }); + } + }); + }, + setSvg(name) { + getSvgIconPathContent(name) + .then(path => { + if (path) { + this.$set(this.svgs, name, `path://${path}`); + } + }) + .catch(e => { + // eslint-disable-next-line no-console, @gitlab/i18n/no-non-i18n-strings + console.error('SVG could not be rendered correctly: ', e); + }); + }, + onChartUpdated(chart) { + [this.primaryColor] = chart.getOption().color; + }, + onResize() { + if (!this.$refs.chart) return; + const { width } = this.$refs.chart.$el.getBoundingClientRect(); + this.width = width; + }, + }, +}; +</script> + +<template> + <div class="prometheus-graph col-12 col-lg-6" :class="[showBorder ? 'p-2' : 'p-0']"> + <div :class="{ 'prometheus-graph-embed w-100 p-3': showBorder }"> + <div class="prometheus-graph-header"> + <h5 class="prometheus-graph-title js-graph-title">{{ graphData.title }}</h5> + <gl-button + v-if="exportMetricsToCsvEnabled" + :href="downloadLink" + :title="__('Download CSV')" + :aria-label="__('Download CSV')" + style="margin-left: 200px;" + download="chart_metrics.csv" + > + {{ __('Download CSV') }} + </gl-button> + <div class="prometheus-graph-widgets js-graph-widgets"> + <slot></slot> + </div> + </div> + + <component + :is="glChartComponent" + ref="chart" + v-bind="$attrs" + :data="chartData" + :option="chartOptions" + :format-tooltip-text="formatTooltipText" + :thresholds="thresholds" + :width="width" + :height="height" + @updated="onChartUpdated" + > + <template v-if="tooltip.isDeployment"> + <template slot="tooltipTitle"> + {{ __('Deployed') }} + </template> + <div slot="tooltipContent" class="d-flex align-items-center"> + <icon name="commit" class="mr-2" /> + <gl-link :href="tooltip.commitUrl">{{ tooltip.sha }}</gl-link> + </div> + </template> + <template v-else> + <template slot="tooltipTitle"> + <div class="text-nowrap"> + {{ tooltip.title }} + </div> + </template> + <template slot="tooltipContent"> + <div + v-for="(content, key) in tooltip.content" + :key="key" + class="d-flex justify-content-between" + > + <gl-chart-series-label :color="isMultiSeries ? content.color : ''"> + {{ content.name }} + </gl-chart-series-label> + <div class="prepend-left-32"> + {{ content.value }} + </div> + </div> + </template> + </template> + </component> + </div> + </div> +</template> diff --git a/app/assets/javascripts/monitoring/components/dashboard.vue b/app/assets/javascripts/monitoring/components/dashboard.vue index c414c26ca61..d330ceb836c 100644 --- a/app/assets/javascripts/monitoring/components/dashboard.vue +++ b/app/assets/javascripts/monitoring/components/dashboard.vue @@ -14,9 +14,9 @@ import { __, s__ } from '~/locale'; import Icon from '~/vue_shared/components/icon.vue'; import { getParameterValues, mergeUrlParams } from '~/lib/utils/url_utility'; import invalidUrl from '~/lib/utils/invalid_url'; -import MonitorAreaChart from './charts/area.vue'; +import PanelType from 'ee_else_ce/monitoring/components/panel_type.vue'; +import MonitorTimeSeriesChart from './charts/time_series.vue'; import MonitorSingleStatChart from './charts/single_stat.vue'; -import PanelType from './panel_type.vue'; import GraphGroup from './graph_group.vue'; import EmptyState from './empty_state.vue'; import { sidebarAnimationDuration, timeWindows } from '../constants'; @@ -26,7 +26,7 @@ let sidebarMutationObserver; export default { components: { - MonitorAreaChart, + MonitorTimeSeriesChart, MonitorSingleStatChart, PanelType, GraphGroup, @@ -141,6 +141,16 @@ export default { required: false, default: false, }, + alertsEndpoint: { + type: String, + required: false, + default: null, + }, + prometheusAlertsAvailable: { + type: Boolean, + required: false, + default: false, + }, }, data() { return { @@ -449,11 +459,13 @@ export default { :clipboard-text="generateLink(groupData.group, graphData.title, graphData.y_label)" :graph-data="graphData" :dashboard-width="elWidth" + :alerts-endpoint="alertsEndpoint" + :prometheus-alerts-available="prometheusAlertsAvailable" :index="`${index}-${graphIndex}`" /> </template> <template v-else> - <monitor-area-chart + <monitor-time-series-chart v-for="(graphData, graphIndex) in chartsWithData(groupData.metrics)" :key="graphIndex" :graph-data="graphData" @@ -461,7 +473,7 @@ export default { :thresholds="getGraphAlertValues(graphData.queries)" :container-width="elWidth" :project-path="projectPath" - group-id="monitor-area-chart" + group-id="monitor-time-series-chart" > <div class="d-flex align-items-center"> <alert-widget @@ -503,7 +515,7 @@ export default { </gl-dropdown-item> </gl-dropdown> </div> - </monitor-area-chart> + </monitor-time-series-chart> </template> </graph-group> </div> diff --git a/app/assets/javascripts/monitoring/constants.js b/app/assets/javascripts/monitoring/constants.js index d7d89522732..13aba3d9f44 100644 --- a/app/assets/javascripts/monitoring/constants.js +++ b/app/assets/javascripts/monitoring/constants.js @@ -8,6 +8,10 @@ export const graphTypes = { deploymentData: 'scatter', }; +export const symbolSizes = { + default: 14, +}; + export const lineTypes = { default: 'solid', }; @@ -21,6 +25,11 @@ export const timeWindows = { oneWeek: __('1 week'), }; +export const dateFormats = { + timeOfDay: 'h:MM TT', + default: 'dd mmm yyyy, h:MMTT', +}; + export const secondsIn = { thirtyMinutes: 60 * 30, threeHours: 60 * 60 * 3, diff --git a/app/assets/javascripts/notes/stores/getters.js b/app/assets/javascripts/notes/stores/getters.js index 52410f18d4a..3d0ec8cd3a7 100644 --- a/app/assets/javascripts/notes/stores/getters.js +++ b/app/assets/javascripts/notes/stores/getters.js @@ -171,26 +171,33 @@ export const isLastUnresolvedDiscussion = (state, getters) => (discussionId, dif return lastDiscussionId === discussionId; }; -// Gets the ID of the discussion following the one provided, respecting order (diff or date) -// @param {Boolean} discussionId - id of the current discussion -// @param {Boolean} diffOrder - is ordered by diff? -export const nextUnresolvedDiscussionId = (state, getters) => (discussionId, diffOrder) => { - const idsOrdered = getters.unresolvedDiscussionsIdsOrdered(diffOrder); - const currentIndex = idsOrdered.indexOf(discussionId); - const slicedIds = idsOrdered.slice(currentIndex + 1, currentIndex + 2); +export const findUnresolvedDiscussionIdNeighbor = (state, getters) => ({ + discussionId, + diffOrder, + step, +}) => { + const ids = getters.unresolvedDiscussionsIdsOrdered(diffOrder); + const index = ids.indexOf(discussionId) + step; + + if (index < 0 && step < 0) { + return ids[ids.length - 1]; + } + + if (index === ids.length && step > 0) { + return ids[0]; + } - // Get the first ID if there is none after the currentIndex - return slicedIds.length ? idsOrdered.slice(currentIndex + 1, currentIndex + 2)[0] : idsOrdered[0]; + return ids[index]; }; -export const previousUnresolvedDiscussionId = (state, getters) => (discussionId, diffOrder) => { - const idsOrdered = getters.unresolvedDiscussionsIdsOrdered(diffOrder); - const currentIndex = idsOrdered.indexOf(discussionId); - const slicedIds = idsOrdered.slice(currentIndex - 1, currentIndex); +// Gets the ID of the discussion following the one provided, respecting order (diff or date) +// @param {Boolean} discussionId - id of the current discussion +// @param {Boolean} diffOrder - is ordered by diff? +export const nextUnresolvedDiscussionId = (state, getters) => (discussionId, diffOrder) => + getters.findUnresolvedDiscussionIdNeighbor({ discussionId, diffOrder, step: 1 }); - // Get the last ID if there is none after the currentIndex - return slicedIds.length ? slicedIds[0] : idsOrdered[idsOrdered.length - 1]; -}; +export const previousUnresolvedDiscussionId = (state, getters) => (discussionId, diffOrder) => + getters.findUnresolvedDiscussionIdNeighbor({ discussionId, diffOrder, step: -1 }); // @param {Boolean} diffOrder - is ordered by diff? export const firstUnresolvedDiscussionId = (state, getters) => diffOrder => { diff --git a/app/assets/javascripts/pages/projects/wikis/wikis.js b/app/assets/javascripts/pages/projects/wikis/wikis.js index 9b58d42b47d..d41199f6374 100644 --- a/app/assets/javascripts/pages/projects/wikis/wikis.js +++ b/app/assets/javascripts/pages/projects/wikis/wikis.js @@ -1,6 +1,5 @@ import bp from '../../../breakpoints'; -import { parseQueryStringIntoObject } from '../../../lib/utils/common_utils'; -import { mergeUrlParams, redirectTo } from '../../../lib/utils/url_utility'; +import { s__, sprintf } from '~/locale'; export default class Wikis { constructor() { @@ -12,32 +11,37 @@ export default class Wikis { sidebarToggles[i].addEventListener('click', e => this.handleToggleSidebar(e)); } - this.newWikiForm = document.querySelector('form.new-wiki-page'); - if (this.newWikiForm) { - this.newWikiForm.addEventListener('submit', e => this.handleNewWikiSubmit(e)); + this.isNewWikiPage = Boolean(document.querySelector('.js-new-wiki-page')); + this.editTitleInput = document.querySelector('form.wiki-form #wiki_title'); + this.commitMessageInput = document.querySelector('form.wiki-form #wiki_message'); + this.commitMessageI18n = this.isNewWikiPage + ? s__('WikiPageCreate|Create %{pageTitle}') + : s__('WikiPageEdit|Update %{pageTitle}'); + + if (this.editTitleInput) { + // Initialize the commit message on load + if (this.editTitleInput.value) this.setWikiCommitMessage(this.editTitleInput.value); + + // Set the commit message as the page title is changed + this.editTitleInput.addEventListener('keyup', e => this.handleWikiTitleChange(e)); } window.addEventListener('resize', () => this.renderSidebar()); this.renderSidebar(); } - handleNewWikiSubmit(e) { - if (!this.newWikiForm) return; - - const slugInput = this.newWikiForm.querySelector('#new_wiki_path'); - - const slug = slugInput.value; + handleWikiTitleChange(e) { + this.setWikiCommitMessage(e.target.value); + } - if (slug.length > 0) { - const wikisPath = slugInput.getAttribute('data-wikis-path'); + setWikiCommitMessage(rawTitle) { + let title = rawTitle; - // If the wiki is empty, we need to merge the current URL params to keep the "create" view. - const params = parseQueryStringIntoObject(window.location.search.substr(1)); - const url = mergeUrlParams(params, `${wikisPath}/${slug}`); - redirectTo(url); + // Replace hyphens with spaces + if (title) title = title.replace(/-+/g, ' '); - e.preventDefault(); - } + const newCommitMessage = sprintf(this.commitMessageI18n, { pageTitle: title }); + this.commitMessageInput.value = newCommitMessage; } handleToggleSidebar(e) { diff --git a/app/assets/javascripts/registry/components/app.vue b/app/assets/javascripts/registry/components/app.vue index efbf0a4e3cf..346dc470a59 100644 --- a/app/assets/javascripts/registry/components/app.vue +++ b/app/assets/javascripts/registry/components/app.vue @@ -1,10 +1,9 @@ <script> import { mapGetters, mapActions } from 'vuex'; -import { GlLoadingIcon } from '@gitlab/ui'; +import { GlLoadingIcon, GlEmptyState } from '@gitlab/ui'; import store from '../stores'; import clipboardButton from '../../vue_shared/components/clipboard_button.vue'; import CollapsibleContainer from './collapsible_container.vue'; -import SvgMessage from './svg_message.vue'; import { s__, sprintf } from '../../locale'; export default { @@ -12,8 +11,8 @@ export default { components: { clipboardButton, CollapsibleContainer, + GlEmptyState, GlLoadingIcon, - SvgMessage, }, props: { endpoint: { @@ -93,7 +92,9 @@ export default { this.setMainEndpoint(this.endpoint); }, mounted() { - this.fetchRepos(); + if (!this.characterError) { + this.fetchRepos(); + } }, methods: { ...mapActions(['setMainEndpoint', 'fetchRepos']), @@ -102,61 +103,63 @@ export default { </script> <template> <div> - <svg-message v-if="characterError" id="invalid-characters" :svg-path="containersErrorImage"> - <h4> - {{ s__('ContainerRegistry|Docker connection error') }} - </h4> - <p v-html="dockerConnectionErrorText"></p> - </svg-message> + <gl-empty-state + v-if="characterError" + :title="s__('ContainerRegistry|Docker connection error')" + :svg-path="containersErrorImage" + > + <template #description> + <p v-html="dockerConnectionErrorText"></p> + </template> + </gl-empty-state> - <gl-loading-icon v-else-if="isLoading && !characterError" size="md" class="prepend-top-16" /> + <gl-loading-icon v-else-if="isLoading" size="md" class="prepend-top-16" /> - <div v-else-if="!isLoading && !characterError && repos.length"> + <div v-else-if="!isLoading && repos.length"> <h4>{{ s__('ContainerRegistry|Container Registry') }}</h4> <p v-html="introText"></p> <collapsible-container v-for="item in repos" :key="item.id" :repo="item" /> </div> - <svg-message - v-else-if="!isLoading && !characterError && !repos.length" - id="no-container-images" + <gl-empty-state + v-else + :title="s__('ContainerRegistry|There are no container images stored for this project')" :svg-path="noContainersImage" + class="container-message" > - <h4> - {{ s__('ContainerRegistry|There are no container images stored for this project') }} - </h4> - <p v-html="noContainerImagesText"></p> - - <h5>{{ s__('ContainerRegistry|Quick Start') }}</h5> - <p> - {{ - s__( - 'ContainerRegistry|You can add an image to this registry with the following commands:', - ) - }} - </p> + <template #description> + <p class="js-no-container-images-text" v-html="noContainerImagesText"></p> + <h5>{{ s__('ContainerRegistry|Quick Start') }}</h5> + <p> + {{ + s__( + 'ContainerRegistry|You can add an image to this registry with the following commands:', + ) + }} + </p> - <div class="input-group append-bottom-10"> - <input :value="dockerBuildCommand" type="text" class="form-control monospace" readonly /> - <span class="input-group-append"> - <clipboard-button - :text="dockerBuildCommand" - :title="s__('ContainerRegistry|Copy build command to clipboard')" - class="input-group-text" - /> - </span> - </div> + <div class="input-group append-bottom-10"> + <input :value="dockerBuildCommand" type="text" class="form-control monospace" readonly /> + <span class="input-group-append"> + <clipboard-button + :text="dockerBuildCommand" + :title="s__('ContainerRegistry|Copy build command to clipboard')" + class="input-group-text" + /> + </span> + </div> - <div class="input-group"> - <input :value="dockerPushCommand" type="text" class="form-control monospace" readonly /> - <span class="input-group-append"> - <clipboard-button - :text="dockerPushCommand" - :title="s__('ContainerRegistry|Copy push command to clipboard')" - class="input-group-text" - /> - </span> - </div> - </svg-message> + <div class="input-group"> + <input :value="dockerPushCommand" type="text" class="form-control monospace" readonly /> + <span class="input-group-append"> + <clipboard-button + :text="dockerPushCommand" + :title="s__('ContainerRegistry|Copy push command to clipboard')" + class="input-group-text" + /> + </span> + </div> + </template> + </gl-empty-state> </div> </template> diff --git a/app/assets/javascripts/registry/components/svg_message.vue b/app/assets/javascripts/registry/components/svg_message.vue deleted file mode 100644 index 617093e054e..00000000000 --- a/app/assets/javascripts/registry/components/svg_message.vue +++ /dev/null @@ -1,26 +0,0 @@ -<script> -export default { - name: 'RegistrySvgMessage', - props: { - id: { - type: String, - required: true, - }, - svgPath: { - type: String, - required: true, - }, - }, -}; -</script> - -<template> - <div :id="id" class="empty-state container-message"> - <div class="svg-content"> - <img :src="svgPath" class="flex-align-self-center" /> - </div> - <div class="text-content"> - <slot></slot> - </div> - </div> -</template> diff --git a/app/assets/javascripts/sidebar/components/assignees/assignee_avatar.vue b/app/assets/javascripts/sidebar/components/assignees/assignee_avatar.vue new file mode 100644 index 00000000000..71a1fc31315 --- /dev/null +++ b/app/assets/javascripts/sidebar/components/assignees/assignee_avatar.vue @@ -0,0 +1,48 @@ +<script> +import { __, sprintf } from '~/locale'; + +export default { + props: { + user: { + type: Object, + required: true, + }, + imgSize: { + type: Number, + required: true, + }, + issuableType: { + type: String, + required: false, + default: 'issue', + }, + }, + computed: { + assigneeAlt() { + return sprintf(__("%{userName}'s avatar"), { userName: this.user.name }); + }, + avatarUrl() { + return this.user.avatar || this.user.avatar_url || gon.default_avatar_url; + }, + isMergeRequest() { + return this.issuableType === 'merge_request'; + }, + hasMergeIcon() { + return this.isMergeRequest && !this.user.can_merge; + }, + }, +}; +</script> + +<template> + <span class="position-relative"> + <img + :alt="assigneeAlt" + :src="avatarUrl" + :width="imgSize" + :class="`s${imgSize}`" + class="avatar avatar-inline m-0" + /> + <i v-if="hasMergeIcon" aria-hidden="true" class="fa fa-exclamation-triangle merge-icon"></i> + </span> +</template> diff --git a/app/assets/javascripts/sidebar/components/assignees/assignee_avatar_link.vue b/app/assets/javascripts/sidebar/components/assignees/assignee_avatar_link.vue new file mode 100644 index 00000000000..6633a63d046 --- /dev/null +++ b/app/assets/javascripts/sidebar/components/assignees/assignee_avatar_link.vue @@ -0,0 +1,83 @@ +<script> +import { __, sprintf } from '~/locale'; +import { GlTooltipDirective, GlLink } from '@gitlab/ui'; +import { joinPaths } from '~/lib/utils/url_utility'; +import AssigneeAvatar from './assignee_avatar.vue'; + +export default { + components: { + AssigneeAvatar, + GlLink, + }, + directives: { + GlTooltip: GlTooltipDirective, + }, + props: { + user: { + type: Object, + required: true, + }, + rootPath: { + type: String, + required: true, + }, + tooltipPlacement: { + type: String, + default: 'bottom', + required: false, + }, + tooltipHasName: { + type: Boolean, + default: true, + required: false, + }, + issuableType: { + type: String, + default: 'issue', + required: false, + }, + }, + computed: { + cannotMerge() { + return this.issuableType === 'merge_request' && !this.user.can_merge; + }, + tooltipTitle() { + if (this.cannotMerge && this.tooltipHasName) { + return sprintf(__('%{userName} (cannot merge)'), { userName: this.user.name }); + } else if (this.cannotMerge) { + return __('Cannot merge'); + } else if (this.tooltipHasName) { + return this.user.name; + } + + return ''; + }, + tooltipOption() { + return { + container: 'body', + placement: this.tooltipPlacement, + boundary: 'viewport', + }; + }, + assigneeUrl() { + return joinPaths(`${this.rootPath}`, `${this.user.username}`); + }, + }, +}; +</script> + +<template> + <!-- must be `d-inline-block` or parent flex-basis causes width issues --> + <gl-link + v-gl-tooltip="tooltipOption" + :href="assigneeUrl" + :title="tooltipTitle" + class="d-inline-block" + > + <!-- use d-flex so that slot can be appropriately styled --> + <span class="d-flex"> + <assignee-avatar :user="user" :img-size="32" :issuable-type="issuableType" /> + <slot :user="user"></slot> + </span> + </gl-link> +</template> diff --git a/app/assets/javascripts/sidebar/components/assignees/assignee_title.vue b/app/assets/javascripts/sidebar/components/assignees/assignee_title.vue index fa6b6bfaef1..63b93a80ead 100644 --- a/app/assets/javascripts/sidebar/components/assignees/assignee_title.vue +++ b/app/assets/javascripts/sidebar/components/assignees/assignee_title.vue @@ -1,5 +1,6 @@ <script> import { n__ } from '~/locale'; +import { trackEvent } from 'ee_else_ce/event_tracking/issue_sidebar'; export default { name: 'AssigneeTitle', @@ -29,13 +30,23 @@ export default { return n__('Assignee', `%d Assignees`, assignees); }, }, + methods: { + trackEdit() { + trackEvent('click_edit_button', 'assignee'); + }, + }, }; </script> <template> <div class="title hide-collapsed"> {{ assigneeTitle }} <i v-if="loading" aria-hidden="true" class="fa fa-spinner fa-spin block-loading"></i> - <a v-if="editable" class="js-sidebar-dropdown-toggle edit-link float-right" href="#"> + <a + v-if="editable" + class="js-sidebar-dropdown-toggle edit-link float-right" + href="#" + @click.prevent="trackEdit" + > {{ __('Edit') }} </a> <a diff --git a/app/assets/javascripts/sidebar/components/assignees/assignees.vue b/app/assets/javascripts/sidebar/components/assignees/assignees.vue index 631e2e28d4d..d9739e8d197 100644 --- a/app/assets/javascripts/sidebar/components/assignees/assignees.vue +++ b/app/assets/javascripts/sidebar/components/assignees/assignees.vue @@ -1,13 +1,14 @@ <script> -import { __, sprintf } from '~/locale'; -import tooltip from '~/vue_shared/directives/tooltip'; +import CollapsedAssigneeList from '../assignees/collapsed_assignee_list.vue'; +import UncollapsedAssigneeList from '../assignees/uncollapsed_assignee_list.vue'; export default { // name: 'Assignees' is a false positive: https://gitlab.com/gitlab-org/frontend/eslint-plugin-i18n/issues/26#possible-false-positives // eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings name: 'Assignees', - directives: { - tooltip, + components: { + CollapsedAssigneeList, + UncollapsedAssigneeList, }, props: { rootPath: { @@ -24,171 +25,34 @@ export default { }, issuableType: { type: String, - require: true, + required: false, default: 'issue', }, }, - data() { - return { - defaultRenderCount: 5, - defaultMaxCounter: 99, - showLess: true, - }; - }, computed: { - firstUser() { - return this.users[0]; - }, - hasMoreThanTwoAssignees() { - return this.users.length > 2; - }, - hasMoreThanOneAssignee() { - return this.users.length > 1; - }, - hasAssignees() { - return this.users.length > 0; - }, hasNoUsers() { return !this.users.length; }, - hasOneUser() { - return this.users.length === 1; - }, - renderShowMoreSection() { - return this.users.length > this.defaultRenderCount; - }, - numberOfHiddenAssignees() { - return this.users.length - this.defaultRenderCount; - }, - isHiddenAssignees() { - return this.numberOfHiddenAssignees > 0; - }, - hiddenAssigneesLabel() { - const { numberOfHiddenAssignees } = this; - return sprintf(__('+ %{numberOfHiddenAssignees} more'), { numberOfHiddenAssignees }); - }, - collapsedTooltipTitle() { - const maxRender = Math.min(this.defaultRenderCount, this.users.length); - const renderUsers = this.users.slice(0, maxRender); - const names = renderUsers.map(u => u.name); - - if (this.users.length > maxRender) { - names.push(`+ ${this.users.length - maxRender} more`); - } - - if (!this.users.length) { - const emptyTooltipLabel = __('Assignee(s)'); - names.push(emptyTooltipLabel); - } - - return names.join(', '); - }, - sidebarAvatarCounter() { - let counter = `+${this.users.length - 1}`; - - if (this.users.length > this.defaultMaxCounter) { - counter = `${this.defaultMaxCounter}+`; - } + sortedAssigness() { + const canMergeUsers = this.users.filter(user => user.can_merge); + const canNotMergeUsers = this.users.filter(user => !user.can_merge); - return counter; - }, - mergeNotAllowedTooltipMessage() { - const assigneesCount = this.users.length; - - if (this.issuableType !== 'merge_request' || assigneesCount === 0) { - return null; - } - - const cannotMergeCount = this.users.filter(u => u.can_merge === false).length; - const canMergeCount = assigneesCount - cannotMergeCount; - - if (canMergeCount === assigneesCount) { - // Everyone can merge - return null; - } else if (cannotMergeCount === assigneesCount && assigneesCount > 1) { - return __('No one can merge'); - } else if (assigneesCount === 1) { - return __('Cannot merge'); - } - - return sprintf(__('%{canMergeCount}/%{assigneesCount} can merge'), { - canMergeCount, - assigneesCount, - }); + return [...canMergeUsers, ...canNotMergeUsers]; }, }, methods: { assignSelf() { this.$emit('assign-self'); }, - toggleShowLess() { - this.showLess = !this.showLess; - }, - renderAssignee(index) { - return !this.showLess || (index < this.defaultRenderCount && this.showLess); - }, - avatarUrl(user) { - return user.avatar || user.avatar_url || gon.default_avatar_url; - }, - assigneeUrl(user) { - return `${this.rootPath}${user.username}`; - }, - assigneeAlt(user) { - return sprintf(__("%{userName}'s avatar"), { userName: user.name }); - }, - assigneeUsername(user) { - return `@${user.username}`; - }, - shouldRenderCollapsedAssignee(index) { - const firstTwo = this.users.length <= 2 && index <= 2; - - return index === 0 || firstTwo; - }, }, }; </script> <template> <div> - <div - v-tooltip - :class="{ 'multiple-users': hasMoreThanOneAssignee }" - :title="collapsedTooltipTitle" - class="sidebar-collapsed-icon sidebar-collapsed-user" - data-container="body" - data-placement="left" - data-boundary="viewport" - > - <i v-if="hasNoUsers" :aria-label="__('None')" class="fa fa-user"> </i> - <button - v-for="(user, index) in users" - v-if="shouldRenderCollapsedAssignee(index)" - :key="user.id" - type="button" - class="btn-link" - > - <img - :alt="assigneeAlt(user)" - :src="avatarUrl(user)" - width="24" - class="avatar avatar-inline s24" - /> - <span class="author"> {{ user.name }} </span> - </button> - <button v-if="hasMoreThanTwoAssignees" class="btn-link" type="button"> - <span class="avatar-counter sidebar-avatar-counter"> {{ sidebarAvatarCounter }} </span> - </button> - </div> + <collapsed-assignee-list :users="sortedAssigness" :issuable-type="issuableType" /> + <div class="value hide-collapsed"> - <span - v-if="mergeNotAllowedTooltipMessage" - v-tooltip - :title="mergeNotAllowedTooltipMessage" - data-placement="left" - class="float-right cannot-be-merged" - > - <i aria-hidden="true" data-hidden="true" class="fa fa-exclamation-triangle"></i> - </span> <template v-if="hasNoUsers"> <span class="assign-yourself no-value qa-assign-yourself"> {{ __('None') }} @@ -200,51 +64,13 @@ export default { </template> </span> </template> - <template v-else-if="hasOneUser"> - <a :href="assigneeUrl(firstUser)" class="author-link bold"> - <img - :alt="assigneeAlt(firstUser)" - :src="avatarUrl(firstUser)" - width="32" - class="avatar avatar-inline s32" - /> - <span class="author"> {{ firstUser.name }} </span> - <span class="username"> {{ assigneeUsername(firstUser) }} </span> - </a> - </template> - <template v-else> - <div class="user-list"> - <div - v-for="(user, index) in users" - v-if="renderAssignee(index)" - :key="user.id" - class="user-item" - > - <a - :href="assigneeUrl(user)" - :data-title="user.name" - class="user-link has-tooltip" - data-container="body" - data-placement="bottom" - > - <img - :alt="assigneeAlt(user)" - :src="avatarUrl(user)" - width="32" - class="avatar avatar-inline s32" - /> - </a> - </div> - </div> - <div v-if="renderShowMoreSection" class="user-list-more"> - <button type="button" class="btn-link" @click="toggleShowLess"> - <template v-if="showLess"> - {{ hiddenAssigneesLabel }} - </template> - <template v-else>{{ __('- show less') }}</template> - </button> - </div> - </template> + + <uncollapsed-assignee-list + v-else + :users="sortedAssigness" + :root-path="rootPath" + :issuable-type="issuableType" + /> </div> </div> </template> diff --git a/app/assets/javascripts/sidebar/components/assignees/collapsed_assignee.vue b/app/assets/javascripts/sidebar/components/assignees/collapsed_assignee.vue new file mode 100644 index 00000000000..2f654409561 --- /dev/null +++ b/app/assets/javascripts/sidebar/components/assignees/collapsed_assignee.vue @@ -0,0 +1,27 @@ +<script> +import AssigneeAvatar from './assignee_avatar.vue'; + +export default { + components: { + AssigneeAvatar, + }, + props: { + user: { + type: Object, + required: true, + }, + issuableType: { + type: String, + required: false, + default: 'issue', + }, + }, +}; +</script> + +<template> + <button type="button" class="btn-link"> + <assignee-avatar :user="user" :img-size="24" :issuable-type="issuableType" /> + <span class="author"> {{ user.name }} </span> + </button> +</template> diff --git a/app/assets/javascripts/sidebar/components/assignees/collapsed_assignee_list.vue b/app/assets/javascripts/sidebar/components/assignees/collapsed_assignee_list.vue new file mode 100644 index 00000000000..5b4a43399ca --- /dev/null +++ b/app/assets/javascripts/sidebar/components/assignees/collapsed_assignee_list.vue @@ -0,0 +1,121 @@ +<script> +import { __, sprintf } from '~/locale'; +import { GlTooltipDirective } from '@gitlab/ui'; +import CollapsedAssignee from './collapsed_assignee.vue'; + +const DEFAULT_MAX_COUNTER = 99; +const DEFAULT_RENDER_COUNT = 5; + +export default { + directives: { + GlTooltip: GlTooltipDirective, + }, + components: { + CollapsedAssignee, + }, + props: { + users: { + type: Array, + required: true, + }, + issuableType: { + type: String, + required: false, + default: 'issue', + }, + }, + computed: { + isMergeRequest() { + return this.issuableType === 'merge_request'; + }, + hasNoUsers() { + return !this.users.length; + }, + hasMoreThanOneAssignee() { + return this.users.length > 1; + }, + hasMoreThanTwoAssignees() { + return this.users.length > 2; + }, + allAssigneesCanMerge() { + return this.users.every(user => user.can_merge); + }, + sidebarAvatarCounter() { + if (this.users.length > DEFAULT_MAX_COUNTER) { + return `${DEFAULT_MAX_COUNTER}+`; + } + + return `+${this.users.length - 1}`; + }, + collapsedUsers() { + const collapsedLength = this.hasMoreThanTwoAssignees ? 1 : this.users.length; + + return this.users.slice(0, collapsedLength); + }, + tooltipTitleMergeStatus() { + if (!this.isMergeRequest) { + return ''; + } + + const mergeLength = this.users.filter(u => u.can_merge).length; + + if (mergeLength === this.users.length) { + return ''; + } else if (mergeLength > 0) { + return sprintf(__('%{mergeLength}/%{usersLength} can merge'), { + mergeLength, + usersLength: this.users.length, + }); + } + + return this.users.length === 1 ? __('cannot merge') : __('no one can merge'); + }, + tooltipTitle() { + const maxRender = Math.min(DEFAULT_RENDER_COUNT, this.users.length); + const renderUsers = this.users.slice(0, maxRender); + const names = renderUsers.map(u => u.name); + + if (!this.users.length) { + return __('Assignee(s)'); + } + + if (this.users.length > names.length) { + names.push(sprintf(__('+ %{amount} more'), { amount: this.users.length - names.length })); + } + + const text = names.join(', '); + + return this.tooltipTitleMergeStatus ? `${text} (${this.tooltipTitleMergeStatus})` : text; + }, + + tooltipOptions() { + return { container: 'body', placement: 'left', boundary: 'viewport' }; + }, + }, +}; +</script> + +<template> + <div + v-gl-tooltip="tooltipOptions" + :class="{ 'multiple-users': hasMoreThanOneAssignee }" + :title="tooltipTitle" + class="sidebar-collapsed-icon sidebar-collapsed-user" + > + <i v-if="hasNoUsers" :aria-label="__('None')" class="fa fa-user"> </i> + <collapsed-assignee + v-for="user in collapsedUsers" + :key="user.id" + :user="user" + :issuable-type="issuableType" + /> + <button v-if="hasMoreThanTwoAssignees" class="btn-link" type="button"> + <span class="avatar-counter sidebar-avatar-counter"> {{ sidebarAvatarCounter }} </span> + <i + v-if="isMergeRequest && !allAssigneesCanMerge" + aria-hidden="true" + class="fa fa-exclamation-triangle merge-icon" + ></i> + </button> + </div> +</template> diff --git a/app/assets/javascripts/sidebar/components/assignees/sidebar_assignees.vue b/app/assets/javascripts/sidebar/components/assignees/sidebar_assignees.vue index be1e4811856..c6cc04a139f 100644 --- a/app/assets/javascripts/sidebar/components/assignees/sidebar_assignees.vue +++ b/app/assets/javascripts/sidebar/components/assignees/sidebar_assignees.vue @@ -29,7 +29,7 @@ export default { }, issuableType: { type: String, - require: true, + required: false, default: 'issue', }, }, diff --git a/app/assets/javascripts/sidebar/components/assignees/uncollapsed_assignee_list.vue b/app/assets/javascripts/sidebar/components/assignees/uncollapsed_assignee_list.vue new file mode 100644 index 00000000000..3a4623121f4 --- /dev/null +++ b/app/assets/javascripts/sidebar/components/assignees/uncollapsed_assignee_list.vue @@ -0,0 +1,96 @@ +<script> +import { __, sprintf } from '~/locale'; +import AssigneeAvatarLink from './assignee_avatar_link.vue'; + +const DEFAULT_RENDER_COUNT = 5; + +export default { + components: { + AssigneeAvatarLink, + }, + props: { + users: { + type: Array, + required: true, + }, + rootPath: { + type: String, + required: true, + }, + issuableType: { + type: String, + required: false, + default: 'issue', + }, + }, + data() { + return { + showLess: true, + }; + }, + computed: { + firstUser() { + return this.users[0]; + }, + hasOneUser() { + return this.users.length === 1; + }, + hiddenAssigneesLabel() { + const { numberOfHiddenAssignees } = this; + return sprintf(__('+ %{numberOfHiddenAssignees} more'), { numberOfHiddenAssignees }); + }, + renderShowMoreSection() { + return this.users.length > DEFAULT_RENDER_COUNT; + }, + numberOfHiddenAssignees() { + return this.users.length - DEFAULT_RENDER_COUNT; + }, + uncollapsedUsers() { + const uncollapsedLength = this.showLess + ? Math.min(this.users.length, DEFAULT_RENDER_COUNT) + : this.users.length; + return this.showLess ? this.users.slice(0, uncollapsedLength) : this.users; + }, + username() { + return `@${this.firstUser.username}`; + }, + }, + methods: { + toggleShowLess() { + this.showLess = !this.showLess; + }, + }, +}; +</script> + +<template> + <assignee-avatar-link + v-if="hasOneUser" + v-slot="{ user }" + tooltip-placement="left" + :tooltip-has-name="false" + :user="firstUser" + :root-path="rootPath" + :issuable-type="issuableType" + > + <div class="ml-2"> + <span class="author"> {{ user.name }} </span> + <span class="username"> {{ username }} </span> + </div> + </assignee-avatar-link> + <div v-else> + <div class="user-list"> + <div v-for="user in uncollapsedUsers" :key="user.id" class="user-item"> + <assignee-avatar-link :user="user" :root-path="rootPath" :issuable-type="issuableType" /> + </div> + </div> + <div v-if="renderShowMoreSection" class="user-list-more"> + <button type="button" class="btn-link" @click="toggleShowLess"> + <template v-if="showLess"> + {{ hiddenAssigneesLabel }} + </template> + <template v-else>{{ __('- show less') }}</template> + </button> + </div> + </div> +</template> diff --git a/app/assets/javascripts/sidebar/components/confidential/confidential_issue_sidebar.vue b/app/assets/javascripts/sidebar/components/confidential/confidential_issue_sidebar.vue index 597b723a9d9..1c75b6148e8 100644 --- a/app/assets/javascripts/sidebar/components/confidential/confidential_issue_sidebar.vue +++ b/app/assets/javascripts/sidebar/components/confidential/confidential_issue_sidebar.vue @@ -5,6 +5,7 @@ import tooltip from '~/vue_shared/directives/tooltip'; import Icon from '~/vue_shared/components/icon.vue'; import eventHub from '~/sidebar/event_hub'; import editForm from './edit_form.vue'; +import { trackEvent } from 'ee_else_ce/event_tracking/issue_sidebar'; export default { components: { @@ -51,6 +52,11 @@ export default { toggleForm() { this.edit = !this.edit; }, + onEditClick() { + this.toggleForm(); + + trackEvent('click_edit_button', 'confidentiality'); + }, updateConfidentialAttribute(confidential) { this.service .update('issue', { confidential }) @@ -82,7 +88,7 @@ export default { v-if="isEditable" class="float-right confidential-edit" href="#" - @click.prevent="toggleForm" + @click.prevent="onEditClick" > {{ __('Edit') }} </a> diff --git a/app/assets/javascripts/sidebar/components/lock/lock_issue_sidebar.vue b/app/assets/javascripts/sidebar/components/lock/lock_issue_sidebar.vue index c5cfa92f3c8..ec2a7b93a98 100644 --- a/app/assets/javascripts/sidebar/components/lock/lock_issue_sidebar.vue +++ b/app/assets/javascripts/sidebar/components/lock/lock_issue_sidebar.vue @@ -6,6 +6,7 @@ import issuableMixin from '~/vue_shared/mixins/issuable'; import Icon from '~/vue_shared/components/icon.vue'; import eventHub from '~/sidebar/event_hub'; import editForm from './edit_form.vue'; +import { trackEvent } from 'ee_else_ce/event_tracking/issue_sidebar'; export default { components: { @@ -65,7 +66,11 @@ export default { toggleForm() { this.mediator.store.isLockDialogOpen = !this.mediator.store.isLockDialogOpen; }, + onEditClick() { + this.toggleForm(); + trackEvent('click_edit_button', 'lock_issue'); + }, updateLockedAttribute(locked) { this.mediator.service .update(this.issuableType, { @@ -109,7 +114,7 @@ export default { v-if="isEditable" class="float-right lock-edit" type="button" - @click.prevent="toggleForm" + @click.prevent="onEditClick" > {{ __('Edit') }} </button> diff --git a/app/assets/javascripts/sidebar/components/subscriptions/subscriptions.vue b/app/assets/javascripts/sidebar/components/subscriptions/subscriptions.vue index 0d1faceef11..1f5f19d1931 100644 --- a/app/assets/javascripts/sidebar/components/subscriptions/subscriptions.vue +++ b/app/assets/javascripts/sidebar/components/subscriptions/subscriptions.vue @@ -4,6 +4,7 @@ import icon from '~/vue_shared/components/icon.vue'; import toggleButton from '~/vue_shared/components/toggle_button.vue'; import tooltip from '~/vue_shared/directives/tooltip'; import eventHub from '../../event_hub'; +import { trackEvent } from 'ee_else_ce/event_tracking/issue_sidebar'; const ICON_ON = 'notifications'; const ICON_OFF = 'notifications-off'; @@ -63,6 +64,8 @@ export default { // Component event emission. this.$emit('toggleSubscription', this.id); + + trackEvent('toggle_button', 'notifications', this.subscribed ? 0 : 1); }, onClickCollapsedIcon() { this.$emit('toggleSidebar'); diff --git a/app/assets/javascripts/users_select.js b/app/assets/javascripts/users_select.js index 33cedf78331..12c939aa70f 100644 --- a/app/assets/javascripts/users_select.js +++ b/app/assets/javascripts/users_select.js @@ -62,6 +62,8 @@ function UsersSelect(currentUser, els, options = {}) { options.showCurrentUser = $dropdown.data('currentUser'); options.todoFilter = $dropdown.data('todoFilter'); options.todoStateFilter = $dropdown.data('todoStateFilter'); + options.iid = $dropdown.data('iid'); + options.issuableType = $dropdown.data('issuableType'); showNullUser = $dropdown.data('nullUser'); defaultNullUser = $dropdown.data('nullUserDefault'); showMenuAbove = $dropdown.data('showMenuAbove'); @@ -239,7 +241,7 @@ function UsersSelect(currentUser, els, options = {}) { '<% if( avatar ) { %> <a class="author-link" href="/<%- username %>"> <img width="24" class="avatar avatar-inline s24" alt="" src="<%- avatar %>"> </a> <% } else { %> <i class="fa fa-user"></i> <% } %>', ); assigneeTemplate = _.template( - `<% if (username) { %> <a class="author-link bold" href="/<%- username %>"> <% if( avatar ) { %> <img width="32" class="avatar avatar-inline s32" alt="" src="<%- avatar %>"> <% } %> <span class="author"><%- name %></span> <span class="username"> @<%- username %> </span> </a> <% } else { %> <span class="no-value assign-yourself"> + `<% if (username) { %> <a class="author-link bold" href="/<%- username %>"> <% if( avatar ) { %> <img width="32" class="avatar avatar-inline s32" alt="" src="<%- avatar %>"> <% } %> <span class="author"><%- name %></span> <span class="username"> @<%- username %> </span> </a> <% } else { %> <span class="no-value assign-yourself"> ${sprintf(s__('UsersSelect|No assignee - %{openingTag} assign yourself %{closingTag}'), { openingTag: '<a href="#" class="js-assign-yourself">', closingTag: '</a>', @@ -423,6 +425,8 @@ function UsersSelect(currentUser, els, options = {}) { const { $el, e, isMarking } = options; const user = options.selectedObj; + $el.tooltip('dispose'); + if ($dropdown.hasClass('js-multiselect')) { const isActive = $el.hasClass('is-active'); const previouslySelected = $dropdown @@ -570,20 +574,11 @@ function UsersSelect(currentUser, els, options = {}) { user.name, )}</a></li>`; } else { - img = "<img src='" + avatar + "' class='avatar avatar-inline' width='32' />"; + // 0 margin, because it's now handled by a wrapper + img = "<img src='" + avatar + "' class='avatar avatar-inline m-0' width='32' />"; } - return ` - <li data-user-id=${user.id}> - <a href='#' class='dropdown-menu-user-link ${selected === true ? 'is-active' : ''}'> - ${img} - <strong class='dropdown-menu-user-full-name'> - ${_.escape(user.name)} - </strong> - ${username ? `<span class='dropdown-menu-user-username'>${username}</span>` : ''} - </a> - </li> - `; + return _this.renderRow(options.issuableType, user, selected, username, img); }, }); }; @@ -764,6 +759,11 @@ UsersSelect.prototype.users = function(query, options, callback) { author_id: options.authorId || null, skip_users: options.skipUsers || null, }; + + if (options.issuableType === 'merge_request') { + params.merge_request_iid = options.iid || null; + } + return axios.get(url, { params }).then(({ data }) => { callback(data); }); @@ -776,4 +776,44 @@ UsersSelect.prototype.buildUrl = function(url) { return url; }; +UsersSelect.prototype.renderRow = function(issuableType, user, selected, username, img) { + const tooltip = issuableType === 'merge_request' && !user.can_merge ? __('Cannot merge') : ''; + const tooltipClass = tooltip ? `has-tooltip` : ''; + const selectedClass = selected === true ? 'is-active' : ''; + const linkClasses = `${selectedClass} ${tooltipClass}`; + const tooltipAttributes = tooltip + ? `data-container="body" data-placement="left" data-title="${tooltip}"` + : ''; + + return ` + <li data-user-id=${user.id}> + <a href="#" class="dropdown-menu-user-link d-flex align-items-center ${linkClasses}" ${tooltipAttributes}> + ${this.renderRowAvatar(issuableType, user, img)} + <span class="d-flex flex-column overflow-hidden"> + <strong class="dropdown-menu-user-full-name"> + ${_.escape(user.name)} + </strong> + ${username ? `<span class="dropdown-menu-user-username">${username}</span>` : ''} + </span> + </a> + </li> + `; +}; + +UsersSelect.prototype.renderRowAvatar = function(issuableType, user, img) { + if (user.beforeDivider) { + return img; + } + + const mergeIcon = + issuableType === 'merge_request' && !user.can_merge + ? '<i class="fa fa-exclamation-triangle merge-icon"></i>' + : ''; + + return `<span class="position-relative mr-2"> + ${img} + ${mergeIcon} + </span>`; +}; + export default UsersSelect; diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue index c7b064b8506..339e154affc 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue @@ -50,6 +50,7 @@ export default { startTag: '<span class="label-branch">', endTag: '</span>', }, + false, ); }, }, diff --git a/app/assets/javascripts/vue_shared/components/recaptcha_modal.vue b/app/assets/javascripts/vue_shared/components/recaptcha_modal.vue index df19906309c..f0aae20477b 100644 --- a/app/assets/javascripts/vue_shared/components/recaptcha_modal.vue +++ b/app/assets/javascripts/vue_shared/components/recaptcha_modal.vue @@ -30,9 +30,16 @@ export default { }, mounted() { + if (window.recaptchaDialogCallback) { + throw new Error('recaptchaDialogCallback is already defined!'); + } window.recaptchaDialogCallback = this.submit.bind(this); }, + beforeDestroy() { + window.recaptchaDialogCallback = null; + }, + methods: { appendRecaptchaScript() { this.removeRecaptchaScript(); diff --git a/app/assets/stylesheets/errors.scss b/app/assets/stylesheets/errors.scss index d287215096e..89029a58d1e 100644 --- a/app/assets/stylesheets/errors.scss +++ b/app/assets/stylesheets/errors.scss @@ -96,7 +96,7 @@ a { } .error-nav { - padding: 0; + padding: $gl-padding 0 0; text-align: center; li { diff --git a/app/assets/stylesheets/pages/container_registry.scss b/app/assets/stylesheets/pages/container_registry.scss index 0f4bdb219a3..b88bd78cf3d 100644 --- a/app/assets/stylesheets/pages/container_registry.scss +++ b/app/assets/stylesheets/pages/container_registry.scss @@ -3,10 +3,6 @@ */ .container-message { - pre { - white-space: pre-line; - } - span .btn { margin: 0; } diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index fa52ce6402d..0e844b0e4a5 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -126,6 +126,16 @@ } } +.assignee { + .merge-icon { + color: $orange-500; + position: absolute; + bottom: 0; + right: 0; + text-shadow: -1px -1px 0 $white-light, 1px -1px 0 $white-light, -1px 1px 0 $white-light, 1px 1px 0 $white-light; + } +} + .right-sidebar { position: fixed; top: $header-height; @@ -202,7 +212,6 @@ &.assignee { .author-link { display: block; - padding-left: 42px; position: relative; &:hover { @@ -210,12 +219,6 @@ text-decoration: underline; } } - - .avatar { - left: 0; - position: absolute; - top: 0; - } } } } @@ -354,13 +357,6 @@ margin-top: 0; } - .assignee .avatar { - float: left; - margin-right: 10px; - margin-bottom: 0; - margin-left: 0; - } - .assignee .user-list .avatar { margin: 0; } @@ -521,6 +517,10 @@ display: none; } + .merge-icon { + font-size: 10px; + } + .multiple-users { position: relative; height: 24px; diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb index f111c7ca8cc..30a567c3bef 100644 --- a/app/controllers/autocomplete_controller.rb +++ b/app/controllers/autocomplete_controller.rb @@ -36,7 +36,7 @@ class AutocompleteController < ApplicationController end def award_emojis - render json: AwardedEmojiFinder.new(current_user).execute + render json: AwardEmojis::CollectUserEmojiService.new(current_user).execute end def merge_request_target_branches diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb index 3489ea78b77..8ea77b994de 100644 --- a/app/controllers/concerns/issuable_collections.rb +++ b/app/controllers/concerns/issuable_collections.rb @@ -2,8 +2,8 @@ module IssuableCollections extend ActiveSupport::Concern - include CookiesHelper include SortingHelper + include SortingPreference include Gitlab::IssuableMetadata include Gitlab::Utils::StrongMemoize @@ -127,47 +127,8 @@ module IssuableCollections 'opened' end - def set_sort_order - set_sort_order_from_user_preference || set_sort_order_from_cookie || default_sort_order - end - - def set_sort_order_from_user_preference - return unless current_user - return unless issuable_sorting_field - - user_preference = current_user.user_preference - - sort_param = params[:sort] - sort_param ||= user_preference[issuable_sorting_field] - - return sort_param if Gitlab::Database.read_only? - - if user_preference[issuable_sorting_field] != sort_param - user_preference.update(issuable_sorting_field => sort_param) - end - - sort_param - end - - # Implement issuable_sorting_field method on controllers - # to choose which column to store the sorting parameter. - def issuable_sorting_field - nil - end - - def set_sort_order_from_cookie - sort_param = params[:sort] if params[:sort].present? - # fallback to legacy cookie value for backward compatibility - sort_param ||= cookies['issuable_sort'] - sort_param ||= cookies[remember_sorting_key] - - sort_value = update_cookie_value(sort_param) - set_secure_cookie(remember_sorting_key, sort_value) - sort_value - end - - def remember_sorting_key - @remember_sorting_key ||= "#{collection_type.downcase}_sort" + def legacy_sort_cookie_name + 'issuable_sort' end def default_sort_order @@ -178,17 +139,6 @@ module IssuableCollections end end - # Update old values to the actual ones. - def update_cookie_value(value) - case value - when 'id_asc' then sort_value_oldest_created - when 'id_desc' then sort_value_recently_created - when 'downvotes_asc' then sort_value_popularity - when 'downvotes_desc' then sort_value_popularity - else value - end - end - def finder @finder ||= issuable_finder_for(finder_type) end diff --git a/app/controllers/concerns/issuable_collections_action.rb b/app/controllers/concerns/issuable_collections_action.rb index 4ad287c4a13..0a6f684a9fc 100644 --- a/app/controllers/concerns/issuable_collections_action.rb +++ b/app/controllers/concerns/issuable_collections_action.rb @@ -32,7 +32,7 @@ module IssuableCollectionsAction private - def issuable_sorting_field + def sorting_field case action_name when 'issues' Issue::SORTING_PREFERENCE_FIELD diff --git a/app/controllers/concerns/sorting_preference.rb b/app/controllers/concerns/sorting_preference.rb new file mode 100644 index 00000000000..a51b68147d5 --- /dev/null +++ b/app/controllers/concerns/sorting_preference.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +module SortingPreference + include SortingHelper + include CookiesHelper + + def set_sort_order + set_sort_order_from_user_preference || set_sort_order_from_cookie || params[:sort] || default_sort_order + end + + # Implement sorting_field method on controllers + # to choose which column to store the sorting parameter. + def sorting_field + nil + end + + # Implement default_sort_order method on controllers + # to choose which default sort should be applied if + # sort param is not provided. + def default_sort_order + nil + end + + # Implement legacy_sort_cookie_name method on controllers + # to set sort from cookie for backwards compatibility. + def legacy_sort_cookie_name + nil + end + + private + + def set_sort_order_from_user_preference + return unless current_user + return unless sorting_field + + user_preference = current_user.user_preference + + sort_param = params[:sort] + sort_param ||= user_preference[sorting_field] + + return sort_param if Gitlab::Database.read_only? + + if user_preference[sorting_field] != sort_param + user_preference.update(sorting_field => sort_param) + end + + sort_param + end + + def set_sort_order_from_cookie + return unless legacy_sort_cookie_name + + sort_param = params[:sort] if params[:sort].present? + # fallback to legacy cookie value for backward compatibility + sort_param ||= cookies[legacy_sort_cookie_name] + sort_param ||= cookies[remember_sorting_key] + + sort_value = update_cookie_value(sort_param) + set_secure_cookie(remember_sorting_key, sort_value) + sort_value + end + + # Convert sorting_field to legacy cookie name for backwards compatibility + # :merge_requests_sort => 'mergerequest_sort' + # :issues_sort => 'issue_sort' + def remember_sorting_key + @remember_sorting_key ||= sorting_field + .to_s + .split('_')[0..-2] + .map(&:singularize) + .join('') + .concat('_sort') + end + + # Update old values to the actual ones. + def update_cookie_value(value) + case value + when 'id_asc' then sort_value_oldest_created + when 'id_desc' then sort_value_recently_created + when 'downvotes_asc' then sort_value_popularity + when 'downvotes_desc' then sort_value_popularity + else value + end + end +end diff --git a/app/controllers/concerns/toggle_award_emoji.rb b/app/controllers/concerns/toggle_award_emoji.rb index 97b343f8b1a..24d178781d6 100644 --- a/app/controllers/concerns/toggle_award_emoji.rb +++ b/app/controllers/concerns/toggle_award_emoji.rb @@ -7,12 +7,9 @@ module ToggleAwardEmoji authenticate_user! name = params.require(:name) - if awardable.user_can_award?(current_user) - awardable.toggle_award_emoji(name, current_user) - - todoable = to_todoable(awardable) - TodoService.new.new_award_emoji(todoable, current_user) if todoable + service = AwardEmojis::ToggleService.new(awardable, name, current_user).execute + if service[:status] == :success render json: { ok: true } else render json: { ok: false } @@ -21,18 +18,6 @@ module ToggleAwardEmoji private - def to_todoable(awardable) - case awardable - when Note - # we don't create todos for personal snippet comments for now - awardable.for_personal_snippet? ? nil : awardable.noteable - when MergeRequest, Issue - awardable - when Snippet - nil - end - end - def awardable raise NotImplementedError end diff --git a/app/controllers/dashboard/projects_controller.rb b/app/controllers/dashboard/projects_controller.rb index daeb8fda417..71f18694613 100644 --- a/app/controllers/dashboard/projects_controller.rb +++ b/app/controllers/dashboard/projects_controller.rb @@ -4,10 +4,12 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController include ParamsBackwardCompatibility include RendersMemberAccess include OnboardingExperimentHelper + include SortingHelper + include SortingPreference prepend_before_action(only: [:index]) { authenticate_sessionless_user!(:rss) } before_action :set_non_archived_param - before_action :default_sorting + before_action :set_sorting before_action :projects, only: [:index] skip_cross_project_access_check :index, :starred @@ -59,11 +61,6 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController end end - def default_sorting - params[:sort] ||= 'latest_activity_desc' - @sort = params[:sort] - end - # rubocop: disable CodeReuse/ActiveRecord def load_projects(finder_params) @total_user_projects_count = ProjectsFinder.new(params: { non_public: true }, current_user: current_user).execute @@ -88,4 +85,17 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController Events::RenderService.new(current_user).execute(@events, atom_request: request.format.atom?) end + + def set_sorting + params[:sort] = set_sort_order + @sort = params[:sort] + end + + def default_sort_order + sort_value_latest_activity + end + + def sorting_field + Project::SORTING_PREFERENCE_FIELD + end end diff --git a/app/controllers/explore/projects_controller.rb b/app/controllers/explore/projects_controller.rb index ef86d5f981a..271f2b4b57d 100644 --- a/app/controllers/explore/projects_controller.rb +++ b/app/controllers/explore/projects_controller.rb @@ -3,12 +3,13 @@ class Explore::ProjectsController < Explore::ApplicationController include ParamsBackwardCompatibility include RendersMemberAccess + include SortingHelper + include SortingPreference before_action :set_non_archived_param + before_action :set_sorting def index - params[:sort] ||= 'latest_activity_desc' - @sort = params[:sort] @projects = load_projects respond_to do |format| @@ -23,7 +24,6 @@ class Explore::ProjectsController < Explore::ApplicationController def trending params[:trending] = true - @sort = params[:sort] @projects = load_projects respond_to do |format| @@ -67,4 +67,17 @@ class Explore::ProjectsController < Explore::ApplicationController prepare_projects_for_rendering(projects) end # rubocop: enable CodeReuse/ActiveRecord + + def set_sorting + params[:sort] = set_sort_order + @sort = params[:sort] + end + + def default_sort_order + sort_value_latest_activity + end + + def sorting_field + Project::SORTING_PREFERENCE_FIELD + end end diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index bc9166b9df3..b7fd286bfe0 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -190,7 +190,7 @@ class Projects::IssuesController < Projects::ApplicationController protected - def issuable_sorting_field + def sorting_field Issue::SORTING_PREFERENCE_FIELD end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index f4d381244d9..f4cc0a5851b 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -219,7 +219,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo alias_method :issuable, :merge_request alias_method :awardable, :merge_request - def issuable_sorting_field + def sorting_field MergeRequest::SORTING_PREFERENCE_FIELD end diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb index d1914c35bd3..b187fdb2723 100644 --- a/app/controllers/projects/wikis_controller.rb +++ b/app/controllers/projects/wikis_controller.rb @@ -16,6 +16,10 @@ class Projects::WikisController < Projects::ApplicationController redirect_to(project_wiki_path(@project, @page)) end + def new + redirect_to project_wiki_path(@project, SecureRandom.uuid, random_title: true) + end + def pages @wiki_pages = Kaminari.paginate_array( @project_wiki.list_pages(sort: params[:sort], direction: params[:direction]) @@ -24,17 +28,25 @@ class Projects::WikisController < Projects::ApplicationController @wiki_entries = WikiPage.group_by_directory(@wiki_pages) end + # `#show` handles a number of scenarios: + # + # - If `id` matches a WikiPage, then show the wiki page. + # - If `id` is a file in the wiki repository, then send the file. + # - If we know the user wants to create a new page with the given `id`, + # then display a create form. + # - Otherwise show the empty wiki page and invite the user to create a page. def show - view_param = @project_wiki.empty? ? params[:view] : 'create' - if @page set_encoding_error unless valid_encoding? render 'show' elsif file_blob send_blob(@project_wiki.repository, file_blob) - elsif can?(current_user, :create_wiki, @project) && view_param == 'create' - @page = build_page(title: params[:id]) + elsif show_create_form? + # Assign a title to the WikiPage unless `id` is a randomly generated slug from #new + title = params[:id] unless params[:random_title].present? + + @page = build_page(title: title) render 'edit' else @@ -110,6 +122,15 @@ class Projects::WikisController < Projects::ApplicationController private + def show_create_form? + can?(current_user, :create_wiki, @project) && + @page.nil? && + # Always show the create form when the wiki has had at least one page created. + # Otherwise, we only show the form when the user has navigated from + # the 'empty wiki' page + (@project_wiki.exists? || params[:view] == 'create') + end + def load_project_wiki @project_wiki = load_wiki @@ -135,7 +156,7 @@ class Projects::WikisController < Projects::ApplicationController params.require(:wiki).permit(:title, :content, :format, :message, :last_commit_sha) end - def build_page(args) + def build_page(args = {}) WikiPage.new(@project_wiki).tap do |page| page.update_attributes(args) # rubocop:disable Rails/ActiveRecordAliases end diff --git a/app/finders/award_emojis_finder.rb b/app/finders/award_emojis_finder.rb new file mode 100644 index 00000000000..7320e035409 --- /dev/null +++ b/app/finders/award_emojis_finder.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +class AwardEmojisFinder + attr_reader :awardable, :params + + def initialize(awardable, params = {}) + @awardable = awardable + @params = params + + validate_params + end + + def execute + awards = awardable.award_emoji + awards = by_name(awards) + awards = by_awarded_by(awards) + awards + end + + private + + def by_name(awards) + return awards unless params[:name] + + awards.named(params[:name]) + end + + def by_awarded_by(awards) + return awards unless params[:awarded_by] + + awards.awarded_by(params[:awarded_by]) + end + + def validate_params + return unless params.present? + + validate_name_param + validate_awarded_by_param + end + + def validate_name_param + return unless params[:name] + + raise ArgumentError, 'Invalid name param' unless params[:name].in?(Gitlab::Emoji.emojis_names) + end + + def validate_awarded_by_param + return unless params[:awarded_by] + + # awarded_by can be a `User`, or an ID + unless params[:awarded_by].is_a?(User) || params[:awarded_by].to_s.match(/\A\d+\Z/) + raise ArgumentError, 'Invalid awarded_by param' + end + end +end diff --git a/app/finders/awarded_emoji_finder.rb b/app/finders/awarded_emoji_finder.rb deleted file mode 100644 index f0cc17f3b26..00000000000 --- a/app/finders/awarded_emoji_finder.rb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -# Class for retrieving information about emoji awarded _by_ a particular user. -class AwardedEmojiFinder - attr_reader :current_user - - # current_user - The User to generate the data for. - def initialize(current_user = nil) - @current_user = current_user - end - - def execute - return [] unless current_user - - # We want the resulting data set to be an Array containing the emoji names - # in descending order, based on how often they were awarded. - AwardEmoji - .award_counts_for_user(current_user) - .map { |name, _| { name: name } } - end -end diff --git a/app/graphql/mutations/award_emojis/add.rb b/app/graphql/mutations/award_emojis/add.rb index 8e050dd6d29..85f3eb065bb 100644 --- a/app/graphql/mutations/award_emojis/add.rb +++ b/app/graphql/mutations/award_emojis/add.rb @@ -10,14 +10,11 @@ module Mutations check_object_is_awardable!(awardable) - # TODO this will be handled by AwardEmoji::AddService - # See https://gitlab.com/gitlab-org/gitlab-ce/issues/63372 and - # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/29782 - award = awardable.create_award_emoji(args[:name], current_user) + service = ::AwardEmojis::AddService.new(awardable, args[:name], current_user).execute { - award_emoji: (award if award.persisted?), - errors: errors_on_object(award) + award_emoji: (service[:award] if service[:status] == :success), + errors: service[:errors] || [] } end end diff --git a/app/graphql/mutations/award_emojis/remove.rb b/app/graphql/mutations/award_emojis/remove.rb index 3ba85e445b8..f8a3d0ce390 100644 --- a/app/graphql/mutations/award_emojis/remove.rb +++ b/app/graphql/mutations/award_emojis/remove.rb @@ -10,22 +10,11 @@ module Mutations check_object_is_awardable!(awardable) - # TODO this check can be removed once AwardEmoji services are available. - # See https://gitlab.com/gitlab-org/gitlab-ce/issues/63372 and - # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/29782 - unless awardable.awarded_emoji?(args[:name], current_user) - raise Gitlab::Graphql::Errors::ResourceNotAvailable, - 'You have not awarded emoji of type name to the awardable' - end - - # TODO this will be handled by AwardEmoji::DestroyService - # See https://gitlab.com/gitlab-org/gitlab-ce/issues/63372 and - # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/29782 - awardable.remove_award_emoji(args[:name], current_user) + service = ::AwardEmojis::DestroyService.new(awardable, args[:name], current_user).execute { # Mutation response is always a `nil` award_emoji - errors: [] + errors: service[:errors] || [] } end end diff --git a/app/graphql/mutations/award_emojis/toggle.rb b/app/graphql/mutations/award_emojis/toggle.rb index c03902e8035..d822048f3a6 100644 --- a/app/graphql/mutations/award_emojis/toggle.rb +++ b/app/graphql/mutations/award_emojis/toggle.rb @@ -15,23 +15,15 @@ module Mutations check_object_is_awardable!(awardable) - # TODO this will be handled by AwardEmoji::ToggleService - # See https://gitlab.com/gitlab-org/gitlab-ce/issues/63372 and - # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/29782 - award = awardable.toggle_award_emoji(args[:name], current_user) - - # Destroy returns a collection :( - award = award.first if award.is_a?(Array) - - errors = errors_on_object(award) + service = ::AwardEmojis::ToggleService.new(awardable, args[:name], current_user).execute toggled_on = awardable.awarded_emoji?(args[:name], current_user) { # For consistency with the AwardEmojis::Remove mutation, only return # the AwardEmoji if it was created and not destroyed - award_emoji: (award if toggled_on), - errors: errors, + award_emoji: (service[:award] if toggled_on), + errors: service[:errors] || [], toggled_on: toggled_on } end diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index e2e007eee50..b88b25eb845 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -405,7 +405,11 @@ module IssuablesHelper placement: is_collapsed ? 'left' : nil, container: is_collapsed ? 'body' : nil, boundary: 'viewport', - is_collapsed: is_collapsed + is_collapsed: is_collapsed, + track_label: "right_sidebar", + track_property: "update_todo", + track_event: "click_button", + track_value: "" } end diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index 5d292094a05..3683f2ea9a9 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -125,9 +125,8 @@ class Notify < BaseMailer def mail_thread(model, headers = {}) add_project_headers add_unsubscription_headers_and_links + add_model_headers(model) - headers["X-GitLab-#{model.class.name}-ID"] = model.id - headers["X-GitLab-#{model.class.name}-IID"] = model.iid if model.respond_to?(:iid) headers['X-GitLab-Reply-Key'] = reply_key @reason = headers['X-GitLab-NotificationReason'] @@ -196,6 +195,18 @@ class Notify < BaseMailer @reply_key ||= SentNotification.reply_key end + # This method applies threading headers to the email to identify + # the instance we are discussing. + # + # All model instances must have `#id`, and may implement `#iid`. + def add_model_headers(object) + # Use replacement so we don't strip the module. + prefix = "X-GitLab-#{object.class.name.gsub(/::/, '-')}" + + headers["#{prefix}-ID"] = object.id + headers["#{prefix}-IID"] = object.iid if object.respond_to?(:iid) + end + def add_project_headers return unless @project diff --git a/app/models/award_emoji.rb b/app/models/award_emoji.rb index e26162f6151..0ab302a0f3e 100644 --- a/app/models/award_emoji.rb +++ b/app/models/award_emoji.rb @@ -16,8 +16,10 @@ class AwardEmoji < ApplicationRecord participant :user - scope :downvotes, -> { where(name: DOWNVOTE_NAME) } - scope :upvotes, -> { where(name: UPVOTE_NAME) } + scope :downvotes, -> { named(DOWNVOTE_NAME) } + scope :upvotes, -> { named(UPVOTE_NAME) } + scope :named, -> (names) { where(name: names) } + scope :awarded_by, -> (users) { where(user: users) } after_save :expire_etag_cache after_destroy :expire_etag_cache diff --git a/app/models/concerns/awardable.rb b/app/models/concerns/awardable.rb index 14bc56f0eee..f229b42ade6 100644 --- a/app/models/concerns/awardable.rb +++ b/app/models/concerns/awardable.rb @@ -106,30 +106,6 @@ module Awardable end def awarded_emoji?(emoji_name, current_user) - award_emoji.where(name: emoji_name, user: current_user).exists? - end - - def create_award_emoji(name, current_user) - return unless emoji_awardable? - - award_emoji.create(name: normalize_name(name), user: current_user) - end - - def remove_award_emoji(name, current_user) - award_emoji.where(name: name, user: current_user).destroy_all # rubocop: disable DestroyAll - end - - def toggle_award_emoji(emoji_name, current_user) - if awarded_emoji?(emoji_name, current_user) - remove_award_emoji(emoji_name, current_user) - else - create_award_emoji(emoji_name, current_user) - end - end - - private - - def normalize_name(name) - Gitlab::Emoji.normalize_emoji_name(name) + award_emoji.named(emoji_name).awarded_by(current_user).exists? end end diff --git a/app/models/project.rb b/app/models/project.rb index 8efe4b06f87..10679fb1f85 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -55,6 +55,8 @@ class Project < ApplicationRecord VALID_MIRROR_PORTS = [22, 80, 443].freeze VALID_MIRROR_PROTOCOLS = %w(http https ssh git).freeze + SORTING_PREFERENCE_FIELD = :projects_sort + cache_markdown_field :description, pipeline: :description delegate :feature_available?, :builds_enabled?, :wiki_enabled?, diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb index c91add6439f..4a19e05bf76 100644 --- a/app/models/project_wiki.rb +++ b/app/models/project_wiki.rb @@ -85,6 +85,10 @@ class ProjectWiki list_pages(limit: 1).empty? end + def exists? + !empty? + end + # Lists wiki pages of the repository. # # limit - max number of pages returned by the method. diff --git a/app/serializers/deployment_entity.rb b/app/serializers/deployment_entity.rb index 6e91317eb20..d6466ad1cdd 100644 --- a/app/serializers/deployment_entity.rb +++ b/app/serializers/deployment_entity.rb @@ -18,7 +18,7 @@ class DeploymentEntity < Grape::Entity end expose :created_at - expose :finished_at + expose :deployed_at expose :tag expose :last? expose :user, using: UserEntity diff --git a/app/serializers/deployment_serializer.rb b/app/serializers/deployment_serializer.rb index 3fd3e1b9cc8..b48037dd53f 100644 --- a/app/serializers/deployment_serializer.rb +++ b/app/serializers/deployment_serializer.rb @@ -4,7 +4,7 @@ class DeploymentSerializer < BaseSerializer entity DeploymentEntity def represent_concise(resource, opts = {}) - opts[:only] = [:iid, :id, :sha, :created_at, :finished_at, :tag, :last?, :id, ref: [:name]] + opts[:only] = [:iid, :id, :sha, :created_at, :deployed_at, :tag, :last?, :id, ref: [:name]] represent(resource, opts) end end diff --git a/app/serializers/issuable_sidebar_basic_entity.rb b/app/serializers/issuable_sidebar_basic_entity.rb index c02fd024345..058c707ef9d 100644 --- a/app/serializers/issuable_sidebar_basic_entity.rb +++ b/app/serializers/issuable_sidebar_basic_entity.rb @@ -4,6 +4,7 @@ class IssuableSidebarBasicEntity < Grape::Entity include RequestAwareEntity expose :id + expose :iid expose :type do |issuable| issuable.to_ability_name end diff --git a/app/serializers/merge_request_serializer.rb b/app/serializers/merge_request_serializer.rb index 8ad1df5dfe0..bd2e682a122 100644 --- a/app/serializers/merge_request_serializer.rb +++ b/app/serializers/merge_request_serializer.rb @@ -8,7 +8,7 @@ class MergeRequestSerializer < BaseSerializer entity ||= case opts[:serializer] when 'sidebar' - IssuableSidebarBasicEntity + MergeRequestSidebarBasicEntity when 'sidebar_extras' MergeRequestSidebarExtrasEntity when 'basic' diff --git a/app/serializers/merge_request_sidebar_basic_entity.rb b/app/serializers/merge_request_sidebar_basic_entity.rb new file mode 100644 index 00000000000..3c911bbe4c8 --- /dev/null +++ b/app/serializers/merge_request_sidebar_basic_entity.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class MergeRequestSidebarBasicEntity < IssuableSidebarBasicEntity + expose :current_user, if: lambda { |_issuable| current_user } do + expose :can_merge do |merge_request| + merge_request.can_be_merged_by?(current_user) + end + end +end diff --git a/app/services/award_emojis/add_service.rb b/app/services/award_emojis/add_service.rb new file mode 100644 index 00000000000..eac15dabbf0 --- /dev/null +++ b/app/services/award_emojis/add_service.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +module AwardEmojis + class AddService < AwardEmojis::BaseService + include Gitlab::Utils::StrongMemoize + + def execute + unless awardable.user_can_award?(current_user) + return error('User cannot award emoji to awardable', status: :forbidden) + end + + unless awardable.emoji_awardable? + return error('Awardable cannot be awarded emoji', status: :unprocessable_entity) + end + + award = awardable.award_emoji.create(name: name, user: current_user) + + if award.persisted? + TodoService.new.new_award_emoji(todoable, current_user) if todoable + success(award: award) + else + error(award.errors.full_messages, award: award) + end + end + + private + + def todoable + strong_memoize(:todoable) do + case awardable + when Note + # We don't create todos for personal snippet comments for now + awardable.noteable unless awardable.for_personal_snippet? + when MergeRequest, Issue + awardable + when Snippet + nil + end + end + end + end +end diff --git a/app/services/award_emojis/base_service.rb b/app/services/award_emojis/base_service.rb new file mode 100644 index 00000000000..a677d03a221 --- /dev/null +++ b/app/services/award_emojis/base_service.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module AwardEmojis + class BaseService < ::BaseService + attr_accessor :awardable, :name + + def initialize(awardable, name, current_user) + @awardable = awardable + @name = normalize_name(name) + + super(awardable.project, current_user) + end + + private + + def normalize_name(name) + Gitlab::Emoji.normalize_emoji_name(name) + end + + # Provide more error state data than what BaseService allows. + # - An array of errors + # - The `AwardEmoji` if present + def error(errors, award: nil, status: nil) + errors = Array.wrap(errors) + + super(errors.to_sentence.presence, status).merge({ + award: award, + errors: errors + }) + end + end +end diff --git a/app/services/award_emojis/collect_user_emoji_service.rb b/app/services/award_emojis/collect_user_emoji_service.rb new file mode 100644 index 00000000000..6cab23f3edf --- /dev/null +++ b/app/services/award_emojis/collect_user_emoji_service.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +# Class for retrieving information about emoji awarded _by_ a particular user. +module AwardEmojis + class CollectUserEmojiService + attr_reader :current_user + + # current_user - The User to generate the data for. + def initialize(current_user = nil) + @current_user = current_user + end + + def execute + return [] unless current_user + + # We want the resulting data set to be an Array containing the emoji names + # in descending order, based on how often they were awarded. + AwardEmoji + .award_counts_for_user(current_user) + .map { |name, _| { name: name } } + end + end +end diff --git a/app/services/award_emojis/destroy_service.rb b/app/services/award_emojis/destroy_service.rb new file mode 100644 index 00000000000..3789a8403bc --- /dev/null +++ b/app/services/award_emojis/destroy_service.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module AwardEmojis + class DestroyService < AwardEmojis::BaseService + def execute + unless awardable.user_can_award?(current_user) + return error('User cannot destroy emoji on the awardable', status: :forbidden) + end + + awards = AwardEmojisFinder.new(awardable, name: name, awarded_by: current_user).execute + + if awards.empty? + return error("User has not awarded emoji of type #{name} on the awardable", status: :forbidden) + end + + award = awards.destroy_all.first # rubocop: disable DestroyAll + + success(award: award) + end + end +end diff --git a/app/services/award_emojis/toggle_service.rb b/app/services/award_emojis/toggle_service.rb new file mode 100644 index 00000000000..812dd1c2889 --- /dev/null +++ b/app/services/award_emojis/toggle_service.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module AwardEmojis + class ToggleService < AwardEmojis::BaseService + def execute + if awardable.awarded_emoji?(name, current_user) + DestroyService.new(awardable, name, current_user).execute + else + AddService.new(awardable, name, current_user).execute + end + end + end +end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 77c2224ee3b..2ab6e88599f 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -344,10 +344,7 @@ class IssuableBaseService < BaseService def toggle_award(issuable) award = params.delete(:emoji_award) - if award - todo_service.new_award_emoji(issuable, current_user) - issuable.toggle_award_emoji(award, current_user) - end + AwardEmojis::ToggleService.new(issuable, award, current_user).execute if award end def associations_before_update(issuable) diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index e30debbbe75..ee7223d6349 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -598,11 +598,11 @@ module SystemNoteService end def zoom_link_added(issue, project, author) - create_note(NoteSummary.new(issue, project, author, _('a Zoom call was added to this issue'), action: 'pinned_embed')) + create_note(NoteSummary.new(issue, project, author, _('added a Zoom call to this issue'), action: 'pinned_embed')) end def zoom_link_removed(issue, project, author) - create_note(NoteSummary.new(issue, project, author, _('a Zoom call was removed from this issue'), action: 'pinned_embed')) + create_note(NoteSummary.new(issue, project, author, _('removed a Zoom call from this issue'), action: 'pinned_embed')) end private diff --git a/app/views/admin/applications/_delete_form.html.haml b/app/views/admin/applications/_delete_form.html.haml index 82781f6716d..86f09bf1cb0 100644 --- a/app/views/admin/applications/_delete_form.html.haml +++ b/app/views/admin/applications/_delete_form.html.haml @@ -1,4 +1,4 @@ - submit_btn_css ||= 'btn btn-link btn-remove btn-sm' = form_tag admin_application_path(application) do %input{ :name => "_method", :type => "hidden", :value => "delete" }/ - = submit_tag 'Destroy', onclick: "return confirm('Are you sure?')", class: submit_btn_css + = submit_tag 'Destroy', class: submit_btn_css, data: { confirm: _('Are you sure?') } diff --git a/app/views/doorkeeper/authorized_applications/_delete_form.html.haml b/app/views/doorkeeper/authorized_applications/_delete_form.html.haml index 69cc510e9c1..9bc5e2ee42f 100644 --- a/app/views/doorkeeper/authorized_applications/_delete_form.html.haml +++ b/app/views/doorkeeper/authorized_applications/_delete_form.html.haml @@ -5,4 +5,4 @@ = form_tag path do %input{ :name => "_method", :type => "hidden", :value => "delete" }/ - = submit_tag _('Revoke'), onclick: "return confirm('#{_('Are you sure?')}')", class: 'btn btn-remove btn-sm' + = submit_tag _('Revoke'), class: 'btn btn-remove btn-sm', data: { confirm: _('Are you sure?') } diff --git a/app/views/errors/access_denied.html.haml b/app/views/errors/access_denied.html.haml index 46931b5932d..1ed7b56db1d 100644 --- a/app/views/errors/access_denied.html.haml +++ b/app/views/errors/access_denied.html.haml @@ -10,7 +10,7 @@ = message %p = s_('403|Please contact your GitLab administrator to get permission.') - .action-container.js-go-back{ style: 'display: none' } - %a{ href: 'javascript:history.back()', class: 'btn btn-success' } + .action-container.js-go-back{ hidden: true } + %button{ type: 'button', class: 'btn btn-success' } = s_('Go Back') = render "errors/footer" diff --git a/app/views/layouts/errors.html.haml b/app/views/layouts/errors.html.haml index 74484005b48..dc924a0e25d 100644 --- a/app/views/layouts/errors.html.haml +++ b/app/views/layouts/errors.html.haml @@ -14,6 +14,10 @@ var goBackElement = document.querySelector('.js-go-back'); if (goBackElement && history.length > 1) { - goBackElement.style.display = 'block'; + goBackElement.removeAttribute('hidden'); + + goBackElement.querySelector('button').addEventListener('click', function() { + history.back(); + }); } }()); diff --git a/app/views/profiles/personal_access_tokens/index.html.haml b/app/views/profiles/personal_access_tokens/index.html.haml index 65ef9690062..08a39fc4f58 100644 --- a/app/views/profiles/personal_access_tokens/index.html.haml +++ b/app/views/profiles/personal_access_tokens/index.html.haml @@ -31,7 +31,7 @@ = s_('AccessTokens|It cannot be used to access any other data.') .col-lg-8.feed-token-reset = label_tag :feed_token, s_('AccessTokens|Feed token'), class: "label-bold" - = text_field_tag :feed_token, current_user.feed_token, class: 'form-control', readonly: true, onclick: 'this.select()' + = text_field_tag :feed_token, current_user.feed_token, class: 'form-control js-select-on-focus', readonly: true %p.form-text.text-muted - reset_link = link_to s_('AccessTokens|reset it'), [:reset, :feed_token, :profile], method: :put, data: { confirm: s_('AccessTokens|Are you sure? Any RSS or calendar URLs currently in use will stop working.') } - reset_message = s_('AccessTokens|Keep this token secret. Anyone who gets ahold of it can read activity and issue RSS feeds or your calendar feed as if they were you. You should %{link_reset_it} if that ever happens.') % { link_reset_it: reset_link } @@ -49,7 +49,7 @@ = s_('AccessTokens|It cannot be used to access any other data.') .col-lg-8.incoming-email-token-reset = label_tag :incoming_email_token, s_('AccessTokens|Incoming email token'), class: "label-bold" - = text_field_tag :incoming_email_token, current_user.incoming_email_token, class: 'form-control', readonly: true, onclick: 'this.select()' + = text_field_tag :incoming_email_token, current_user.incoming_email_token, class: 'form-control js-select-on-focus', readonly: true %p.form-text.text-muted - reset_link = link_to s_('AccessTokens|reset it'), [:reset, :incoming_email_token, :profile], method: :put, data: { confirm: s_('AccessTokens|Are you sure? Any issue email addresses currently in use will stop working.') } - reset_message = s_('AccessTokens|Keep this token secret. Anyone who gets ahold of it can create issues as if they were you. You should %{link_reset_it} if that ever happens.') % { link_reset_it: reset_link } diff --git a/app/views/projects/wikis/_form.html.haml b/app/views/projects/wikis/_form.html.haml index 858731b2dda..a153f527ee0 100644 --- a/app/views/projects/wikis/_form.html.haml +++ b/app/views/projects/wikis/_form.html.haml @@ -1,8 +1,8 @@ -- commit_message = @page.persisted? ? s_("WikiPageEdit|Update %{page_title}") : s_("WikiPageCreate|Create %{page_title}") -- commit_message = commit_message % { page_title: @page.title } +- form_classes = 'wiki-form common-note-form prepend-top-default js-quick-submit' +- form_classes += ' js-new-wiki-page' unless @page.persisted? = form_for [@project.namespace.becomes(Namespace), @project, @page], method: @page.persisted? ? :put : :post, - html: { class: 'wiki-form common-note-form prepend-top-default js-quick-submit' }, + html: { class: form_classes }, data: { uploads_path: uploads_path } do |f| = form_errors(@page) @@ -12,12 +12,14 @@ .form-group.row .col-sm-12= f.label :title, class: 'control-label-full-width' .col-sm-12 - = f.text_field :title, class: 'form-control qa-wiki-title-textbox', value: @page.title - - if @page.persisted? - %span.d-inline-block.mw-100.prepend-top-5 - = icon('lightbulb-o') + = f.text_field :title, class: 'form-control qa-wiki-title-textbox', value: @page.title, required: true, autofocus: !@page.persisted?, placeholder: _('Wiki|Page title') + %span.d-inline-block.mw-100.prepend-top-5 + = icon('lightbulb-o') + - if @page.persisted? = s_("WikiEditPageTip|Tip: You can move this page by adding the path to the beginning of the title.") = link_to icon('question-circle'), help_page_path('user/project/wiki/index', anchor: 'moving-a-wiki-page'), target: '_blank' + - else + = s_("WikiNewPageTip|Tip: You can specify the full path for the new file. We will automatically create any missing directories.") .form-group.row .col-sm-12= f.label :format, class: 'control-label-full-width' .col-sm-12 @@ -43,7 +45,7 @@ .form-group.row .col-sm-12= f.label :commit_message, class: 'control-label-full-width' - .col-sm-12= f.text_field :message, class: 'form-control qa-wiki-message-textbox', rows: 18, value: commit_message + .col-sm-12= f.text_field :message, class: 'form-control qa-wiki-message-textbox', rows: 18, value: nil .form-actions - if @page && @page.persisted? diff --git a/app/views/projects/wikis/_main_links.html.haml b/app/views/projects/wikis/_main_links.html.haml index 643b51e01d1..2e1e176c42a 100644 --- a/app/views/projects/wikis/_main_links.html.haml +++ b/app/views/projects/wikis/_main_links.html.haml @@ -1,9 +1,9 @@ - if (@page && @page.persisted?) - if can?(current_user, :create_wiki, @project) - = link_to '#modal-new-wiki', class: "add-new-wiki btn btn-success", "data-toggle" => "modal" do + = link_to project_wikis_new_path(@project), class: "add-new-wiki btn btn-success", role: "button" do = s_("Wiki|New page") - = link_to project_wiki_history_path(@project, @page), class: "btn" do + = link_to project_wiki_history_path(@project, @page), class: "btn", role: "button" do = s_("Wiki|Page history") - if can?(current_user, :create_wiki, @project) && @page.latest? && @valid_encoding - = link_to project_wiki_edit_path(@project, @page), class: "btn js-wiki-edit" do + = link_to project_wiki_edit_path(@project, @page), class: "btn js-wiki-edit", role: "button" do = _("Edit") diff --git a/app/views/projects/wikis/_new.html.haml b/app/views/projects/wikis/_new.html.haml deleted file mode 100644 index 2c675c0de9c..00000000000 --- a/app/views/projects/wikis/_new.html.haml +++ /dev/null @@ -1,18 +0,0 @@ -#modal-new-wiki.modal - .modal-dialog - .modal-content - .modal-header - %h3.page-title= s_("WikiNewPageTitle|New Wiki Page") - %button.close{ type: "button", "data-dismiss": "modal", "aria-label" => _('Close') } - %span{ "aria-hidden": true } × - .modal-body - %form.new-wiki-page - .form-group - = label_tag :new_wiki_path do - %span= s_("WikiPage|Page slug") - = text_field_tag :new_wiki_path, nil, placeholder: s_("WikiNewPagePlaceholder|how-to-setup"), class: 'form-control', required: true, :'data-wikis-path' => project_wikis_path(@project), autofocus: true - %span.d-inline-block.mw-100.prepend-top-5 - = icon('lightbulb-o') - = s_("WikiNewPageTip|Tip: You can specify the full path for the new file. We will automatically create any missing directories.") - .form-actions - = button_tag s_("Wiki|Create page"), class: "build-new-wiki btn btn-success" diff --git a/app/views/projects/wikis/_sidebar.html.haml b/app/views/projects/wikis/_sidebar.html.haml index 28353927135..a9d21470944 100644 --- a/app/views/projects/wikis/_sidebar.html.haml +++ b/app/views/projects/wikis/_sidebar.html.haml @@ -19,5 +19,3 @@ .block = link_to project_wikis_pages_path(@project), class: 'btn btn-block' do = s_("Wiki|More Pages") - -= render 'projects/wikis/new' diff --git a/app/views/projects/wikis/edit.html.haml b/app/views/projects/wikis/edit.html.haml index e8b59a3b8c4..815b4a51261 100644 --- a/app/views/projects/wikis/edit.html.haml +++ b/app/views/projects/wikis/edit.html.haml @@ -13,14 +13,11 @@ %h2.wiki-page-title - if @page.persisted? = link_to @page.human_title, project_wiki_path(@project, @page) - - else - = @page.human_title - %span.light - · - - if @page.persisted? + %span.light + · = s_("Wiki|Edit Page") - - else - = s_("Wiki|Create Page") + - else + = s_("Wiki|Create New Page") .nav-controls - if @page.persisted? diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index 825088a58e7..aea09bf8d4d 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -38,7 +38,7 @@ = _('Milestone') = icon('spinner spin', class: 'hidden block-loading', 'aria-hidden': 'true') - if can_edit_issuable - = link_to _('Edit'), '#', class: 'js-sidebar-dropdown-toggle edit-link float-right' + = link_to _('Edit'), '#', class: 'js-sidebar-dropdown-toggle edit-link float-right', data: { track_label: "right_sidebar", track_property: "milestone", track_event: "click_edit_button", track_value: "" } .value.hide-collapsed - if milestone.present? = link_to milestone[:title], milestone[:web_url], class: "bold has-tooltip", title: sidebar_milestone_remaining_days(milestone), data: { container: "body", html: 'true', boundary: 'viewport' } @@ -66,7 +66,7 @@ = _('Due date') = icon('spinner spin', class: 'hidden block-loading', 'aria-hidden': 'true') - if can_edit_issuable - = link_to _('Edit'), '#', class: 'js-sidebar-dropdown-toggle edit-link float-right' + = link_to _('Edit'), '#', class: 'js-sidebar-dropdown-toggle edit-link float-right', data: { track_label: "right_sidebar", track_property: "due_date", track_event: "click_edit_button", track_value: "" } .value.hide-collapsed %span.value-content - if issuable_sidebar[:due_date] @@ -102,7 +102,7 @@ = _('Labels') = icon('spinner spin', class: 'hidden block-loading', 'aria-hidden': 'true') - if can_edit_issuable - = link_to _('Edit'), '#', class: 'js-sidebar-dropdown-toggle edit-link qa-edit-link-labels float-right' + = link_to _('Edit'), '#', class: 'js-sidebar-dropdown-toggle edit-link qa-edit-link-labels float-right', data: { track_label: "right_sidebar", track_property: "labels", track_event: "click_edit_button", track_value: "" } .value.issuable-show-labels.dont-hide.hide-collapsed.qa-labels-block{ class: ("has-labels" if selected_labels.any?) } - if selected_labels.any? - selected_labels.each do |label_hash| @@ -160,7 +160,7 @@ = custom_icon('icon_arrow_right') .dropdown.sidebar-move-issue-dropdown.hide-collapsed %button.btn.btn-default.btn-block.js-sidebar-dropdown-toggle.js-move-issue{ type: 'button', - data: { toggle: 'dropdown', display: 'static' } } + data: { toggle: 'dropdown', display: 'static', track_label: "right_sidebar", track_property: "move_issue", track_event: "click_button", track_value: "" } } = _('Move issue') .dropdown-menu.dropdown-menu-selectable.dropdown-extended-height = dropdown_title(_('Move issue')) diff --git a/app/views/shared/issuable/_sidebar_assignees.html.haml b/app/views/shared/issuable/_sidebar_assignees.html.haml index ab01094ed6e..1dc538826dc 100644 --- a/app/views/shared/issuable/_sidebar_assignees.html.haml +++ b/app/views/shared/issuable/_sidebar_assignees.html.haml @@ -20,6 +20,8 @@ placeholder: _('Search users'), data: { first_user: issuable_sidebar.dig(:current_user, :username), current_user: true, + iid: issuable_sidebar[:iid], + issuable_type: issuable_type, project_id: issuable_sidebar[:project_id], author_id: issuable_sidebar[:author_id], field_name: "#{issuable_type}[assignee_ids][]", diff --git a/changelogs/unreleased/32032-html-code-shown-in-merge-request.yml b/changelogs/unreleased/32032-html-code-shown-in-merge-request.yml new file mode 100644 index 00000000000..ffd58067784 --- /dev/null +++ b/changelogs/unreleased/32032-html-code-shown-in-merge-request.yml @@ -0,0 +1,5 @@ +--- +title: Fix HTML rendering for fast-forward rebases in merge request widget +merge_request: 32032 +author: +type: fixed diff --git a/changelogs/unreleased/46299-wiki-page-creation.yml b/changelogs/unreleased/46299-wiki-page-creation.yml new file mode 100644 index 00000000000..2e8f2accf45 --- /dev/null +++ b/changelogs/unreleased/46299-wiki-page-creation.yml @@ -0,0 +1,5 @@ +--- +title: Remove wiki page slug dialog step when creating wiki page +merge_request: 31362 +author: +type: changed diff --git a/changelogs/unreleased/56130-deployed_at.yml b/changelogs/unreleased/56130-deployed_at.yml new file mode 100644 index 00000000000..c53658de752 --- /dev/null +++ b/changelogs/unreleased/56130-deployed_at.yml @@ -0,0 +1,5 @@ +--- +title: Replace finished_at with deployed_at for the internal API Deployment entity +merge_request: 32000 +author: +type: other diff --git a/changelogs/unreleased/59786-show-renamed-file-in-mr.yml b/changelogs/unreleased/59786-show-renamed-file-in-mr.yml new file mode 100644 index 00000000000..e8c52b592d2 --- /dev/null +++ b/changelogs/unreleased/59786-show-renamed-file-in-mr.yml @@ -0,0 +1,5 @@ +--- +title: Fix to show renamed file in mr +merge_request: 31888 +author: +type: changed diff --git a/changelogs/unreleased/64383-pattern-matching-with-variables-causes-gitlabs-ci-lint-to-throw-500.yml b/changelogs/unreleased/64383-pattern-matching-with-variables-causes-gitlabs-ci-lint-to-throw-500.yml new file mode 100644 index 00000000000..ba4bd614170 --- /dev/null +++ b/changelogs/unreleased/64383-pattern-matching-with-variables-causes-gitlabs-ci-lint-to-throw-500.yml @@ -0,0 +1,5 @@ +--- +title: Fix 500 errors caused by pattern matching with variables in CI Lint +merge_request: 31719 +author: +type: fixed diff --git a/changelogs/unreleased/65412-add-support-for-line-charts.yml b/changelogs/unreleased/65412-add-support-for-line-charts.yml new file mode 100644 index 00000000000..cb9043596b7 --- /dev/null +++ b/changelogs/unreleased/65412-add-support-for-line-charts.yml @@ -0,0 +1,5 @@ +--- +title: Create component to display area and line charts in monitor dashboards +merge_request: 31639 +author: +type: added diff --git a/changelogs/unreleased/65427-improve-system-notes-for-zoom-links.yml b/changelogs/unreleased/65427-improve-system-notes-for-zoom-links.yml new file mode 100644 index 00000000000..d081620c710 --- /dev/null +++ b/changelogs/unreleased/65427-improve-system-notes-for-zoom-links.yml @@ -0,0 +1,5 @@ +--- +title: Improve system notes for Zoom links +merge_request: 31410 +author: Jacopo Beschi @jacopo-beschi +type: changed diff --git a/changelogs/unreleased/ce-22058-improve-ux-multi-assignees-in-mr.yml b/changelogs/unreleased/ce-22058-improve-ux-multi-assignees-in-mr.yml new file mode 100644 index 00000000000..e7453a2b9bd --- /dev/null +++ b/changelogs/unreleased/ce-22058-improve-ux-multi-assignees-in-mr.yml @@ -0,0 +1,5 @@ +--- +title: Update assignee (cannot merge) style +merge_request: 31545 +author: +type: changed diff --git a/changelogs/unreleased/georgekoltsov-18720-persistent-dashboard-sort.yml b/changelogs/unreleased/georgekoltsov-18720-persistent-dashboard-sort.yml new file mode 100644 index 00000000000..7eed8550acd --- /dev/null +++ b/changelogs/unreleased/georgekoltsov-18720-persistent-dashboard-sort.yml @@ -0,0 +1,5 @@ +--- +title: Add persistance to last choice of projects sorting on projects dashboard page +merge_request: 31669 +author: +type: added diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index 7217f098fd9..9f3e104bc2b 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -28,11 +28,13 @@ if Rails.env.development? end enable_json_logs = Gitlab.config.sidekiq.log_format == 'json' +enable_sidekiq_monitor = ENV.fetch("SIDEKIQ_MONITOR_WORKER", 0).to_i.nonzero? Sidekiq.configure_server do |config| config.redis = queues_config_hash config.server_middleware do |chain| + chain.add Gitlab::SidekiqMiddleware::Monitor if enable_sidekiq_monitor chain.add Gitlab::SidekiqMiddleware::Metrics if Settings.monitoring.sidekiq_exporter chain.add Gitlab::SidekiqMiddleware::ArgumentsLogger if ENV['SIDEKIQ_LOG_ARGUMENTS'] && !enable_json_logs chain.add Gitlab::SidekiqMiddleware::MemoryKiller if ENV['SIDEKIQ_MEMORY_KILLER_MAX_RSS'] @@ -57,6 +59,8 @@ Sidekiq.configure_server do |config| # Clear any connections that might have been obtained before starting # Sidekiq (e.g. in an initializer). ActiveRecord::Base.clear_all_connections! + + Gitlab::SidekiqMonitor.instance.start if enable_sidekiq_monitor end if enable_reliable_fetch? diff --git a/config/prometheus/common_metrics.yml b/config/prometheus/common_metrics.yml index 32475ef8380..08504d6f7d5 100644 --- a/config/prometheus/common_metrics.yml +++ b/config/prometheus/common_metrics.yml @@ -166,7 +166,7 @@ panel_groups: label: Total (cores) unit: "cores" - title: "Memory Usage (Pod average)" - type: "area-chart" + type: "line-chart" y_label: "Memory Used per Pod (MB)" weight: 2 metrics: @@ -175,7 +175,7 @@ panel_groups: label: Pod average (MB) unit: MB - title: "Canary: Memory Usage (Pod Average)" - type: "area-chart" + type: "line-chart" y_label: "Memory Used per Pod (MB)" weight: 2 metrics: @@ -185,7 +185,7 @@ panel_groups: unit: MB track: canary - title: "Core Usage (Pod Average)" - type: "area-chart" + type: "line-chart" y_label: "Cores per Pod" weight: 1 metrics: @@ -194,7 +194,7 @@ panel_groups: label: Pod average (cores) unit: "cores" - title: "Canary: Core Usage (Pod Average)" - type: "area-chart" + type: "line-chart" y_label: "Cores per Pod" weight: 1 metrics: diff --git a/config/routes/wiki.rb b/config/routes/wiki.rb index 2ca52e55fca..d439c99270e 100644 --- a/config/routes/wiki.rb +++ b/config/routes/wiki.rb @@ -2,6 +2,7 @@ scope(controller: :wikis) do scope(path: 'wikis', as: :wikis) do get :git_access get :pages + get :new get '/', to: redirect('%{namespace_id}/%{project_id}/wikis/home') post '/', to: 'wikis#create' end diff --git a/danger/only_documentation/Dangerfile b/danger/only_documentation/Dangerfile index ff65f8713d2..dad12c0d29c 100644 --- a/danger/only_documentation/Dangerfile +++ b/danger/only_documentation/Dangerfile @@ -1,7 +1,7 @@ # rubocop:disable Style/SignalException # frozen_string_literal: true -has_only_docs_changes = helper.all_changed_files.all? { |file| file.start_with?('doc/', '.gitlab/ci/docs.gitlab-ci.yml', '.mdlrc') } +has_only_docs_changes = helper.all_changed_files.all? { |file| file.start_with?('doc/', '.gitlab/ci/docs.gitlab-ci.yml', '.mdlrc') || file.end_with?('.md') } is_docs_only_branch = gitlab.branch_for_head =~ /(^docs[\/-].*|.*-docs$)/ if is_docs_only_branch && !has_only_docs_changes diff --git a/db/fixtures/development/15_award_emoji.rb b/db/fixtures/development/15_award_emoji.rb index 137a036edaf..a9dcc048586 100644 --- a/db/fixtures/development/15_award_emoji.rb +++ b/db/fixtures/development/15_award_emoji.rb @@ -1,35 +1,22 @@ require './spec/support/sidekiq' Gitlab::Seeder.quiet do - emoji = Gitlab::Emoji.emojis.keys + EMOJI = Gitlab::Emoji.emojis.keys - Issue.order(Gitlab::Database.random).limit(Issue.count / 2).each do |issue| - project = issue.project + def seed_award_emoji(klass) + klass.order(Gitlab::Database.random).limit(klass.count / 2).each do |awardable| + awardable.project.authorized_users.where('project_authorizations.access_level > ?', Gitlab::Access::GUEST).sample(2).each do |user| + AwardEmojis::AddService.new(awardable, EMOJI.sample, user).execute - project.team.users.sample(2).each do |user| - issue.create_award_emoji(emoji.sample, user) + awardable.notes.user.sample(2).each do |note| + AwardEmojis::AddService.new(note, EMOJI.sample, user).execute + end - issue.notes.sample(2).each do |note| - next if note.system? - note.create_award_emoji(emoji.sample, user) + print '.' end - - print '.' end end - MergeRequest.order(Gitlab::Database.random).limit(MergeRequest.count / 2).each do |mr| - project = mr.project - - project.team.users.sample(2).each do |user| - mr.create_award_emoji(emoji.sample, user) - - mr.notes.sample(2).each do |note| - next if note.system? - note.create_award_emoji(emoji.sample, user) - end - - print '.' - end - end + seed_award_emoji(Issue) + seed_award_emoji(MergeRequest) end diff --git a/db/migrate/20190808152507_add_projects_sorting_field_to_user_preferences.rb b/db/migrate/20190808152507_add_projects_sorting_field_to_user_preferences.rb new file mode 100644 index 00000000000..941fead655e --- /dev/null +++ b/db/migrate/20190808152507_add_projects_sorting_field_to_user_preferences.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class AddProjectsSortingFieldToUserPreferences < ActiveRecord::Migration[5.2] + DOWNTIME = false + + def up + add_column :user_preferences, :projects_sort, :string, limit: 64 + end + + def down + remove_column :user_preferences, :projects_sort + end +end diff --git a/db/migrate/20190814205640_import_common_metrics_line_charts.rb b/db/migrate/20190814205640_import_common_metrics_line_charts.rb new file mode 100644 index 00000000000..1c28d686a42 --- /dev/null +++ b/db/migrate/20190814205640_import_common_metrics_line_charts.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class ImportCommonMetricsLineCharts < ActiveRecord::Migration[5.2] + DOWNTIME = false + + def up + ::Gitlab::DatabaseImporters::CommonMetrics::Importer.new.execute + end + + def down + # no-op + end +end diff --git a/db/schema.rb b/db/schema.rb index ce5fd38129a..3f7917654cf 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -3411,6 +3411,7 @@ ActiveRecord::Schema.define(version: 2019_08_15_093949) do t.string "epics_sort" t.integer "roadmap_epics_state" t.string "roadmaps_sort" + t.string "projects_sort", limit: 64 t.index ["user_id"], name: "index_user_preferences_on_user_id", unique: true end diff --git a/doc/administration/container_registry.md b/doc/administration/container_registry.md index d0adeb89543..d43b3718bf9 100644 --- a/doc/administration/container_registry.md +++ b/doc/administration/container_registry.md @@ -137,6 +137,15 @@ otherwise you will run into conflicts. 1. Save the file and [reconfigure GitLab][] for the changes to take effect. +1. Validate using: + + ```sh + openssl s_client -showcerts -servername gitlab.example.com -connect gitlab.example.com:443 > cacert.pem + ``` + +NOTE: **Note:** +If your certificate provider provides the CA Bundle certificates, append them to the TLS certificate file. + **Installations from source** 1. Open `/home/git/gitlab/config/gitlab.yml`, find the `registry` entry and @@ -163,9 +172,13 @@ docker login gitlab.example.com:4567 If the Registry is configured to use its own domain, you will need a TLS certificate for that specific domain (e.g., `registry.example.com`) or maybe -a wildcard certificate if hosted under a subdomain of your existing GitLab +a wildcard certificate if hosted under a subdomain of your existing GitLab domain (e.g., `registry.gitlab.example.com`). +NOTE: **Note:** +As well as manually generated SSL certificates (explained here), certificates automatically +generated by Let's Encrypt are also [supported in Omnibus installs](https://docs.gitlab.com/omnibus/settings/ssl.html#host-services). + Let's assume that you want the container Registry to be accessible at `https://registry.gitlab.example.com`. diff --git a/doc/administration/gitaly/index.md b/doc/administration/gitaly/index.md index 150494c47e5..878f0ef842d 100644 --- a/doc/administration/gitaly/index.md +++ b/doc/administration/gitaly/index.md @@ -64,6 +64,7 @@ The following list depicts what the network architecture of Gitaly is: topology. - A `(Gitaly address, Gitaly token)` corresponds to a Gitaly server. - A Gitaly server hosts one or more storages. +- A GitLab server can use one or more Gitaly servers. - Gitaly addresses must be specified in such a way that they resolve correctly for ALL Gitaly clients. - Gitaly clients are: Unicorn, Sidekiq, gitlab-workhorse, @@ -77,14 +78,16 @@ The following list depicts what the network architecture of Gitaly is: - Authentication is done through a static token which is shared among the Gitaly and GitLab Rails nodes. -Below we describe how to configure a Gitaly server at address -`gitaly.internal:8075` with secret token `abc123secret`. We assume -your GitLab installation has two repository storages, `default` and -`storage1`. +Below we describe how to configure two Gitaly servers one at +`gitaly1.internal` and the other at `gitaly2.internal` +with secret token `abc123secret`. We assume +your GitLab installation has three repository storages: `default`, +`storage1` and `storage2`. ### 1. Installation -First install Gitaly using either Omnibus GitLab or install it from source: +First install Gitaly on each Gitaly server using either +Omnibus GitLab or install it from source: - For Omnibus GitLab: [Download/install](https://about.gitlab.com/install/) the Omnibus GitLab package you want using **steps 1 and 2** from the GitLab downloads page but @@ -119,7 +122,7 @@ Configure a token on the instance that runs the GitLab Rails application. ### 3. Gitaly server configuration -Next, on the Gitaly server, you need to configure storage paths, enable +Next, on the Gitaly servers, you need to configure storage paths, enable the network listener and configure the token. NOTE: **Note:** if you want to reduce the risk of downtime when you enable @@ -175,15 +178,29 @@ Check the directory layout on your Gitaly server to be sure. gitaly['listen_addr'] = "0.0.0.0:8075" gitaly['auth_token'] = 'abc123secret' + # To use TLS for Gitaly you need to add + gitaly['tls_listen_addr'] = "0.0.0.0:9999" + gitaly['certificate_path'] = "path/to/cert.pem" + gitaly['key_path'] = "path/to/key.pem" + ``` + +1. Append the following to `/etc/gitlab/gitlab.rb` for each respective server: + + For `gitaly1.internal`: + + ``` gitaly['storage'] = [ { 'name' => 'default' }, { 'name' => 'storage1' }, ] + ``` + + For `gitaly2.internal`: - # To use TLS for Gitaly you need to add - gitaly['tls_listen_addr'] = "0.0.0.0:9999" - gitaly['certificate_path'] = "path/to/cert.pem" - gitaly['key_path'] = "path/to/key.pem" + ``` + gitaly['storage'] = [ + { 'name' => 'storage2' }, + ] ``` NOTE: **Note:** @@ -206,13 +223,26 @@ Check the directory layout on your Gitaly server to be sure. [auth] token = 'abc123secret' + ``` + +1. Append the following to `/home/git/gitaly/config.toml` for each respective server: + + For `gitaly1.internal`: + ```toml [[storage]] name = 'default' [[storage]] name = 'storage1' ``` + + For `gitaly2.internal`: + + ```toml + [[storage]] + name = 'storage2' + ``` NOTE: **Note:** In some cases, you'll have to set `path` for each `[[storage]]` in the @@ -231,9 +261,13 @@ then all Gitaly requests will fail. Additionally, you need to [disable Rugged if previously manually enabled](../high_availability/nfs.md#improving-nfs-performance-with-gitlab). -We assume that your Gitaly server can be reached at -`gitaly.internal:8075` from your GitLab server, and that Gitaly can read and -write to `/mnt/gitlab/default` and `/mnt/gitlab/storage1` respectively. +We assume that your `gitaly1.internal` Gitaly server can be reached at +`gitaly1.internal:8075` from your GitLab server, and that Gitaly server +can read and write to `/mnt/gitlab/default` and `/mnt/gitlab/storage1`. + +We assume also that your `gitaly2.internal` Gitaly server can be reached at +`gitaly2.internal:8075` from your GitLab server, and that Gitaly server +can read and write to `/mnt/gitlab/storage2`. **For Omnibus GitLab** @@ -241,8 +275,9 @@ write to `/mnt/gitlab/default` and `/mnt/gitlab/storage1` respectively. ```ruby git_data_dirs({ - 'default' => { 'gitaly_address' => 'tcp://gitaly.internal:8075' }, - 'storage1' => { 'gitaly_address' => 'tcp://gitaly.internal:8075' }, + 'default' => { 'gitaly_address' => 'tcp://gitaly1.internal:8075' }, + 'storage1' => { 'gitaly_address' => 'tcp://gitaly1.internal:8075' }, + 'storage2' => { 'gitaly_address' => 'tcp://gitaly2.internal:8075' }, }) gitlab_rails['gitaly_token'] = 'abc123secret' @@ -268,9 +303,11 @@ write to `/mnt/gitlab/default` and `/mnt/gitlab/storage1` respectively. repositories: storages: default: - gitaly_address: tcp://gitaly.internal:8075 + gitaly_address: tcp://gitaly1.internal:8075 storage1: - gitaly_address: tcp://gitaly.internal:8075 + gitaly_address: tcp://gitaly1.internal:8075 + storage2: + gitaly_address: tcp://gitaly2.internal:8075 gitaly: token: 'abc123secret' @@ -350,8 +387,9 @@ To configure Gitaly with TLS: ```ruby git_data_dirs({ - 'default' => { 'gitaly_address' => 'tls://gitaly.internal:9999' }, - 'storage1' => { 'gitaly_address' => 'tls://gitaly.internal:9999' }, + 'default' => { 'gitaly_address' => 'tls://gitaly1.internal:9999' }, + 'storage1' => { 'gitaly_address' => 'tls://gitaly1.internal:9999' }, + 'storage2' => { 'gitaly_address' => 'tls://gitaly2.internal:9999' }, }) gitlab_rails['gitaly_token'] = 'abc123secret' @@ -377,9 +415,11 @@ To configure Gitaly with TLS: repositories: storages: default: - gitaly_address: tls://gitaly.internal:9999 + gitaly_address: tls://gitaly1.internal:9999 storage1: - gitaly_address: tls://gitaly.internal:9999 + gitaly_address: tls://gitaly1.internal:9999 + storage2: + gitaly_address: tls://gitaly2.internal:9999 gitaly: token: 'abc123secret' diff --git a/doc/administration/high_availability/load_balancer.md b/doc/administration/high_availability/load_balancer.md index 9e9f604317a..f11d27487d1 100644 --- a/doc/administration/high_availability/load_balancer.md +++ b/doc/administration/high_availability/load_balancer.md @@ -60,11 +60,21 @@ for details on managing SSL certificates and configuring Nginx. ### Basic ports -| LB Port | Backend Port | Protocol | -| ------- | ------------ | --------------- | -| 80 | 80 | HTTP [^1] | -| 443 | 443 | TCP or HTTPS [^1] [^2] | -| 22 | 22 | TCP | +| LB Port | Backend Port | Protocol | +| ------- | ------------ | ------------------------ | +| 80 | 80 | HTTP (*1*) | +| 443 | 443 | TCP or HTTPS (*1*) (*2*) | +| 22 | 22 | TCP | + +- (*1*): [Web terminal](../../ci/environments.md#web-terminals) support requires + your load balancer to correctly handle WebSocket connections. When using + HTTP or HTTPS proxying, this means your load balancer must be configured + to pass through the `Connection` and `Upgrade` hop-by-hop headers. See the + [web terminal](../integration/terminal.md) integration guide for + more details. +- (*2*): When using HTTPS protocol for port 443, you will need to add an SSL + certificate to the load balancers. If you wish to terminate SSL at the + GitLab application server instead, use TCP protocol. ### GitLab Pages Ports @@ -72,12 +82,19 @@ If you're using GitLab Pages with custom domain support you will need some additional port configurations. GitLab Pages requires a separate virtual IP address. Configure DNS to point the `pages_external_url` from `/etc/gitlab/gitlab.rb` at the new virtual IP address. See the -[GitLab Pages documentation][gitlab-pages] for more information. +[GitLab Pages documentation](../pages/index.md) for more information. -| LB Port | Backend Port | Protocol | -| ------- | ------------ | -------- | -| 80 | Varies [^3] | HTTP | -| 443 | Varies [^3] | TCP [^4] | +| LB Port | Backend Port | Protocol | +| ------- | ------------- | --------- | +| 80 | Varies (*1*) | HTTP | +| 443 | Varies (*1*) | TCP (*2*) | + +- (*1*): The backend port for GitLab Pages depends on the + `gitlab_pages['external_http']` and `gitlab_pages['external_https']` + setting. See [GitLab Pages documentation](../pages/index.md) for more details. +- (*2*): Port 443 for GitLab Pages should always use the TCP protocol. Users can + configure custom domains with custom SSL, which would not be possible + if SSL was terminated at the load balancer. ### Alternate SSH Port @@ -86,7 +103,7 @@ it may be helpful to configure an alternate SSH hostname that allows users to use SSH on port 443. An alternate SSH hostname will require a new virtual IP address compared to the other GitLab HTTP configuration above. -Configure DNS for an alternate SSH hostname such as altssh.gitlab.example.com. +Configure DNS for an alternate SSH hostname such as `altssh.gitlab.example.com`. | LB Port | Backend Port | Protocol | | ------- | ------------ | -------- | @@ -101,24 +118,6 @@ Read more on high-availability configuration: 1. [Configure NFS](nfs.md) 1. [Configure the GitLab application servers](gitlab.md) -[^1]: [Web terminal](../../ci/environments.md#web-terminals) support requires - your load balancer to correctly handle WebSocket connections. When using - HTTP or HTTPS proxying, this means your load balancer must be configured - to pass through the `Connection` and `Upgrade` hop-by-hop headers. See the - [web terminal](../integration/terminal.md) integration guide for - more details. -[^2]: When using HTTPS protocol for port 443, you will need to add an SSL - certificate to the load balancers. If you wish to terminate SSL at the - GitLab application server instead, use TCP protocol. -[^3]: The backend port for GitLab Pages depends on the - `gitlab_pages['external_http']` and `gitlab_pages['external_https']` - setting. See [GitLab Pages documentation][gitlab-pages] for more details. -[^4]: Port 443 for GitLab Pages should always use the TCP protocol. Users can - configure custom domains with custom SSL, which would not be possible - if SSL was terminated at the load balancer. - -[gitlab-pages]: ../pages/index.md - <!-- ## Troubleshooting Include any troubleshooting steps that you can foresee. If you know beforehand what issues diff --git a/doc/administration/monitoring/gitlab_instance_administration_project/index.md b/doc/administration/monitoring/gitlab_instance_administration_project/index.md index 8e33cea6217..d445b68721d 100644 --- a/doc/administration/monitoring/gitlab_instance_administration_project/index.md +++ b/doc/administration/monitoring/gitlab_instance_administration_project/index.md @@ -27,7 +27,7 @@ If that's not the case or if you have an external Prometheus instance or an HA s you should [configure it manually](../../../user/project/integrations/prometheus.md#manual-configuration-of-prometheus). -## Taking action on Prometheus alerts **[ULTIMATE]** +## Taking action on Prometheus alerts **(ULTIMATE)** You can [add a webhook](../../../user/project/integrations/prometheus.md#external-prometheus-instances) to the Prometheus config in order for GitLab to receive notifications of any alerts. diff --git a/doc/administration/monitoring/performance/request_profiling.md b/doc/administration/monitoring/performance/request_profiling.md index 9f671e0db11..c32edb60f9d 100644 --- a/doc/administration/monitoring/performance/request_profiling.md +++ b/doc/administration/monitoring/performance/request_profiling.md @@ -5,7 +5,7 @@ 1. Grab the profiling token from **Monitoring > Requests Profiles** admin page (highlighted in a blue in the image below).  -1. Pass the header `X-Profile-Token: <token>` and `X-Profile-Mode: <mode>`(where <mode> can be `execution` or `memory`) to the request you want to profile. You can use: +1. Pass the header `X-Profile-Token: <token>` and `X-Profile-Mode: <mode>`(where `<mode>` can be `execution` or `memory`) to the request you want to profile. You can use: - Browser extensions. For example, [ModHeader](https://chrome.google.com/webstore/detail/modheader/idgpnmonknjnojddfkpgkljpfnnfcklj) Chrome extension. - `curl`. For example, `curl --header 'X-Profile-Token: <token>' --header 'X-Profile-Mode: <mode>' https://gitlab.example.com/group/project`. 1. Once request is finished (which will take a little longer than usual), you can diff --git a/doc/administration/troubleshooting/sidekiq.md b/doc/administration/troubleshooting/sidekiq.md index 7067958ecb4..9b016c64e29 100644 --- a/doc/administration/troubleshooting/sidekiq.md +++ b/doc/administration/troubleshooting/sidekiq.md @@ -169,3 +169,121 @@ The PostgreSQL wiki has details on the query you can run to see blocking queries. The query is different based on PostgreSQL version. See [Lock Monitoring](https://wiki.postgresql.org/wiki/Lock_Monitoring) for the query details. + +## Managing Sidekiq queues + +It is possible to use [Sidekiq API](https://github.com/mperham/sidekiq/wiki/API) +to perform a number of troubleshoting on Sidekiq. + +These are the administrative commands and it should only be used if currently +admin interface is not suitable due to scale of installation. + +All this commands should be run using `gitlab-rails console`. + +### View the queue size + +```ruby +Sidekiq::Queue.new("pipeline_processing:build_queue").size +``` + +### Enumerate all enqueued jobs + +```ruby +queue = Sidekiq::Queue.new("chaos:chaos_sleep") +queue.each do |job| + # job.klass # => 'MyWorker' + # job.args # => [1, 2, 3] + # job.jid # => jid + # job.queue # => chaos:chaos_sleep + # job["retry"] # => 3 + # job.item # => { + # "class"=>"Chaos::SleepWorker", + # "args"=>[1000], + # "retry"=>3, + # "queue"=>"chaos:chaos_sleep", + # "backtrace"=>true, + # "queue_namespace"=>"chaos", + # "jid"=>"39bc482b823cceaf07213523", + # "created_at"=>1566317076.266069, + # "correlation_id"=>"c323b832-a857-4858-b695-672de6f0e1af", + # "enqueued_at"=>1566317076.26761}, + # } + + # job.delete if job.jid == 'abcdef1234567890' +end +``` + +### Enumerate currently running jobs + +```ruby +workers = Sidekiq::Workers.new +workers.each do |process_id, thread_id, work| + # process_id is a unique identifier per Sidekiq process + # thread_id is a unique identifier per thread + # work is a Hash which looks like: + # {"queue"=>"chaos:chaos_sleep", + # "payload"=> + # { "class"=>"Chaos::SleepWorker", + # "args"=>[1000], + # "retry"=>3, + # "queue"=>"chaos:chaos_sleep", + # "backtrace"=>true, + # "queue_namespace"=>"chaos", + # "jid"=>"b2a31e3eac7b1a99ff235869", + # "created_at"=>1566316974.9215662, + # "correlation_id"=>"e484fb26-7576-45f9-bf21-b99389e1c53c", + # "enqueued_at"=>1566316974.9229589}, + # "run_at"=>1566316974}], +end +``` + +### Remove sidekiq jobs for given parameters (destructive) + +```ruby +# for jobs like this: +# RepositoryImportWorker.new.perform_async(100) +id_list = [100] + +queue = Sidekiq::Queue.new('repository_import') +queue.each do |job| + job.delete if id_list.include?(job.args[0]) +end +``` + +### Remove specific job ID (destructive) + +```ruby +queue = Sidekiq::Queue.new('repository_import') +queue.each do |job| + job.delete if job.jid == 'my-job-id' +end +``` + +## Canceling running jobs (destructive) + +> Introduced in GitLab 12.3. + +This is highly risky operation and use it as last resort. +Doing that might result in data corruption, as the job +is interrupted mid-execution and it is not guaranteed +that proper rollback of transactions is implemented. + +```ruby +Gitlab::SidekiqMonitor.cancel_job('job-id') +``` + +> This requires the Sidekiq to be run with `SIDEKIQ_MONITOR_WORKER=1` +> environment variable. + +To perform of the interrupt we use `Thread.raise` which +has number of drawbacks, as mentioned in [Why Ruby’s Timeout is dangerous (and Thread.raise is terrifying)](https://jvns.ca/blog/2015/11/27/why-rubys-timeout-is-dangerous-and-thread-dot-raise-is-terrifying/): + +> This is where the implications get interesting, and terrifying. This means that an exception can get raised: +> +> * during a network request (ok, as long as the surrounding code is prepared to catch Timeout::Error) +> * during the cleanup for the network request +> * during a rescue block +> * while creating an object to save to the database afterwards +> * in any of your code, regardless of whether it could have possibly raised an exception before +> +> Nobody writes code to defend against an exception being raised on literally any line. That’s not even possible. So Thread.raise is basically like a sneak attack on your code that could result in almost anything. It would probably be okay if it were pure-functional code that did not modify any state. But this is Ruby, so that’s unlikely :) diff --git a/doc/api/api_resources.md b/doc/api/api_resources.md index b32f11464ef..9af5430f1c8 100644 --- a/doc/api/api_resources.md +++ b/doc/api/api_resources.md @@ -67,7 +67,7 @@ The following API resources are available in the project context: | [Search](search.md) | `/projects/:id/search` (also available for groups and standalone) | | [Services](services.md) | `/projects/:id/services` | | [Tags](tags.md) | `/projects/:id/repository/tags` | -| [Vulnerabilities](vulnerabilities.md) **(ULTIMATE)** | `/projects/:id/vulnerabilities` (also available for groups) | +| [Vulnerabilities](vulnerabilities.md) **(ULTIMATE)** | `/projects/:id/vulnerabilities` | [Wikis](wikis.md) | `/projects/:id/wikis` | ## Group resources diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 335f3292444..f7a67931793 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -518,10 +518,24 @@ Four keys are available: - `changes` - `kubernetes` -If you use multiple keys under `only` or `except`, they act as an AND. The logic is: +If you use multiple keys under `only` or `except`, the keys will be evaluated as a +single conjoined expression. That is: + +- `only:` means "include this job if all of the conditions match". +- `except:` means "exclude this job if any of the conditions match". + +With `only`, individual keys are logically joined by an AND: > (any of refs) AND (any of variables) AND (any of changes) AND (if kubernetes is active) +`except` is implemented as a negation of this complete expression: + +> NOT((any of refs) AND (any of variables) AND (any of changes) AND (if kubernetes is active)) + +This, more intuitively, means the keys join by an OR. A functionally equivalent expression: + +> (any of refs) OR (any of variables) OR (any of changes) OR (if kubernetes is active) + #### `only:refs`/`except:refs` > `refs` policy introduced in GitLab 10.0. @@ -1721,7 +1735,7 @@ This example creates three paths of execution: 1. If `needs:` is set to point to a job that is not instantiated because of `only/except` rules or otherwise does not exist, the job will fail. -1. Note that on day one of the launch, we are temporarily limiting the +1. Note that on day one of the launch, we are temporarily limiting the maximum number of jobs that a single job can need in the `needs:` array. Track our [infrastructure issue](https://gitlab.com/gitlab-com/gl-infra/infrastructure/issues/7541) for details on the current limit. diff --git a/doc/development/testing_guide/frontend_testing.md b/doc/development/testing_guide/frontend_testing.md index ed867c41f59..7dc89a3fcdb 100644 --- a/doc/development/testing_guide/frontend_testing.md +++ b/doc/development/testing_guide/frontend_testing.md @@ -619,6 +619,42 @@ See also [Notes on testing Vue components](../fe_guide/vue.html#testing-vue-comp Unit tests are on the lowest abstraction level and typically test functionality that is not directly perceivable by a user. +```mermaid +graph RL + plain[Plain JavaScript]; + Vue[Vue Components]; + feature-flags[Feature Flags]; + license-checks[License Checks]; + + plain---Vuex; + plain---GraphQL; + Vue---plain; + Vue---Vuex; + Vue---GraphQL; + browser---plain; + browser---Vue; + plain---backend; + Vuex---backend; + GraphQL---backend; + Vue---backend; + backend---database; + backend---feature-flags; + backend---license-checks; + + class plain tested; + class Vuex tested; + + classDef node color:#909090,fill:#f0f0f0,stroke-width:2px,stroke:#909090 + classDef label stroke-width:0; + classDef tested color:#000000,fill:#a0c0ff,stroke:#6666ff,stroke-width:2px,stroke-dasharray: 5, 5; + + subgraph " " + tested; + mocked; + class tested tested; + end +``` + #### When to use unit tests <details> @@ -711,6 +747,41 @@ Unit tests are on the lowest abstraction level and typically test functionality Component tests cover the state of a single component that is perceivable by a user depending on external signals such as user input, events fired from other components, or application state. +```mermaid +graph RL + plain[Plain JavaScript]; + Vue[Vue Components]; + feature-flags[Feature Flags]; + license-checks[License Checks]; + + plain---Vuex; + plain---GraphQL; + Vue---plain; + Vue---Vuex; + Vue---GraphQL; + browser---plain; + browser---Vue; + plain---backend; + Vuex---backend; + GraphQL---backend; + Vue---backend; + backend---database; + backend---feature-flags; + backend---license-checks; + + class Vue tested; + + classDef node color:#909090,fill:#f0f0f0,stroke-width:2px,stroke:#909090 + classDef label stroke-width:0; + classDef tested color:#000000,fill:#a0c0ff,stroke:#6666ff,stroke-width:2px,stroke-dasharray: 5, 5; + + subgraph " " + tested; + mocked; + class tested tested; + end +``` + #### When to use component tests - Vue components @@ -781,6 +852,46 @@ Component tests cover the state of a single component that is perceivable by a u Integration tests cover the interaction between all components on a single page. Their abstraction level is comparable to how a user would interact with the UI. +```mermaid +graph RL + plain[Plain JavaScript]; + Vue[Vue Components]; + feature-flags[Feature Flags]; + license-checks[License Checks]; + + plain---Vuex; + plain---GraphQL; + Vue---plain; + Vue---Vuex; + Vue---GraphQL; + browser---plain; + browser---Vue; + plain---backend; + Vuex---backend; + GraphQL---backend; + Vue---backend; + backend---database; + backend---feature-flags; + backend---license-checks; + + class plain tested; + class Vue tested; + class Vuex tested; + class GraphQL tested; + class browser tested; + linkStyle 0,1,2,3,4,5,6 stroke:#6666ff,stroke-width:2px,stroke-dasharray: 5, 5; + + classDef node color:#909090,fill:#f0f0f0,stroke-width:2px,stroke:#909090 + classDef label stroke-width:0; + classDef tested color:#000000,fill:#a0c0ff,stroke:#6666ff,stroke-width:2px,stroke-dasharray: 5, 5; + + subgraph " " + tested; + mocked; + class tested tested; + end +``` + #### When to use integration tests <details> @@ -838,6 +949,47 @@ This also implies that database queries are executed which makes this category s See also the [RSpec testing guidelines](../testing_guide/best_practices.md#rspec). +```mermaid +graph RL + plain[Plain JavaScript]; + Vue[Vue Components]; + feature-flags[Feature Flags]; + license-checks[License Checks]; + + plain---Vuex; + plain---GraphQL; + Vue---plain; + Vue---Vuex; + Vue---GraphQL; + browser---plain; + browser---Vue; + plain---backend; + Vuex---backend; + GraphQL---backend; + Vue---backend; + backend---database; + backend---feature-flags; + backend---license-checks; + + class backend tested; + class plain tested; + class Vue tested; + class Vuex tested; + class GraphQL tested; + class browser tested; + linkStyle 0,1,2,3,4,5,6,7,8,9,10 stroke:#6666ff,stroke-width:2px,stroke-dasharray: 5, 5; + + classDef node color:#909090,fill:#f0f0f0,stroke-width:2px,stroke:#909090 + classDef label stroke-width:0; + classDef tested color:#000000,fill:#a0c0ff,stroke:#6666ff,stroke-width:2px,stroke-dasharray: 5, 5; + + subgraph " " + tested; + mocked; + class tested tested; + end +``` + #### When to use feature tests - Use cases that require a backend and cannot be tested using fixtures. diff --git a/doc/gitlab-basics/start-using-git.md b/doc/gitlab-basics/start-using-git.md index 3e3f96fb31f..5de173abbff 100644 --- a/doc/gitlab-basics/start-using-git.md +++ b/doc/gitlab-basics/start-using-git.md @@ -63,8 +63,8 @@ git config --global user.email You'll need to do this only once, since you are using the `--global` option. It tells Git to always use this information for anything you do on that system. If you want -to override this with a different username or email address for specific projects, -you can run the command without the `--global` option when you’re in that project. +to override this with a different username or email address for specific projects or repositories, +you can run the command without the `--global` option when you’re in that project, and that will default to `--local`. You can read more on how Git manages configurations in the [Git Config](https://git-scm.com/book/en/v2/Customizing-Git-Git-Configuration) documentation. ## Check your information @@ -102,8 +102,7 @@ files to your local computer, automatically preserving the Git connection with t remote repository. You can either clone it via HTTPS or [SSH](../ssh/README.md). If you chose to clone -it via HTTPS, you'll have to enter your credentials every time you pull and push. -With SSH, you enter your credentials only once. +it via HTTPS, you'll have to enter your credentials every time you pull and push. You can read more about credential storage in the [Git Credentials documentation](https://git-scm.com/book/en/v2/Git-Tools-Credential-Storage). With SSH, you enter your credentials only once. You can find both paths (HTTPS and SSH) by navigating to your project's landing page and clicking **Clone**. GitLab will prompt you with both paths, from which you can copy @@ -152,13 +151,15 @@ to get the main branch code, or the branch name of the branch you are currently in. ```bash -git pull REMOTE <name-of-branch> +git pull <REMOTE> <name-of-branch> ``` -When you first clone a repository, REMOTE is typically `origin`. This is where the +When you clone a repository, `REMOTE` is typically `origin`. This is where the repository was cloned from, and it indicates the SSH or HTTPS URL of the repository on the remote server. `<name-of-branch>` is usually `master`, but it may be any existing -branch. +branch. You can create additional named remotes and branches as necessary. + +You can learn more on how Git manages remote repositories in the [Git Remote documentation](https://git-scm.com/book/en/v2/Git-Basics-Working-with-Remotes). ### View your remote repositories @@ -168,6 +169,8 @@ To view your remote repositories, type: git remote -v ``` +The `-v` flag stands for verbose. + ### Add a remote repository To add a link to a remote repository: @@ -186,7 +189,7 @@ following (spaces won't be recognized in the branch name, so you will need to us hyphen or underscore): ```bash -git checkout -b <name-of-branch>> +git checkout -b <name-of-branch> ``` ### Work on an existing branch @@ -238,7 +241,7 @@ git commit -m "COMMENT TO DESCRIBE THE INTENTION OF THE COMMIT" ``` NOTE: **Note:** -The `.` character typically means _all_ in Git. +The `.` character means _all file changes in the current directory and all subdirectories_. ### Send changes to GitLab.com diff --git a/doc/install/azure/index.md b/doc/install/azure/index.md index 543a222bd25..c5939dc6856 100644 --- a/doc/install/azure/index.md +++ b/doc/install/azure/index.md @@ -407,7 +407,7 @@ on any cloud service you choose. ## Where to next? -Check out our other [Technical Articles](../../articles/index.md) or browse the [GitLab Documentation][GitLab-Docs](../../README.md) to learn more about GitLab. +Check out our other [Technical Articles](../../articles/index.md) or browse the [GitLab Documentation](../../README.md) to learn more about GitLab. ### Useful links diff --git a/doc/user/group/saml_sso/index.md b/doc/user/group/saml_sso/index.md index e3f657af564..bd50367681e 100644 --- a/doc/user/group/saml_sso/index.md +++ b/doc/user/group/saml_sso/index.md @@ -39,6 +39,25 @@ However, users will not be prompted to log via SSO on each visit. GitLab will ch We intend to add a similar SSO requirement for [Git and API activity](https://gitlab.com/gitlab-org/gitlab-ee/issues/9152) in the future. +#### Group-managed accounts + +[Introduced in GitLab 12.1](https://gitlab.com/groups/gitlab-org/-/epics/709). + +When SSO is being enforced, groups can enable an additional level of protection by enforcing the creation of dedicated user accounts to access the group. + +Without group-managed accounts, users can link their SAML identity with any existing user on the instance. With group-managed accounts enabled, users are required to create a new, dedicated user linked to the group. The notification email address associated with the user is locked to the email address received from the configured identity provider. + +When this option is enabled: + +- All existing and new users in the group will be required to log in via the SSO URL associated with the group. +- On successfully authenticating, GitLab will prompt the user to create a new, dedicated account using the email address received from the configured identity provider. +- After the group managed account has been created, group activity will require the use of this user account. + +Since use of the group managed account requires the use of SSO, users of group managed accounts will lose access to these accounts when they are no longer able to authenticate with the connected identity provider. In the case of an offboarded employee who has been removed from your identity provider: + +- The user will be unable to access the group (their credentials will no longer work on the identity provider when prompted to SSO). +- Contributions in the group (e.g. issues, merge requests) will remain intact. + ### NameID GitLab.com uses the SAML NameID to identify users. The NameID element: diff --git a/doc/user/profile/account/two_factor_authentication.md b/doc/user/profile/account/two_factor_authentication.md index e2b1a20a605..f7ba921aa7d 100644 --- a/doc/user/profile/account/two_factor_authentication.md +++ b/doc/user/profile/account/two_factor_authentication.md @@ -48,6 +48,7 @@ To enable 2FA: - [andOTP](https://github.com/andOTP/andOTP): feature rich open source app for Android which supports PGP encrypted backups. - [FreeOTP](https://freeotp.github.io/): open source app for Android. - [Google Authenticator](https://support.google.com/accounts/answer/1066447?hl=en): proprietary app for iOS and Android. + - [SailOTP](https://openrepos.net/content/seiichiro0185/sailotp): open source app for SailFish OS. 1. In the application, add a new entry in one of two ways: - Scan the code presented in GitLab with your device's camera to add the entry automatically. diff --git a/doc/user/project/integrations/img/download_as_csv.png b/doc/user/project/integrations/img/download_as_csv.png Binary files differnew file mode 100644 index 00000000000..0ed5ab8db89 --- /dev/null +++ b/doc/user/project/integrations/img/download_as_csv.png diff --git a/doc/user/project/integrations/img/generate_link_to_chart.png b/doc/user/project/integrations/img/generate_link_to_chart.png Binary files differnew file mode 100644 index 00000000000..03e018969b1 --- /dev/null +++ b/doc/user/project/integrations/img/generate_link_to_chart.png diff --git a/doc/user/project/integrations/prometheus.md b/doc/user/project/integrations/prometheus.md index aa7db97c413..e2f45b8a32f 100644 --- a/doc/user/project/integrations/prometheus.md +++ b/doc/user/project/integrations/prometheus.md @@ -269,6 +269,12 @@ Note the following properties:  +### Downloading data as CSV + +Data from Prometheus charts on the metrics dashboard can be downloaded as CSV. + + + ### Setting up alerts for Prometheus metrics **(ULTIMATE)** #### Managed Prometheus instances @@ -363,6 +369,10 @@ It is possible to display metrics charts within [GitLab Flavored Markdown](../.. To display a metric chart, include a link of the form `https://<root_url>/<project>/environments/<environment_id>/metrics`. +A single chart may also be embedded. You can generate a link to the chart via the dropdown located on the right side of the chart: + + + The following requirements must be met for the metric to unfurl: - The `<environment_id>` must correspond to a real environment. diff --git a/doc/user/project/members/img/access_requests_management.png b/doc/user/project/members/img/access_requests_management.png Binary files differindex 8996d9564d7..9a1c9621e41 100644 --- a/doc/user/project/members/img/access_requests_management.png +++ b/doc/user/project/members/img/access_requests_management.png diff --git a/doc/user/project/members/img/request_access_button.png b/doc/user/project/members/img/request_access_button.png Binary files differindex e8b490b91b8..e693f9a9ac2 100644 --- a/doc/user/project/members/img/request_access_button.png +++ b/doc/user/project/members/img/request_access_button.png diff --git a/doc/user/project/members/img/withdraw_access_request_button.png b/doc/user/project/members/img/withdraw_access_request_button.png Binary files differindex 6a3172dfcdb..e5a8fe0b356 100644 --- a/doc/user/project/members/img/withdraw_access_request_button.png +++ b/doc/user/project/members/img/withdraw_access_request_button.png diff --git a/doc/user/project/wiki/index.md b/doc/user/project/wiki/index.md index e3c6cd6d6ff..d1bab47de25 100644 --- a/doc/user/project/wiki/index.md +++ b/doc/user/project/wiki/index.md @@ -28,11 +28,12 @@ NOTE: **Note:** Requires Developer [permissions](../../permissions.md). Create a new page by clicking the **New page** button that can be found -in all wiki pages. You will be asked to fill in the page name from which GitLab -will create the path to the page. You can specify a full path for the new file -and any missing directories will be created automatically. +in all wiki pages. - +You will be asked to fill in a title for your new wiki page. Wiki titles +also determine the path to the wiki page. You can specify a full path +(using "`/`" for subdirectories) for the new title and any missing +directories will be created automatically. Once you enter the page name, it's time to fill in its content. GitLab wikis support Markdown, RDoc and AsciiDoc. For Markdown based pages, all the diff --git a/lib/api/award_emoji.rb b/lib/api/award_emoji.rb index a1851ba3627..89b7e5c5e4b 100644 --- a/lib/api/award_emoji.rb +++ b/lib/api/award_emoji.rb @@ -69,12 +69,12 @@ module API post endpoint do not_found!('Award Emoji') unless can_read_awardable? && can_award_awardable? - award = awardable.create_award_emoji(params[:name], current_user) + service = AwardEmojis::AddService.new(awardable, params[:name], current_user).execute - if award.persisted? - present award, with: Entities::AwardEmoji + if service[:status] == :success + present service[:award], with: Entities::AwardEmoji else - not_found!("Award Emoji #{award.errors.messages}") + not_found!("Award Emoji #{service[:message]}") end end diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb b/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb index 942e4e55323..f7b0720d4a9 100644 --- a/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb +++ b/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb @@ -11,8 +11,9 @@ module Gitlab def evaluate(variables = {}) text = @left.evaluate(variables) regexp = @right.evaluate(variables) + return false unless regexp - regexp.scan(text.to_s).any? + regexp.scan(text.to_s).present? end def self.build(_value, behind, ahead) diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb b/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb index 831c27fa0ea..02479ed28a4 100644 --- a/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb +++ b/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb @@ -11,8 +11,9 @@ module Gitlab def evaluate(variables = {}) text = @left.evaluate(variables) regexp = @right.evaluate(variables) + return true unless regexp - regexp.scan(text.to_s).none? + regexp.scan(text.to_s).empty? end def self.build(_value, behind, ahead) diff --git a/lib/gitlab/daemon.rb b/lib/gitlab/daemon.rb index 6d5fc4219fb..2f4ae010e74 100644 --- a/lib/gitlab/daemon.rb +++ b/lib/gitlab/daemon.rb @@ -46,7 +46,10 @@ module Gitlab if thread thread.wakeup if thread.alive? - thread.join unless Thread.current == thread + begin + thread.join unless Thread.current == thread + rescue Exception # rubocop:disable Lint/RescueException + end @thread = nil end end diff --git a/lib/gitlab/sidekiq_middleware/monitor.rb b/lib/gitlab/sidekiq_middleware/monitor.rb new file mode 100644 index 00000000000..0d88fe760d3 --- /dev/null +++ b/lib/gitlab/sidekiq_middleware/monitor.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Gitlab + module SidekiqMiddleware + class Monitor + def call(worker, job, queue) + Gitlab::SidekiqMonitor.instance.within_job(job['jid'], queue) do + yield + end + rescue Gitlab::SidekiqMonitor::CancelledError + # ignore retries + raise Sidekiq::JobRetry::Skip + end + end + end +end diff --git a/lib/gitlab/sidekiq_monitor.rb b/lib/gitlab/sidekiq_monitor.rb new file mode 100644 index 00000000000..9842f1f53f7 --- /dev/null +++ b/lib/gitlab/sidekiq_monitor.rb @@ -0,0 +1,182 @@ +# frozen_string_literal: true + +module Gitlab + class SidekiqMonitor < Daemon + include ::Gitlab::Utils::StrongMemoize + + NOTIFICATION_CHANNEL = 'sidekiq:cancel:notifications'.freeze + CANCEL_DEADLINE = 24.hours.seconds + RECONNECT_TIME = 3.seconds + + # We use exception derived from `Exception` + # to consider this as an very low-level exception + # that should not be caught by application + CancelledError = Class.new(Exception) # rubocop:disable Lint/InheritException + + attr_reader :jobs_thread + attr_reader :jobs_mutex + + def initialize + super + + @jobs_thread = {} + @jobs_mutex = Mutex.new + end + + def within_job(jid, queue) + jobs_mutex.synchronize do + jobs_thread[jid] = Thread.current + end + + if cancelled?(jid) + Sidekiq.logger.warn( + class: self.class.to_s, + action: 'run', + queue: queue, + jid: jid, + canceled: true + ) + raise CancelledError + end + + yield + ensure + jobs_mutex.synchronize do + jobs_thread.delete(jid) + end + end + + def self.cancel_job(jid) + payload = { + action: 'cancel', + jid: jid + }.to_json + + ::Gitlab::Redis::SharedState.with do |redis| + redis.setex(cancel_job_key(jid), CANCEL_DEADLINE, 1) + redis.publish(NOTIFICATION_CHANNEL, payload) + end + end + + private + + def start_working + Sidekiq.logger.info( + class: self.class.to_s, + action: 'start', + message: 'Starting Monitor Daemon' + ) + + while enabled? + process_messages + sleep(RECONNECT_TIME) + end + + ensure + Sidekiq.logger.warn( + class: self.class.to_s, + action: 'stop', + message: 'Stopping Monitor Daemon' + ) + end + + def stop_working + thread.raise(Interrupt) if thread.alive? + end + + def process_messages + ::Gitlab::Redis::SharedState.with do |redis| + redis.subscribe(NOTIFICATION_CHANNEL) do |on| + on.message do |channel, message| + process_message(message) + end + end + end + rescue Exception => e # rubocop:disable Lint/RescueException + Sidekiq.logger.warn( + class: self.class.to_s, + action: 'exception', + message: e.message + ) + + # we re-raise system exceptions + raise e unless e.is_a?(StandardError) + end + + def process_message(message) + Sidekiq.logger.info( + class: self.class.to_s, + channel: NOTIFICATION_CHANNEL, + message: 'Received payload on channel', + payload: message + ) + + message = safe_parse(message) + return unless message + + case message['action'] + when 'cancel' + process_job_cancel(message['jid']) + else + # unknown message + end + end + + def safe_parse(message) + JSON.parse(message) + rescue JSON::ParserError + end + + def process_job_cancel(jid) + return unless jid + + # try to find thread without lock + return unless find_thread_unsafe(jid) + + Thread.new do + # try to find a thread, but with guaranteed + # that handle for thread corresponds to actually + # running job + find_thread_with_lock(jid) do |thread| + Sidekiq.logger.warn( + class: self.class.to_s, + action: 'cancel', + message: 'Canceling thread with CancelledError', + jid: jid, + thread_id: thread.object_id + ) + + thread&.raise(CancelledError) + end + end + end + + # This method needs to be thread-safe + # This is why it passes thread in block, + # to ensure that we do process this thread + def find_thread_unsafe(jid) + jobs_thread[jid] + end + + def find_thread_with_lock(jid) + # don't try to lock if we cannot find the thread + return unless find_thread_unsafe(jid) + + jobs_mutex.synchronize do + find_thread_unsafe(jid).tap do |thread| + yield(thread) if thread + end + end + end + + def cancelled?(jid) + ::Gitlab::Redis::SharedState.with do |redis| + redis.exists(self.class.cancel_job_key(jid)) + end + end + + def self.cancel_job_key(jid) + "sidekiq:cancel:#{jid}" + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 26e6cb524bd..1c1a3a51932 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -128,9 +128,6 @@ msgstr[1] "" msgid "%{actionText} & %{openOrClose} %{noteable}" msgstr "" -msgid "%{canMergeCount}/%{assigneesCount} can merge" -msgstr "" - msgid "%{commit_author_link} authored %{commit_timeago}" msgstr "" @@ -202,6 +199,9 @@ msgstr "" msgid "%{lock_path} is locked by GitLab User %{lock_user_id}" msgstr "" +msgid "%{mergeLength}/%{usersLength} can merge" +msgstr "" + msgid "%{mrText}, this issue will be closed automatically." msgstr "" @@ -279,6 +279,9 @@ msgstr "" msgid "%{usage_ping_link_start}Learn more%{usage_ping_link_end} about what information is shared with GitLab Inc." msgstr "" +msgid "%{userName} (cannot merge)" +msgstr "" + msgid "%{userName}'s avatar" msgstr "" @@ -306,6 +309,9 @@ msgstr "" msgid "(external source)" msgstr "" +msgid "+ %{amount} more" +msgstr "" + msgid "+ %{count} more" msgstr "" @@ -7439,9 +7445,6 @@ msgstr "" msgid "No milestones to show" msgstr "" -msgid "No one can merge" -msgstr "" - msgid "No other labels with such name or description" msgstr "" @@ -12888,15 +12891,9 @@ msgstr "" msgid "WikiMarkdownTip|To link to a (new) page, simply type %{link_example}" msgstr "" -msgid "WikiNewPagePlaceholder|how-to-setup" -msgstr "" - msgid "WikiNewPageTip|Tip: You can specify the full path for the new file. We will automatically create any missing directories." msgstr "" -msgid "WikiNewPageTitle|New Wiki Page" -msgstr "" - msgid "WikiPageConfirmDelete|Are you sure you want to delete this page?" msgstr "" @@ -12912,19 +12909,16 @@ msgstr "" msgid "WikiPageConflictMessage|the page" msgstr "" -msgid "WikiPageCreate|Create %{page_title}" -msgstr "" - -msgid "WikiPageEdit|Update %{page_title}" +msgid "WikiPageCreate|Create %{pageTitle}" msgstr "" -msgid "WikiPage|Page slug" +msgid "WikiPageEdit|Update %{pageTitle}" msgstr "" msgid "WikiPage|Write your content or drag files here…" msgstr "" -msgid "Wiki|Create Page" +msgid "Wiki|Create New Page" msgstr "" msgid "Wiki|Create page" @@ -12945,6 +12939,9 @@ msgstr "" msgid "Wiki|Page history" msgstr "" +msgid "Wiki|Page title" +msgstr "" + msgid "Wiki|Page version" msgstr "" @@ -13401,18 +13398,15 @@ msgstr "" msgid "Your request for access has been queued for review." msgstr "" -msgid "a Zoom call was added to this issue" -msgstr "" - -msgid "a Zoom call was removed from this issue" -msgstr "" - msgid "a deleted user" msgstr "" msgid "added %{created_at_timeago}" msgstr "" +msgid "added a Zoom call to this issue" +msgstr "" + msgid "ago" msgstr "" @@ -13452,6 +13446,9 @@ msgstr "" msgid "cannot include leading slash or directory traversal." msgstr "" +msgid "cannot merge" +msgstr "" + msgid "comment" msgstr "" @@ -13879,6 +13876,9 @@ msgstr "" msgid "no contributions" msgstr "" +msgid "no one can merge" +msgstr "" + msgid "none" msgstr "" @@ -13949,6 +13949,9 @@ msgstr "" msgid "remove due date" msgstr "" +msgid "removed a Zoom call from this issue" +msgstr "" + msgid "rendered diff" msgstr "" diff --git a/qa/qa/specs/features/browser_ui/1_manage/project/add_project_member_spec.rb b/qa/qa/specs/features/browser_ui/1_manage/project/add_project_member_spec.rb index f51c16f472c..1c1f552e224 100644 --- a/qa/qa/specs/features/browser_ui/1_manage/project/add_project_member_spec.rb +++ b/qa/qa/specs/features/browser_ui/1_manage/project/add_project_member_spec.rb @@ -19,7 +19,7 @@ module QA page.add_member(user.username) end - expect(page).to have_content(/#{user.name} (. )?@#{user.username} Given access/) + expect(page).to have_content(/@#{user.username}(\n| )?Given access/) end end end diff --git a/spec/controllers/concerns/issuable_collections_spec.rb b/spec/controllers/concerns/issuable_collections_spec.rb index f210537aad5..7bdf5c49425 100644 --- a/spec/controllers/concerns/issuable_collections_spec.rb +++ b/spec/controllers/concerns/issuable_collections_spec.rb @@ -24,78 +24,6 @@ describe IssuableCollections do controller end - describe '#set_sort_order_from_user_preference' do - describe 'when sort param given' do - let(:params) { { sort: 'updated_desc' } } - - context 'when issuable_sorting_field is defined' do - before do - controller.class.define_method(:issuable_sorting_field) { :issues_sort} - end - - it 'sets user_preference with the right value' do - controller.send(:set_sort_order_from_user_preference) - - expect(user.user_preference.reload.issues_sort).to eq('updated_desc') - end - end - - context 'when no issuable_sorting_field is defined on the controller' do - it 'does not touch user_preference' do - allow(user).to receive(:user_preference) - - controller.send(:set_sort_order_from_user_preference) - - expect(user).not_to have_received(:user_preference) - end - end - end - - context 'when a user sorting preference exists' do - let(:params) { {} } - - before do - controller.class.define_method(:issuable_sorting_field) { :issues_sort } - end - - it 'returns the set preference' do - user.user_preference.update(issues_sort: 'updated_asc') - - sort_preference = controller.send(:set_sort_order_from_user_preference) - - expect(sort_preference).to eq('updated_asc') - end - end - end - - describe '#set_set_order_from_cookie' do - describe 'when sort param given' do - let(:cookies) { {} } - let(:params) { { sort: 'downvotes_asc' } } - - it 'sets the cookie with the right values and flags' do - allow(controller).to receive(:cookies).and_return(cookies) - - controller.send(:set_sort_order_from_cookie) - - expect(cookies['issue_sort']).to eq({ value: 'popularity', secure: false, httponly: false }) - end - end - - describe 'when cookie exists' do - let(:cookies) { { 'issue_sort' => 'id_asc' } } - let(:params) { {} } - - it 'sets the cookie with the right values and flags' do - allow(controller).to receive(:cookies).and_return(cookies) - - controller.send(:set_sort_order_from_cookie) - - expect(cookies['issue_sort']).to eq({ value: 'created_asc', secure: false, httponly: false }) - end - end - end - describe '#page_count_for_relation' do let(:params) { { state: 'opened' } } diff --git a/spec/controllers/concerns/sorting_preference_spec.rb b/spec/controllers/concerns/sorting_preference_spec.rb new file mode 100644 index 00000000000..a36124c6776 --- /dev/null +++ b/spec/controllers/concerns/sorting_preference_spec.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe SortingPreference do + let(:user) { create(:user) } + + let(:controller_class) do + Class.new do + def self.helper_method(name); end + + include SortingPreference + include SortingHelper + end + end + + let(:controller) { controller_class.new } + + before do + allow(controller).to receive(:params).and_return(ActionController::Parameters.new(params)) + allow(controller).to receive(:current_user).and_return(user) + allow(controller).to receive(:legacy_sort_cookie_name).and_return('issuable_sort') + allow(controller).to receive(:sorting_field).and_return(:issues_sort) + end + + describe '#set_sort_order_from_user_preference' do + subject { controller.send(:set_sort_order_from_user_preference) } + + context 'when sort param given' do + let(:params) { { sort: 'updated_desc' } } + + context 'when sorting_field is defined' do + it 'sets user_preference with the right value' do + is_expected.to eq('updated_desc') + end + end + + context 'when no sorting_field is defined on the controller' do + before do + allow(controller).to receive(:sorting_field).and_return(nil) + end + + it 'does not touch user_preference' do + expect(user).not_to receive(:user_preference) + + subject + end + end + end + + context 'when a user sorting preference exists' do + let(:params) { {} } + + before do + user.user_preference.update!(issues_sort: 'updated_asc') + end + + it 'returns the set preference' do + is_expected.to eq('updated_asc') + end + end + end + + describe '#set_set_order_from_cookie' do + subject { controller.send(:set_sort_order_from_cookie) } + + before do + allow(controller).to receive(:cookies).and_return(cookies) + end + + context 'when sort param given' do + let(:cookies) { {} } + let(:params) { { sort: 'downvotes_asc' } } + + it 'sets the cookie with the right values and flags' do + subject + + expect(cookies['issue_sort']).to eq(value: 'popularity', secure: false, httponly: false) + end + end + + context 'when cookie exists' do + let(:cookies) { { 'issue_sort' => 'id_asc' } } + let(:params) { {} } + + it 'sets the cookie with the right values and flags' do + subject + + expect(cookies['issue_sort']).to eq(value: 'created_asc', secure: false, httponly: false) + end + end + end +end diff --git a/spec/controllers/dashboard/projects_controller_spec.rb b/spec/controllers/dashboard/projects_controller_spec.rb index 6591901a9dc..8b95c9f2496 100644 --- a/spec/controllers/dashboard/projects_controller_spec.rb +++ b/spec/controllers/dashboard/projects_controller_spec.rb @@ -40,6 +40,14 @@ describe Dashboard::ProjectsController do expect(assigns(:projects)).to eq([project, project2]) end + + context 'project sorting' do + let(:project) { create(:project) } + + it_behaves_like 'set sort order from user preference' do + let(:sorting_param) { 'created_asc' } + end + end end end diff --git a/spec/controllers/explore/projects_controller_spec.rb b/spec/controllers/explore/projects_controller_spec.rb index 463586ee422..6752d2b8ebd 100644 --- a/spec/controllers/explore/projects_controller_spec.rb +++ b/spec/controllers/explore/projects_controller_spec.rb @@ -3,56 +3,91 @@ require 'spec_helper' describe Explore::ProjectsController do - describe 'GET #index.json' do - render_views + shared_examples 'explore projects' do + describe 'GET #index.json' do + render_views - before do - get :index, format: :json + before do + get :index, format: :json + end + + it { is_expected.to respond_with(:success) } end - it { is_expected.to respond_with(:success) } - end + describe 'GET #trending.json' do + render_views - describe 'GET #trending.json' do - render_views + before do + get :trending, format: :json + end - before do - get :trending, format: :json + it { is_expected.to respond_with(:success) } + end + + describe 'GET #starred.json' do + render_views + + before do + get :starred, format: :json + end + + it { is_expected.to respond_with(:success) } end - it { is_expected.to respond_with(:success) } + describe 'GET #trending' do + context 'sorting by update date' do + let(:project1) { create(:project, :public, updated_at: 3.days.ago) } + let(:project2) { create(:project, :public, updated_at: 1.day.ago) } + + before do + create(:trending_project, project: project1) + create(:trending_project, project: project2) + end + + it 'sorts by last updated' do + get :trending, params: { sort: 'updated_desc' } + + expect(assigns(:projects)).to eq [project2, project1] + end + + it 'sorts by oldest updated' do + get :trending, params: { sort: 'updated_asc' } + + expect(assigns(:projects)).to eq [project1, project2] + end + end + end end - describe 'GET #starred.json' do - render_views + context 'when user is signed in' do + let(:user) { create(:user) } before do - get :starred, format: :json + sign_in(user) end - it { is_expected.to respond_with(:success) } - end + include_examples 'explore projects' - describe 'GET #trending' do - context 'sorting by update date' do - let(:project1) { create(:project, :public, updated_at: 3.days.ago) } - let(:project2) { create(:project, :public, updated_at: 1.day.ago) } + context 'user preference sorting' do + let(:project) { create(:project) } - before do - create(:trending_project, project: project1) - create(:trending_project, project: project2) + it_behaves_like 'set sort order from user preference' do + let(:sorting_param) { 'created_asc' } end + end + end - it 'sorts by last updated' do - get :trending, params: { sort: 'updated_desc' } + context 'when user is not signed in' do + include_examples 'explore projects' - expect(assigns(:projects)).to eq [project2, project1] - end + context 'user preference sorting' do + let(:project) { create(:project) } + let(:sorting_param) { 'created_asc' } - it 'sorts by oldest updated' do - get :trending, params: { sort: 'updated_asc' } + it 'does not set sort order from user preference' do + expect_any_instance_of(UserPreference).not_to receive(:update) - expect(assigns(:projects)).to eq [project1, project2] + get :index, params: { sort: sorting_param } end end end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index fab47aa4701..187c7864ad7 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1104,18 +1104,39 @@ describe Projects::IssuesController do project.add_developer(user) end + subject do + post(:toggle_award_emoji, params: { + namespace_id: project.namespace, + project_id: project, + id: issue.iid, + name: emoji_name + }) + end + let(:emoji_name) { 'thumbsup' } + it "toggles the award emoji" do expect do - post(:toggle_award_emoji, params: { - namespace_id: project.namespace, - project_id: project, - id: issue.iid, - name: "thumbsup" - }) + subject end.to change { issue.award_emoji.count }.by(1) expect(response).to have_gitlab_http_status(200) end + + it "removes the already awarded emoji" do + create(:award_emoji, awardable: issue, name: emoji_name, user: user) + + expect { subject }.to change { AwardEmoji.count }.by(-1) + + expect(response).to have_gitlab_http_status(200) + end + + it 'marks Todos on the Issue as done' do + todo = create(:todo, target: issue, project: project, user: user) + + subject + + expect(todo.reload).to be_done + end end describe 'POST create_merge_request' do diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 9ab565dc2e8..4500c412521 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -543,23 +543,32 @@ describe Projects::NotesController do project.add_developer(user) end + subject { post(:toggle_award_emoji, params: request_params.merge(name: emoji_name)) } + let(:emoji_name) { 'thumbsup' } + it "toggles the award emoji" do expect do - post(:toggle_award_emoji, params: request_params.merge(name: "thumbsup")) + subject end.to change { note.award_emoji.count }.by(1) expect(response).to have_gitlab_http_status(200) end it "removes the already awarded emoji" do - post(:toggle_award_emoji, params: request_params.merge(name: "thumbsup")) + create(:award_emoji, awardable: note, name: emoji_name, user: user) - expect do - post(:toggle_award_emoji, params: request_params.merge(name: "thumbsup")) - end.to change { AwardEmoji.count }.by(-1) + expect { subject }.to change { AwardEmoji.count }.by(-1) expect(response).to have_gitlab_http_status(200) end + + it 'marks Todos on the Noteable as done' do + todo = create(:todo, target: note.noteable, project: project, user: user) + + subject + + expect(todo.reload).to be_done + end end describe "resolving and unresolving" do diff --git a/spec/controllers/projects/wikis_controller_spec.rb b/spec/controllers/projects/wikis_controller_spec.rb index fbca1d5740f..6fea6bca4f2 100644 --- a/spec/controllers/projects/wikis_controller_spec.rb +++ b/spec/controllers/projects/wikis_controller_spec.rb @@ -3,11 +3,11 @@ require 'spec_helper' describe Projects::WikisController do - let(:project) { create(:project, :public, :repository) } - let(:user) { project.owner } + set(:project) { create(:project, :public, :repository) } + set(:user) { project.owner } let(:project_wiki) { ProjectWiki.new(project, user) } let(:wiki) { project_wiki.wiki } - let(:wiki_title) { 'page-title-test' } + let(:wiki_title) { 'page title test' } before do create_page(wiki_title, 'hello world') @@ -19,6 +19,21 @@ describe Projects::WikisController do destroy_page(wiki_title) end + describe 'GET #new' do + subject { get :new, params: { namespace_id: project.namespace, project_id: project } } + + it 'redirects to #show and appends a `random_title` param' do + subject + + expect(response).to have_http_status(302) + expect(Rails.application.routes.recognize_path(response.redirect_url)).to include( + controller: 'projects/wikis', + action: 'show' + ) + expect(response.redirect_url).to match(/\?random_title=true\Z/) + end + end + describe 'GET #pages' do subject { get :pages, params: { namespace_id: project.namespace, project_id: project, id: wiki_title } } @@ -75,40 +90,62 @@ describe Projects::WikisController do describe 'GET #show' do render_views - subject { get :show, params: { namespace_id: project.namespace, project_id: project, id: wiki_title } } + let(:random_title) { nil } - it 'limits the retrieved pages for the sidebar' do - expect(controller).to receive(:load_wiki).and_return(project_wiki) + subject { get :show, params: { namespace_id: project.namespace, project_id: project, id: id, random_title: random_title } } - # empty? call - expect(project_wiki).to receive(:list_pages).with(limit: 1).and_call_original - # Sidebar entries - expect(project_wiki).to receive(:list_pages).with(limit: 15).and_call_original + context 'when page exists' do + let(:id) { wiki_title } - subject + it 'limits the retrieved pages for the sidebar' do + expect(controller).to receive(:load_wiki).and_return(project_wiki) + expect(project_wiki).to receive(:list_pages).with(limit: 15).and_call_original + + subject + + expect(response).to have_http_status(:ok) + expect(assigns(:page).title).to eq(wiki_title) + end + + context 'when page content encoding is invalid' do + it 'sets flash error' do + allow(controller).to receive(:valid_encoding?).and_return(false) - expect(response).to have_http_status(:ok) - expect(response.body).to include(wiki_title) + subject + + expect(response).to have_http_status(:ok) + expect(flash[:notice]).to eq('The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository.') + end + end end - context 'when page content encoding is invalid' do - it 'sets flash error' do - allow(controller).to receive(:valid_encoding?).and_return(false) + context 'when the page does not exist' do + let(:id) { 'does not exist' } + before do subject + end - expect(response).to have_http_status(:ok) - expect(flash[:notice]).to eq 'The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository.' + it 'builds a new wiki page with the id as the title' do + expect(assigns(:page).title).to eq(id) + end + + context 'when a random_title param is present' do + let(:random_title) { true } + + it 'builds a new wiki page with no title' do + expect(assigns(:page).title).to be_empty + end end end context 'when page is a file' do include WikiHelpers - let(:path) { upload_file_to_wiki(project, user, file_name) } + let(:id) { upload_file_to_wiki(project, user, file_name) } before do - get :show, params: { namespace_id: project.namespace, project_id: project, id: path } + subject end context 'when file is an image' do diff --git a/spec/controllers/snippets/notes_controller_spec.rb b/spec/controllers/snippets/notes_controller_spec.rb index 652533ac49f..fd4b95ce226 100644 --- a/spec/controllers/snippets/notes_controller_spec.rb +++ b/spec/controllers/snippets/notes_controller_spec.rb @@ -288,11 +288,13 @@ describe Snippets::NotesController do describe 'POST toggle_award_emoji' do let(:note) { create(:note_on_personal_snippet, noteable: public_snippet) } + let(:emoji_name) { 'thumbsup'} + before do sign_in(user) end - subject { post(:toggle_award_emoji, params: { snippet_id: public_snippet, id: note.id, name: "thumbsup" }) } + subject { post(:toggle_award_emoji, params: { snippet_id: public_snippet, id: note.id, name: emoji_name }) } it "toggles the award emoji" do expect { subject }.to change { note.award_emoji.count }.by(1) @@ -301,7 +303,7 @@ describe Snippets::NotesController do end it "removes the already awarded emoji when it exists" do - note.toggle_award_emoji('thumbsup', user) # create award emoji before + create(:award_emoji, awardable: note, name: emoji_name, user: user) expect { subject }.to change { AwardEmoji.count }.by(-1) diff --git a/spec/features/projects/wiki/markdown_preview_spec.rb b/spec/features/projects/wiki/markdown_preview_spec.rb index 7b511c4d3d5..5c6b04a7141 100644 --- a/spec/features/projects/wiki/markdown_preview_spec.rb +++ b/spec/features/projects/wiki/markdown_preview_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe 'Projects > Wiki > User previews markdown changes', :js do - let(:user) { create(:user) } + set(:user) { create(:user) } let(:project) { create(:project, :wiki_repo, namespace: user.namespace) } let(:wiki_page) { create(:wiki_page, wiki: project.wiki, attrs: { title: 'home', content: '[some link](other-page)' }) } let(:wiki_content) do @@ -20,23 +20,12 @@ describe 'Projects > Wiki > User previews markdown changes', :js do project.add_maintainer(user) sign_in(user) - - visit project_wiki_path(project, wiki_page) end context "while creating a new wiki page" do context "when there are no spaces or hyphens in the page name" do it "rewrites relative links as expected" do - find('.add-new-wiki').click - page.within '#modal-new-wiki' do - fill_in :new_wiki_path, with: 'a/b/c/d' - click_button 'Create page' - end - - page.within '.wiki-form' do - fill_in :wiki_content, with: wiki_content - click_on "Preview" - end + create_wiki_page('a/b/c/d', content: wiki_content) expect(page).to have_content("regular link") @@ -50,16 +39,7 @@ describe 'Projects > Wiki > User previews markdown changes', :js do context "when there are spaces in the page name" do it "rewrites relative links as expected" do - click_link 'New page' - page.within '#modal-new-wiki' do - fill_in :new_wiki_path, with: 'a page/b page/c page/d page' - click_button 'Create page' - end - - page.within '.wiki-form' do - fill_in :wiki_content, with: wiki_content - click_on "Preview" - end + create_wiki_page('a page/b page/c page/d page', content: wiki_content) expect(page).to have_content("regular link") @@ -73,16 +53,7 @@ describe 'Projects > Wiki > User previews markdown changes', :js do context "when there are hyphens in the page name" do it "rewrites relative links as expected" do - click_link 'New page' - page.within '#modal-new-wiki' do - fill_in :new_wiki_path, with: 'a-page/b-page/c-page/d-page' - click_button 'Create page' - end - - page.within '.wiki-form' do - fill_in :wiki_content, with: wiki_content - click_on "Preview" - end + create_wiki_page('a-page/b-page/c-page/d-page', content: wiki_content) expect(page).to have_content("regular link") @@ -96,23 +67,9 @@ describe 'Projects > Wiki > User previews markdown changes', :js do end context "while editing a wiki page" do - def create_wiki_page(path) - find('.add-new-wiki').click - - page.within '#modal-new-wiki' do - fill_in :new_wiki_path, with: path - click_button 'Create page' - end - - page.within '.wiki-form' do - fill_in :wiki_content, with: 'content' - click_on "Create page" - end - end - context "when there are no spaces or hyphens in the page name" do it "rewrites relative links as expected" do - create_wiki_page 'a/b/c/d' + create_wiki_page('a/b/c/d') click_link 'Edit' fill_in :wiki_content, with: wiki_content @@ -130,7 +87,7 @@ describe 'Projects > Wiki > User previews markdown changes', :js do context "when there are spaces in the page name" do it "rewrites relative links as expected" do - create_wiki_page 'a page/b page/c page/d page' + create_wiki_page('a page/b page/c page/d page') click_link 'Edit' fill_in :wiki_content, with: wiki_content @@ -148,7 +105,7 @@ describe 'Projects > Wiki > User previews markdown changes', :js do context "when there are hyphens in the page name" do it "rewrites relative links as expected" do - create_wiki_page 'a-page/b-page/c-page/d-page' + create_wiki_page('a-page/b-page/c-page/d-page') click_link 'Edit' fill_in :wiki_content, with: wiki_content @@ -166,7 +123,7 @@ describe 'Projects > Wiki > User previews markdown changes', :js do context 'when rendering the preview' do it 'renders content with CommonMark' do - create_wiki_page 'a-page/b-page/c-page/common-mark' + create_wiki_page('a-page/b-page/c-page/common-mark') click_link 'Edit' fill_in :wiki_content, with: "1. one\n - sublist\n" @@ -180,25 +137,31 @@ describe 'Projects > Wiki > User previews markdown changes', :js do end it "does not linkify double brackets inside code blocks as expected" do - click_link 'New page' - page.within '#modal-new-wiki' do - fill_in :new_wiki_path, with: 'linkify_test' - click_button 'Create page' - end + wiki_content = <<-HEREDOC + `[[do_not_linkify]]` + ``` + [[also_do_not_linkify]] + ``` + HEREDOC - page.within '.wiki-form' do - fill_in :wiki_content, with: <<-HEREDOC - `[[do_not_linkify]]` - ``` - [[also_do_not_linkify]] - ``` - HEREDOC - click_on "Preview" - end + create_wiki_page('linkify_test', wiki_content) expect(page).to have_content("do_not_linkify") expect(page.html).to include('[[do_not_linkify]]') expect(page.html).to include('[[also_do_not_linkify]]') end + + private + + def create_wiki_page(path, content = 'content') + visit project_wiki_path(project, wiki_page) + + click_link 'New page' + + fill_in :wiki_title, with: path + fill_in :wiki_content, with: content + + click_button 'Create page' + end end diff --git a/spec/features/projects/wiki/user_creates_wiki_page_spec.rb b/spec/features/projects/wiki/user_creates_wiki_page_spec.rb index cc6dbaa6eb8..56d0518015d 100644 --- a/spec/features/projects/wiki/user_creates_wiki_page_spec.rb +++ b/spec/features/projects/wiki/user_creates_wiki_page_spec.rb @@ -42,10 +42,10 @@ describe "User creates wiki page" do click_link("link test") - expect(page).to have_content("Create Page") + expect(page).to have_content("Create New Page") end - it "shows non-escaped link in the pages list", :js, :quarantine do + it "shows non-escaped link in the pages list", :quarantine do fill_in(:wiki_title, with: "one/two/three-test") page.within(".wiki-form") do @@ -58,7 +58,9 @@ describe "User creates wiki page" do expect(page).to have_xpath("//a[@href='/#{project.full_path}/wikis/one/two/three-test']") end - it "has `Create home` as a commit message" do + it "has `Create home` as a commit message", :js do + wait_for_requests + expect(page).to have_field("wiki[message]", with: "Create home") end @@ -81,7 +83,7 @@ describe "User creates wiki page" do expect(current_path).to eq(project_wiki_path(project, "test")) page.within(:css, ".nav-text") do - expect(page).to have_content("test").and have_content("Create Page") + expect(page).to have_content("Create New Page") end click_link("Home") @@ -93,7 +95,7 @@ describe "User creates wiki page" do expect(current_path).to eq(project_wiki_path(project, "api")) page.within(:css, ".nav-text") do - expect(page).to have_content("Create").and have_content("api") + expect(page).to have_content("Create") end click_link("Home") @@ -105,7 +107,7 @@ describe "User creates wiki page" do expect(current_path).to eq(project_wiki_path(project, "raketasks")) page.within(:css, ".nav-text") do - expect(page).to have_content("Create").and have_content("rake") + expect(page).to have_content("Create") end end @@ -150,6 +152,8 @@ describe "User creates wiki page" do let(:project) { create(:project, :wiki_repo, namespace: create(:group, :public)) } it "has `Create home` as a commit message" do + wait_for_requests + expect(page).to have_field("wiki[message]", with: "Create home") end @@ -181,20 +185,15 @@ describe "User creates wiki page" do it "creates a page with a single word" do click_link("New page") - page.within("#modal-new-wiki") do - fill_in(:new_wiki_path, with: "foo") - - click_button("Create page") + page.within(".wiki-form") do + fill_in(:wiki_title, with: "foo") + fill_in(:wiki_content, with: "My awesome wiki!") end # Commit message field should have correct value. expect(page).to have_field("wiki[message]", with: "Create foo") - page.within(".wiki-form") do - fill_in(:wiki_content, with: "My awesome wiki!") - - click_button("Create page") - end + click_button("Create page") expect(page).to have_content("foo") .and have_content("Last edited by #{user.name}") @@ -204,20 +203,15 @@ describe "User creates wiki page" do it "creates a page with spaces in the name" do click_link("New page") - page.within("#modal-new-wiki") do - fill_in(:new_wiki_path, with: "Spaces in the name") - - click_button("Create page") + page.within(".wiki-form") do + fill_in(:wiki_title, with: "Spaces in the name") + fill_in(:wiki_content, with: "My awesome wiki!") end # Commit message field should have correct value. expect(page).to have_field("wiki[message]", with: "Create Spaces in the name") - page.within(".wiki-form") do - fill_in(:wiki_content, with: "My awesome wiki!") - - click_button("Create page") - end + click_button("Create page") expect(page).to have_content("Spaces in the name") .and have_content("Last edited by #{user.name}") @@ -227,10 +221,9 @@ describe "User creates wiki page" do it "creates a page with hyphens in the name" do click_link("New page") - page.within("#modal-new-wiki") do - fill_in(:new_wiki_path, with: "hyphens-in-the-name") - - click_button("Create page") + page.within(".wiki-form") do + fill_in(:wiki_title, with: "hyphens-in-the-name") + fill_in(:wiki_content, with: "My awesome wiki!") end # Commit message field should have correct value. @@ -251,12 +244,6 @@ describe "User creates wiki page" do it "shows the emoji autocompletion dropdown" do click_link("New page") - page.within("#modal-new-wiki") do - fill_in(:new_wiki_path, with: "test-autocomplete") - - click_button("Create page") - end - page.within(".wiki-form") do find("#wiki_content").native.send_keys("") @@ -274,20 +261,15 @@ describe "User creates wiki page" do it "creates a page" do click_link("New page") - page.within("#modal-new-wiki") do - fill_in(:new_wiki_path, with: "foo") - - click_button("Create page") + page.within(".wiki-form") do + fill_in(:wiki_title, with: "foo") + fill_in(:wiki_content, with: "My awesome wiki!") end # Commit message field should have correct value. expect(page).to have_field("wiki[message]", with: "Create foo") - page.within(".wiki-form") do - fill_in(:wiki_content, with: "My awesome wiki!") - - click_button("Create page") - end + click_button("Create page") expect(page).to have_content("foo") .and have_content("Last edited by #{user.name}") diff --git a/spec/features/projects/wiki/user_updates_wiki_page_spec.rb b/spec/features/projects/wiki/user_updates_wiki_page_spec.rb index 2aab8fda62d..3f3711f9eb8 100644 --- a/spec/features/projects/wiki/user_updates_wiki_page_spec.rb +++ b/spec/features/projects/wiki/user_updates_wiki_page_spec.rb @@ -70,7 +70,7 @@ describe 'User updates wiki page' do context 'in a user namespace' do let(:project) { create(:project, :wiki_repo) } - it 'updates a page' do + it 'updates a page', :js do # Commit message field should have correct value. expect(page).to have_field('wiki[message]', with: 'Update home') @@ -82,6 +82,18 @@ describe 'User updates wiki page' do expect(page).to have_content('My awesome wiki!') end + it 'updates the commit message as the title is changed', :js do + fill_in(:wiki_title, with: 'Wiki title') + + expect(page).to have_field('wiki[message]', with: 'Update Wiki title') + end + + it 'does not allow XSS', :js do + fill_in(:wiki_title, with: '<script>') + + expect(page).to have_field('wiki[message]', with: 'Update <script>') + end + it 'shows a validation error message' do fill_in(:wiki_content, with: '') click_button('Save changes') @@ -129,7 +141,7 @@ describe 'User updates wiki page' do context 'in a group namespace' do let(:project) { create(:project, :wiki_repo, namespace: create(:group, :public)) } - it 'updates a page' do + it 'updates a page', :js do # Commit message field should have correct value. expect(page).to have_field('wiki[message]', with: 'Update home') diff --git a/spec/features/projects/wiki/user_views_wiki_page_spec.rb b/spec/features/projects/wiki/user_views_wiki_page_spec.rb index 05742b63c43..77e725e7f11 100644 --- a/spec/features/projects/wiki/user_views_wiki_page_spec.rb +++ b/spec/features/projects/wiki/user_views_wiki_page_spec.rb @@ -101,8 +101,7 @@ describe 'User views a wiki page' do click_on('image') expect(current_path).to match("wikis/#{path}") - expect(page).to have_content('New Wiki Page') - expect(page).to have_content('Create page') + expect(page).to have_content('Create New Page') end end @@ -156,6 +155,6 @@ describe 'User views a wiki page' do find('.shortcuts-wiki').click click_link "Create your first page" - expect(page).to have_content('Home · Create Page') + expect(page).to have_content('Create New Page') end end diff --git a/spec/finders/award_emojis_finder_spec.rb b/spec/finders/award_emojis_finder_spec.rb new file mode 100644 index 00000000000..ccac475daad --- /dev/null +++ b/spec/finders/award_emojis_finder_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe AwardEmojisFinder do + set(:issue_1) { create(:issue) } + set(:issue_1_thumbsup) { create(:award_emoji, name: 'thumbsup', awardable: issue_1) } + set(:issue_1_thumbsdown) { create(:award_emoji, name: 'thumbsdown', awardable: issue_1) } + # Create a matching set of emoji for a second issue. + # These should never appear in our finder results + set(:issue_2) { create(:issue) } + set(:issue_2_thumbsup) { create(:award_emoji, name: 'thumbsup', awardable: issue_2) } + set(:issue_2_thumbsdown) { create(:award_emoji, name: 'thumbsdown', awardable: issue_2) } + + describe 'param validation' do + it 'raises an error if `name` is invalid' do + expect { described_class.new(issue_1, { name: 'invalid' }).execute }.to raise_error( + ArgumentError, + 'Invalid name param' + ) + end + + it 'raises an error if `awarded_by` is invalid' do + expectation = [ArgumentError, 'Invalid awarded_by param'] + + expect { described_class.new(issue_1, { awarded_by: issue_2 }).execute }.to raise_error(*expectation) + expect { described_class.new(issue_1, { awarded_by: 'not-an-id' }).execute }.to raise_error(*expectation) + expect { described_class.new(issue_1, { awarded_by: 1.123 }).execute }.to raise_error(*expectation) + end + end + + describe '#execute' do + it 'scopes to the awardable' do + expect(described_class.new(issue_1).execute).to contain_exactly( + issue_1_thumbsup, issue_1_thumbsdown + ) + end + + it 'filters by emoji name' do + expect(described_class.new(issue_1, { name: 'thumbsup' }).execute).to contain_exactly(issue_1_thumbsup) + expect(described_class.new(issue_1, { name: '8ball' }).execute).to be_empty + end + + it 'filters by user' do + expect(described_class.new(issue_1, { awarded_by: issue_1_thumbsup.user }).execute).to contain_exactly(issue_1_thumbsup) + expect(described_class.new(issue_1, { awarded_by: issue_2_thumbsup.user }).execute).to be_empty + end + end +end diff --git a/spec/fixtures/api/schemas/deployment.json b/spec/fixtures/api/schemas/deployment.json index 9216ad0060b..fe725b97c21 100644 --- a/spec/fixtures/api/schemas/deployment.json +++ b/spec/fixtures/api/schemas/deployment.json @@ -3,7 +3,7 @@ "required": [ "sha", "created_at", - "finished_at", + "deployed_at", "iid", "tag", "last?", @@ -12,7 +12,7 @@ ], "properties": { "created_at": { "type": "string" }, - "finished_at": { "type": ["string", "null"] }, + "deployed_at": { "type": ["string", "null"] }, "id": { "type": "integer" }, "iid": { "type": "integer" }, "last?": { "type": "boolean" }, diff --git a/spec/fixtures/api/schemas/entities/merge_request_sidebar.json b/spec/fixtures/api/schemas/entities/merge_request_sidebar.json index 214b67a9a0f..9945de8a856 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_sidebar.json +++ b/spec/fixtures/api/schemas/entities/merge_request_sidebar.json @@ -2,6 +2,7 @@ "type": "object", "properties" : { "id": { "type": "integer" }, + "iid": { "type": "integer" }, "type": { "type": "string" }, "author_id": { "type": "integer" }, "project_id": { "type": "integer" }, diff --git a/spec/frontend/autosave_spec.js b/spec/frontend/autosave_spec.js index 4d9c8f96d62..33d402388c9 100644 --- a/spec/frontend/autosave_spec.js +++ b/spec/frontend/autosave_spec.js @@ -63,12 +63,15 @@ describe('Autosave', () => { expect(field.trigger).toHaveBeenCalled(); }); - it('triggers native event', done => { - autosave.field.get(0).addEventListener('change', () => { - done(); - }); + it('triggers native event', () => { + const fieldElement = autosave.field.get(0); + const eventHandler = jest.fn(); + fieldElement.addEventListener('change', eventHandler); Autosave.prototype.restore.call(autosave); + + expect(eventHandler).toHaveBeenCalledTimes(1); + fieldElement.removeEventListener('change', eventHandler); }); }); diff --git a/spec/frontend/sidebar/components/assignees/assignee_avatar_link_spec.js b/spec/frontend/sidebar/components/assignees/assignee_avatar_link_spec.js new file mode 100644 index 00000000000..452d4cd07cc --- /dev/null +++ b/spec/frontend/sidebar/components/assignees/assignee_avatar_link_spec.js @@ -0,0 +1,85 @@ +import { shallowMount } from '@vue/test-utils'; +import { joinPaths } from '~/lib/utils/url_utility'; +import { TEST_HOST } from 'helpers/test_constants'; +import AssigneeAvatarLink from '~/sidebar/components/assignees/assignee_avatar_link.vue'; +import AssigneeAvatar from '~/sidebar/components/assignees/assignee_avatar.vue'; +import userDataMock from '../../user_data_mock'; + +const TOOLTIP_PLACEMENT = 'bottom'; +const { name: USER_NAME, username: USER_USERNAME } = userDataMock(); +const TEST_ISSUABLE_TYPE = 'merge_request'; + +describe('AssigneeAvatarLink component', () => { + let wrapper; + + function createComponent(props = {}) { + const propsData = { + user: userDataMock(), + showLess: true, + rootPath: TEST_HOST, + tooltipPlacement: TOOLTIP_PLACEMENT, + singleUser: false, + issuableType: TEST_ISSUABLE_TYPE, + ...props, + }; + + wrapper = shallowMount(AssigneeAvatarLink, { + propsData, + sync: false, + }); + } + + afterEach(() => { + wrapper.destroy(); + }); + + const findTooltipText = () => wrapper.attributes('data-original-title'); + + it('has the root url present in the assigneeUrl method', () => { + createComponent(); + const assigneeUrl = joinPaths(TEST_HOST, USER_USERNAME); + + expect(wrapper.attributes().href).toEqual(assigneeUrl); + }); + + it('renders assignee avatar', () => { + createComponent(); + + expect(wrapper.find(AssigneeAvatar).props()).toEqual( + expect.objectContaining({ + issuableType: TEST_ISSUABLE_TYPE, + user: userDataMock(), + }), + ); + }); + + describe.each` + issuableType | tooltipHasName | canMerge | expected + ${'merge_request'} | ${true} | ${true} | ${USER_NAME} + ${'merge_request'} | ${true} | ${false} | ${`${USER_NAME} (cannot merge)`} + ${'merge_request'} | ${false} | ${true} | ${''} + ${'merge_request'} | ${false} | ${false} | ${'Cannot merge'} + ${'issue'} | ${true} | ${true} | ${USER_NAME} + ${'issue'} | ${true} | ${false} | ${USER_NAME} + ${'issue'} | ${false} | ${true} | ${''} + ${'issue'} | ${false} | ${false} | ${''} + `( + 'with $issuableType and tooltipHasName=$tooltipHasName and canMerge=$canMerge', + ({ issuableType, tooltipHasName, canMerge, expected }) => { + beforeEach(() => { + createComponent({ + issuableType, + tooltipHasName, + user: { + ...userDataMock(), + can_merge: canMerge, + }, + }); + }); + + it('sets tooltip', () => { + expect(findTooltipText()).toBe(expected); + }); + }, + ); +}); diff --git a/spec/frontend/sidebar/components/assignees/assignee_avatar_spec.js b/spec/frontend/sidebar/components/assignees/assignee_avatar_spec.js new file mode 100644 index 00000000000..d60ae17733b --- /dev/null +++ b/spec/frontend/sidebar/components/assignees/assignee_avatar_spec.js @@ -0,0 +1,78 @@ +import { shallowMount } from '@vue/test-utils'; +import AssigneeAvatar from '~/sidebar/components/assignees/assignee_avatar.vue'; +import { TEST_HOST } from 'helpers/test_constants'; +import userDataMock from '../../user_data_mock'; + +const TEST_AVATAR = `${TEST_HOST}/avatar.png`; +const TEST_DEFAULT_AVATAR_URL = `${TEST_HOST}/default/avatar/url.png`; + +describe('AssigneeAvatar', () => { + let origGon; + let wrapper; + + function createComponent(props = {}) { + const propsData = { + user: userDataMock(), + imgSize: 24, + issuableType: 'merge_request', + ...props, + }; + + wrapper = shallowMount(AssigneeAvatar, { + propsData, + sync: false, + }); + } + + beforeEach(() => { + origGon = window.gon; + window.gon = { default_avatar_url: TEST_DEFAULT_AVATAR_URL }; + }); + + afterEach(() => { + window.gon = origGon; + wrapper.destroy(); + }); + + const findImg = () => wrapper.find('img'); + + it('does not show warning icon if assignee can merge', () => { + createComponent(); + + expect(wrapper.find('.merge-icon').exists()).toBe(false); + }); + + it('shows warning icon if assignee cannot merge', () => { + createComponent({ + user: { + can_merge: false, + }, + }); + + expect(wrapper.find('.merge-icon').exists()).toBe(true); + }); + + it('does not show warning icon for issuableType = "issue"', () => { + createComponent({ + issuableType: 'issue', + }); + + expect(wrapper.find('.merge-icon').exists()).toBe(false); + }); + + it.each` + avatar | avatar_url | expected | desc + ${TEST_AVATAR} | ${null} | ${TEST_AVATAR} | ${'with avatar'} + ${null} | ${TEST_AVATAR} | ${TEST_AVATAR} | ${'with avatar_url'} + ${null} | ${null} | ${TEST_DEFAULT_AVATAR_URL} | ${'with no avatar'} + `('$desc', ({ avatar, avatar_url, expected }) => { + createComponent({ + user: { + avatar, + avatar_url, + }, + }); + + expect(findImg().attributes('src')).toEqual(expected); + }); +}); diff --git a/spec/frontend/sidebar/components/assignees/collapsed_assignee_list_spec.js b/spec/frontend/sidebar/components/assignees/collapsed_assignee_list_spec.js new file mode 100644 index 00000000000..ff0c8d181b5 --- /dev/null +++ b/spec/frontend/sidebar/components/assignees/collapsed_assignee_list_spec.js @@ -0,0 +1,189 @@ +import { shallowMount } from '@vue/test-utils'; +import CollapsedAssigneeList from '~/sidebar/components/assignees/collapsed_assignee_list.vue'; +import CollapsedAssignee from '~/sidebar/components/assignees/collapsed_assignee.vue'; +import UsersMockHelper from 'helpers/user_mock_data_helper'; + +const DEFAULT_MAX_COUNTER = 99; + +describe('CollapsedAssigneeList component', () => { + let wrapper; + + function createComponent(props = {}) { + const propsData = { + users: [], + issuableType: 'merge_request', + ...props, + }; + + wrapper = shallowMount(CollapsedAssigneeList, { + propsData, + sync: false, + }); + } + + const findNoUsersIcon = () => wrapper.find('i[aria-label=None]'); + const findAvatarCounter = () => wrapper.find('.avatar-counter'); + const findAssignees = () => wrapper.findAll(CollapsedAssignee); + const getTooltipTitle = () => wrapper.attributes('data-original-title'); + + afterEach(() => { + wrapper.destroy(); + }); + + describe('No assignees/users', () => { + beforeEach(() => { + createComponent({ + users: [], + }); + }); + + it('has no users', () => { + expect(findNoUsersIcon().exists()).toBe(true); + }); + }); + + describe('One assignee/user', () => { + let users; + + beforeEach(() => { + users = UsersMockHelper.createNumberRandomUsers(1); + }); + + it('should not show no users icon', () => { + createComponent({ users }); + + expect(findNoUsersIcon().exists()).toBe(false); + }); + + it('has correct "cannot merge" tooltip when user cannot merge', () => { + users[0].can_merge = false; + + createComponent({ users }); + + expect(getTooltipTitle()).toContain('cannot merge'); + }); + + it('does not have "merge" word in tooltip if user can merge', () => { + users[0].can_merge = true; + + createComponent({ users }); + + expect(getTooltipTitle()).not.toContain('merge'); + }); + }); + + describe('More than one assignees/users', () => { + let users; + + beforeEach(() => { + users = UsersMockHelper.createNumberRandomUsers(2); + + createComponent({ users }); + }); + + it('has multiple-users class', () => { + expect(wrapper.classes('multiple-users')).toBe(true); + }); + + it('does not display an avatar count', () => { + expect(findAvatarCounter().exists()).toBe(false); + }); + + it('returns just two collapsed users', () => { + expect(findAssignees().length).toBe(2); + }); + }); + + describe('More than two assignees/users', () => { + let users; + let userNames; + + beforeEach(() => { + users = UsersMockHelper.createNumberRandomUsers(3); + userNames = users.map(x => x.name).join(', '); + }); + + describe('default', () => { + beforeEach(() => { + createComponent({ users }); + }); + + it('does display an avatar count', () => { + expect(findAvatarCounter().exists()).toBe(true); + expect(findAvatarCounter().text()).toEqual('+2'); + }); + + it('returns one collapsed users', () => { + expect(findAssignees().length).toBe(1); + }); + }); + + it('has corrent "no one can merge" tooltip when no one can merge', () => { + users[0].can_merge = false; + users[1].can_merge = false; + users[2].can_merge = false; + + createComponent({ + users, + }); + + expect(getTooltipTitle()).toEqual(`${userNames} (no one can merge)`); + }); + + it('has correct "cannot merge" tooltip when one user can merge', () => { + users[0].can_merge = true; + users[1].can_merge = false; + users[2].can_merge = false; + + createComponent({ + users, + }); + + expect(getTooltipTitle()).toEqual(`${userNames} (1/3 can merge)`); + }); + + it('has correct "cannot merge" tooltip when more than one user can merge', () => { + users[0].can_merge = false; + users[1].can_merge = true; + users[2].can_merge = true; + + createComponent({ + users, + }); + + expect(getTooltipTitle()).toEqual(`${userNames} (2/3 can merge)`); + }); + + it('does not have "merge" in tooltip if everyone can merge', () => { + users[0].can_merge = true; + users[1].can_merge = true; + users[2].can_merge = true; + + createComponent({ + users, + }); + + expect(getTooltipTitle()).toEqual(userNames); + }); + + it('displays the correct avatar count', () => { + users = UsersMockHelper.createNumberRandomUsers(5); + + createComponent({ + users, + }); + + expect(findAvatarCounter().text()).toEqual(`+${users.length - 1}`); + }); + + it('displays the correct avatar count via a computed property if more than default max counter', () => { + users = UsersMockHelper.createNumberRandomUsers(100); + + createComponent({ + users, + }); + + expect(findAvatarCounter().text()).toEqual(`${DEFAULT_MAX_COUNTER}+`); + }); + }); +}); diff --git a/spec/frontend/sidebar/components/assignees/collapsed_assignee_spec.js b/spec/frontend/sidebar/components/assignees/collapsed_assignee_spec.js new file mode 100644 index 00000000000..f9ca7bc1ecb --- /dev/null +++ b/spec/frontend/sidebar/components/assignees/collapsed_assignee_spec.js @@ -0,0 +1,49 @@ +import { shallowMount } from '@vue/test-utils'; +import CollapsedAssignee from '~/sidebar/components/assignees/collapsed_assignee.vue'; +import AssigneeAvatar from '~/sidebar/components/assignees/assignee_avatar.vue'; +import userDataMock from '../../user_data_mock'; + +const TEST_USER = userDataMock(); +const TEST_ISSUABLE_TYPE = 'merge_request'; + +describe('CollapsedAssignee assignee component', () => { + let wrapper; + + function createComponent(props = {}) { + const propsData = { + user: userDataMock(), + issuableType: TEST_ISSUABLE_TYPE, + ...props, + }; + + wrapper = shallowMount(CollapsedAssignee, { + propsData, + sync: false, + }); + } + + afterEach(() => { + wrapper.destroy(); + }); + + it('has author name', () => { + createComponent(); + + expect( + wrapper + .find('.author') + .text() + .trim(), + ).toEqual(TEST_USER.name); + }); + + it('has assignee avatar', () => { + createComponent(); + + expect(wrapper.find(AssigneeAvatar).props()).toEqual({ + imgSize: 24, + user: TEST_USER, + issuableType: TEST_ISSUABLE_TYPE, + }); + }); +}); diff --git a/spec/frontend/sidebar/components/assignees/uncollapsed_assignee_list_spec.js b/spec/frontend/sidebar/components/assignees/uncollapsed_assignee_list_spec.js new file mode 100644 index 00000000000..6398351834c --- /dev/null +++ b/spec/frontend/sidebar/components/assignees/uncollapsed_assignee_list_spec.js @@ -0,0 +1,103 @@ +import { mount } from '@vue/test-utils'; +import UncollapsedAssigneeList from '~/sidebar/components/assignees/uncollapsed_assignee_list.vue'; +import AssigneeAvatarLink from '~/sidebar/components/assignees/assignee_avatar_link.vue'; +import { TEST_HOST } from 'helpers/test_constants'; +import userDataMock from '../../user_data_mock'; +import UsersMockHelper from '../../../helpers/user_mock_data_helper'; + +const DEFAULT_RENDER_COUNT = 5; + +describe('UncollapsedAssigneeList component', () => { + let wrapper; + + function createComponent(props = {}) { + const propsData = { + users: [], + rootPath: TEST_HOST, + ...props, + }; + + wrapper = mount(UncollapsedAssigneeList, { + sync: false, + propsData, + }); + } + + afterEach(() => { + wrapper.destroy(); + }); + + const findMoreButton = () => wrapper.find('.user-list-more button'); + + describe('One assignee/user', () => { + let user; + + beforeEach(() => { + user = userDataMock(); + + createComponent({ + users: [user], + }); + }); + + it('only has one user', () => { + expect(wrapper.findAll(AssigneeAvatarLink).length).toBe(1); + }); + + it('calls the AssigneeAvatarLink with the proper props', () => { + expect(wrapper.find(AssigneeAvatarLink).exists()).toBe(true); + expect(wrapper.find(AssigneeAvatarLink).props().tooltipPlacement).toEqual('left'); + }); + + it('Shows one user with avatar, username and author name', () => { + expect(wrapper.text()).toContain(user.name); + expect(wrapper.text()).toContain(`@${user.username}`); + }); + }); + + describe('n+ more label', () => { + describe('when users count is rendered users', () => { + beforeEach(() => { + createComponent({ + users: UsersMockHelper.createNumberRandomUsers(DEFAULT_RENDER_COUNT), + }); + }); + + it('does not show more label', () => { + expect(findMoreButton().exists()).toBe(false); + }); + }); + + describe('when more than rendered users', () => { + beforeEach(() => { + createComponent({ + users: UsersMockHelper.createNumberRandomUsers(DEFAULT_RENDER_COUNT + 1), + }); + }); + + it('shows "+1 more" label', () => { + expect(findMoreButton().text()).toBe('+ 1 more'); + }); + + it('shows truncated users', () => { + expect(wrapper.findAll(AssigneeAvatarLink).length).toBe(DEFAULT_RENDER_COUNT); + }); + + describe('when more button is clicked', () => { + beforeEach(() => { + findMoreButton().trigger('click'); + + return wrapper.vm.$nextTick(); + }); + + it('shows "show less" label', () => { + expect(findMoreButton().text()).toBe('- show less'); + }); + + it('shows all users', () => { + expect(wrapper.findAll(AssigneeAvatarLink).length).toBe(DEFAULT_RENDER_COUNT + 1); + }); + }); + }); + }); +}); diff --git a/spec/frontend/sidebar/user_data_mock.js b/spec/frontend/sidebar/user_data_mock.js new file mode 100644 index 00000000000..8ad70bb3499 --- /dev/null +++ b/spec/frontend/sidebar/user_data_mock.js @@ -0,0 +1,9 @@ +export default () => ({ + avatar_url: 'mock_path', + id: 1, + name: 'Root', + state: 'active', + username: 'root', + web_url: '', + can_merge: true, +}); diff --git a/spec/frontend/test_setup.js b/spec/frontend/test_setup.js index df8a625319b..d52aeb1fe6b 100644 --- a/spec/frontend/test_setup.js +++ b/spec/frontend/test_setup.js @@ -93,3 +93,9 @@ Object.assign(global, { clearTimeout(id); }, }); + +// make sure that each test actually tests something +// see https://jestjs.io/docs/en/expect#expecthasassertions +beforeEach(() => { + expect.hasAssertions(); +}); diff --git a/spec/frontend/wikis_spec.js b/spec/frontend/wikis_spec.js new file mode 100644 index 00000000000..b2475488d97 --- /dev/null +++ b/spec/frontend/wikis_spec.js @@ -0,0 +1,74 @@ +import Wikis from '~/pages/projects/wikis/wikis'; +import { setHTMLFixture } from './helpers/fixtures'; + +describe('Wikis', () => { + describe('setting the commit message when the title changes', () => { + const editFormHtmlFixture = args => `<form class="wiki-form ${ + args.newPage ? 'js-new-wiki-page' : '' + }"> + <input type="text" id="wiki_title" value="My title" /> + <input type="text" id="wiki_message" /> + </form>`; + + let wikis; + let titleInput; + let messageInput; + + describe('when the wiki page is being created', () => { + const formHtmlFixture = editFormHtmlFixture({ newPage: true }); + + beforeEach(() => { + setHTMLFixture(formHtmlFixture); + + titleInput = document.getElementById('wiki_title'); + messageInput = document.getElementById('wiki_message'); + wikis = new Wikis(); + }); + + it('binds an event listener to the title input', () => { + wikis.handleWikiTitleChange = jest.fn(); + + titleInput.dispatchEvent(new Event('keyup')); + + expect(wikis.handleWikiTitleChange).toHaveBeenCalled(); + }); + + it('sets the commit message when title changes', () => { + titleInput.value = 'My title'; + messageInput.value = ''; + + titleInput.dispatchEvent(new Event('keyup')); + + expect(messageInput.value).toEqual('Create My title'); + }); + + it('replaces hyphens with spaces', () => { + titleInput.value = 'my-hyphenated-title'; + titleInput.dispatchEvent(new Event('keyup')); + + expect(messageInput.value).toEqual('Create my hyphenated title'); + }); + }); + + describe('when the wiki page is being updated', () => { + const formHtmlFixture = editFormHtmlFixture({ newPage: false }); + + beforeEach(() => { + setHTMLFixture(formHtmlFixture); + + titleInput = document.getElementById('wiki_title'); + messageInput = document.getElementById('wiki_message'); + wikis = new Wikis(); + }); + + it('sets the commit message when title changes, prefixing with "Update"', () => { + titleInput.value = 'My title'; + messageInput.value = ''; + + titleInput.dispatchEvent(new Event('keyup')); + + expect(messageInput.value).toEqual('Update My title'); + }); + }); + }); +}); diff --git a/spec/javascripts/diffs/components/diff_file_header_spec.js b/spec/javascripts/diffs/components/diff_file_header_spec.js index d4280d3ec2c..356e7a8f1fe 100644 --- a/spec/javascripts/diffs/components/diff_file_header_spec.js +++ b/spec/javascripts/diffs/components/diff_file_header_spec.js @@ -372,7 +372,7 @@ describe('diff_file_header', () => { }); it('displays old and new path if the file was renamed', () => { - props.diffFile.viewer.name = diffViewerModes.renamed; + props.diffFile.renamed_file = true; vm = mountComponentWithStore(Component, { props, store }); diff --git a/spec/javascripts/issue_show/components/description_spec.js b/spec/javascripts/issue_show/components/description_spec.js index 7e00fbf2745..e10426a9858 100644 --- a/spec/javascripts/issue_show/components/description_spec.js +++ b/spec/javascripts/issue_show/components/description_spec.js @@ -1,5 +1,6 @@ import $ from 'jquery'; import Vue from 'vue'; +import '~/behaviors/markdown/render_gfm'; import Description from '~/issue_show/components/description.vue'; import mountComponent from 'spec/helpers/vue_mount_component_helper'; @@ -91,6 +92,7 @@ describe('Description component', () => { let TaskList; beforeEach(() => { + vm.$destroy(); vm = mountComponent( DescriptionComponent, Object.assign({}, props, { diff --git a/spec/javascripts/monitoring/charts/time_series_spec.js b/spec/javascripts/monitoring/charts/time_series_spec.js new file mode 100644 index 00000000000..d145a64e8d0 --- /dev/null +++ b/spec/javascripts/monitoring/charts/time_series_spec.js @@ -0,0 +1,335 @@ +import { shallowMount } from '@vue/test-utils'; +import { createStore } from '~/monitoring/stores'; +import { GlLink } from '@gitlab/ui'; +import { GlAreaChart, GlLineChart, GlChartSeriesLabel } from '@gitlab/ui/dist/charts'; +import { shallowWrapperContainsSlotText } from 'spec/helpers/vue_test_utils_helper'; +import TimeSeries from '~/monitoring/components/charts/time_series.vue'; +import * as types from '~/monitoring/stores/mutation_types'; +import { TEST_HOST } from 'spec/test_constants'; +import MonitoringMock, { deploymentData, mockProjectPath } from '../mock_data'; + +describe('Time series component', () => { + const mockSha = 'mockSha'; + const mockWidgets = 'mockWidgets'; + const mockSvgPathContent = 'mockSvgPathContent'; + const projectPath = `${TEST_HOST}${mockProjectPath}`; + const commitUrl = `${projectPath}/commit/${mockSha}`; + let mockGraphData; + let makeTimeSeriesChart; + let spriteSpy; + let store; + + beforeEach(() => { + store = createStore(); + store.commit(`monitoringDashboard/${types.RECEIVE_METRICS_DATA_SUCCESS}`, MonitoringMock.data); + store.commit(`monitoringDashboard/${types.RECEIVE_DEPLOYMENTS_DATA_SUCCESS}`, deploymentData); + store.dispatch('monitoringDashboard/setFeatureFlags', { exportMetricsToCsvEnabled: true }); + [mockGraphData] = store.state.monitoringDashboard.groups[0].metrics; + + makeTimeSeriesChart = (graphData, type) => + shallowMount(TimeSeries, { + propsData: { + graphData: { ...graphData, type }, + containerWidth: 0, + deploymentData: store.state.monitoringDashboard.deploymentData, + projectPath, + }, + slots: { + default: mockWidgets, + }, + sync: false, + store, + }); + + spriteSpy = spyOnDependency(TimeSeries, 'getSvgIconPathContent').and.callFake( + () => new Promise(resolve => resolve(mockSvgPathContent)), + ); + }); + + describe('general functions', () => { + let timeSeriesChart; + + beforeEach(() => { + timeSeriesChart = makeTimeSeriesChart(mockGraphData, 'area-chart'); + }); + + it('renders chart title', () => { + expect(timeSeriesChart.find('.js-graph-title').text()).toBe(mockGraphData.title); + }); + + it('contains graph widgets from slot', () => { + expect(timeSeriesChart.find('.js-graph-widgets').text()).toBe(mockWidgets); + }); + + describe('when exportMetricsToCsvEnabled is disabled', () => { + beforeEach(() => { + store.dispatch('monitoringDashboard/setFeatureFlags', { exportMetricsToCsvEnabled: false }); + }); + + it('does not render the Download CSV button', done => { + timeSeriesChart.vm.$nextTick(() => { + expect(timeSeriesChart.contains('glbutton-stub')).toBe(false); + done(); + }); + }); + }); + + describe('methods', () => { + describe('formatTooltipText', () => { + const mockDate = deploymentData[0].created_at; + const mockCommitUrl = deploymentData[0].commitUrl; + const generateSeriesData = type => ({ + seriesData: [ + { + seriesName: timeSeriesChart.vm.chartData[0].name, + componentSubType: type, + value: [mockDate, 5.55555], + seriesIndex: 0, + }, + ], + value: mockDate, + }); + + describe('when series is of line type', () => { + beforeEach(done => { + timeSeriesChart.vm.formatTooltipText(generateSeriesData('line')); + timeSeriesChart.vm.$nextTick(done); + }); + + it('formats tooltip title', () => { + expect(timeSeriesChart.vm.tooltip.title).toBe('31 May 2017, 9:23PM'); + }); + + it('formats tooltip content', () => { + const name = 'Core Usage'; + const value = '5.556'; + const seriesLabel = timeSeriesChart.find(GlChartSeriesLabel); + + expect(seriesLabel.vm.color).toBe(''); + expect(shallowWrapperContainsSlotText(seriesLabel, 'default', name)).toBe(true); + expect(timeSeriesChart.vm.tooltip.content).toEqual([{ name, value, color: undefined }]); + expect( + shallowWrapperContainsSlotText( + timeSeriesChart.find(GlAreaChart), + 'tooltipContent', + value, + ), + ).toBe(true); + }); + }); + + describe('when series is of scatter type', () => { + beforeEach(() => { + timeSeriesChart.vm.formatTooltipText(generateSeriesData('scatter')); + }); + + it('formats tooltip title', () => { + expect(timeSeriesChart.vm.tooltip.title).toBe('31 May 2017, 9:23PM'); + }); + + it('formats tooltip sha', () => { + expect(timeSeriesChart.vm.tooltip.sha).toBe('f5bcd1d9'); + }); + + it('formats tooltip commit url', () => { + expect(timeSeriesChart.vm.tooltip.commitUrl).toBe(mockCommitUrl); + }); + }); + }); + + describe('setSvg', () => { + const mockSvgName = 'mockSvgName'; + + beforeEach(done => { + timeSeriesChart.vm.setSvg(mockSvgName); + timeSeriesChart.vm.$nextTick(done); + }); + + it('gets svg path content', () => { + expect(spriteSpy).toHaveBeenCalledWith(mockSvgName); + }); + + it('sets svg path content', () => { + timeSeriesChart.vm.$nextTick(() => { + expect(timeSeriesChart.vm.svgs[mockSvgName]).toBe(`path://${mockSvgPathContent}`); + }); + }); + }); + + describe('onResize', () => { + const mockWidth = 233; + + beforeEach(() => { + spyOn(Element.prototype, 'getBoundingClientRect').and.callFake(() => ({ + width: mockWidth, + })); + timeSeriesChart.vm.onResize(); + }); + + it('sets area chart width', () => { + expect(timeSeriesChart.vm.width).toBe(mockWidth); + }); + }); + }); + + describe('computed', () => { + describe('chartData', () => { + let chartData; + const seriesData = () => chartData[0]; + + beforeEach(() => { + ({ chartData } = timeSeriesChart.vm); + }); + + it('utilizes all data points', () => { + const { values } = mockGraphData.queries[0].result[0]; + + expect(chartData.length).toBe(1); + expect(seriesData().data.length).toBe(values.length); + }); + + it('creates valid data', () => { + const { data } = seriesData(); + + expect( + data.filter( + ([time, value]) => new Date(time).getTime() > 0 && typeof value === 'number', + ).length, + ).toBe(data.length); + }); + + it('formats line width correctly', () => { + expect(chartData[0].lineStyle.width).toBe(2); + }); + }); + + describe('chartOptions', () => { + describe('yAxis formatter', () => { + let format; + + beforeEach(() => { + format = timeSeriesChart.vm.chartOptions.yAxis.axisLabel.formatter; + }); + + it('rounds to 3 decimal places', () => { + expect(format(0.88888)).toBe('0.889'); + }); + }); + }); + + describe('scatterSeries', () => { + it('utilizes deployment data', () => { + expect(timeSeriesChart.vm.scatterSeries.data).toEqual([ + ['2017-05-31T21:23:37.881Z', 0], + ['2017-05-30T20:08:04.629Z', 0], + ['2017-05-30T17:42:38.409Z', 0], + ]); + + expect(timeSeriesChart.vm.scatterSeries.symbolSize).toBe(14); + }); + }); + + describe('yAxisLabel', () => { + it('constructs a label for the chart y-axis', () => { + expect(timeSeriesChart.vm.yAxisLabel).toBe('CPU'); + }); + }); + + describe('csvText', () => { + it('converts data from json to csv', () => { + const header = `timestamp,${mockGraphData.y_label}`; + const data = mockGraphData.queries[0].result[0].values; + const firstRow = `${data[0][0]},${data[0][1]}`; + + expect(timeSeriesChart.vm.csvText).toMatch(`^${header}\r\n${firstRow}`); + }); + }); + + describe('downloadLink', () => { + it('produces a link to download metrics as csv', () => { + const link = timeSeriesChart.vm.downloadLink; + + expect(link).toContain('blob:'); + }); + }); + }); + + afterEach(() => { + timeSeriesChart.destroy(); + }); + }); + + describe('wrapped components', () => { + const glChartComponents = [ + { + chartType: 'area-chart', + component: GlAreaChart, + }, + { + chartType: 'line-chart', + component: GlLineChart, + }, + ]; + + glChartComponents.forEach(dynamicComponent => { + describe(`GitLab UI: ${dynamicComponent.chartType}`, () => { + let timeSeriesAreaChart; + let glChart; + + beforeEach(done => { + timeSeriesAreaChart = makeTimeSeriesChart(mockGraphData, dynamicComponent.chartType); + glChart = timeSeriesAreaChart.find(dynamicComponent.component); + timeSeriesAreaChart.vm.$nextTick(done); + }); + + it('is a Vue instance', () => { + expect(glChart.exists()).toBe(true); + expect(glChart.isVueInstance()).toBe(true); + }); + + it('receives data properties needed for proper chart render', () => { + const props = glChart.props(); + + expect(props.data).toBe(timeSeriesAreaChart.vm.chartData); + expect(props.option).toBe(timeSeriesAreaChart.vm.chartOptions); + expect(props.formatTooltipText).toBe(timeSeriesAreaChart.vm.formatTooltipText); + expect(props.thresholds).toBe(timeSeriesAreaChart.vm.thresholds); + }); + + it('recieves a tooltip title', done => { + const mockTitle = 'mockTitle'; + timeSeriesAreaChart.vm.tooltip.title = mockTitle; + + timeSeriesAreaChart.vm.$nextTick(() => { + expect(shallowWrapperContainsSlotText(glChart, 'tooltipTitle', mockTitle)).toBe(true); + done(); + }); + }); + + describe('when tooltip is showing deployment data', () => { + beforeEach(done => { + timeSeriesAreaChart.vm.tooltip.isDeployment = true; + timeSeriesAreaChart.vm.$nextTick(done); + }); + + it('uses deployment title', () => { + expect(shallowWrapperContainsSlotText(glChart, 'tooltipTitle', 'Deployed')).toBe(true); + }); + + it('renders clickable commit sha in tooltip content', done => { + timeSeriesAreaChart.vm.tooltip.sha = mockSha; + timeSeriesAreaChart.vm.tooltip.commitUrl = commitUrl; + + timeSeriesAreaChart.vm.$nextTick(() => { + const commitLink = timeSeriesAreaChart.find(GlLink); + + expect(shallowWrapperContainsSlotText(commitLink, 'default', mockSha)).toBe(true); + expect(commitLink.attributes('href')).toEqual(commitUrl); + done(); + }); + }); + }); + }); + }); + }); +}); diff --git a/spec/javascripts/monitoring/dashboard_spec.js b/spec/javascripts/monitoring/components/dashboard_spec.js index 02c3f303912..f3ec7520c6f 100644 --- a/spec/javascripts/monitoring/dashboard_spec.js +++ b/spec/javascripts/monitoring/components/dashboard_spec.js @@ -13,7 +13,7 @@ import MonitoringMock, { environmentData, singleGroupResponse, dashboardGitResponse, -} from './mock_data'; +} from '../mock_data'; const localVue = createLocalVue(); const propsData = { diff --git a/spec/javascripts/monitoring/mock_data.js b/spec/javascripts/monitoring/mock_data.js index 85e660d3925..17e7314e214 100644 --- a/spec/javascripts/monitoring/mock_data.js +++ b/spec/javascripts/monitoring/mock_data.js @@ -1,5 +1,7 @@ export const mockApiEndpoint = `${gl.TEST_HOST}/monitoring/mock`; +export const mockProjectPath = '/frontend-fixtures/environments-project'; + export const metricsGroupsAPIResponse = { success: true, data: [ @@ -902,7 +904,7 @@ export const metricsDashboardResponse = { }, { title: 'Memory Usage (Pod average)', - type: 'area-chart', + type: 'line-chart', y_label: 'Memory Used per Pod', weight: 2, metrics: [ diff --git a/spec/javascripts/notes/stores/getters_spec.js b/spec/javascripts/notes/stores/getters_spec.js index 71dcba114a9..d69f469c7c7 100644 --- a/spec/javascripts/notes/stores/getters_spec.js +++ b/spec/javascripts/notes/stores/getters_spec.js @@ -14,6 +14,13 @@ import { const discussionWithTwoUnresolvedNotes = 'merge_requests/resolved_diff_discussion.json'; +// Helper function to ensure that we're using the same schema across tests. +const createDiscussionNeighborParams = (discussionId, diffOrder, step) => ({ + discussionId, + diffOrder, + step, +}); + describe('Getters Notes Store', () => { let state; @@ -25,7 +32,6 @@ describe('Getters Notes Store', () => { targetNoteHash: 'hash', lastFetchedAt: 'timestamp', isNotesFetched: false, - notesData: notesDataMock, userData: userDataMock, noteableData: noteableDataMock, @@ -244,62 +250,104 @@ describe('Getters Notes Store', () => { }); }); - describe('nextUnresolvedDiscussionId', () => { - const localGetters = { - unresolvedDiscussionsIdsOrdered: () => ['123', '456', '789'], - }; + describe('findUnresolvedDiscussionIdNeighbor', () => { + let localGetters; + beforeEach(() => { + localGetters = { + unresolvedDiscussionsIdsOrdered: () => ['123', '456', '789'], + }; + }); - it('should return the ID of the discussion after the ID provided', () => { - expect(getters.nextUnresolvedDiscussionId(state, localGetters)('123')).toBe('456'); - expect(getters.nextUnresolvedDiscussionId(state, localGetters)('456')).toBe('789'); - expect(getters.nextUnresolvedDiscussionId(state, localGetters)('789')).toBe('123'); + [ + { step: 1, id: '123', expected: '456' }, + { step: 1, id: '456', expected: '789' }, + { step: 1, id: '789', expected: '123' }, + { step: -1, id: '123', expected: '789' }, + { step: -1, id: '456', expected: '123' }, + { step: -1, id: '789', expected: '456' }, + ].forEach(({ step, id, expected }) => { + it(`with step ${step} and id ${id}, returns next value`, () => { + const params = createDiscussionNeighborParams(id, true, step); + + expect(getters.findUnresolvedDiscussionIdNeighbor(state, localGetters)(params)).toBe( + expected, + ); + }); }); - }); - describe('previousUnresolvedDiscussionId', () => { - describe('with unresolved discussions', () => { - const localGetters = { - unresolvedDiscussionsIdsOrdered: () => ['123', '456', '789'], - }; + describe('with 1 unresolved discussion', () => { + beforeEach(() => { + localGetters = { + unresolvedDiscussionsIdsOrdered: () => ['123'], + }; + }); + + [{ step: 1, id: '123', expected: '123' }, { step: -1, id: '123', expected: '123' }].forEach( + ({ step, id, expected }) => { + it(`with step ${step} and match, returns only value`, () => { + const params = createDiscussionNeighborParams(id, true, step); - it('with bogus returns falsey', () => { - expect(getters.previousUnresolvedDiscussionId(state, localGetters)('bogus')).toBe('456'); + expect(getters.findUnresolvedDiscussionIdNeighbor(state, localGetters)(params)).toBe( + expected, + ); + }); + }, + ); + + it('with no match, returns only value', () => { + const params = createDiscussionNeighborParams('bogus', true, 1); + + expect(getters.findUnresolvedDiscussionIdNeighbor(state, localGetters)(params)).toBe('123'); }); + }); - [ - { id: '123', expected: '789' }, - { id: '456', expected: '123' }, - { id: '789', expected: '456' }, - ].forEach(({ id, expected }) => { - it(`with ${id}, returns previous value`, () => { - expect(getters.previousUnresolvedDiscussionId(state, localGetters)(id)).toBe(expected); + describe('with 0 unresolved discussions', () => { + beforeEach(() => { + localGetters = { + unresolvedDiscussionsIdsOrdered: () => [], + }; + }); + + [{ step: 1 }, { step: -1 }].forEach(({ step }) => { + it(`with step ${step}, returns undefined`, () => { + const params = createDiscussionNeighborParams('bogus', true, step); + + expect( + getters.findUnresolvedDiscussionIdNeighbor(state, localGetters)(params), + ).toBeUndefined(); }); }); }); + }); - describe('with 1 unresolved discussion', () => { - const localGetters = { - unresolvedDiscussionsIdsOrdered: () => ['123'], - }; + describe('findUnresolvedDiscussionIdNeighbor aliases', () => { + let neighbor; + let findUnresolvedDiscussionIdNeighbor; + let localGetters; - it('with bogus returns id', () => { - expect(getters.previousUnresolvedDiscussionId(state, localGetters)('bogus')).toBe('123'); - }); + beforeEach(() => { + neighbor = {}; + findUnresolvedDiscussionIdNeighbor = jasmine.createSpy().and.returnValue(neighbor); + localGetters = { findUnresolvedDiscussionIdNeighbor }; + }); - it('with match, returns value', () => { - expect(getters.previousUnresolvedDiscussionId(state, localGetters)('123')).toEqual('123'); + describe('nextUnresolvedDiscussionId', () => { + it('should return result of find neighbor', () => { + const expectedParams = createDiscussionNeighborParams('123', true, 1); + const result = getters.nextUnresolvedDiscussionId(state, localGetters)('123', true); + + expect(findUnresolvedDiscussionIdNeighbor).toHaveBeenCalledWith(expectedParams); + expect(result).toBe(neighbor); }); }); - describe('with 0 unresolved discussions', () => { - const localGetters = { - unresolvedDiscussionsIdsOrdered: () => [], - }; + describe('previosuUnresolvedDiscussionId', () => { + it('should return result of find neighbor', () => { + const expectedParams = createDiscussionNeighborParams('123', true, -1); + const result = getters.previousUnresolvedDiscussionId(state, localGetters)('123', true); - it('returns undefined', () => { - expect( - getters.previousUnresolvedDiscussionId(state, localGetters)('bogus'), - ).toBeUndefined(); + expect(findUnresolvedDiscussionIdNeighbor).toHaveBeenCalledWith(expectedParams); + expect(result).toBe(neighbor); }); }); }); diff --git a/spec/javascripts/registry/components/app_spec.js b/spec/javascripts/registry/components/app_spec.js index e7675669f7a..5ea3f85a247 100644 --- a/spec/javascripts/registry/components/app_spec.js +++ b/spec/javascripts/registry/components/app_spec.js @@ -84,12 +84,7 @@ describe('Registry List', () => { it('should render empty message', done => { setTimeout(() => { - expect( - vm.$el - .querySelector('p') - .textContent.trim() - .replace(/[\r\n]+/g, ' '), - ).toEqual( + expect(vm.$el.querySelector('.js-no-container-images-text').textContent).toEqual( 'With the Container Registry, every project can have its own space to store its Docker images. More Information', ); done(); @@ -124,7 +119,9 @@ describe('Registry List', () => { it('should render invalid characters error message', done => { setTimeout(() => { - expect(vm.$el.querySelector('.container-message')).not.toBe(null); + expect(vm.$el.querySelector('p')).not.toContain( + 'We are having trouble connecting to Docker, which could be due to an issue with your project name or path. More information', + ); done(); }); }); diff --git a/spec/javascripts/sidebar/assignee_title_spec.js b/spec/javascripts/sidebar/assignee_title_spec.js index 509edba2036..7fff7c075d9 100644 --- a/spec/javascripts/sidebar/assignee_title_spec.js +++ b/spec/javascripts/sidebar/assignee_title_spec.js @@ -4,8 +4,10 @@ import AssigneeTitle from '~/sidebar/components/assignees/assignee_title.vue'; describe('AssigneeTitle component', () => { let component; let AssigneeTitleComponent; + let statsSpy; beforeEach(() => { + statsSpy = spyOnDependency(AssigneeTitle, 'trackEvent'); AssigneeTitleComponent = Vue.extend(AssigneeTitle); }); @@ -102,4 +104,16 @@ describe('AssigneeTitle component', () => { expect(component.$el.querySelector('.edit-link')).not.toBeNull(); }); + + it('calls trackEvent when edit is clicked', () => { + component = new AssigneeTitleComponent({ + propsData: { + numberOfAssignees: 0, + editable: true, + }, + }).$mount(); + component.$el.querySelector('.js-sidebar-dropdown-toggle').click(); + + expect(statsSpy).toHaveBeenCalled(); + }); }); diff --git a/spec/javascripts/sidebar/assignees_spec.js b/spec/javascripts/sidebar/assignees_spec.js index 4ae2141d5f0..a1df5389a38 100644 --- a/spec/javascripts/sidebar/assignees_spec.js +++ b/spec/javascripts/sidebar/assignees_spec.js @@ -94,115 +94,9 @@ describe('Assignee component', () => { expect(assignee.querySelector('.author').innerText.trim()).toEqual(UsersMock.user.name); }); - - it('Shows one user with avatar, username and author name', () => { - component = new AssigneeComponent({ - propsData: { - rootPath: 'http://localhost:3000/', - users: [UsersMock.user], - editable: true, - }, - }).$mount(); - - expect(component.$el.querySelector('.author-link')).not.toBeNull(); - // The image - expect(component.$el.querySelector('.author-link img').getAttribute('src')).toEqual( - UsersMock.user.avatar, - ); - // Author name - expect(component.$el.querySelector('.author-link .author').innerText.trim()).toEqual( - UsersMock.user.name, - ); - // Username - expect(component.$el.querySelector('.author-link .username').innerText.trim()).toEqual( - `@${UsersMock.user.username}`, - ); - }); - - it('has the root url present in the assigneeUrl method', () => { - component = new AssigneeComponent({ - propsData: { - rootPath: 'http://localhost:3000/', - users: [UsersMock.user], - editable: true, - }, - }).$mount(); - - expect(component.assigneeUrl(UsersMock.user).indexOf('http://localhost:3000/')).not.toEqual( - -1, - ); - }); - - it('has correct "cannot merge" tooltip when user cannot merge', () => { - const user = Object.assign({}, UsersMock.user, { can_merge: false }); - - component = new AssigneeComponent({ - propsData: { - rootPath: 'http://localhost:3000/', - users: [user], - editable: true, - issuableType: 'merge_request', - }, - }).$mount(); - - expect(component.mergeNotAllowedTooltipMessage).toEqual('Cannot merge'); - }); }); describe('Two or more assignees/users', () => { - it('has correct "cannot merge" tooltip when one user can merge', () => { - const users = UsersMockHelper.createNumberRandomUsers(3); - users[0].can_merge = true; - users[1].can_merge = false; - users[2].can_merge = false; - - component = new AssigneeComponent({ - propsData: { - rootPath: 'http://localhost:3000/', - users, - editable: true, - issuableType: 'merge_request', - }, - }).$mount(); - - expect(component.mergeNotAllowedTooltipMessage).toEqual('1/3 can merge'); - }); - - it('has correct "cannot merge" tooltip when no user can merge', () => { - const users = UsersMockHelper.createNumberRandomUsers(2); - users[0].can_merge = false; - users[1].can_merge = false; - - component = new AssigneeComponent({ - propsData: { - rootPath: 'http://localhost:3000/', - users, - editable: true, - issuableType: 'merge_request', - }, - }).$mount(); - - expect(component.mergeNotAllowedTooltipMessage).toEqual('No one can merge'); - }); - - it('has correct "cannot merge" tooltip when more than one user can merge', () => { - const users = UsersMockHelper.createNumberRandomUsers(3); - users[0].can_merge = false; - users[1].can_merge = true; - users[2].can_merge = true; - - component = new AssigneeComponent({ - propsData: { - rootPath: 'http://localhost:3000/', - users, - editable: true, - issuableType: 'merge_request', - }, - }).$mount(); - - expect(component.mergeNotAllowedTooltipMessage).toEqual('2/3 can merge'); - }); - it('has no "cannot merge" tooltip when every user can merge', () => { const users = UsersMockHelper.createNumberRandomUsers(2); users[0].can_merge = true; @@ -217,7 +111,7 @@ describe('Assignee component', () => { }, }).$mount(); - expect(component.mergeNotAllowedTooltipMessage).toEqual(null); + expect(component.collapsedTooltipTitle).not.toContain('cannot merge'); }); it('displays two assignee icons when collapsed', () => { @@ -295,8 +189,12 @@ describe('Assignee component', () => { expect(component.$el.querySelector('.user-list-more')).toBe(null); }); - it('sets tooltip container to body', () => { - const users = UsersMockHelper.createNumberRandomUsers(2); + it('shows sorted assignee where "can merge" users are sorted first', () => { + const users = UsersMockHelper.createNumberRandomUsers(3); + users[0].can_merge = false; + users[1].can_merge = false; + users[2].can_merge = true; + component = new AssigneeComponent({ propsData: { rootPath: 'http://localhost:3000', @@ -305,98 +203,46 @@ describe('Assignee component', () => { }, }).$mount(); - expect(component.$el.querySelector('.user-link').getAttribute('data-container')).toBe('body'); + expect(component.sortedAssigness[0].can_merge).toBe(true); }); - it('Shows the "show-less" assignees label', done => { - const users = UsersMockHelper.createNumberRandomUsers(6); + it('passes the sorted assignees to the uncollapsed-assignee-list', () => { + const users = UsersMockHelper.createNumberRandomUsers(3); + users[0].can_merge = false; + users[1].can_merge = false; + users[2].can_merge = true; + component = new AssigneeComponent({ propsData: { rootPath: 'http://localhost:3000', users, - editable: true, + editable: false, }, }).$mount(); - expect(component.$el.querySelectorAll('.user-item').length).toEqual( - component.defaultRenderCount, - ); - - expect(component.$el.querySelector('.user-list-more')).not.toBe(null); - const usersLabelExpectation = users.length - component.defaultRenderCount; + const userItems = component.$el.querySelectorAll('.user-list .user-item a'); - expect(component.$el.querySelector('.user-list-more .btn-link').innerText.trim()).not.toBe( - `+${usersLabelExpectation} more`, - ); - component.toggleShowLess(); - Vue.nextTick(() => { - expect(component.$el.querySelector('.user-list-more .btn-link').innerText.trim()).toBe( - '- show less', - ); - done(); - }); + expect(userItems.length).toBe(3); + expect(userItems[0].dataset.originalTitle).toBe(users[2].name); }); - it('Shows the "show-less" when "n+ more " label is clicked', done => { - const users = UsersMockHelper.createNumberRandomUsers(6); - component = new AssigneeComponent({ - propsData: { - rootPath: 'http://localhost:3000', - users, - editable: true, - }, - }).$mount(); - - component.$el.querySelector('.user-list-more .btn-link').click(); - Vue.nextTick(() => { - expect(component.$el.querySelector('.user-list-more .btn-link').innerText.trim()).toBe( - '- show less', - ); - done(); - }); - }); + it('passes the sorted assignees to the collapsed-assignee-list', () => { + const users = UsersMockHelper.createNumberRandomUsers(3); + users[0].can_merge = false; + users[1].can_merge = false; + users[2].can_merge = true; - it('gets the count of avatar via a computed property ', () => { - const users = UsersMockHelper.createNumberRandomUsers(6); component = new AssigneeComponent({ propsData: { rootPath: 'http://localhost:3000', users, - editable: true, + editable: false, }, }).$mount(); - expect(component.sidebarAvatarCounter).toEqual(`+${users.length - 1}`); - }); + const collapsedButton = component.$el.querySelector('.sidebar-collapsed-user button'); - describe('n+ more label', () => { - beforeEach(() => { - const users = UsersMockHelper.createNumberRandomUsers(6); - component = new AssigneeComponent({ - propsData: { - rootPath: 'http://localhost:3000', - users, - editable: true, - }, - }).$mount(); - }); - - it('shows "+1 more" label', () => { - expect(component.$el.querySelector('.user-list-more .btn-link').innerText.trim()).toBe( - '+ 1 more', - ); - }); - - it('shows "show less" label', done => { - component.toggleShowLess(); - - Vue.nextTick(() => { - expect(component.$el.querySelector('.user-list-more .btn-link').innerText.trim()).toBe( - '- show less', - ); - done(); - }); - }); + expect(collapsedButton.innerText.trim()).toBe(users[2].name); }); }); }); diff --git a/spec/javascripts/sidebar/confidential_issue_sidebar_spec.js b/spec/javascripts/sidebar/confidential_issue_sidebar_spec.js index 486a7241e33..ea9e5677bc5 100644 --- a/spec/javascripts/sidebar/confidential_issue_sidebar_spec.js +++ b/spec/javascripts/sidebar/confidential_issue_sidebar_spec.js @@ -4,8 +4,10 @@ import confidentialIssueSidebar from '~/sidebar/components/confidential/confiden describe('Confidential Issue Sidebar Block', () => { let vm1; let vm2; + let statsSpy; beforeEach(() => { + statsSpy = spyOnDependency(confidentialIssueSidebar, 'trackEvent'); const Component = Vue.extend(confidentialIssueSidebar); const service = { update: () => Promise.resolve(true), @@ -67,4 +69,10 @@ describe('Confidential Issue Sidebar Block', () => { done(); }); }); + + it('calls trackEvent when "Edit" is clicked', () => { + vm1.$el.querySelector('.confidential-edit').click(); + + expect(statsSpy).toHaveBeenCalled(); + }); }); diff --git a/spec/javascripts/sidebar/lock/lock_issue_sidebar_spec.js b/spec/javascripts/sidebar/lock/lock_issue_sidebar_spec.js index ca882032bdf..2d930428230 100644 --- a/spec/javascripts/sidebar/lock/lock_issue_sidebar_spec.js +++ b/spec/javascripts/sidebar/lock/lock_issue_sidebar_spec.js @@ -4,8 +4,10 @@ import lockIssueSidebar from '~/sidebar/components/lock/lock_issue_sidebar.vue'; describe('LockIssueSidebar', () => { let vm1; let vm2; + let statsSpy; beforeEach(() => { + statsSpy = spyOnDependency(lockIssueSidebar, 'trackEvent'); const Component = Vue.extend(lockIssueSidebar); const mediator = { @@ -59,6 +61,12 @@ describe('LockIssueSidebar', () => { }); }); + it('calls trackEvent when "Edit" is clicked', () => { + vm1.$el.querySelector('.lock-edit').click(); + + expect(statsSpy).toHaveBeenCalled(); + }); + it('displays the edit form when opened from collapsed state', done => { expect(vm1.isLockDialogOpen).toBe(false); diff --git a/spec/javascripts/sidebar/subscriptions_spec.js b/spec/javascripts/sidebar/subscriptions_spec.js index 32728e58b06..2efa13f3fe8 100644 --- a/spec/javascripts/sidebar/subscriptions_spec.js +++ b/spec/javascripts/sidebar/subscriptions_spec.js @@ -6,8 +6,10 @@ import mountComponent from 'spec/helpers/vue_mount_component_helper'; describe('Subscriptions', function() { let vm; let Subscriptions; + let statsSpy; beforeEach(() => { + statsSpy = spyOnDependency(subscriptions, 'trackEvent'); Subscriptions = Vue.extend(subscriptions); }); @@ -58,6 +60,13 @@ describe('Subscriptions', function() { expect(vm.$emit).toHaveBeenCalledWith('toggleSubscription', jasmine.any(Object)); }); + it('calls trackEvent when toggled', () => { + vm = mountComponent(Subscriptions, { subscribed: true }); + vm.toggleSubscription(); + + expect(statsSpy).toHaveBeenCalled(); + }); + it('onClickCollapsedIcon method emits `toggleSidebar` event on component', () => { vm = mountComponent(Subscriptions, { subscribed: true }); spyOn(vm, '$emit'); diff --git a/spec/javascripts/vue_mr_widget/components/mr_widget_rebase_spec.js b/spec/javascripts/vue_mr_widget/components/mr_widget_rebase_spec.js index 212519743aa..7216ad00cc1 100644 --- a/spec/javascripts/vue_mr_widget/components/mr_widget_rebase_spec.js +++ b/spec/javascripts/vue_mr_widget/components/mr_widget_rebase_spec.js @@ -83,6 +83,24 @@ describe('Merge request widget rebase component', () => { expect(text).toContain('foo'); expect(text.replace(/\s\s+/g, ' ')).toContain('to allow this merge request to be merged.'); }); + + it('should render the correct target branch name', () => { + const targetBranch = 'fake-branch-to-test-with'; + vm = mountComponent(Component, { + mr: { + rebaseInProgress: false, + canPushToSourceBranch: false, + targetBranch, + }, + service: {}, + }); + + const elem = vm.$el.querySelector('.rebase-state-find-class-convention span'); + + expect(elem.innerHTML).toContain( + `Fast-forward merge is not possible. Rebase the source branch onto <span class="label-branch">${targetBranch}</span> to allow this merge request to be merged.`, + ); + }); }); describe('methods', () => { diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb index 4e4f1bf6ad3..a527783ffac 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb @@ -69,6 +69,34 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::Matches do it { is_expected.to eq(false) } end + context 'when right is nil' do + let(:left_value) { 'my-awesome-string' } + let(:right_value) { nil } + + it { is_expected.to eq(false) } + end + + context 'when left and right are nil' do + let(:left_value) { nil } + let(:right_value) { nil } + + it { is_expected.to eq(false) } + end + + context 'when left is an empty string' do + let(:left_value) { '' } + let(:right_value) { Gitlab::UntrustedRegexp.new('pattern') } + + it { is_expected.to eq(false) } + end + + context 'when left and right are empty strings' do + let(:left_value) { '' } + let(:right_value) { Gitlab::UntrustedRegexp.new('') } + + it { is_expected.to eq(true) } + end + context 'when left is a multiline string and matches right' do let(:left_value) do <<~TEXT diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexeme/not_matches_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexeme/not_matches_spec.rb index 6b81008ffb1..fb4238ecaf3 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/not_matches_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/not_matches_spec.rb @@ -69,6 +69,34 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::NotMatches do it { is_expected.to eq(true) } end + context 'when right is nil' do + let(:left_value) { 'my-awesome-string' } + let(:right_value) { nil } + + it { is_expected.to eq(true) } + end + + context 'when left and right are nil' do + let(:left_value) { nil } + let(:right_value) { nil } + + it { is_expected.to eq(true) } + end + + context 'when left is an empty string' do + let(:left_value) { '' } + let(:right_value) { Gitlab::UntrustedRegexp.new('pattern') } + + it { is_expected.to eq(true) } + end + + context 'when left and right are empty strings' do + let(:left_value) { '' } + let(:right_value) { Gitlab::UntrustedRegexp.new('') } + + it { is_expected.to eq(false) } + end + context 'when left is a multiline string and matches right' do let(:left_value) do <<~TEXT diff --git a/spec/lib/gitlab/daemon_spec.rb b/spec/lib/gitlab/daemon_spec.rb index d3e73314b87..0372b770844 100644 --- a/spec/lib/gitlab/daemon_spec.rb +++ b/spec/lib/gitlab/daemon_spec.rb @@ -34,12 +34,12 @@ describe Gitlab::Daemon do end end - describe 'when Daemon is enabled' do + context 'when Daemon is enabled' do before do allow(subject).to receive(:enabled?).and_return(true) end - describe 'when Daemon is stopped' do + context 'when Daemon is stopped' do describe '#start' do it 'starts the Daemon' do expect { subject.start.join }.to change { subject.thread? }.from(false).to(true) @@ -57,14 +57,14 @@ describe Gitlab::Daemon do end end - describe 'when Daemon is running' do + context 'when Daemon is running' do before do - subject.start.join + subject.start end describe '#start' do it "doesn't start running Daemon" do - expect { subject.start.join }.not_to change { subject.thread? } + expect { subject.start.join }.not_to change { subject.thread } expect(subject).to have_received(:start_working).once end @@ -76,11 +76,29 @@ describe Gitlab::Daemon do expect(subject).to have_received(:stop_working) end + + context 'when stop_working raises exception' do + before do + allow(subject).to receive(:start_working) do + sleep(1000) + end + end + + it 'shutdowns Daemon' do + expect(subject).to receive(:stop_working) do + subject.thread.raise(Interrupt) + end + + expect(subject.thread).to be_alive + expect { subject.stop }.not_to raise_error + expect(subject.thread).to be_nil + end + end end end end - describe 'when Daemon is disabled' do + context 'when Daemon is disabled' do before do allow(subject).to receive(:enabled?).and_return(false) end diff --git a/spec/lib/gitlab/sidekiq_middleware/monitor_spec.rb b/spec/lib/gitlab/sidekiq_middleware/monitor_spec.rb new file mode 100644 index 00000000000..2933d26a387 --- /dev/null +++ b/spec/lib/gitlab/sidekiq_middleware/monitor_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::SidekiqMiddleware::Monitor do + let(:monitor) { described_class.new } + + describe '#call' do + let(:worker) { double } + let(:job) { { 'jid' => 'job-id' } } + let(:queue) { 'my-queue' } + + it 'calls SidekiqMonitor' do + expect(Gitlab::SidekiqMonitor.instance).to receive(:within_job) + .with('job-id', 'my-queue') + .and_call_original + + expect { |blk| monitor.call(worker, job, queue, &blk) }.to yield_control + end + + it 'passthroughs the return value' do + result = monitor.call(worker, job, queue) do + 'value' + end + + expect(result).to eq('value') + end + + context 'when cancel happens' do + subject do + monitor.call(worker, job, queue) do + raise Gitlab::SidekiqMonitor::CancelledError + end + end + + it 'does skip this job' do + expect { subject }.to raise_error(Sidekiq::JobRetry::Skip) + end + end + end +end diff --git a/spec/lib/gitlab/sidekiq_monitor_spec.rb b/spec/lib/gitlab/sidekiq_monitor_spec.rb new file mode 100644 index 00000000000..bbd7bf90217 --- /dev/null +++ b/spec/lib/gitlab/sidekiq_monitor_spec.rb @@ -0,0 +1,261 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::SidekiqMonitor do + let(:monitor) { described_class.new } + + describe '#within_job' do + it 'tracks thread' do + blk = proc do + expect(monitor.jobs_thread['jid']).not_to be_nil + + "OK" + end + + expect(monitor.within_job('jid', 'queue', &blk)).to eq("OK") + end + + context 'when job is canceled' do + let(:jid) { SecureRandom.hex } + + before do + described_class.cancel_job(jid) + end + + it 'does not execute a block' do + expect do |blk| + monitor.within_job(jid, 'queue', &blk) + rescue described_class::CancelledError + end.not_to yield_control + end + + it 'raises exception' do + expect { monitor.within_job(jid, 'queue') }.to raise_error( + described_class::CancelledError) + end + end + end + + describe '#start_working' do + subject { monitor.send(:start_working) } + + before do + # we want to run at most once cycle + # we toggle `enabled?` flag after the first call + stub_const('Gitlab::SidekiqMonitor::RECONNECT_TIME', 0) + allow(monitor).to receive(:enabled?).and_return(true, false) + + allow(Sidekiq.logger).to receive(:info) + allow(Sidekiq.logger).to receive(:warn) + end + + context 'when structured logging is used' do + it 'logs start message' do + expect(Sidekiq.logger).to receive(:info) + .with( + class: described_class.to_s, + action: 'start', + message: 'Starting Monitor Daemon') + + expect(::Gitlab::Redis::SharedState).to receive(:with) + + subject + end + + it 'logs stop message' do + expect(Sidekiq.logger).to receive(:warn) + .with( + class: described_class.to_s, + action: 'stop', + message: 'Stopping Monitor Daemon') + + expect(::Gitlab::Redis::SharedState).to receive(:with) + + subject + end + + it 'logs StandardError message' do + expect(Sidekiq.logger).to receive(:warn) + .with( + class: described_class.to_s, + action: 'exception', + message: 'My Exception') + + expect(::Gitlab::Redis::SharedState).to receive(:with) + .and_raise(StandardError, 'My Exception') + + expect { subject }.not_to raise_error + end + + it 'logs and raises Exception message' do + expect(Sidekiq.logger).to receive(:warn) + .with( + class: described_class.to_s, + action: 'exception', + message: 'My Exception') + + expect(::Gitlab::Redis::SharedState).to receive(:with) + .and_raise(Exception, 'My Exception') + + expect { subject }.to raise_error(Exception, 'My Exception') + end + end + + context 'when StandardError is raised' do + it 'does retry connection' do + expect(::Gitlab::Redis::SharedState).to receive(:with) + .and_raise(StandardError, 'My Exception') + + expect(::Gitlab::Redis::SharedState).to receive(:with) + + # we expect to run `process_messages` twice + expect(monitor).to receive(:enabled?).and_return(true, true, false) + + subject + end + end + + context 'when message is published' do + let(:subscribed) { double } + + before do + expect_any_instance_of(::Redis).to receive(:subscribe) + .and_yield(subscribed) + + expect(subscribed).to receive(:message) + .and_yield( + described_class::NOTIFICATION_CHANNEL, + payload + ) + + expect(Sidekiq.logger).to receive(:info) + .with( + class: described_class.to_s, + action: 'start', + message: 'Starting Monitor Daemon') + + expect(Sidekiq.logger).to receive(:info) + .with( + class: described_class.to_s, + channel: described_class::NOTIFICATION_CHANNEL, + message: 'Received payload on channel', + payload: payload + ) + end + + context 'and message is valid' do + let(:payload) { '{"action":"cancel","jid":"my-jid"}' } + + it 'processes cancel' do + expect(monitor).to receive(:process_job_cancel).with('my-jid') + + subject + end + end + + context 'and message is not valid json' do + let(:payload) { '{"action"}' } + + it 'skips processing' do + expect(monitor).not_to receive(:process_job_cancel) + + subject + end + end + end + end + + describe '#stop' do + let!(:monitor_thread) { monitor.start } + + it 'does stop the thread' do + expect(monitor_thread).to be_alive + + expect { monitor.stop }.not_to raise_error + + expect(monitor_thread).not_to be_alive + expect { monitor_thread.value }.to raise_error(Interrupt) + end + end + + describe '#process_job_cancel' do + subject { monitor.send(:process_job_cancel, jid) } + + context 'when jid is missing' do + let(:jid) { nil } + + it 'does not run thread' do + expect(subject).to be_nil + end + end + + context 'when jid is provided' do + let(:jid) { 'my-jid' } + + context 'when jid is not found' do + it 'does not log cancellation message' do + expect(Sidekiq.logger).not_to receive(:warn) + expect(subject).to be_nil + end + end + + context 'when jid is found' do + let(:thread) { Thread.new { sleep 1000 } } + + before do + monitor.jobs_thread[jid] = thread + end + + after do + thread.kill + rescue + end + + it 'does log cancellation message' do + expect(Sidekiq.logger).to receive(:warn) + .with( + class: described_class.to_s, + action: 'cancel', + message: 'Canceling thread with CancelledError', + jid: 'my-jid', + thread_id: thread.object_id) + + expect(subject).to be_a(Thread) + + subject.join + end + + it 'does cancel the thread' do + expect(subject).to be_a(Thread) + + subject.join + + # we wait for the thread to be cancelled + # by `process_job_cancel` + expect { thread.join(5) }.to raise_error(described_class::CancelledError) + end + end + end + end + + describe '.cancel_job' do + subject { described_class.cancel_job('my-jid') } + + it 'sets a redis key' do + expect_any_instance_of(::Redis).to receive(:setex) + .with('sidekiq:cancel:my-jid', anything, 1) + + subject + end + + it 'notifies all workers' do + payload = '{"action":"cancel","jid":"my-jid"}' + + expect_any_instance_of(::Redis).to receive(:publish) + .with('sidekiq:cancel:notifications', payload) + + subject + end + end +end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index dcc4b70a382..6cba7df114c 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -543,6 +543,73 @@ describe Notify do end end + describe '#mail_thread' do + set(:mail_thread_note) { create(:note) } + + let(:headers) do + { + from: 'someone@test.com', + to: 'someone-else@test.com', + subject: 'something', + template_name: '_note_email' # re-use this for testing + } + end + + let(:mailer) do + mailer = described_class.new + mailer.instance_variable_set(:@note, mail_thread_note) + mailer + end + + context 'the model has no namespace' do + class TopLevelThing + include Referable + include Noteable + + def to_reference(*_args) + 'tlt-ref' + end + + def id + 'tlt-id' + end + end + + subject do + mailer.send(:mail_thread, TopLevelThing.new, headers) + end + + it 'has X-GitLab-Namespaced-Thing-ID header' do + expect(subject.header['X-GitLab-TopLevelThing-ID'].value).to eq('tlt-id') + end + end + + context 'the model has a namespace' do + module Namespaced + class Thing + include Referable + include Noteable + + def to_reference(*_args) + 'some-reference' + end + + def id + 'some-id' + end + end + end + + subject do + mailer.send(:mail_thread, Namespaced::Thing.new, headers) + end + + it 'has X-GitLab-Namespaced-Thing-ID header' do + expect(subject.header['X-GitLab-Namespaced-Thing-ID'].value).to eq('some-id') + end + end + end + context 'for issue notes' do let(:host) { Gitlab.config.gitlab.host } diff --git a/spec/models/award_emoji_spec.rb b/spec/models/award_emoji_spec.rb index 8452ac69734..b15b26b1630 100644 --- a/spec/models/award_emoji_spec.rb +++ b/spec/models/award_emoji_spec.rb @@ -44,6 +44,29 @@ describe AwardEmoji do end end + describe 'scopes' do + set(:thumbsup) { create(:award_emoji, name: 'thumbsup') } + set(:thumbsdown) { create(:award_emoji, name: 'thumbsdown') } + + describe '.upvotes' do + it { expect(described_class.upvotes).to contain_exactly(thumbsup) } + end + + describe '.downvotes' do + it { expect(described_class.downvotes).to contain_exactly(thumbsdown) } + end + + describe '.named' do + it { expect(described_class.named('thumbsup')).to contain_exactly(thumbsup) } + it { expect(described_class.named(%w[thumbsup thumbsdown])).to contain_exactly(thumbsup, thumbsdown) } + end + + describe '.awarded_by' do + it { expect(described_class.awarded_by(thumbsup.user)).to contain_exactly(thumbsup) } + it { expect(described_class.awarded_by([thumbsup.user, thumbsdown.user])).to contain_exactly(thumbsup, thumbsdown) } + end + end + describe 'expiring ETag cache' do context 'on a note' do let(:note) { create(:note_on_issue) } diff --git a/spec/models/concerns/awardable_spec.rb b/spec/models/concerns/awardable_spec.rb index 9e7106281ee..76da42cf243 100644 --- a/spec/models/concerns/awardable_spec.rb +++ b/spec/models/concerns/awardable_spec.rb @@ -82,16 +82,6 @@ describe Awardable do end end - describe "#toggle_award_emoji" do - it "adds an emoji if it isn't awarded yet" do - expect { issue.toggle_award_emoji("thumbsup", award_emoji.user) }.to change { AwardEmoji.count }.by(1) - end - - it "toggles already awarded emoji" do - expect { issue.toggle_award_emoji("thumbsdown", award_emoji.user) }.to change { AwardEmoji.count }.by(-1) - end - end - describe 'querying award_emoji on an Awardable' do let(:issue) { create(:issue) } diff --git a/spec/requests/api/award_emoji_spec.rb b/spec/requests/api/award_emoji_spec.rb index 6c67d84b59b..342fcfa1041 100644 --- a/spec/requests/api/award_emoji_spec.rb +++ b/spec/requests/api/award_emoji_spec.rb @@ -155,6 +155,14 @@ describe API::AwardEmoji do expect(json_response['user']['username']).to eq(user.username) end + it 'marks Todos on the Issue as done' do + todo = create(:todo, target: issue, project: project, user: user) + + post api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji", user), params: { name: '8ball' } + + expect(todo.reload).to be_done + end + it "returns a 400 bad request error if the name is not given" do post api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji", user) @@ -209,6 +217,14 @@ describe API::AwardEmoji do expect(json_response['user']['username']).to eq(user.username) end + it 'marks Todos on the Noteable as done' do + todo = create(:todo, target: note2.noteable, project: project, user: user) + + post api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{note.id}/award_emoji", user), params: { name: 'rocket' } + + expect(todo.reload).to be_done + end + it "normalizes +1 as thumbsup award" do post api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{note.id}/award_emoji", user), params: { name: '+1' } diff --git a/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb index 3982125a38a..5b910d5bfe0 100644 --- a/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb +++ b/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb @@ -5,9 +5,9 @@ require 'spec_helper' describe 'Adding an AwardEmoji' do include GraphqlHelpers - let(:current_user) { create(:user) } - let(:awardable) { create(:note) } - let(:project) { awardable.project } + set(:current_user) { create(:user) } + set(:project) { create(:project) } + set(:awardable) { create(:note, project: project) } let(:emoji_name) { 'thumbsup' } let(:mutation) do variables = { @@ -43,7 +43,7 @@ describe 'Adding an AwardEmoji' do end context 'when the given awardable is not an Awardable' do - let(:awardable) { create(:label) } + let(:awardable) { create(:label, project: project) } it_behaves_like 'a mutation that does not create an AwardEmoji' @@ -52,7 +52,7 @@ describe 'Adding an AwardEmoji' do end context 'when the given awardable is an Awardable but still cannot be awarded an emoji' do - let(:awardable) { create(:system_note) } + let(:awardable) { create(:system_note, project: project) } it_behaves_like 'a mutation that does not create an AwardEmoji' @@ -73,6 +73,13 @@ describe 'Adding an AwardEmoji' do expect(mutation_response['awardEmoji']['name']).to eq(emoji_name) end + describe 'marking Todos as done' do + let(:user) { current_user} + subject { post_graphql_mutation(mutation, current_user: user) } + + include_examples 'creating award emojis marks Todos as done' + end + context 'when there were active record validation errors' do before do expect_next_instance_of(AwardEmoji) do |award| diff --git a/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb index 31145730f10..ae628d3e56c 100644 --- a/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb +++ b/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb @@ -5,9 +5,9 @@ require 'spec_helper' describe 'Toggling an AwardEmoji' do include GraphqlHelpers - let(:current_user) { create(:user) } - let(:awardable) { create(:note) } - let(:project) { awardable.project } + set(:current_user) { create(:user) } + set(:project) { create(:project) } + set(:awardable) { create(:note, project: project) } let(:emoji_name) { 'thumbsup' } let(:mutation) do variables = { @@ -40,7 +40,7 @@ describe 'Toggling an AwardEmoji' do end context 'when the given awardable is not an Awardable' do - let(:awardable) { create(:label) } + let(:awardable) { create(:label, project: project) } it_behaves_like 'a mutation that does not create or destroy an AwardEmoji' @@ -49,7 +49,7 @@ describe 'Toggling an AwardEmoji' do end context 'when the given awardable is an Awardable but still cannot be awarded an emoji' do - let(:awardable) { create(:system_note) } + let(:awardable) { create(:system_note, project: project) } it_behaves_like 'a mutation that does not create or destroy an AwardEmoji' @@ -81,6 +81,13 @@ describe 'Toggling an AwardEmoji' do expect(mutation_response['toggledOn']).to eq(true) end + describe 'marking Todos as done' do + let(:user) { current_user} + subject { post_graphql_mutation(mutation, current_user: user) } + + include_examples 'creating award emojis marks Todos as done' + end + context 'when there were active record validation errors' do before do expect_next_instance_of(AwardEmoji) do |award| diff --git a/spec/serializers/deployment_entity_spec.rb b/spec/serializers/deployment_entity_spec.rb index c0ea2b3c389..1b19eac9a97 100644 --- a/spec/serializers/deployment_entity_spec.rb +++ b/spec/serializers/deployment_entity_spec.rb @@ -32,8 +32,8 @@ describe DeploymentEntity do expect(subject).to include(:created_at) end - it 'exposes finished_at' do - expect(subject).to include(:finished_at) + it 'exposes deployed_at' do + expect(subject).to include(:deployed_at) end context 'when the pipeline has another manual action' do diff --git a/spec/serializers/merge_request_sidebar_basic_entity_spec.rb b/spec/serializers/merge_request_sidebar_basic_entity_spec.rb new file mode 100644 index 00000000000..b364b1a3306 --- /dev/null +++ b/spec/serializers/merge_request_sidebar_basic_entity_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe MergeRequestSidebarBasicEntity do + let(:project) { create :project, :repository } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let(:user) { create(:user) } + + let(:request) { double('request', current_user: user, project: project) } + + let(:entity) { described_class.new(merge_request, request: request).as_json } + + describe '#current_user' do + it 'contains attributes related to the current user' do + expect(entity[:current_user].keys).to contain_exactly( + :id, :name, :username, :state, :avatar_url, :web_url, :todo, + :can_edit, :can_move, :can_admin_label, :can_merge + ) + end + end +end diff --git a/spec/services/award_emojis/add_service_spec.rb b/spec/services/award_emojis/add_service_spec.rb new file mode 100644 index 00000000000..037db39ba80 --- /dev/null +++ b/spec/services/award_emojis/add_service_spec.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe AwardEmojis::AddService do + set(:user) { create(:user) } + set(:project) { create(:project) } + set(:awardable) { create(:note, project: project) } + let(:name) { 'thumbsup' } + subject(:service) { described_class.new(awardable, name, user) } + + describe '#execute' do + context 'when user is not authorized' do + it 'does not add an emoji' do + expect { service.execute }.not_to change { AwardEmoji.count } + end + + it 'returns an error state' do + result = service.execute + + expect(result[:status]).to eq(:error) + expect(result[:http_status]).to eq(:forbidden) + end + end + + context 'when user is authorized' do + before do + project.add_developer(user) + end + + it 'creates an award emoji' do + expect { service.execute }.to change { AwardEmoji.count }.by(1) + end + + it 'returns the award emoji' do + result = service.execute + + expect(result[:award]).to be_kind_of(AwardEmoji) + end + + it 'return a success status' do + result = service.execute + + expect(result[:status]).to eq(:success) + end + + it 'sets the correct properties on the award emoji' do + award = service.execute[:award] + + expect(award.name).to eq(name) + expect(award.user).to eq(user) + end + + describe 'marking Todos as done' do + subject { service.execute } + + include_examples 'creating award emojis marks Todos as done' + end + + context 'when the awardable cannot have emoji awarded to it' do + before do + expect(awardable).to receive(:emoji_awardable?).and_return(false) + end + + it 'does not add an emoji' do + expect { service.execute }.not_to change { AwardEmoji.count } + end + + it 'returns an error status' do + result = service.execute + + expect(result[:status]).to eq(:error) + expect(result[:http_status]).to eq(:unprocessable_entity) + end + end + + context 'when the awardable is invalid' do + before do + expect_next_instance_of(AwardEmoji) do |award| + expect(award).to receive(:valid?).and_return(false) + expect(award).to receive_message_chain(:errors, :full_messages).and_return(['Error 1', 'Error 2']) + end + end + + it 'does not add an emoji' do + expect { service.execute }.not_to change { AwardEmoji.count } + end + + it 'returns an error status' do + result = service.execute + + expect(result[:status]).to eq(:error) + end + + it 'returns an error message' do + result = service.execute + + expect(result[:message]).to eq('Error 1 and Error 2') + end + end + end + end +end diff --git a/spec/finders/awarded_emoji_finder_spec.rb b/spec/services/award_emojis/collect_user_emoji_service_spec.rb index d4479df7418..a0dea31b403 100644 --- a/spec/finders/awarded_emoji_finder_spec.rb +++ b/spec/services/award_emojis/collect_user_emoji_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe AwardedEmojiFinder do +describe AwardEmojis::CollectUserEmojiService do describe '#execute' do it 'returns an Array containing the awarded emoji names' do user = create(:user) diff --git a/spec/services/award_emojis/destroy_service_spec.rb b/spec/services/award_emojis/destroy_service_spec.rb new file mode 100644 index 00000000000..c4a7d5ec20e --- /dev/null +++ b/spec/services/award_emojis/destroy_service_spec.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe AwardEmojis::DestroyService do + set(:user) { create(:user) } + set(:awardable) { create(:note) } + set(:project) { awardable.project } + let(:name) { 'thumbsup' } + let!(:award_from_other_user) do + create(:award_emoji, name: name, awardable: awardable, user: create(:user)) + end + subject(:service) { described_class.new(awardable, name, user) } + + describe '#execute' do + shared_examples_for 'a service that does not authorize the user' do |error:| + it 'does not remove the emoji' do + expect { service.execute }.not_to change { AwardEmoji.count } + end + + it 'returns an error state' do + result = service.execute + + expect(result[:status]).to eq(:error) + expect(result[:http_status]).to eq(:forbidden) + end + + it 'returns a nil award' do + result = service.execute + + expect(result).to have_key(:award) + expect(result[:award]).to be_nil + end + + it 'returns the error' do + result = service.execute + + expect(result[:message]).to eq(error) + expect(result[:errors]).to eq([error]) + end + end + + context 'when user is not authorized' do + it_behaves_like 'a service that does not authorize the user', + error: 'User cannot destroy emoji on the awardable' + end + + context 'when the user is authorized' do + before do + project.add_developer(user) + end + + context 'when user has not awarded an emoji to the awardable' do + let!(:award_from_user) { create(:award_emoji, name: name, user: user) } + + it_behaves_like 'a service that does not authorize the user', + error: 'User has not awarded emoji of type thumbsup on the awardable' + end + + context 'when user has awarded an emoji to the awardable' do + let!(:award_from_user) { create(:award_emoji, name: name, awardable: awardable, user: user) } + + it 'removes the emoji' do + expect { service.execute }.to change { AwardEmoji.count }.by(-1) + end + + it 'returns a success status' do + result = service.execute + + expect(result[:status]).to eq(:success) + end + + it 'returns no errors' do + result = service.execute + + expect(result).not_to have_key(:error) + expect(result).not_to have_key(:errors) + end + + it 'returns the destroyed award' do + result = service.execute + + expect(result[:award]).to eq(award_from_user) + expect(result[:award]).to be_destroyed + end + end + end + end +end diff --git a/spec/services/award_emojis/toggle_service_spec.rb b/spec/services/award_emojis/toggle_service_spec.rb new file mode 100644 index 00000000000..972a1d5fc06 --- /dev/null +++ b/spec/services/award_emojis/toggle_service_spec.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe AwardEmojis::ToggleService do + set(:user) { create(:user) } + set(:project) { create(:project, :public) } + set(:awardable) { create(:note, project: project) } + let(:name) { 'thumbsup' } + subject(:service) { described_class.new(awardable, name, user) } + + describe '#execute' do + context 'when user has awarded an emoji' do + let!(:award_from_other_user) { create(:award_emoji, name: name, awardable: awardable, user: create(:user)) } + let!(:award) { create(:award_emoji, name: name, awardable: awardable, user: user) } + + it 'calls AwardEmojis::DestroyService' do + expect(AwardEmojis::AddService).not_to receive(:new) + + expect_next_instance_of(AwardEmojis::DestroyService) do |service| + expect(service).to receive(:execute) + end + + service.execute + end + + it 'destroys an AwardEmoji' do + expect { service.execute }.to change { AwardEmoji.count }.by(-1) + end + + it 'returns the result of DestroyService#execute' do + mock_result = double(foo: true) + + expect_next_instance_of(AwardEmojis::DestroyService) do |service| + expect(service).to receive(:execute).and_return(mock_result) + end + + result = service.execute + + expect(result).to eq(mock_result) + end + end + + context 'when user has not awarded an emoji' do + it 'calls AwardEmojis::AddService' do + expect_next_instance_of(AwardEmojis::AddService) do |service| + expect(service).to receive(:execute) + end + + expect(AwardEmojis::DestroyService).not_to receive(:new) + + service.execute + end + + it 'creates an AwardEmoji' do + expect { service.execute }.to change { AwardEmoji.count }.by(1) + end + + it 'returns the result of AddService#execute' do + mock_result = double(foo: true) + + expect_next_instance_of(AwardEmojis::AddService) do |service| + expect(service).to receive(:execute).and_return(mock_result) + end + + result = service.execute + + expect(result).to eq(mock_result) + end + end + end +end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index d9f35afee06..fd9a63b79cc 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -229,10 +229,10 @@ describe Issues::UpdateService, :mailer do 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('a Zoom call was added') + note = find_note('added a Zoom call') expect(note).not_to be_nil - expect(note.note).to eq('a Zoom call was added to this issue') + expect(note.note).to eq('added a Zoom call to this issue') end context 'when issue turns confidential' do diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index b0b74407812..d4fa62fa85d 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -78,6 +78,7 @@ describe Projects::CreateService, '#execute' do expect(project).to be_valid expect(project.owner).to eq(group) expect(project.namespace).to eq(group) + expect(project.team.owners).to include(user) expect(user.authorized_projects).to include(project) end end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 486d0ca0c56..f46f9633c1c 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -521,7 +521,7 @@ describe SystemNoteService do end it 'sets the zoom link added note text' do - expect(subject.note).to eq('a Zoom call was added to this issue') + expect(subject.note).to eq('added a Zoom call to this issue') end end @@ -533,7 +533,7 @@ describe SystemNoteService do end it 'sets the zoom link removed note text' do - expect(subject.note).to eq('a Zoom call was removed from this issue') + expect(subject.note).to eq('removed a Zoom call from this issue') end end diff --git a/spec/support/shared_examples/award_emoji_todo_shared_examples.rb b/spec/support/shared_examples/award_emoji_todo_shared_examples.rb new file mode 100644 index 00000000000..88ad37d232f --- /dev/null +++ b/spec/support/shared_examples/award_emoji_todo_shared_examples.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +# Shared examples to that test code that creates AwardEmoji also mark Todos +# as done. +# +# The examples expect these to be defined in the calling spec: +# - `subject` the callable code that executes the creation of an AwardEmoji +# - `user` +# - `project` +RSpec.shared_examples 'creating award emojis marks Todos as done' do + using RSpec::Parameterized::TableSyntax + + before do + project.add_developer(user) + end + + where(:type, :expectation) do + :issue | true + :merge_request | true + :project_snippet | false + end + + with_them do + let(:project) { awardable.project } + let(:awardable) { create(type) } + let!(:todo) { create(:todo, target: awardable, project: project, user: user) } + + it do + subject + + expect(todo.reload.done?).to eq(expectation) + end + end + + # Notes have more complicated rules than other Todoables + describe 'for notes' do + let!(:todo) { create(:todo, target: awardable.noteable, project: project, user: user) } + + context 'regular Notes' do + let(:awardable) { create(:note, project: project) } + + it 'marks the Todo as done' do + subject + + expect(todo.reload.done?).to eq(true) + end + end + + context 'PersonalSnippet Notes' do + let(:awardable) { create(:note, noteable: create(:personal_snippet, author: user)) } + + it 'does not mark the Todo as done' do + subject + + expect(todo.reload.done?).to eq(false) + end + end + end +end diff --git a/spec/support/shared_examples/controllers/set_sort_order_from_user_preference_shared_examples.rb b/spec/support/shared_examples/controllers/set_sort_order_from_user_preference_shared_examples.rb index 1cd14ea2251..d89eded6e69 100644 --- a/spec/support/shared_examples/controllers/set_sort_order_from_user_preference_shared_examples.rb +++ b/spec/support/shared_examples/controllers/set_sort_order_from_user_preference_shared_examples.rb @@ -2,14 +2,14 @@ shared_examples 'set sort order from user preference' do describe '#set_sort_order_from_user_preference' do - # There is no issuable_sorting_field defined in any CE controllers yet, + # There is no sorting_field defined in any CE controllers yet, # however any other field present in user_preferences table can be used for testing. context 'when database is in read-only mode' do it 'does not update user preference' do allow(Gitlab::Database).to receive(:read_only?).and_return(true) - expect_any_instance_of(UserPreference).not_to receive(:update).with({ controller.send(:issuable_sorting_field) => sorting_param }) + expect_any_instance_of(UserPreference).not_to receive(:update).with({ controller.send(:sorting_field) => sorting_param }) get :index, params: { namespace_id: project.namespace, project_id: project, sort: sorting_param } end @@ -19,7 +19,7 @@ shared_examples 'set sort order from user preference' do it 'updates user preference' do allow(Gitlab::Database).to receive(:read_only?).and_return(false) - expect_any_instance_of(UserPreference).to receive(:update).with({ controller.send(:issuable_sorting_field) => sorting_param }) + expect_any_instance_of(UserPreference).to receive(:update).with({ controller.send(:sorting_field) => sorting_param }) get :index, params: { namespace_id: project.namespace, project_id: project, sort: sorting_param } end |
