diff options
91 files changed, 1673 insertions, 384 deletions
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index c42d11a860e..77ad4753c84 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -80,11 +80,6 @@ Lint/InterpolationCheck: Lint/MissingCopEnableDirective: Enabled: false -# Offense count: 1 -Lint/ReturnInVoidContext: - Exclude: - - 'app/models/project.rb' - # Offense count: 9 Lint/UriEscapeUnescape: Exclude: diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 63e799cf451..092afa15df4 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -1.14.1 +1.17.0 @@ -113,7 +113,7 @@ gem 'seed-fu', '~> 2.3.7' # Markdown and HTML processing gem 'html-pipeline', '~> 2.8' -gem 'deckar01-task_list', '2.0.1' +gem 'deckar01-task_list', '2.2.0' gem 'gitlab-markup', '~> 1.6.5' gem 'github-markup', '~> 1.7.0', require: 'github/markup' gem 'redcarpet', '~> 3.4' diff --git a/Gemfile.lock b/Gemfile.lock index 1c28176ac62..e6b563b5cb8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -143,7 +143,7 @@ GEM database_cleaner (1.7.0) debug_inspector (0.0.3) debugger-ruby_core_source (1.3.8) - deckar01-task_list (2.0.1) + deckar01-task_list (2.2.0) html-pipeline declarative (0.0.10) declarative-option (0.1.0) @@ -982,7 +982,7 @@ DEPENDENCIES connection_pool (~> 2.0) creole (~> 0.5.0) database_cleaner (~> 1.7.0) - deckar01-task_list (= 2.0.1) + deckar01-task_list (= 2.2.0) device_detector devise (~> 4.4) devise-two-factor (~> 3.0.0) diff --git a/app/assets/javascripts/ide/stores/modules/commit/actions.js b/app/assets/javascripts/ide/stores/modules/commit/actions.js index d97a950a8b2..24c2f71ae2b 100644 --- a/app/assets/javascripts/ide/stores/modules/commit/actions.js +++ b/app/assets/javascripts/ide/stores/modules/commit/actions.js @@ -202,7 +202,7 @@ export const commitChanges = ({ commit, state, getters, dispatch, rootState, roo dispatch( 'setErrorMessage', { - text: __('An error accured whilst committing your changes.'), + text: __('An error occurred whilst committing your changes.'), action: () => dispatch('commitChanges').then(() => dispatch('setErrorMessage', null, { root: true }), diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue index cd569eb3045..fea7f0d77a5 100644 --- a/app/assets/javascripts/issue_show/components/app.vue +++ b/app/assets/javascripts/issue_show/components/app.vue @@ -1,5 +1,7 @@ <script> import Visibility from 'visibilityjs'; +import { __, s__, sprintf } from '~/locale'; +import createFlash from '~/flash'; import { visitUrl } from '../../lib/utils/url_utility'; import Poll from '../../lib/utils/poll'; import eventHub from '../event_hub'; @@ -10,7 +12,6 @@ import descriptionComponent from './description.vue'; import editedComponent from './edited.vue'; import formComponent from './form.vue'; import recaptchaModalImplementor from '../../vue_shared/mixins/recaptcha_modal_implementor'; -import { __ } from '~/locale'; export default { components: { @@ -130,6 +131,11 @@ export default { required: false, default: true, }, + lockVersion: { + type: Number, + required: false, + default: 0, + }, }, data() { const store = new Store({ @@ -141,6 +147,7 @@ export default { updatedByName: this.updatedByName, updatedByPath: this.updatedByPath, taskStatus: this.initialTaskStatus, + lock_version: this.lockVersion, }); return { @@ -161,6 +168,9 @@ export default { const titleChanged = this.initialTitleText !== this.store.formState.title; return descriptionChanged || titleChanged; }, + defaultErrorMessage() { + return sprintf(s__('Error updating %{issuableType}'), { issuableType: this.issuableType }); + }, }, created() { this.service = new Service(this.endpoint); @@ -207,6 +217,17 @@ export default { } return undefined; }, + updateStoreState() { + return this.service + .getData() + .then(res => res.data) + .then(data => { + this.store.updateState(data); + }) + .catch(() => { + createFlash(this.defaultErrorMessage); + }); + }, openForm() { if (!this.showForm) { @@ -214,6 +235,7 @@ export default { this.store.setFormState({ title: this.state.titleText, description: this.state.descriptionText, + lock_version: this.state.lock_version, lockedWarningVisible: false, updateLoading: false, }); @@ -232,20 +254,24 @@ export default { if (window.location.pathname !== data.web_url) { visitUrl(data.web_url); } - - return this.service.getData(); }) - .then(res => res.data) - .then(data => { - this.store.updateState(data); + .then(this.updateStoreState) + .then(() => { eventHub.$emit('close.form'); }) - .catch(error => { - if (error && error.name === 'SpamError') { + .catch((error = {}) => { + const { name, response = {} } = error; + + if (name === 'SpamError') { this.openRecaptcha(); } else { - eventHub.$emit('close.form'); - window.Flash(`Error updating ${this.issuableType}`); + let errMsg = this.defaultErrorMessage; + + if (response.data && response.data.errors) { + errMsg += `. ${response.data.errors.join(' ')}`; + } + + createFlash(errMsg); } }); }, @@ -269,8 +295,9 @@ export default { visitUrl(data.web_url); }) .catch(() => { - eventHub.$emit('close.form'); - window.Flash(`Error deleting ${this.issuableType}`); + createFlash( + sprintf(s__('Error deleting %{issuableType}'), { issuableType: this.issuableType }), + ); }); }, }, @@ -314,6 +341,8 @@ export default { :task-status="state.taskStatus" :issuable-type="issuableType" :update-url="updateEndpoint" + :lock-version="state.lock_version" + @taskListUpdateFailed="updateStoreState" /> <edited-component v-if="hasUpdated" diff --git a/app/assets/javascripts/issue_show/components/description.vue b/app/assets/javascripts/issue_show/components/description.vue index 5ca88d75063..e664269b199 100644 --- a/app/assets/javascripts/issue_show/components/description.vue +++ b/app/assets/javascripts/issue_show/components/description.vue @@ -1,5 +1,6 @@ <script> import $ from 'jquery'; +import { __ } from '~/locale'; import animateMixin from '../mixins/animate'; import TaskList from '../../task_list'; import recaptchaModalImplementor from '../../vue_shared/mixins/recaptcha_modal_implementor'; @@ -35,6 +36,11 @@ export default { required: false, default: null, }, + lockVersion: { + type: Number, + required: false, + default: 0, + }, }, data() { return { @@ -67,8 +73,10 @@ export default { new TaskList({ dataType: this.issuableType, fieldName: 'description', + lockVersion: this.lockVersion, selector: '.detail-page-description', onSuccess: this.taskListUpdateSuccess.bind(this), + onError: this.taskListUpdateError.bind(this), }); } }, @@ -82,6 +90,16 @@ export default { } }, + taskListUpdateError() { + window.Flash( + __( + 'Someone edited this issue at the same time you did. The description has been updated and you will need to make your changes again.', + ), + ); + + this.$emit('taskListUpdateFailed'); + }, + updateTaskStatusText() { const taskRegexMatches = this.taskStatus.match(/(\d+) of ((?!0)\d+)/); const $issuableHeader = $('.issuable-meta'); diff --git a/app/assets/javascripts/issue_show/stores/index.js b/app/assets/javascripts/issue_show/stores/index.js index 32044d6da25..3c17e73ccec 100644 --- a/app/assets/javascripts/issue_show/stores/index.js +++ b/app/assets/javascripts/issue_show/stores/index.js @@ -1,3 +1,5 @@ +import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; + export default class Store { constructor(initialState) { this.state = initialState; @@ -6,6 +8,7 @@ export default class Store { description: '', lockedWarningVisible: false, updateLoading: false, + lock_version: 0, }; } @@ -14,14 +17,10 @@ export default class Store { this.formState.lockedWarningVisible = true; } + Object.assign(this.state, convertObjectPropsToCamelCase(data)); this.state.titleHtml = data.title; - this.state.titleText = data.title_text; this.state.descriptionHtml = data.description; - this.state.descriptionText = data.description_text; - this.state.taskStatus = data.task_status; - this.state.updatedAt = data.updated_at; - this.state.updatedByName = data.updated_by_name; - this.state.updatedByPath = data.updated_by_path; + this.state.lock_version = data.lock_version; } stateShouldUpdate(data) { diff --git a/app/assets/javascripts/jobs/components/stages_dropdown.vue b/app/assets/javascripts/jobs/components/stages_dropdown.vue index 91332c21b52..c5076d65ff9 100644 --- a/app/assets/javascripts/jobs/components/stages_dropdown.vue +++ b/app/assets/javascripts/jobs/components/stages_dropdown.vue @@ -39,7 +39,9 @@ export default { <ci-icon :status="pipeline.details.status" class="vertical-align-middle" /> <span class="font-weight-bold">{{ __('Pipeline') }}</span> - <a :href="pipeline.path" class="js-pipeline-path link-commit">#{{ pipeline.id }}</a> + <a :href="pipeline.path" class="js-pipeline-path link-commit qa-pipeline-path" + >#{{ pipeline.id }}</a + > <template v-if="hasRef"> {{ __('from') }} <a :href="pipeline.ref.path" class="link-commit ref-name">{{ pipeline.ref.name }}</a> diff --git a/app/assets/javascripts/merge_request.js b/app/assets/javascripts/merge_request.js index 0deae478deb..ac3b47cd218 100644 --- a/app/assets/javascripts/merge_request.js +++ b/app/assets/javascripts/merge_request.js @@ -35,6 +35,7 @@ function MergeRequest(opts) { dataType: 'merge_request', fieldName: 'description', selector: '.detail-page-description', + lockVersion: this.$el.data('lockVersion'), onSuccess: result => { document.querySelector('#task_status').innerText = result.task_status; document.querySelector('#task_status_short').innerText = result.task_status_short; diff --git a/app/assets/javascripts/monitoring/components/charts/area.vue b/app/assets/javascripts/monitoring/components/charts/area.vue index e2cffe0b4b4..5ca561259b6 100644 --- a/app/assets/javascripts/monitoring/components/charts/area.vue +++ b/app/assets/javascripts/monitoring/components/charts/area.vue @@ -35,13 +35,7 @@ export default { computed: { chartData() { return this.graphData.queries.reduce((accumulator, query) => { - const xLabel = `${query.unit}`; - accumulator[xLabel] = {}; - query.result.forEach(res => - res.values.forEach(v => { - accumulator[xLabel][v.time.toISOString()] = v.value; - }), - ); + accumulator[query.unit] = query.result.reduce((acc, res) => acc.concat(res.values), []); return accumulator; }, {}); }, @@ -51,14 +45,17 @@ export default { name: 'Time', type: 'time', axisLabel: { - formatter: date => dateFormat(date, 'h:MMtt'), + formatter: date => dateFormat(date, 'h:MM TT'), + }, + axisPointer: { + snap: true, }, nameTextStyle: { padding: [18, 0, 0, 0], }, }, yAxis: { - name: this.graphData.y_label, + name: this.yAxisLabel, axisLabel: { formatter: value => value.toFixed(3), }, @@ -74,6 +71,10 @@ export default { xAxisLabel() { return this.graphData.queries.map(query => query.label).join(', '); }, + yAxisLabel() { + const [query] = this.graphData.queries; + return `${this.graphData.y_label} (${query.unit})`; + }, }, methods: { formatTooltipText(params) { @@ -85,7 +86,7 @@ export default { </script> <template> - <div class="prometheus-graph"> + <div class="prometheus-graph col-12 col-lg-6"> <div class="prometheus-graph-header"> <h5 class="prometheus-graph-title">{{ graphData.title }}</h5> <div class="prometheus-graph-widgets"><slot></slot></div> diff --git a/app/assets/javascripts/monitoring/stores/monitoring_store.js b/app/assets/javascripts/monitoring/stores/monitoring_store.js index 96ecc5ab8a8..70635059bd9 100644 --- a/app/assets/javascripts/monitoring/stores/monitoring_store.js +++ b/app/assets/javascripts/monitoring/stores/monitoring_store.js @@ -13,7 +13,7 @@ function checkQueryEmptyData(query) { result: query.result.filter(timeSeries => { const newTimeSeries = timeSeries; const hasValue = series => - !Number.isNaN(series.value) && (series.value !== null || series.value !== undefined); + !Number.isNaN(series[1]) && (series[1] !== null || series[1] !== undefined); const hasNonNullValue = timeSeries.values.find(hasValue); newTimeSeries.values = hasNonNullValue ? newTimeSeries.values : []; @@ -33,10 +33,10 @@ function normalizeMetrics(metrics) { ...query, result: query.result.map(result => ({ ...result, - values: result.values.map(([timestamp, value]) => ({ - time: new Date(timestamp * 1000), - value: Number(value), - })), + values: result.values.map(([timestamp, value]) => [ + new Date(timestamp * 1000).toISOString(), + Number(value), + ]), })), })); diff --git a/app/assets/javascripts/pages/profiles/show/index.js b/app/assets/javascripts/pages/profiles/show/index.js index c7ce4675573..0dd0d5336fc 100644 --- a/app/assets/javascripts/pages/profiles/show/index.js +++ b/app/assets/javascripts/pages/profiles/show/index.js @@ -1,6 +1,7 @@ import $ from 'jquery'; import createFlash from '~/flash'; import GfmAutoComplete from '~/gfm_auto_complete'; +import emojiRegex from 'emoji-regex'; import EmojiMenu from './emoji_menu'; const defaultStatusEmoji = 'speech_balloon'; @@ -42,6 +43,17 @@ document.addEventListener('DOMContentLoaded', () => { const emojiAutocomplete = new GfmAutoComplete(); emojiAutocomplete.setup($(statusMessageField), { emojis: true }); + const userNameInput = document.getElementById('user_name'); + userNameInput.addEventListener('input', () => { + const EMOJI_REGEX = emojiRegex(); + if (EMOJI_REGEX.test(userNameInput.value)) { + // set field to invalid so it gets detected by GlFieldErrors + userNameInput.setCustomValidity('Invalid field'); + } else { + userNameInput.setCustomValidity(''); + } + }); + import(/* webpackChunkName: 'emoji' */ '~/emoji') .then(Emoji => { const emojiMenu = new EmojiMenu( diff --git a/app/assets/javascripts/pipelines/components/graph/job_item.vue b/app/assets/javascripts/pipelines/components/graph/job_item.vue index cf9db89e32b..2b32a6e4a98 100644 --- a/app/assets/javascripts/pipelines/components/graph/job_item.vue +++ b/app/assets/javascripts/pipelines/components/graph/job_item.vue @@ -108,7 +108,7 @@ export default { :href="status.details_path" :title="tooltipText" :class="cssClassJobName" - class="js-pipeline-graph-job-link" + class="js-pipeline-graph-job-link qa-job-link" > <job-name-component :name="job.name" :status="job.status" /> </gl-link> diff --git a/app/assets/javascripts/task_list.js b/app/assets/javascripts/task_list.js index edefb3735d7..5172a1ef3d6 100644 --- a/app/assets/javascripts/task_list.js +++ b/app/assets/javascripts/task_list.js @@ -1,5 +1,6 @@ import $ from 'jquery'; import 'deckar01-task_list'; +import { __ } from '~/locale'; import axios from './lib/utils/axios_utils'; import Flash from './flash'; @@ -8,46 +9,79 @@ export default class TaskList { this.selector = options.selector; this.dataType = options.dataType; this.fieldName = options.fieldName; + this.lockVersion = options.lockVersion; + this.taskListContainerSelector = `${this.selector} .js-task-list-container`; + this.updateHandler = this.update.bind(this); this.onSuccess = options.onSuccess || (() => {}); - this.onError = function showFlash(e) { - let errorMessages = ''; + this.onError = + options.onError || + function showFlash(e) { + let errorMessages = ''; - if (e.response.data && typeof e.response.data === 'object') { - errorMessages = e.response.data.errors.join(' '); - } + if (e.response.data && typeof e.response.data === 'object') { + errorMessages = e.response.data.errors.join(' '); + } - return new Flash(errorMessages || 'Update failed', 'alert'); - }; + return new Flash(errorMessages || __('Update failed'), 'alert'); + }; this.init(); } init() { - // Prevent duplicate event bindings - this.disable(); - $(`${this.selector} .js-task-list-container`).taskList('enable'); - $(document).on( - 'tasklist:changed', - `${this.selector} .js-task-list-container`, - this.update.bind(this), - ); + this.disable(); // Prevent duplicate event bindings + + $(this.taskListContainerSelector).taskList('enable'); + $(document).on('tasklist:changed', this.taskListContainerSelector, this.updateHandler); + } + + getTaskListTarget(e) { + return e && e.currentTarget ? $(e.currentTarget) : $(this.taskListContainerSelector); + } + + disableTaskListItems(e) { + this.getTaskListTarget(e).taskList('disable'); + } + + enableTaskListItems(e) { + this.getTaskListTarget(e).taskList('enable'); } disable() { - $(`${this.selector} .js-task-list-container`).taskList('disable'); - $(document).off('tasklist:changed', `${this.selector} .js-task-list-container`); + this.disableTaskListItems(); + $(document).off('tasklist:changed', this.taskListContainerSelector); } update(e) { const $target = $(e.target); + const { index, checked, lineNumber, lineSource } = e.detail; const patchData = {}; + patchData[this.dataType] = { [this.fieldName]: $target.val(), + lock_version: this.lockVersion, + update_task: { + index, + checked, + line_number: lineNumber, + line_source: lineSource, + }, }; + this.disableTaskListItems(e); + return axios .patch($target.data('updateUrl') || $('form.js-issuable-update').attr('action'), patchData) - .then(({ data }) => this.onSuccess(data)) - .catch(err => this.onError(err)); + .then(({ data }) => { + this.lockVersion = data.lock_version; + this.enableTaskListItems(e); + + return this.onSuccess(data); + }) + .catch(({ response }) => { + this.enableTaskListItems(e); + + return this.onError(response.data); + }); } } diff --git a/app/assets/stylesheets/pages/environments.scss b/app/assets/stylesheets/pages/environments.scss index b6abb792709..61ecf133b02 100644 --- a/app/assets/stylesheets/pages/environments.scss +++ b/app/assets/stylesheets/pages/environments.scss @@ -240,18 +240,7 @@ } .prometheus-graph { - flex: 1 0 auto; - min-width: 450px; - max-width: 100%; padding: $gl-padding / 2; - - h5 { - font-size: 16px; - } - - @include media-breakpoint-down(sm) { - min-width: 100%; - } } .prometheus-graph-header { @@ -261,6 +250,7 @@ margin-bottom: $gl-padding-8; h5 { + font-size: $gl-font-size-large; margin: 0; } } diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index 3d64ae8b775..8ef3b6502df 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/app/controllers/concerns/issuable_actions.rb @@ -58,7 +58,8 @@ module IssuableActions title_text: issuable.title, description: view_context.markdown_field(issuable, :description), description_text: issuable.description, - task_status: issuable.task_status + task_status: issuable.task_status, + lock_version: issuable.lock_version } if issuable.edited? diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 1a1b024d766..4e85de25c6b 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -182,11 +182,11 @@ class Projects::EnvironmentsController < Projects::ApplicationController end def serialize_environments(request, response, nested = false) - serializer = EnvironmentSerializer + EnvironmentSerializer .new(project: @project, current_user: @current_user) + .tap { |serializer| serializer.within_folders if nested } .with_pagination(request, response) - serializer = serializer.within_folders if nested - serializer.represent(@environments) + .represent(@environments) end def authorize_stop_environment! diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 69f983f7ccd..f9a80aa3cfb 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -246,7 +246,7 @@ class Projects::IssuesController < Projects::ApplicationController task_num lock_version discussion_locked - ] + [{ label_ids: [], assignee_ids: [] }] + ] + [{ label_ids: [], assignee_ids: [], update_task: [:index, :checked, :line_number, :line_source] }] end def store_uri diff --git a/app/graphql/resolvers/issues_resolver.rb b/app/graphql/resolvers/issues_resolver.rb index 4ab3c13787a..95e66fb3b7c 100644 --- a/app/graphql/resolvers/issues_resolver.rb +++ b/app/graphql/resolvers/issues_resolver.rb @@ -4,6 +4,10 @@ module Resolvers class IssuesResolver < BaseResolver extend ActiveSupport::Concern + argument :iids, [GraphQL::ID_TYPE], + required: false, + description: 'The list of IIDs of issues, e.g., [1, 2]' + argument :search, GraphQL::STRING_TYPE, required: false argument :sort, Types::Sort, diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index f8176facce9..0fee29bf7c7 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -269,6 +269,7 @@ module IssuablesHelper markdownPreviewPath: preview_markdown_path(parent), markdownDocsPath: help_page_path('user/markdown'), markdownVersion: issuable.cached_markdown_version, + lockVersion: issuable.lock_version, issuableTemplates: issuable_templates(issuable), initialTitleHtml: markdown_field(issuable, :title), initialTitleText: issuable.title, diff --git a/app/models/concerns/cache_markdown_field.rb b/app/models/concerns/cache_markdown_field.rb index 002f3e17891..588204c7470 100644 --- a/app/models/concerns/cache_markdown_field.rb +++ b/app/models/concerns/cache_markdown_field.rb @@ -130,13 +130,17 @@ module CacheMarkdownField def latest_cached_markdown_version return CacheMarkdownField::CACHE_COMMONMARK_VERSION unless cached_markdown_version - if cached_markdown_version < CacheMarkdownField::CACHE_COMMONMARK_VERSION_START + if legacy_markdown? CacheMarkdownField::CACHE_REDCARPET_VERSION else CacheMarkdownField::CACHE_COMMONMARK_VERSION end end + def legacy_markdown? + cached_markdown_version && cached_markdown_version.between?(1, CacheMarkdownField::CACHE_COMMONMARK_VERSION_START - 1) + end + included do cattr_reader :cached_markdown_fields do FieldData.new @@ -178,7 +182,9 @@ module CacheMarkdownField # author and project invalidate the cache in all circumstances. define_method(invalidation_method) do changed_fields = changed_attributes.keys - invalidations = changed_fields & [markdown_field.to_s, *INVALIDATED_BY] + invalidations = changed_fields & [markdown_field.to_s, *INVALIDATED_BY] + invalidations.delete(markdown_field.to_s) if changed_fields.include?("#{markdown_field}_html") + !invalidations.empty? || !cached_html_up_to_date?(markdown_field) end end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 0d363ec68b7..b1cf03551f6 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -270,26 +270,29 @@ module Issuable def to_hook_data(user, old_associations: {}) changes = previous_changes - old_labels = old_associations.fetch(:labels, []) - old_assignees = old_associations.fetch(:assignees, []) - if old_labels != labels - changes[:labels] = [old_labels.map(&:hook_attrs), labels.map(&:hook_attrs)] - end + if old_associations + old_labels = old_associations.fetch(:labels, []) + old_assignees = old_associations.fetch(:assignees, []) - if old_assignees != assignees - if self.is_a?(Issue) - changes[:assignees] = [old_assignees.map(&:hook_attrs), assignees.map(&:hook_attrs)] - else - changes[:assignee] = [old_assignees&.first&.hook_attrs, assignee&.hook_attrs] + if old_labels != labels + changes[:labels] = [old_labels.map(&:hook_attrs), labels.map(&:hook_attrs)] end - end - if self.respond_to?(:total_time_spent) - old_total_time_spent = old_associations.fetch(:total_time_spent, nil) + if old_assignees != assignees + if self.is_a?(Issue) + changes[:assignees] = [old_assignees.map(&:hook_attrs), assignees.map(&:hook_attrs)] + else + changes[:assignee] = [old_assignees&.first&.hook_attrs, assignee&.hook_attrs] + end + end + + if self.respond_to?(:total_time_spent) + old_total_time_spent = old_associations.fetch(:total_time_spent, nil) - if old_total_time_spent != total_time_spent - changes[:total_time_spent] = [old_total_time_spent, total_time_spent] + if old_total_time_spent != total_time_spent + changes[:total_time_spent] = [old_total_time_spent, total_time_spent] + end end end diff --git a/app/models/concerns/taskable.rb b/app/models/concerns/taskable.rb index 603d4d62578..f147ce8ad6b 100644 --- a/app/models/concerns/taskable.rb +++ b/app/models/concerns/taskable.rb @@ -9,9 +9,11 @@ require 'task_list/filter' # # Used by MergeRequest and Issue module Taskable - COMPLETED = 'completed'.freeze - INCOMPLETE = 'incomplete'.freeze - ITEM_PATTERN = %r{ + COMPLETED = 'completed'.freeze + INCOMPLETE = 'incomplete'.freeze + COMPLETE_PATTERN = /(\[[xX]\])/.freeze + INCOMPLETE_PATTERN = /(\[[\s]\])/.freeze + ITEM_PATTERN = %r{ ^ \s*(?:[-+*]|(?:\d+\.)) # list prefix required - task item has to be always in a list \s+ # whitespace prefix has to be always presented for a list item diff --git a/app/models/error_tracking/project_error_tracking_setting.rb b/app/models/error_tracking/project_error_tracking_setting.rb index 7f4947ba27a..31084c54bdc 100644 --- a/app/models/error_tracking/project_error_tracking_setting.rb +++ b/app/models/error_tracking/project_error_tracking_setting.rb @@ -8,9 +8,13 @@ module ErrorTracking belongs_to :project - validates :api_url, length: { maximum: 255 }, public_url: true, url: { enforce_sanitization: true } + validates :api_url, length: { maximum: 255 }, public_url: true, url: { enforce_sanitization: true, ascii_only: true }, allow_nil: true - validate :validate_api_url_path + validates :api_url, presence: true, if: :enabled + + validate :validate_api_url_path, if: :enabled + + validates :token, presence: true, if: :enabled attr_encrypted :token, mode: :per_attribute_iv, @@ -19,6 +23,31 @@ module ErrorTracking after_save :clear_reactive_cache! + def project_name + super || project_name_from_slug + end + + def organization_name + super || organization_name_from_slug + end + + def project_slug + project_slug_from_api_url + end + + def organization_slug + organization_slug_from_api_url + end + + def self.build_api_url_from(api_host:, project_slug:, organization_slug:) + uri = Addressable::URI.parse("#{api_host}/api/0/projects/#{organization_slug}/#{project_slug}/") + uri.path = uri.path.squeeze('/') + + uri.to_s + rescue Addressable::URI::InvalidURIError + api_host + end + def sentry_client Sentry::Client.new(api_url, token) end @@ -33,6 +62,10 @@ module ErrorTracking end end + def list_sentry_projects + { projects: sentry_client.list_projects } + end + def calculate_reactive_cache(request, opts) case request when 'list_issues' @@ -47,13 +80,53 @@ module ErrorTracking url.sub('api/0/projects/', '') end + def api_host + return if api_url.blank? + + # This returns http://example.com/ + Addressable::URI.join(api_url, '/').to_s + end + private + def project_name_from_slug + @project_name_from_slug ||= project_slug_from_api_url&.titleize + end + + def organization_name_from_slug + @organization_name_from_slug ||= organization_slug_from_api_url&.titleize + end + + def project_slug_from_api_url + extract_slug(:project) + end + + def organization_slug_from_api_url + extract_slug(:organization) + end + + def extract_slug(capture) + return if api_url.blank? + + begin + url = Addressable::URI.parse(api_url) + rescue Addressable::URI::InvalidURIError + return nil + end + + @slug_match ||= url.path.match(%r{^/api/0/projects/+(?<organization>[^/]+)/+(?<project>[^/|$]+)}) || {} + @slug_match[capture] + end + def validate_api_url_path - unless URI(api_url).path.starts_with?('/api/0/projects') - errors.add(:api_url, 'path needs to start with /api/0/projects') + return if api_url.blank? + + begin + unless Addressable::URI.parse(api_url).path.starts_with?('/api/0/projects') + errors.add(:api_url, 'path needs to start with /api/0/projects') + end + rescue Addressable::URI::InvalidURIError end - rescue URI::InvalidURIError end end end diff --git a/app/models/project.rb b/app/models/project.rb index b385b89449d..f6218519533 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -738,11 +738,13 @@ class Project < ActiveRecord::Base end def import_url=(value) - return super(value) unless Gitlab::UrlSanitizer.valid?(value) - - import_url = Gitlab::UrlSanitizer.new(value) - super(import_url.sanitized_url) - create_or_update_import_data(credentials: import_url.credentials) + if Gitlab::UrlSanitizer.valid?(value) + import_url = Gitlab::UrlSanitizer.new(value) + super(import_url.sanitized_url) + create_or_update_import_data(credentials: import_url.credentials) + else + super(value) + end end def import_url @@ -1066,7 +1068,7 @@ class Project < ActiveRecord::Base # rubocop: disable CodeReuse/ServiceClass def create_labels Label.templates.each do |label| - params = label.attributes.except('id', 'template', 'created_at', 'updated_at') + params = label.attributes.except('id', 'template', 'created_at', 'updated_at', 'type') Labels::FindOrCreateService.new(nil, self, params).execute(skip_authorization: true) end end diff --git a/app/serializers/merge_request_basic_entity.rb b/app/serializers/merge_request_basic_entity.rb index 084627f9dbe..178e72f4f0a 100644 --- a/app/serializers/merge_request_basic_entity.rb +++ b/app/serializers/merge_request_basic_entity.rb @@ -11,4 +11,5 @@ class MergeRequestBasicEntity < Grape::Entity expose :labels, using: LabelEntity expose :assignee, using: API::Entities::UserBasic expose :task_status, :task_status_short + expose :lock_version, :lock_version end diff --git a/app/services/issuable/common_system_notes_service.rb b/app/services/issuable/common_system_notes_service.rb index 885e14bba8f..77f38f8882e 100644 --- a/app/services/issuable/common_system_notes_service.rb +++ b/app/services/issuable/common_system_notes_service.rb @@ -20,7 +20,7 @@ module Issuable create_due_date_note if issuable.previous_changes.include?('due_date') create_milestone_note if issuable.previous_changes.include?('milestone_id') - create_labels_note(old_labels) if issuable.labels != old_labels + create_labels_note(old_labels) if old_labels && issuable.labels != old_labels end private diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 805bb5b510d..842d59d26a0 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -235,6 +235,63 @@ class IssuableBaseService < BaseService issuable end + def update_task(issuable) + filter_params(issuable) + + if issuable.changed? || params.present? + issuable.assign_attributes(params.merge(updated_by: current_user, + last_edited_at: Time.now, + last_edited_by: current_user)) + + before_update(issuable) + + if issuable.with_transaction_returning_status { issuable.save } + # We do not touch as it will affect a update on updated_at field + ActiveRecord::Base.no_touching do + Issuable::CommonSystemNotesService.new(project, current_user).execute(issuable, old_labels: nil) + end + + handle_task_changes(issuable) + invalidate_cache_counts(issuable, users: issuable.assignees.to_a) + after_update(issuable) + execute_hooks(issuable, 'update', old_associations: nil) + end + end + + issuable + end + + # Handle the `update_task` event sent from UI. Attempts to update a specific + # line in the markdown and cached html, bypassing any unnecessary updates or checks. + def update_task_event(issuable) + update_task_params = params.delete(:update_task) + return unless update_task_params + + tasklist_toggler = TaskListToggleService.new(issuable.description, issuable.description_html, + line_source: update_task_params[:line_source], + line_number: update_task_params[:line_number].to_i, + toggle_as_checked: update_task_params[:checked], + index: update_task_params[:index].to_i, + sourcepos: !issuable.legacy_markdown?) + + unless tasklist_toggler.execute + # if we make it here, the data is much newer than we thought it was - fail fast + raise ActiveRecord::StaleObjectError + end + + # by updating the description_html field at the same time, + # the markdown cache won't be considered invalid + params[:description] = tasklist_toggler.updated_markdown + params[:description_html] = tasklist_toggler.updated_markdown_html + + # since we're updating a very specific line, we don't care whether + # the `lock_version` sent from the FE is the same or not. Just + # make sure the data hasn't changed since we queried it + params[:lock_version] = issuable.lock_version + + update_task(issuable) + end + def labels_changing?(old_label_ids, new_label_ids) old_label_ids.sort != new_label_ids.sort end @@ -318,6 +375,10 @@ class IssuableBaseService < BaseService end # override if needed + def handle_task_changes(issuable) + end + + # override if needed def execute_hooks(issuable, action = 'open', params = {}) end diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index e992d682c79..cec5b5734c0 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -8,7 +8,7 @@ module Issues handle_move_between_ids(issue) filter_spam_check_params change_issue_duplicate(issue) - move_issue_to_new_project(issue) || update(issue) + move_issue_to_new_project(issue) || update_task_event(issue) || update(issue) end def update(issue) @@ -63,6 +63,11 @@ module Issues end end + def handle_task_changes(issuable) + todo_service.mark_pending_todos_as_done(issuable, current_user) + todo_service.update_issue(issuable, current_user) + end + def handle_move_between_ids(issue) return unless params[:move_between_ids] @@ -78,6 +83,8 @@ module Issues # rubocop: disable CodeReuse/ActiveRecord def change_issue_duplicate(issue) canonical_issue_id = params.delete(:canonical_issue_id) + return unless canonical_issue_id + canonical_issue = IssuesFinder.new(current_user).find_by(id: canonical_issue_id) if canonical_issue diff --git a/app/services/members/create_service.rb b/app/services/members/create_service.rb index 714b8586737..cf710fef52b 100644 --- a/app/services/members/create_service.rb +++ b/app/services/members/create_service.rb @@ -19,9 +19,19 @@ module Members current_user: current_user ) - members.each { |member| after_execute(member: member) } + errors = [] - success + members.each do |member| + if member.errors.any? + errors << "#{member.user.username}: #{member.errors.full_messages.to_sentence}" + else + after_execute(member: member) + end + end + + return success unless errors.any? + + error(errors.to_sentence) end private diff --git a/app/services/task_list_toggle_service.rb b/app/services/task_list_toggle_service.rb new file mode 100644 index 00000000000..2717fc9035a --- /dev/null +++ b/app/services/task_list_toggle_service.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +# Finds the correct checkbox in the passed in markdown/html and toggles it's state, +# returning the updated markdown/html. +# We don't care if the text has changed above or below the specific checkbox, as long +# the checkbox still exists at exactly the same line number and the text is equal. +# If successful, new values are available in `updated_markdown` and `updated_markdown_html` +# +# Note: once we've removed RedCarpet support, we can remove the `index` and `sourcepos` +# parameters +class TaskListToggleService + attr_reader :updated_markdown, :updated_markdown_html + + def initialize(markdown, markdown_html, line_source:, line_number:, toggle_as_checked:, index:, sourcepos: true) + @markdown, @markdown_html = markdown, markdown_html + @line_source, @line_number = line_source, line_number + @toggle_as_checked = toggle_as_checked + @index, @use_sourcepos = index, sourcepos + + @updated_markdown, @updated_markdown_html = nil + end + + def execute + return false unless markdown && markdown_html + + toggle_markdown && toggle_markdown_html + end + + private + + attr_reader :markdown, :markdown_html, :index, :toggle_as_checked + attr_reader :line_source, :line_number, :use_sourcepos + + def toggle_markdown + source_lines = markdown.split("\n") + source_line_index = line_number - 1 + markdown_task = source_lines[source_line_index] + + return unless markdown_task == line_source + return unless source_checkbox = Taskable::ITEM_PATTERN.match(markdown_task) + + currently_checked = TaskList::Item.new(source_checkbox[1]).complete? + + # Check `toggle_as_checked` to make sure we don't accidentally replace + # any `[ ]` or `[x]` in the middle of the text + if currently_checked + markdown_task.sub!(Taskable::COMPLETE_PATTERN, '[ ]') unless toggle_as_checked + else + markdown_task.sub!(Taskable::INCOMPLETE_PATTERN, '[x]') if toggle_as_checked + end + + source_lines[source_line_index] = markdown_task + @updated_markdown = source_lines.join("\n") + end + + def toggle_markdown_html + html = Nokogiri::HTML.fragment(markdown_html) + html_checkbox = get_html_checkbox(html) + return unless html_checkbox + + if toggle_as_checked + html_checkbox[:checked] = 'checked' + else + html_checkbox.remove_attribute('checked') + end + + @updated_markdown_html = html.to_html + end + + # When using CommonMark, we should be able to use the embedded `sourcepos` attribute to + # target the exact line in the DOM. For RedCarpet, we need to use the index of the checkbox + # that was checked and match it with what we think is the same checkbox. + # The reason `sourcepos` is slightly more reliable is the case where a line of text is + # changed from a regular line into a checkbox (or vice versa). Then the checked index + # in the UI will be off from the list of checkboxes we've calculated locally. + # It's a rare circumstance, but since we can account for it, we do. + def get_html_checkbox(html) + if use_sourcepos + html.css(".task-list-item[data-sourcepos^='#{line_number}:'] > input.task-list-item-checkbox").first + else + html.css('.task-list-item-checkbox')[index - 1] + end + end +end diff --git a/app/views/admin/appearances/_form.html.haml b/app/views/admin/appearances/_form.html.haml index 544f09048f5..77e84abd76e 100644 --- a/app/views/admin/appearances/_form.html.haml +++ b/app/views/admin/appearances/_form.html.haml @@ -1,86 +1,96 @@ -= form_for @appearance, url: admin_appearances_path do |f| += form_for @appearance, url: admin_appearances_path, html: { class: 'prepend-top-default' } do |f| = form_errors(@appearance) - %fieldset.app_logo - %legend - Navigation bar: - .form-group.row - = f.label :header_logo, 'Header logo', class: 'col-sm-2 col-form-label pt-0' - .col-sm-10 - - if @appearance.header_logo? - = image_tag @appearance.header_logo_path, class: 'appearance-light-logo-preview' - - if @appearance.persisted? - %br - = link_to 'Remove header logo', header_logos_admin_appearances_path, data: { confirm: "Header logo will be removed. Are you sure?"}, method: :delete, class: "btn btn-inverted btn-remove btn-sm remove-logo" - %hr - = f.hidden_field :header_logo_cache - = f.file_field :header_logo, class: "" - .hint - Maximum file size is 1MB. Pages are optimized for a 28px tall header logo - %fieldset.app_logo - %legend - Favicon: - .form-group.row - = f.label :favicon, 'Favicon', class: 'col-sm-2 col-form-label pt-0' - .col-sm-10 - - if @appearance.favicon? - = image_tag @appearance.favicon_path, class: 'appearance-light-logo-preview' - - if @appearance.persisted? - %br - = link_to 'Remove favicon', favicon_admin_appearances_path, data: { confirm: "Favicon will be removed. Are you sure?"}, method: :delete, class: "btn btn-inverted btn-remove btn-sm remove-logo" - %hr - = f.hidden_field :favicon_cache - = f.file_field :favicon, class: '' - .hint - Maximum file size is 1MB. Image size must be 32x32px. Allowed image formats are #{favicon_extension_whitelist}. - %br - Images with incorrect dimensions are not resized automatically, and may result in unexpected behavior. + .row + .col-lg-4.profile-settings-sidebar + %h4.prepend-top-0 Navigation bar + + .col-lg-8 + .form-group + = f.label :header_logo, 'Header logo', class: 'col-form-label label-bold pt-0' + %p + - if @appearance.header_logo? + = image_tag @appearance.header_logo_path, class: 'appearance-light-logo-preview' + - if @appearance.persisted? + %br + = link_to 'Remove header logo', header_logos_admin_appearances_path, data: { confirm: "Header logo will be removed. Are you sure?"}, method: :delete, class: "btn btn-inverted btn-remove btn-sm remove-logo" + %hr + = f.hidden_field :header_logo_cache + = f.file_field :header_logo, class: "" + .hint + Maximum file size is 1MB. Pages are optimized for a 28px tall header logo + %hr + .row + .col-lg-4.profile-settings-sidebar + %h4.prepend-top-0 Favicon + + .col-lg-8 + .form-group + = f.label :favicon, 'Favicon', class: 'col-form-label label-bold pt-0' + %p + - if @appearance.favicon? + = image_tag @appearance.favicon_path, class: 'appearance-light-logo-preview' + - if @appearance.persisted? + %br + = link_to 'Remove favicon', favicon_admin_appearances_path, data: { confirm: "Favicon will be removed. Are you sure?"}, method: :delete, class: "btn btn-inverted btn-remove btn-sm remove-logo" + %hr + = f.hidden_field :favicon_cache + = f.file_field :favicon, class: '' + .hint + Maximum file size is 1MB. Image size must be 32x32px. Allowed image formats are #{favicon_extension_whitelist}. + %br + Images with incorrect dimensions are not resized automatically, and may result in unexpected behavior. - %fieldset.sign-in - %legend - Sign in/Sign up pages: - .form-group.row - = f.label :title, class: 'col-sm-2 col-form-label' - .col-sm-10 - = f.text_field :title, class: "form-control" - .form-group.row - = f.label :description, class: 'col-sm-2 col-form-label' - .col-sm-10 - = f.text_area :description, class: "form-control", rows: 10 - .hint - Description parsed with #{link_to "GitLab Flavored Markdown", help_page_path('user/markdown'), target: '_blank'}. - .form-group.row - = f.label :logo, class: 'col-sm-2 col-form-label pt-0' - .col-sm-10 - - if @appearance.logo? - = image_tag @appearance.logo_path, class: 'appearance-logo-preview' - - if @appearance.persisted? - %br - = link_to 'Remove logo', logo_admin_appearances_path, data: { confirm: "Logo will be removed. Are you sure?"}, method: :delete, class: "btn btn-inverted btn-remove btn-sm remove-logo" - %hr - = f.hidden_field :logo_cache - = f.file_field :logo, class: "" - .hint - Maximum file size is 1MB. Pages are optimized for a 640x360 px logo. - %fieldset - %legend - New project pages: - .form-group.row - = f.label :new_project_guidelines, class: 'col-sm-2 col-form-label' - .col-sm-10 - = f.text_area :new_project_guidelines, class: "form-control", rows: 10 - .hint - Guidelines parsed with #{link_to "GitLab Flavored Markdown", help_page_path('user/markdown'), target: '_blank'}. + %hr + .row + .col-lg-4.profile-settings-sidebar + %h4.prepend-top-0 Sign in/Sign up pages - .form-actions - = f.submit 'Save', class: 'btn btn-success append-right-10' - - if @appearance.persisted? - Preview last save: - = link_to 'Sign-in page', preview_sign_in_admin_appearances_path, class: 'btn', target: '_blank', rel: 'noopener noreferrer' - = link_to 'New project page', new_project_path, class: 'btn', target: '_blank', rel: 'noopener noreferrer' + .col-lg-8 + .form-group + = f.label :title, class: 'col-form-label label-bold' + = f.text_field :title, class: "form-control" + .form-group + = f.label :description, class: 'col-form-label label-bold' + = f.text_area :description, class: "form-control", rows: 10 + .hint + Description parsed with #{link_to "GitLab Flavored Markdown", help_page_path('user/markdown'), target: '_blank'}. + .form-group + = f.label :logo, class: 'col-form-label label-bold pt-0' + %p + - if @appearance.logo? + = image_tag @appearance.logo_path, class: 'appearance-logo-preview' + - if @appearance.persisted? + %br + = link_to 'Remove logo', logo_admin_appearances_path, data: { confirm: "Logo will be removed. Are you sure?"}, method: :delete, class: "btn btn-inverted btn-remove btn-sm remove-logo" + %hr + = f.hidden_field :logo_cache + = f.file_field :logo, class: "" + .hint + Maximum file size is 1MB. Pages are optimized for a 640x360 px logo. + + %hr + .row + .col-lg-4.profile-settings-sidebar + %h4.prepend-top-0 New project pages + + .col-lg-8 + .form-group + = f.label :new_project_guidelines, class: 'col-form-label label-bold' + %p + = f.text_area :new_project_guidelines, class: "form-control", rows: 10 + .hint + Guidelines parsed with #{link_to "GitLab Flavored Markdown", help_page_path('user/markdown'), target: '_blank'}. + + .prepend-top-default.append-bottom-default + = f.submit 'Update appearance settings', class: 'btn btn-success' + - if @appearance.persisted? + Preview last save: + = link_to 'Sign-in page', preview_sign_in_admin_appearances_path, class: 'btn', target: '_blank', rel: 'noopener noreferrer' + = link_to 'New project page', new_project_path, class: 'btn', target: '_blank', rel: 'noopener noreferrer' - - if @appearance.updated_at - %span.float-right - Last edit #{time_ago_with_tooltip(@appearance.updated_at)} + - if @appearance.updated_at + %span.float-right + Last edit #{time_ago_with_tooltip(@appearance.updated_at)} diff --git a/app/views/admin/appearances/show.html.haml b/app/views/admin/appearances/show.html.haml index 454b779842c..ccf6f960cf2 100644 --- a/app/views/admin/appearances/show.html.haml +++ b/app/views/admin/appearances/show.html.haml @@ -1,9 +1,4 @@ - page_title "Appearance" - -%h3.page-title - Appearance settings -%p.light - You can modify the look and feel of GitLab here -%hr +- @content_class = "limit-container-width" unless fluid_layout = render 'form' diff --git a/app/views/devise/sessions/_new_ldap.html.haml b/app/views/devise/sessions/_new_ldap.html.haml index 796c0cadda8..f856773526d 100644 --- a/app/views/devise/sessions/_new_ldap.html.haml +++ b/app/views/devise/sessions/_new_ldap.html.haml @@ -1,3 +1,5 @@ +- server = local_assigns.fetch(:server) + = form_tag(omniauth_callback_path(:user, server['provider_name']), id: 'new_ldap_user', class: "gl-show-field-errors") do .form-group = label_tag :username, "#{server['label']} Username" diff --git a/app/views/profiles/show.html.haml b/app/views/profiles/show.html.haml index 2629b374e7c..753316b27e2 100644 --- a/app/views/profiles/show.html.haml +++ b/app/views/profiles/show.html.haml @@ -2,7 +2,7 @@ - @content_class = "limit-container-width" unless fluid_layout - gravatar_link = link_to Gitlab.config.gravatar.host, 'https://' + Gitlab.config.gravatar.host -= bootstrap_form_for @user, url: profile_path, method: :put, html: { multipart: true, class: 'edit-user prepend-top-default js-quick-submit' }, authenticity_token: true do |f| += bootstrap_form_for @user, url: profile_path, method: :put, html: { multipart: true, class: 'edit-user prepend-top-default js-quick-submit gl-show-field-errors' }, authenticity_token: true do |f| = form_errors(@user) .row @@ -77,10 +77,10 @@ .col-lg-8 .row - if @user.read_only_attribute?(:name) - = f.text_field :name, required: true, readonly: true, wrapper: { class: 'col-md-9' }, + = f.text_field :name, required: true, readonly: true, wrapper: { class: 'col-md-9 qa-full-name' }, help: s_("Profiles|Your name was automatically set based on your %{provider_label} account, so people you know can recognize you") % { provider_label: attribute_provider_label(:name) } - else - = f.text_field :name, label: 'Full name', required: true, wrapper: { class: 'col-md-9' }, help: s_("Profiles|Enter your name, so people you know can recognize you") + = f.text_field :name, label: 'Full name', required: true, title: s_("Profiles|Using emojis in names seems fun, but please try to set a status message instead"), wrapper: { class: 'col-md-9 qa-full-name' }, help: s_("Profiles|Enter your name, so people you know can recognize you") = f.text_field :id, readonly: true, label: 'User ID', wrapper: { class: 'col-md-3' } - if @user.read_only_attribute?(:email) diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml index d6f340d0ee2..0b720e5d542 100644 --- a/app/views/projects/merge_requests/show.html.haml +++ b/app/views/projects/merge_requests/show.html.haml @@ -7,7 +7,7 @@ - page_card_attributes @merge_request.card_attributes - suggest_changes_help_path = help_page_path('user/discussions/index.md', anchor: 'suggest-changes') -.merge-request{ data: { mr_action: j(params[:tab].presence || 'show'), url: merge_request_path(@merge_request, format: :json), project_path: project_path(@merge_request.project) } } +.merge-request{ data: { mr_action: j(params[:tab].presence || 'show'), url: merge_request_path(@merge_request, format: :json), project_path: project_path(@merge_request.project), lock_version: @merge_request.lock_version } } = render "projects/merge_requests/mr_title" .merge-request-details.issuable-details{ data: { id: @merge_request.project.id } } diff --git a/changelogs/unreleased/19745-forms-with-task-lists-can-be-overwritten-when-editing-simultaneously.yml b/changelogs/unreleased/19745-forms-with-task-lists-can-be-overwritten-when-editing-simultaneously.yml new file mode 100644 index 00000000000..b1177e1717e --- /dev/null +++ b/changelogs/unreleased/19745-forms-with-task-lists-can-be-overwritten-when-editing-simultaneously.yml @@ -0,0 +1,5 @@ +--- +title: Increase reliability and performance of toggling task items +merge_request: 23938 +author: +type: fixed diff --git a/changelogs/unreleased/50521-block-emojis-and-symbol-characters-from-user-s-full-names-2.yml b/changelogs/unreleased/50521-block-emojis-and-symbol-characters-from-user-s-full-names-2.yml new file mode 100644 index 00000000000..04caf8262c6 --- /dev/null +++ b/changelogs/unreleased/50521-block-emojis-and-symbol-characters-from-user-s-full-names-2.yml @@ -0,0 +1,5 @@ +--- +title: Block emojis and symbol characters from users full names +merge_request: 24523 +author: +type: other diff --git a/changelogs/unreleased/55098-ui-bug-adding-group-members-with-lower-permissions.yml b/changelogs/unreleased/55098-ui-bug-adding-group-members-with-lower-permissions.yml new file mode 100644 index 00000000000..f22524ef4b2 --- /dev/null +++ b/changelogs/unreleased/55098-ui-bug-adding-group-members-with-lower-permissions.yml @@ -0,0 +1,5 @@ +--- +title: Resolve UI bug adding group members with lower permissions +merge_request: 24820 +author: +type: fixed diff --git a/changelogs/unreleased/57063-implement-new-arguments-iid-for-issuesresolver-in-graphql.yml b/changelogs/unreleased/57063-implement-new-arguments-iid-for-issuesresolver-in-graphql.yml new file mode 100644 index 00000000000..b05ab07e14c --- /dev/null +++ b/changelogs/unreleased/57063-implement-new-arguments-iid-for-issuesresolver-in-graphql.yml @@ -0,0 +1,5 @@ +--- +title: Add argument iids for issues in GraphQL +merge_request: 24802 +author: +type: added diff --git a/changelogs/unreleased/issue_55744.yml b/changelogs/unreleased/issue_55744.yml new file mode 100644 index 00000000000..6a643732b18 --- /dev/null +++ b/changelogs/unreleased/issue_55744.yml @@ -0,0 +1,5 @@ +--- +title: Fix template labels not being created on new projects +merge_request: 24803 +author: +type: fixed diff --git a/changelogs/unreleased/update-gitaly.yml b/changelogs/unreleased/update-gitaly.yml new file mode 100644 index 00000000000..4ba42a689a7 --- /dev/null +++ b/changelogs/unreleased/update-gitaly.yml @@ -0,0 +1,5 @@ +--- +title: Update Gitaly to v1.17.0 +merge_request: 24873 +author: +type: other diff --git a/changelogs/unreleased/update-ui-admin-appearance.yml b/changelogs/unreleased/update-ui-admin-appearance.yml new file mode 100644 index 00000000000..7bc35029d77 --- /dev/null +++ b/changelogs/unreleased/update-ui-admin-appearance.yml @@ -0,0 +1,5 @@ +--- +title: Update UI for admin appearance settings +merge_request: 24685 +author: +type: other diff --git a/db/migrate/20190115092821_add_columns_project_error_tracking_settings.rb b/db/migrate/20190115092821_add_columns_project_error_tracking_settings.rb new file mode 100644 index 00000000000..190b6f958fd --- /dev/null +++ b/db/migrate/20190115092821_add_columns_project_error_tracking_settings.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class AddColumnsProjectErrorTrackingSettings < ActiveRecord::Migration[5.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :project_error_tracking_settings, :project_name, :string + add_column :project_error_tracking_settings, :organization_name, :string + + change_column_default :project_error_tracking_settings, :enabled, from: true, to: false + + change_column_null :project_error_tracking_settings, :api_url, true + end +end diff --git a/db/post_migrate/20190131122559_fix_null_type_labels.rb b/db/post_migrate/20190131122559_fix_null_type_labels.rb new file mode 100644 index 00000000000..83bb613990c --- /dev/null +++ b/db/post_migrate/20190131122559_fix_null_type_labels.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class FixNullTypeLabels < ActiveRecord::Migration[5.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + update_column_in_batches(:labels, :type, 'ProjectLabel') do |table, query| + query.where( + table[:project_id].not_eq(nil) + .and(table[:template].eq(false)) + .and(table[:type].eq(nil)) + ) + end + end + + def down + # no action + end +end diff --git a/db/schema.rb b/db/schema.rb index 7c1733becb9..4b6e4992056 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20190124200344) do +ActiveRecord::Schema.define(version: 20190131122559) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1574,10 +1574,12 @@ ActiveRecord::Schema.define(version: 20190124200344) do end create_table "project_error_tracking_settings", primary_key: "project_id", id: :integer, force: :cascade do |t| - t.boolean "enabled", default: true, null: false - t.string "api_url", null: false + t.boolean "enabled", default: false, null: false + t.string "api_url" t.string "encrypted_token" t.string "encrypted_token_iv" + t.string "project_name" + t.string "organization_name" end create_table "project_features", force: :cascade do |t| diff --git a/doc/ci/examples/browser_performance.md b/doc/ci/examples/browser_performance.md index 7c3b3a65675..b47038011de 100644 --- a/doc/ci/examples/browser_performance.md +++ b/doc/ci/examples/browser_performance.md @@ -41,7 +41,7 @@ The above example will create a `performance` job in your CI/CD pipeline and wil Sitespeed.io against the webpage you defined in `URL` to gather key metrics. The [GitLab plugin](https://gitlab.com/gitlab-org/gl-performance) for Sitespeed.io is downloaded in order to save the report as a -[Performance report artifact](https://docs.gitlab.com/ee//ci/yaml/README.html#artifactsreportsperformance) +[Performance report artifact](../yaml/README.md#artifactsreportsperformance-premium) that you can later download and analyze. Due to implementation limitations we always take the latest Performance artifact available. diff --git a/doc/ci/examples/code_quality.md b/doc/ci/examples/code_quality.md index ae000b9d30d..3e7d6e7e3f7 100644 --- a/doc/ci/examples/code_quality.md +++ b/doc/ci/examples/code_quality.md @@ -36,7 +36,7 @@ code_quality: The above example will create a `code_quality` job in your CI/CD pipeline which will scan your source code for code quality issues. The report will be saved as a -[Code Quality report artifact](../../ci/yaml/README.md#artifactsreportscodequality) +[Code Quality report artifact](../yaml/README.md#artifactsreportscodequality-starter) that you can later download and analyze. Due to implementation limitations we always take the latest Code Quality artifact available. diff --git a/doc/ci/examples/container_scanning.md b/doc/ci/examples/container_scanning.md index 31c3df81fef..e8e9c73d1b2 100644 --- a/doc/ci/examples/container_scanning.md +++ b/doc/ci/examples/container_scanning.md @@ -51,7 +51,7 @@ The above example will create a `container_scanning` job in your CI/CD pipeline, the image from the [Container Registry](../../user/project/container_registry.md) (whose name is defined from the two `CI_APPLICATION_` variables) and scan it for possible vulnerabilities. The report will be saved as a -[Container Scanning report artifact](https://docs.gitlab.com/ee//ci/yaml/README.html#artifactsreportscontainer_scanning) +[Container Scanning report artifact](../yaml/README.md#artifactsreportscontainer_scanning-ultimate) that you can later download and analyze. Due to implementation limitations we always take the latest Container Scanning artifact available. diff --git a/doc/ci/examples/dast.md b/doc/ci/examples/dast.md index 0ca89eb6700..ab0ca13d2cf 100644 --- a/doc/ci/examples/dast.md +++ b/doc/ci/examples/dast.md @@ -40,7 +40,7 @@ dast: The above example will create a `dast` job in your CI/CD pipeline which will run the tests on the URL defined in the `website` variable (change it to use your own) and scan it for possible vulnerabilities. The report will be saved as a -[DAST report artifact](https://docs.gitlab.com/ee//ci/yaml/README.html#artifactsreportsdast) +[DAST report artifact](../yaml/README.md#artifactsreportsdast-ultimate) that you can later download and analyze. Due to implementation limitations we always take the latest DAST artifact available. diff --git a/doc/ci/pipelines.md b/doc/ci/pipelines.md index d2a00b9218d..c41f3b7e82d 100644 --- a/doc/ci/pipelines.md +++ b/doc/ci/pipelines.md @@ -3,7 +3,7 @@ > Introduced in GitLab 8.8. NOTE: **Note:** -If you have a [mirrored repository where GitLab pulls from](https://docs.gitlab.com/ee/workflow/repository_mirroring.html#pulling-from-a-remote-repository), +If you have a [mirrored repository where GitLab pulls from](https://docs.gitlab.com/ee/workflow/repository_mirroring.html#pulling-from-a-remote-repository-starter), you may need to enable pipeline triggering in your project's **Settings > Repository > Pull from a remote repository > Trigger pipelines for mirror updates**. diff --git a/doc/ci/quick_start/README.md b/doc/ci/quick_start/README.md index 1ec8a8c89c9..9684cb6ed98 100644 --- a/doc/ci/quick_start/README.md +++ b/doc/ci/quick_start/README.md @@ -127,7 +127,7 @@ Now if you go to the **Pipelines** page you will see that the pipeline is pending. NOTE: **Note:** -If you have a [mirrored repository where GitLab pulls from](https://docs.gitlab.com/ee/workflow/repository_mirroring.html#pulling-from-a-remote-repository), +If you have a [mirrored repository where GitLab pulls from](https://docs.gitlab.com/ee/workflow/repository_mirroring.html#pulling-from-a-remote-repository-starter), you may need to enable pipeline triggering in your project's **Settings > Repository > Pull from a remote repository > Trigger pipelines for mirror updates**. diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 4c39b14b1d0..df14376dd36 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -11,7 +11,7 @@ If you want a quick introduction to GitLab CI, follow our [quick start guide](../quick_start/README.md). NOTE: **Note:** -If you have a [mirrored repository where GitLab pulls from](https://docs.gitlab.com/ee/workflow/repository_mirroring.html#pulling-from-a-remote-repository), +If you have a [mirrored repository where GitLab pulls from](https://docs.gitlab.com/ee/workflow/repository_mirroring.html#pulling-from-a-remote-repository-starter), you may need to enable pipeline triggering in your project's **Settings > Repository > Pull from a remote repository > Trigger pipelines for mirror updates**. diff --git a/doc/development/documentation/index.md b/doc/development/documentation/index.md index 828f9bfeec6..436d0a38f31 100644 --- a/doc/development/documentation/index.md +++ b/doc/development/documentation/index.md @@ -27,7 +27,7 @@ The source of the documentation is maintained in the following repository locati | Project | Path | | --- | --- | | [GitLab Community Edition](https://gitlab.com/gitlab-org/gitlab-ce/) | [`/doc`](https://gitlab.com/gitlab-org/gitlab-ce/tree/master/doc) | -| [GitLab Enterprise Edition](https://gitlab.com/gitlab-org/gitlab-ce/) | [`/doc`](https://gitlab.com/gitlab-org/gitlab-ee/tree/master/doc) | +| [GitLab Enterprise Edition](https://gitlab.com/gitlab-org/gitlab-ee/) | [`/doc`](https://gitlab.com/gitlab-org/gitlab-ee/tree/master/doc) | | [GitLab Runner](https://gitlab.com/gitlab-org/gitlab-runner/) | [`/docs`](https://gitlab.com/gitlab-org/gitlab-runner/tree/master/docs) | | [Omnibus GitLab](https://gitlab.com/gitlab-org/omnibus-gitlab/) | [`/doc`](https://gitlab.com/gitlab-org/gitlab-ee/tree/master/doc) | diff --git a/doc/user/project/pages/getting_started_part_three.md b/doc/user/project/pages/getting_started_part_three.md index cea9628966d..68dd3330d7a 100644 --- a/doc/user/project/pages/getting_started_part_three.md +++ b/doc/user/project/pages/getting_started_part_three.md @@ -264,7 +264,7 @@ your Pages project are the same. 1. A PEM certificate 1. An intermediate certificate -1. A public key +1. A private key  @@ -280,7 +280,7 @@ Usually it's combined with the PEM certificate, but there are some cases in which you need to add them manually. [CloudFlare certs](https://about.gitlab.com/2017/02/07/setting-up-gitlab-pages-with-cloudflare-certificates/) are one of these cases. -- A public key is an encrypted key which validates +- A private key is an encrypted key which validates your PEM against your domain. ### Now what? @@ -293,7 +293,7 @@ of this, it's simple: and paste the root certificate (usually available from your CA website) and paste it in the [same field as your PEM certificate](https://about.gitlab.com/2017/02/07/setting-up-gitlab-pages-with-cloudflare-certificates/), just jumping a line between them. -- Copy your public key and paste it in the last field +- Copy your private key and paste it in the last field >**Note:** **Do not** open certificates or encryption keys in diff --git a/doc/workflow/repository_mirroring.md b/doc/workflow/repository_mirroring.md index ac26aeab137..1213474b7d8 100644 --- a/doc/workflow/repository_mirroring.md +++ b/doc/workflow/repository_mirroring.md @@ -55,7 +55,7 @@ When push mirroring is enabled, only push commits directly to the mirrored repos mirror diverging. All changes will end up in the mirrored repository whenever: - Commits are pushed to GitLab. -- A [forced update](#forcing-an-update) is initiated. +- A [forced update](#forcing-an-update-core) is initiated. Changes pushed to files in the repository are automatically pushed to the remote mirror at least: @@ -122,7 +122,7 @@ directly to the repository on GitLab. Instead, any commits should be pushed to t Changes pushed to the upstream repository will be pulled into the GitLab repository, either: - Automatically within a certain period of time. -- When a [forced update](#forcing-an-update) is initiated. +- When a [forced update](#forcing-an-update-core) is initiated. CAUTION: **Caution:** If you do manually update a branch in the GitLab repository, the branch will become diverged from @@ -259,7 +259,7 @@ failed. This will become visible in either the: - Pull mirror settings page. When a project is hard failed, it will no longer get picked up for mirroring. A user can resume the -project mirroring again by [Forcing an update](#forcing-an-update). +project mirroring again by [Forcing an update](#forcing-an-update-core). ### Trigger update using API **[STARTER]** @@ -292,8 +292,8 @@ them and how they will be resolved. Rewriting any mirrored commit on either remote will cause conflicts and mirroring to fail. This can be prevented by: -- [Pulling only protected branches](#pull-only-protected-branches). -- [Pushing only protected branches](#push-only-protected-branches). +- [Pulling only protected branches](#only-mirror-protected-branches-starter). +- [Pushing only protected branches](#push-only-protected-branches-core). You should [protect the branches](../user/project/protected_branches.md) you wish to mirror on both remotes to prevent conflicts caused by rewriting history. diff --git a/lib/gitlab/auth/ldap/adapter.rb b/lib/gitlab/auth/ldap/adapter.rb index 42c657afe6a..15b9d5ad6e9 100644 --- a/lib/gitlab/auth/ldap/adapter.rb +++ b/lib/gitlab/auth/ldap/adapter.rb @@ -30,14 +30,7 @@ module Gitlab def users(fields, value, limit = nil) options = user_options(Array(fields), value, limit) - - entries = ldap_search(options).select do |entry| - entry.respond_to? config.uid - end - - entries.map do |entry| - Gitlab::Auth::LDAP::Person.new(entry, provider) - end + users_search(options) end def user(*args) @@ -90,6 +83,16 @@ module Gitlab SEARCH_RETRY_FACTOR[retry_number] * config.timeout end + def users_search(options) + entries = ldap_search(options).select do |entry| + entry.respond_to? config.uid + end + + entries.map do |entry| + Gitlab::Auth::LDAP::Person.new(entry, provider) + end + end + def user_options(fields, value, limit) options = { attributes: Gitlab::Auth::LDAP::Person.ldap_attributes(config), diff --git a/lib/gitlab/background_migration.rb b/lib/gitlab/background_migration.rb index 6cf40e2d4ca..5251e0fadf9 100644 --- a/lib/gitlab/background_migration.rb +++ b/lib/gitlab/background_migration.rb @@ -16,11 +16,18 @@ module Gitlab # re-raises the exception. # # steal_class - The name of the class for which to steal jobs. - def self.steal(steal_class) - enqueued = Sidekiq::Queue.new(self.queue) - scheduled = Sidekiq::ScheduledSet.new + def self.steal(steal_class, retry_dead_jobs: false) + queues = [ + Sidekiq::ScheduledSet.new, + Sidekiq::Queue.new(self.queue) + ] + + if retry_dead_jobs + queues << Sidekiq::RetrySet.new + queues << Sidekiq::DeadSet.new + end - [scheduled, enqueued].each do |queue| + queues.each do |queue| queue.each do |job| migration_class, migration_args = job.args diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index 7987533978c..add7ee58da6 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -166,6 +166,7 @@ excluded_attributes: error_tracking_setting: - :encrypted_token - :encrypted_token_iv + - :enabled methods: labels: diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 8caa876e6b0..ccdbc63c51e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -552,9 +552,6 @@ msgstr "" msgid "An empty GitLab User field will add the FogBugz user's full name (e.g. \"By John Smith\") in the description of all issues and comments. It will also associate and/or assign these issues and comments with the project creator." msgstr "" -msgid "An error accured whilst committing your changes." -msgstr "" - msgid "An error has occurred" msgstr "" @@ -639,6 +636,9 @@ msgstr "" msgid "An error occurred while validating username" msgstr "" +msgid "An error occurred whilst committing your changes." +msgstr "" + msgid "An error occurred whilst fetching the job trace." msgstr "" @@ -2992,6 +2992,9 @@ msgstr "" msgid "Error Tracking" msgstr "" +msgid "Error deleting %{issuableType}" +msgstr "" + msgid "Error fetching contributors data." msgstr "" @@ -3040,6 +3043,9 @@ msgstr "" msgid "Error saving label update." msgstr "" +msgid "Error updating %{issuableType}" +msgstr "" + msgid "Error updating status for all todos." msgstr "" @@ -5513,6 +5519,9 @@ msgstr "" msgid "Profiles|Username successfully changed" msgstr "" +msgid "Profiles|Using emojis in names seems fun, but please try to set a status message instead" +msgstr "" + msgid "Profiles|What's your status?" msgstr "" @@ -6524,6 +6533,9 @@ msgstr "" msgid "Snippets" msgstr "" +msgid "Someone edited this issue at the same time you did. The description has been updated and you will need to make your changes again." +msgstr "" + msgid "Something went wrong on our end" msgstr "" @@ -7710,6 +7722,9 @@ msgstr "" msgid "Update" msgstr "" +msgid "Update failed" +msgstr "" + msgid "Update now" msgstr "" diff --git a/package.json b/package.json index 13c0527c4a3..4d884470cc4 100644 --- a/package.json +++ b/package.json @@ -56,11 +56,12 @@ "d3-time": "^1.0.8", "d3-time-format": "^2.1.1", "dateformat": "^3.0.3", - "deckar01-task_list": "^2.0.1", + "deckar01-task_list": "^2.2.0", "diff": "^3.4.0", "document-register-element": "1.3.0", "dropzone": "^4.2.0", "echarts": "^4.2.0-rc.2", + "emoji-regex": "^7.0.3", "emoji-unicode-version": "^0.2.1", "exports-loader": "^0.7.0", "file-loader": "^3.0.1", diff --git a/qa/qa/page/project/job/show.rb b/qa/qa/page/project/job/show.rb index d688f15914c..49c676c01f2 100644 --- a/qa/qa/page/project/job/show.rb +++ b/qa/qa/page/project/job/show.rb @@ -16,11 +16,19 @@ module QA::Page element :status_badge end + view 'app/assets/javascripts/jobs/components/stages_dropdown.vue' do + element :pipeline_path + end + def completed? COMPLETED_STATUSES.include?(status_badge) end - def passed? + def successful?(timeout: 60) + wait(reload: false, max: timeout) do + completed? && !trace_loading? + end + status_badge == PASSED_STATUS end diff --git a/qa/qa/page/project/pipeline/show.rb b/qa/qa/page/project/pipeline/show.rb index b22396fd67a..f192f1fc64b 100644 --- a/qa/qa/page/project/pipeline/show.rb +++ b/qa/qa/page/project/pipeline/show.rb @@ -11,7 +11,7 @@ module QA::Page view 'app/assets/javascripts/pipelines/components/graph/job_item.vue' do element :job_component, /class.*ci-job-component.*/ # rubocop:disable QA/ElementWithPattern - element :job_link, /class.*js-pipeline-graph-job-link.*/ # rubocop:disable QA/ElementWithPattern + element :job_link end view 'app/assets/javascripts/vue_shared/components/ci_icon.vue' do @@ -32,6 +32,10 @@ module QA::Page end end + def go_to_job(job_name) + find_element(:job_link, job_name).click + end + def go_to_first_job css = '.js-pipeline-graph-job-link' diff --git a/qa/qa/specs/features/browser_ui/6_release/deploy_key/clone_using_deploy_key_spec.rb b/qa/qa/specs/features/browser_ui/6_release/deploy_key/clone_using_deploy_key_spec.rb index e2320c92343..11a9653db81 100644 --- a/qa/qa/specs/features/browser_ui/6_release/deploy_key/clone_using_deploy_key_spec.rb +++ b/qa/qa/specs/features/browser_ui/6_release/deploy_key/clone_using_deploy_key_spec.rb @@ -95,11 +95,7 @@ module QA Page::Project::Pipeline::Show.act { go_to_first_job } Page::Project::Job::Show.perform do |job| - job.wait(reload: false) do - job.completed? && !job.trace_loading? - end - - expect(job.passed?).to be_truthy, "Job status did not become \"passed\"." + expect(job).to be_successful, "Job status did not become \"passed\"." expect(job.output).to include(sha1sum) end end diff --git a/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb b/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb index 553550eef8b..b0ff83db86b 100644 --- a/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb +++ b/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb @@ -75,9 +75,30 @@ module QA Page::Project::Pipeline::Index.act { go_to_latest_pipeline } Page::Project::Pipeline::Show.perform do |pipeline| - expect(pipeline).to have_build('build', status: :success, wait: 600) - expect(pipeline).to have_build('test', status: :success, wait: 600) - expect(pipeline).to have_build('production', status: :success, wait: 1200) + pipeline.go_to_job('build') + end + Page::Project::Job::Show.perform do |job| + expect(job).to be_sucessful(timeout: 600), "Job did not pass" + + job.click_element(:pipeline_path) + end + + Page::Project::Pipeline::Show.perform do |pipeline| + pipeline.go_to_job('test') + end + Page::Project::Job::Show.perform do |job| + expect(job).to be_sucessful(timeout: 600), "Job did not pass" + + job.click_element(:pipeline_path) + end + + Page::Project::Pipeline::Show.perform do |pipeline| + pipeline.go_to_job('production') + end + Page::Project::Job::Show.perform do |job| + expect(job).to be_sucessful(timeout: 1200), "Job did not pass" + + job.click_element(:pipeline_path) end Page::Project::Menu.act { click_operations_environments } @@ -115,9 +136,30 @@ module QA Page::Project::Pipeline::Index.act { go_to_latest_pipeline } Page::Project::Pipeline::Show.perform do |pipeline| - expect(pipeline).to have_build('build', status: :success, wait: 600) - expect(pipeline).to have_build('test', status: :success, wait: 600) - expect(pipeline).to have_build('production', status: :success, wait: 1200) + pipeline.go_to_job('build') + end + Page::Project::Job::Show.perform do |job| + expect(job).to be_sucessful(timeout: 600), "Job did not pass" + + job.click_element(:pipeline_path) + end + + Page::Project::Pipeline::Show.perform do |pipeline| + pipeline.go_to_job('test') + end + Page::Project::Job::Show.perform do |job| + expect(job).to be_sucessful(timeout: 600), "Job did not pass" + + job.click_element(:pipeline_path) + end + + Page::Project::Pipeline::Show.perform do |pipeline| + pipeline.go_to_job('production') + end + Page::Project::Job::Show.perform do |job| + expect(job).to be_sucessful(timeout: 1200), "Job did not pass" + + job.click_element(:pipeline_path) end Page::Project::Menu.act { click_operations_environments } diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 4743ad04339..c34d7c13d57 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -379,6 +379,23 @@ describe Projects::IssuesController do expect(response).to have_gitlab_http_status(200) end end + + context 'when getting the changes' do + before do + project.add_developer(user) + + sign_in(user) + end + + it 'returns the necessary data' do + go(id: issue.iid) + + data = JSON.parse(response.body) + + expect(data).to include('title_text', 'description', 'description_text') + expect(data).to include('task_status', 'lock_version') + end + end end describe 'Confidential Issues' do diff --git a/spec/factories/project_error_tracking_settings.rb b/spec/factories/project_error_tracking_settings.rb index fbd8dfd395c..be30bd0260a 100644 --- a/spec/factories/project_error_tracking_settings.rb +++ b/spec/factories/project_error_tracking_settings.rb @@ -6,5 +6,7 @@ FactoryBot.define do api_url 'https://gitlab.com/api/0/projects/sentry-org/sentry-project' enabled true token 'access_token_123' + project_name 'Sentry Project' + organization_name 'Sentry Org' end end diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 1906c06a211..18fab395cc2 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -1,7 +1,7 @@ require_relative '../support/helpers/test_env' FactoryBot.define do - PAGES_ACCESS_LEVEL_SCHEMA_VERSION = 20180423204600 + PAGES_ACCESS_LEVEL_SCHEMA_VERSION ||= 20180423204600 # Project without repository # diff --git a/spec/features/admin/admin_appearance_spec.rb b/spec/features/admin/admin_appearance_spec.rb index 3c2ae5b3c6a..57215c0d1e9 100644 --- a/spec/features/admin/admin_appearance_spec.rb +++ b/spec/features/admin/admin_appearance_spec.rb @@ -10,10 +10,10 @@ describe 'Admin Appearance' do fill_in 'appearance_title', with: 'MyCompany' fill_in 'appearance_description', with: 'dev server' fill_in 'appearance_new_project_guidelines', with: 'Custom project guidelines' - click_button 'Save' + click_button 'Update appearance settings' expect(current_path).to eq admin_appearances_path - expect(page).to have_content 'Appearance settings' + expect(page).to have_content 'Appearance' expect(page).to have_field('appearance_title', with: 'MyCompany') expect(page).to have_field('appearance_description', with: 'dev server') @@ -57,7 +57,7 @@ describe 'Admin Appearance' do visit admin_appearances_path attach_file(:appearance_logo, logo_fixture) - click_button 'Save' + click_button 'Update appearance settings' expect(page).to have_css(logo_selector) click_link 'Remove logo' @@ -69,7 +69,7 @@ describe 'Admin Appearance' do visit admin_appearances_path attach_file(:appearance_header_logo, logo_fixture) - click_button 'Save' + click_button 'Update appearance settings' expect(page).to have_css(header_logo_selector) click_link 'Remove header logo' @@ -81,7 +81,7 @@ describe 'Admin Appearance' do visit admin_appearances_path attach_file(:appearance_favicon, logo_fixture) - click_button 'Save' + click_button 'Update appearance settings' expect(page).to have_css('.appearance-light-logo-preview') @@ -91,7 +91,7 @@ describe 'Admin Appearance' do # allowed file types attach_file(:appearance_favicon, Rails.root.join('spec', 'fixtures', 'sanitized.svg')) - click_button 'Save' + click_button 'Update appearance settings' expect(page).to have_content 'Favicon You are not allowed to upload "svg" files, allowed types: png, ico' end diff --git a/spec/features/profiles/user_edit_profile_spec.rb b/spec/features/profiles/user_edit_profile_spec.rb index f45bcabd196..b43711f6ef6 100644 --- a/spec/features/profiles/user_edit_profile_spec.rb +++ b/spec/features/profiles/user_edit_profile_spec.rb @@ -10,6 +10,7 @@ describe 'User edit profile' do def submit_settings click_button 'Update profile settings' + wait_for_requests if respond_to?(:wait_for_requests) end it 'changes user profile' do @@ -35,6 +36,17 @@ describe 'User edit profile' do expect(page).to have_content('Profile was successfully updated') end + it 'shows an error if the full name contains an emoji', :js do + simulate_input('#user_name', 'Martin 😀') + submit_settings + + page.within('.qa-full-name') do + expect(page).to have_css '.gl-field-error-outline' + expect(find('.gl-field-error')).not_to have_selector('.hidden') + expect(find('.gl-field-error')).to have_content('Using emojis in names seems fun, but please try to set a status message instead') + end + end + context 'user avatar' do before do attach_file(:user_avatar, Rails.root.join('spec', 'fixtures', 'banana_sample.gif')) @@ -61,6 +73,11 @@ describe 'User edit profile' do end context 'user status', :js do + def visit_user + visit user_path(user) + wait_for_requests + end + def select_emoji(emoji_name, is_modal = false) emoji_menu_class = is_modal ? '.js-modal-status-emoji-menu' : '.js-status-emoji-menu' toggle_button = find('.js-toggle-emoji-menu') @@ -71,18 +88,16 @@ describe 'User edit profile' do context 'profile edit form' do it 'shows the user status form' do - visit(profile_path) - expect(page).to have_content('Current status') end it 'adds emoji to user status' do emoji = 'biohazard' - visit(profile_path) select_emoji(emoji) submit_settings - visit user_path(user) + visit_user + within('.cover-status') do expect(page).to have_emoji(emoji) end @@ -90,11 +105,11 @@ describe 'User edit profile' do it 'adds message to user status' do message = 'I have something to say' - visit(profile_path) fill_in 'js-status-message-field', with: message submit_settings - visit user_path(user) + visit_user + within('.cover-status') do expect(page).to have_emoji('speech_balloon') expect(page).to have_content message @@ -104,12 +119,12 @@ describe 'User edit profile' do it 'adds message and emoji to user status' do emoji = 'tanabata_tree' message = 'Playing outside' - visit(profile_path) select_emoji(emoji) fill_in 'js-status-message-field', with: message submit_settings - visit user_path(user) + visit_user + within('.cover-status') do expect(page).to have_emoji(emoji) expect(page).to have_content message @@ -119,7 +134,8 @@ describe 'User edit profile' do it 'clears the user status' do user_status = create(:user_status, user: user, message: 'Eating bread', emoji: 'stuffed_flatbread') - visit user_path(user) + visit_user + within('.cover-status') do expect(page).to have_emoji(user_status.emoji) expect(page).to have_content user_status.message @@ -129,15 +145,13 @@ describe 'User edit profile' do click_button 'js-clear-user-status-button' submit_settings - wait_for_requests + visit_user - visit user_path(user) expect(page).not_to have_selector '.cover-status' end it 'displays a default emoji if only message is entered' do message = 'a status without emoji' - visit(profile_path) fill_in 'js-status-message-field', with: message within('.js-toggle-emoji-menu') do @@ -162,6 +176,7 @@ describe 'User edit profile' do page.within "#set-user-status-modal" do click_button 'Set status' end + wait_for_requests end before do @@ -202,7 +217,8 @@ describe 'User edit profile' do select_emoji(emoji, true) set_user_status_in_modal - visit user_path(user) + visit_user + within('.cover-status') do expect(page).to have_emoji(emoji) end @@ -225,7 +241,8 @@ describe 'User edit profile' do find('.js-status-message-field').native.send_keys(message) set_user_status_in_modal - visit user_path(user) + visit_user + within('.cover-status') do expect(page).to have_emoji('speech_balloon') expect(page).to have_content message @@ -240,7 +257,8 @@ describe 'User edit profile' do find('.js-status-message-field').native.send_keys(message) set_user_status_in_modal - visit user_path(user) + visit_user + within('.cover-status') do expect(page).to have_emoji(emoji) expect(page).to have_content message @@ -250,7 +268,9 @@ describe 'User edit profile' do it 'clears the user status with the "X" button' do user_status = create(:user_status, user: user, message: 'Eating bread', emoji: 'stuffed_flatbread') - visit user_path(user) + visit_user + wait_for_requests + within('.cover-status') do expect(page).to have_emoji(user_status.emoji) expect(page).to have_content user_status.message @@ -265,14 +285,18 @@ describe 'User edit profile' do find('.js-clear-user-status-button').click set_user_status_in_modal - visit user_path(user) + visit_user + wait_for_requests + expect(page).not_to have_selector '.cover-status' end it 'clears the user status with the "Remove status" button' do user_status = create(:user_status, user: user, message: 'Eating bread', emoji: 'stuffed_flatbread') - visit user_path(user) + visit_user + wait_for_requests + within('.cover-status') do expect(page).to have_emoji(user_status.emoji) expect(page).to have_content user_status.message @@ -288,7 +312,8 @@ describe 'User edit profile' do click_button 'Remove status' end - visit user_path(user) + visit_user + expect(page).not_to have_selector '.cover-status' end diff --git a/spec/features/task_lists_spec.rb b/spec/features/task_lists_spec.rb index 9c9127980a1..b549f2b5c62 100644 --- a/spec/features/task_lists_spec.rb +++ b/spec/features/task_lists_spec.rb @@ -170,8 +170,11 @@ describe 'Task Lists' do expect(page).to have_content("2 of 7 tasks completed") page.find('li.task-list-item', text: 'Task b').find('input').click + wait_for_requests page.find('li.task-list-item ul li.task-list-item', text: 'Task a.2').find('input').click + wait_for_requests page.find('li.task-list-item ol li.task-list-item', text: 'Task 1.1').find('input').click + wait_for_requests expect(page).to have_content("5 of 7 tasks completed") @@ -184,25 +187,24 @@ describe 'Task Lists' do end describe 'nested tasks', :js do - context 'with Redcarpet' do - let(:issue) { create(:issue, description: nested_tasks_markdown_redcarpet, author: user, project: project) } + let(:cache_version) { CacheMarkdownField::CACHE_COMMONMARK_VERSION } + let!(:issue) do + create(:issue, description: nested_tasks_markdown, author: user, project: project, + cached_markdown_version: cache_version) + end + + before do + visit_issue(project, issue) + end - before do - allow_any_instance_of(Banzai::Filter::MarkdownFilter).to receive(:engine).and_return('Redcarpet') - visit_issue(project, issue) - end + context 'with Redcarpet' do + let(:cache_version) { CacheMarkdownField::CACHE_REDCARPET_VERSION } + let(:nested_tasks_markdown) { nested_tasks_markdown_redcarpet } it_behaves_like 'shared nested tasks' end context 'with CommonMark' do - let(:issue) { create(:issue, description: nested_tasks_markdown, author: user, project: project) } - - before do - allow_any_instance_of(Banzai::Filter::MarkdownFilter).to receive(:engine).and_return('CommonMark') - visit_issue(project, issue) - end - it_behaves_like 'shared nested tasks' end end diff --git a/spec/fixtures/api/schemas/entities/merge_request_basic.json b/spec/fixtures/api/schemas/entities/merge_request_basic.json index 4c04c838cb8..3006b482d41 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_basic.json +++ b/spec/fixtures/api/schemas/entities/merge_request_basic.json @@ -22,7 +22,8 @@ "type": [ "array", "null" ] }, "task_status": { "type": "string" }, - "task_status_short": { "type": "string" } + "task_status_short": { "type": "string" }, + "lock_version": { "type": ["string", "null"] } }, "additionalProperties": false } diff --git a/spec/graphql/resolvers/issues_resolver_spec.rb b/spec/graphql/resolvers/issues_resolver_spec.rb index ca90673521c..1a54ab540fc 100644 --- a/spec/graphql/resolvers/issues_resolver_spec.rb +++ b/spec/graphql/resolvers/issues_resolver_spec.rb @@ -32,6 +32,26 @@ describe Resolvers::IssuesResolver do expect(resolve_issues).to contain_exactly(issue, issue2) end + + it 'finds a specific issue with iids' do + expect(resolve_issues(iids: issue.iid)).to contain_exactly(issue) + end + + it 'finds multiple issues with iids' do + expect(resolve_issues(iids: [issue.iid, issue2.iid])) + .to contain_exactly(issue, issue2) + end + + it 'finds only the issues within the project we are looking at' do + another_project = create(:project) + iids = [issue, issue2].map(&:iid) + + iids.each do |iid| + create(:issue, project: another_project, iid: iid) + end + + expect(resolve_issues(iids: iids)).to contain_exactly(issue, issue2) + end end def resolve_issues(args = {}, context = { current_user: current_user }) diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb index 03e3a72a82f..af319e5ebfe 100644 --- a/spec/helpers/issuables_helper_spec.rb +++ b/spec/helpers/issuables_helper_spec.rb @@ -190,6 +190,7 @@ describe IssuablesHelper do markdownDocsPath: '/help/user/markdown', markdownVersion: CacheMarkdownField::CACHE_COMMONMARK_VERSION, issuableTemplates: [], + lockVersion: issue.lock_version, projectPath: @project.path, projectNamespace: @project.namespace.path, initialTitleHtml: issue.title, diff --git a/spec/javascripts/issue_show/components/app_spec.js b/spec/javascripts/issue_show/components/app_spec.js index 2bd1b3996dc..0ccf771c7ef 100644 --- a/spec/javascripts/issue_show/components/app_spec.js +++ b/spec/javascripts/issue_show/components/app_spec.js @@ -18,9 +18,13 @@ describe('Issuable output', () => { let realtimeRequestCount = 0; let vm; - document.body.innerHTML = '<span id="task_status"></span>'; - beforeEach(done => { + setFixtures(` + <div> + <div class="flash-container"></div> + <span id="task_status"></span> + </div> + `); spyOn(eventHub, '$emit'); const IssuableDescriptionComponent = Vue.extend(issuableApp); @@ -43,6 +47,7 @@ describe('Issuable output', () => { initialTitleText: '', initialDescriptionHtml: 'test', initialDescriptionText: 'test', + lockVersion: 1, markdownPreviewPath: '/', markdownDocsPath: '/', projectNamespace: '/', @@ -78,6 +83,7 @@ describe('Issuable output', () => { expect(formatText(editedText.innerText)).toMatch(/Edited[\s\S]+?by Some User/); expect(editedText.querySelector('.author-link').href).toMatch(/\/some_user$/); expect(editedText.querySelector('time')).toBeTruthy(); + expect(vm.state.lock_version).toEqual(1); }) .then(() => { vm.poll.makeRequest(); @@ -95,6 +101,7 @@ describe('Issuable output', () => { expect(editedText.querySelector('.author-link').href).toMatch(/\/other_user$/); expect(editedText.querySelector('time')).toBeTruthy(); + expect(vm.state.lock_version).toEqual(2); }) .then(done) .catch(done.fail); @@ -137,21 +144,17 @@ describe('Issuable output', () => { describe('updateIssuable', () => { it('fetches new data after update', done => { + spyOn(vm, 'updateStoreState').and.callThrough(); spyOn(vm.service, 'getData').and.callThrough(); - spyOn(vm.service, 'updateIssuable').and.callFake( - () => - new Promise(resolve => { - resolve({ - data: { - confidential: false, - web_url: window.location.pathname, - }, - }); - }), + spyOn(vm.service, 'updateIssuable').and.returnValue( + Promise.resolve({ + data: { web_url: window.location.pathname }, + }), ); vm.updateIssuable() .then(() => { + expect(vm.updateStoreState).toHaveBeenCalled(); expect(vm.service.getData).toHaveBeenCalled(); }) .then(done) @@ -159,11 +162,10 @@ describe('Issuable output', () => { }); it('correctly updates issuable data', done => { - spyOn(vm.service, 'updateIssuable').and.callFake( - () => - new Promise(resolve => { - resolve(); - }), + spyOn(vm.service, 'updateIssuable').and.returnValue( + Promise.resolve({ + data: { web_url: window.location.pathname }, + }), ); vm.updateIssuable() @@ -177,16 +179,13 @@ describe('Issuable output', () => { it('does not redirect if issue has not moved', done => { const visitUrl = spyOnDependency(issuableApp, 'visitUrl'); - spyOn(vm.service, 'updateIssuable').and.callFake( - () => - new Promise(resolve => { - resolve({ - data: { - web_url: window.location.pathname, - confidential: vm.isConfidential, - }, - }); - }), + spyOn(vm.service, 'updateIssuable').and.returnValue( + Promise.resolve({ + data: { + web_url: window.location.pathname, + confidential: vm.isConfidential, + }, + }), ); vm.updateIssuable(); @@ -199,16 +198,13 @@ describe('Issuable output', () => { it('redirects if returned web_url has changed', done => { const visitUrl = spyOnDependency(issuableApp, 'visitUrl'); - spyOn(vm.service, 'updateIssuable').and.callFake( - () => - new Promise(resolve => { - resolve({ - data: { - web_url: '/testing-issue-move', - confidential: vm.isConfidential, - }, - }); - }), + spyOn(vm.service, 'updateIssuable').and.returnValue( + Promise.resolve({ + data: { + web_url: '/testing-issue-move', + confidential: vm.isConfidential, + }, + }), ); vm.updateIssuable(); @@ -227,6 +223,7 @@ describe('Issuable output', () => { vm.handleBeforeUnloadEvent(e); Vue.nextTick(() => { expect(e.returnValue).not.toBeNull(); + done(); }); }); @@ -238,6 +235,7 @@ describe('Issuable output', () => { vm.handleBeforeUnloadEvent(e); Vue.nextTick(() => { expect(e.returnValue).not.toBeNull(); + done(); }); }); @@ -247,49 +245,61 @@ describe('Issuable output', () => { vm.handleBeforeUnloadEvent(e); Vue.nextTick(() => { expect(e.returnValue).toBeNull(); + done(); }); }); }); describe('error when updating', () => { - beforeEach(() => { - spyOn(window, 'Flash').and.callThrough(); - spyOn(vm.service, 'updateIssuable').and.callFake( - () => - new Promise((resolve, reject) => { - reject(); - }), - ); - }); - it('closes form on error', done => { + spyOn(vm.service, 'updateIssuable').and.callFake(() => Promise.reject()); vm.updateIssuable(); setTimeout(() => { - expect(eventHub.$emit).toHaveBeenCalledWith('close.form'); - - expect(window.Flash).toHaveBeenCalledWith('Error updating issue'); + expect(eventHub.$emit).not.toHaveBeenCalledWith('close.form'); + expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe( + `Error updating issue`, + ); done(); }); }); it('returns the correct error message for issuableType', done => { + spyOn(vm.service, 'updateIssuable').and.callFake(() => Promise.reject()); vm.issuableType = 'merge request'; Vue.nextTick(() => { vm.updateIssuable(); setTimeout(() => { - expect(eventHub.$emit).toHaveBeenCalledWith('close.form'); - - expect(window.Flash).toHaveBeenCalledWith('Error updating merge request'); + expect(eventHub.$emit).not.toHaveBeenCalledWith('close.form'); + expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe( + `Error updating merge request`, + ); done(); }); }); }); + + it('shows error mesage from backend if exists', done => { + const msg = 'Custom error message from backend'; + spyOn(vm.service, 'updateIssuable').and.callFake( + // eslint-disable-next-line prefer-promise-reject-errors + () => Promise.reject({ response: { data: { errors: [msg] } } }), + ); + + vm.updateIssuable(); + setTimeout(() => { + expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe( + `${vm.defaultErrorMessage}. ${msg}`, + ); + + done(); + }); + }); }); }); @@ -342,21 +352,19 @@ describe('Issuable output', () => { describe('deleteIssuable', () => { it('changes URL when deleted', done => { const visitUrl = spyOnDependency(issuableApp, 'visitUrl'); - spyOn(vm.service, 'deleteIssuable').and.callFake( - () => - new Promise(resolve => { - resolve({ - data: { - web_url: '/test', - }, - }); - }), + spyOn(vm.service, 'deleteIssuable').and.returnValue( + Promise.resolve({ + data: { + web_url: '/test', + }, + }), ); vm.deleteIssuable(); setTimeout(() => { expect(visitUrl).toHaveBeenCalledWith('/test'); + done(); }); }); @@ -364,40 +372,33 @@ describe('Issuable output', () => { it('stops polling when deleting', done => { spyOnDependency(issuableApp, 'visitUrl'); spyOn(vm.poll, 'stop').and.callThrough(); - spyOn(vm.service, 'deleteIssuable').and.callFake( - () => - new Promise(resolve => { - resolve({ - data: { - web_url: '/test', - }, - }); - }), + spyOn(vm.service, 'deleteIssuable').and.returnValue( + Promise.resolve({ + data: { + web_url: '/test', + }, + }), ); vm.deleteIssuable(); setTimeout(() => { expect(vm.poll.stop).toHaveBeenCalledWith(); + done(); }); }); it('closes form on error', done => { - spyOn(window, 'Flash').and.callThrough(); - spyOn(vm.service, 'deleteIssuable').and.callFake( - () => - new Promise((resolve, reject) => { - reject(); - }), - ); + spyOn(vm.service, 'deleteIssuable').and.returnValue(Promise.reject()); vm.deleteIssuable(); setTimeout(() => { - expect(eventHub.$emit).toHaveBeenCalledWith('close.form'); - - expect(window.Flash).toHaveBeenCalledWith('Error deleting issue'); + expect(eventHub.$emit).not.toHaveBeenCalledWith('close.form'); + expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe( + 'Error deleting issue', + ); done(); }); @@ -420,6 +421,7 @@ describe('Issuable output', () => { .then(vm.$nextTick) .then(() => { expect(vm.formState.lockedWarningVisible).toEqual(true); + expect(vm.formState.lock_version).toEqual(1); expect(vm.$el.querySelector('.alert')).not.toBeNull(); }) .then(done) @@ -438,4 +440,34 @@ describe('Issuable output', () => { expect(vm.$el.querySelector('.title-container .note-action-button')).toBeDefined(); }); }); + + describe('updateStoreState', () => { + it('should make a request and update the state of the store', done => { + const data = { foo: 1 }; + spyOn(vm.store, 'updateState'); + spyOn(vm.service, 'getData').and.returnValue(Promise.resolve({ data })); + + vm.updateStoreState() + .then(() => { + expect(vm.service.getData).toHaveBeenCalled(); + expect(vm.store.updateState).toHaveBeenCalledWith(data); + }) + .then(done) + .catch(done.fail); + }); + + it('should show error message if store update fails', done => { + spyOn(vm.service, 'getData').and.returnValue(Promise.reject()); + vm.issuableType = 'merge request'; + + vm.updateStoreState() + .then(() => { + expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe( + `Error updating ${vm.issuableType}`, + ); + }) + .then(done) + .catch(done.fail); + }); + }); }); diff --git a/spec/javascripts/issue_show/components/description_spec.js b/spec/javascripts/issue_show/components/description_spec.js index 463f3c89926..72716b97f5f 100644 --- a/spec/javascripts/issue_show/components/description_spec.js +++ b/spec/javascripts/issue_show/components/description_spec.js @@ -123,7 +123,10 @@ describe('Description component', () => { fieldName: 'description', selector: '.detail-page-description', onSuccess: jasmine.any(Function), + onError: jasmine.any(Function), + lockVersion: 0, }); + done(); }); }); @@ -184,4 +187,18 @@ describe('Description component', () => { it('sets data-update-url', () => { expect(vm.$el.querySelector('textarea').dataset.updateUrl).toEqual(gl.TEST_HOST); }); + + describe('taskListUpdateError', () => { + it('should create flash notification and emit an event to parent', () => { + const msg = + 'Someone edited this issue at the same time you did. The description has been updated and you will need to make your changes again.'; + spyOn(window, 'Flash'); + spyOn(vm, '$emit'); + + vm.taskListUpdateError(); + + expect(window.Flash).toHaveBeenCalledWith(msg); + expect(vm.$emit).toHaveBeenCalledWith('taskListUpdateFailed'); + }); + }); }); diff --git a/spec/javascripts/issue_show/mock_data.js b/spec/javascripts/issue_show/mock_data.js index 74b3efb014b..f4475aadb8b 100644 --- a/spec/javascripts/issue_show/mock_data.js +++ b/spec/javascripts/issue_show/mock_data.js @@ -8,6 +8,7 @@ export default { updated_at: '2015-05-15T12:31:04.428Z', updated_by_name: 'Some User', updated_by_path: '/some_user', + lock_version: 1, }, secondRequest: { title: '<p>2</p>', @@ -18,5 +19,6 @@ export default { updated_at: '2016-05-15T12:31:04.428Z', updated_by_name: 'Other User', updated_by_path: '/other_user', + lock_version: 2, }, }; diff --git a/spec/javascripts/merge_request_spec.js b/spec/javascripts/merge_request_spec.js index 1cb49b49ca7..32623d1781a 100644 --- a/spec/javascripts/merge_request_spec.js +++ b/spec/javascripts/merge_request_spec.js @@ -41,15 +41,28 @@ describe('MergeRequest', function() { }); it('submits an ajax request on tasklist:changed', done => { - $('.js-task-list-field').trigger('tasklist:changed'); + const lineNumber = 8; + const lineSource = '- [ ] item 8'; + const index = 3; + const checked = true; + + $('.js-task-list-field').trigger({ + type: 'tasklist:changed', + detail: { lineNumber, lineSource, index, checked }, + }); setTimeout(() => { expect(axios.patch).toHaveBeenCalledWith( `${gl.TEST_HOST}/frontend-fixtures/merge-requests-project/merge_requests/1.json`, { - merge_request: { description: '- [ ] Task List Item' }, + merge_request: { + description: '- [ ] Task List Item', + lock_version: undefined, + update_task: { line_number: lineNumber, line_source: lineSource, index, checked }, + }, }, ); + done(); }); }); diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index 694f581150f..7c869d4c326 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -89,10 +89,25 @@ describe('Notes', function() { }); it('submits an ajax request on tasklist:changed', function(done) { - $('.js-task-list-container').trigger('tasklist:changed'); + const lineNumber = 8; + const lineSource = '- [ ] item 8'; + const index = 3; + const checked = true; + + $('.js-task-list-container').trigger({ + type: 'tasklist:changed', + detail: { lineNumber, lineSource, index, checked }, + }); setTimeout(() => { - expect(axios.patch).toHaveBeenCalled(); + expect(axios.patch).toHaveBeenCalledWith(undefined, { + note: { + note: '', + lock_version: undefined, + update_task: { index, checked, line_number: lineNumber, line_source: lineSource }, + }, + }); + done(); }); }); diff --git a/spec/javascripts/task_list_spec.js b/spec/javascripts/task_list_spec.js new file mode 100644 index 00000000000..563f402de58 --- /dev/null +++ b/spec/javascripts/task_list_spec.js @@ -0,0 +1,156 @@ +import $ from 'jquery'; +import TaskList from '~/task_list'; +import axios from '~/lib/utils/axios_utils'; + +describe('TaskList', () => { + let taskList; + let currentTarget; + const taskListOptions = { + selector: '.task-list', + dataType: 'issue', + fieldName: 'description', + lockVersion: 2, + }; + const createTaskList = () => new TaskList(taskListOptions); + + beforeEach(() => { + setFixtures(` + <div class="task-list"> + <div class="js-task-list-container"></div> + </div> + `); + + currentTarget = $('<div></div>'); + taskList = createTaskList(); + }); + + it('should call init when the class constructed', () => { + spyOn(TaskList.prototype, 'init').and.callThrough(); + spyOn(TaskList.prototype, 'disable'); + spyOn($.prototype, 'taskList'); + spyOn($.prototype, 'on'); + + taskList = createTaskList(); + const $taskListEl = $(taskList.taskListContainerSelector); + + expect(taskList.init).toHaveBeenCalled(); + expect(taskList.disable).toHaveBeenCalled(); + expect($taskListEl.taskList).toHaveBeenCalledWith('enable'); + expect($(document).on).toHaveBeenCalledWith( + 'tasklist:changed', + taskList.taskListContainerSelector, + taskList.updateHandler, + ); + }); + + describe('getTaskListTarget', () => { + it('should return currentTarget from event object if exists', () => { + const $target = taskList.getTaskListTarget({ currentTarget }); + + expect($target).toEqual(currentTarget); + }); + + it('should return element of the taskListContainerSelector', () => { + const $target = taskList.getTaskListTarget(); + + expect($target).toEqual($(taskList.taskListContainerSelector)); + }); + }); + + describe('disableTaskListItems', () => { + it('should call taskList method with disable param', () => { + spyOn($.prototype, 'taskList'); + + taskList.disableTaskListItems({ currentTarget }); + + expect(currentTarget.taskList).toHaveBeenCalledWith('disable'); + }); + }); + + describe('enableTaskListItems', () => { + it('should call taskList method with enable param', () => { + spyOn($.prototype, 'taskList'); + + taskList.enableTaskListItems({ currentTarget }); + + expect(currentTarget.taskList).toHaveBeenCalledWith('enable'); + }); + }); + + describe('disable', () => { + it('should disable task list items and off document event', () => { + spyOn(taskList, 'disableTaskListItems'); + spyOn($.prototype, 'off'); + + taskList.disable(); + + expect(taskList.disableTaskListItems).toHaveBeenCalled(); + expect($(document).off).toHaveBeenCalledWith( + 'tasklist:changed', + taskList.taskListContainerSelector, + ); + }); + }); + + describe('update', () => { + it('should disable task list items and make a patch request then enable them again', done => { + const response = { data: { lock_version: 3 } }; + spyOn(taskList, 'enableTaskListItems'); + spyOn(taskList, 'disableTaskListItems'); + spyOn(taskList, 'onSuccess'); + spyOn(axios, 'patch').and.returnValue(Promise.resolve(response)); + + const value = 'hello world'; + const endpoint = '/foo'; + const target = $(`<input data-update-url="${endpoint}" value="${value}" />`); + const detail = { + index: 2, + checked: true, + lineNumber: 8, + lineSource: '- [ ] check item', + }; + const event = { target, detail }; + const patchData = { + [taskListOptions.dataType]: { + [taskListOptions.fieldName]: value, + lock_version: taskListOptions.lockVersion, + update_task: { + index: detail.index, + checked: detail.checked, + line_number: detail.lineNumber, + line_source: detail.lineSource, + }, + }, + }; + + taskList + .update(event) + .then(() => { + expect(taskList.disableTaskListItems).toHaveBeenCalledWith(event); + expect(axios.patch).toHaveBeenCalledWith(endpoint, patchData); + expect(taskList.enableTaskListItems).toHaveBeenCalledWith(event); + expect(taskList.onSuccess).toHaveBeenCalledWith(response.data); + expect(taskList.lockVersion).toEqual(response.data.lock_version); + }) + .then(done) + .catch(done.fail); + }); + }); + + it('should handle request error and enable task list items', done => { + const response = { data: { error: 1 } }; + spyOn(taskList, 'enableTaskListItems'); + spyOn(taskList, 'onError'); + spyOn(axios, 'patch').and.returnValue(Promise.reject({ response })); // eslint-disable-line prefer-promise-reject-errors + + const event = { detail: {} }; + taskList + .update(event) + .then(() => { + expect(taskList.enableTaskListItems).toHaveBeenCalledWith(event); + expect(taskList.onError).toHaveBeenCalledWith(response.data); + }) + .then(done) + .catch(done.fail); + }); +}); diff --git a/spec/lib/gitlab/background_migration_spec.rb b/spec/lib/gitlab/background_migration_spec.rb index 8a83b76fd94..7d3d8a949ef 100644 --- a/spec/lib/gitlab/background_migration_spec.rb +++ b/spec/lib/gitlab/background_migration_spec.rb @@ -104,6 +104,38 @@ describe Gitlab::BackgroundMigration do end end end + + context 'when retry_dead_jobs is true', :sidekiq, :redis do + let(:retry_queue) do + [double(args: ['Object', [3]], queue: described_class.queue, delete: true)] + end + let(:dead_queue) do + [double(args: ['Object', [4]], queue: described_class.queue, delete: true)] + end + + before do + allow(Sidekiq::RetrySet).to receive(:new).and_return(retry_queue) + allow(Sidekiq::DeadSet).to receive(:new).and_return(dead_queue) + end + + it 'steals from the dead and retry queue' do + Sidekiq::Testing.disable! do + expect(described_class).to receive(:perform) + .with('Object', [1]).ordered + expect(described_class).to receive(:perform) + .with('Object', [2]).ordered + expect(described_class).to receive(:perform) + .with('Object', [3]).ordered + expect(described_class).to receive(:perform) + .with('Object', [4]).ordered + + BackgroundMigrationWorker.perform_async('Object', [2]) + BackgroundMigrationWorker.perform_in(10.minutes, 'Object', [1]) + + described_class.steal('Object', retry_dead_jobs: true) + end + end + end end describe '.perform' do diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index fe2087e8fc3..baca8f6d542 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -603,5 +603,6 @@ ResourceLabelEvent: ErrorTracking::ProjectErrorTrackingSetting: - id - api_url -- enabled - project_id +- project_name +- organization_name diff --git a/spec/migrations/fix_null_type_labels_spec.rb b/spec/migrations/fix_null_type_labels_spec.rb new file mode 100644 index 00000000000..462ae9b913f --- /dev/null +++ b/spec/migrations/fix_null_type_labels_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20190131122559_fix_null_type_labels') + +describe FixNullTypeLabels, :migration do + let(:migration) { described_class.new } + let(:projects) { table(:projects) } + let(:namespaces) { table(:namespaces) } + let(:labels) { table(:labels) } + + before do + group = namespaces.create(name: 'labels-test-project', path: 'labels-test-project', type: 'Group') + project = projects.create!(namespace_id: group.id, name: 'labels-test-group', path: 'labels-test-group') + + @template_label = labels.create(title: 'template', template: true) + @project_label = labels.create(title: 'project label', project_id: project.id, type: 'ProjectLabel') + @group_label = labels.create(title: 'group_label', group_id: group.id, type: 'GroupLabel') + @broken_label_1 = labels.create(title: 'broken 1', project_id: project.id) + @broken_label_2 = labels.create(title: 'broken 2', project_id: project.id) + end + + describe '#up' do + it 'fix labels with type missing' do + migration.up + + # Labels that requires type change + expect(@broken_label_1.reload.type).to eq('ProjectLabel') + expect(@broken_label_2.reload.type).to eq('ProjectLabel') + # Labels out of scope + expect(@template_label.reload.type).to be_nil + expect(@project_label.reload.type).to eq('ProjectLabel') + expect(@group_label.reload.type).to eq('GroupLabel') + end + end +end diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb index ef6af232999..925e2ab0955 100644 --- a/spec/models/concerns/cache_markdown_field_spec.rb +++ b/spec/models/concerns/cache_markdown_field_spec.rb @@ -133,6 +133,15 @@ describe CacheMarkdownField do end end + context 'when a markdown field and html field are both changed' do + it do + expect(thing).not_to receive(:refresh_markdown_cache) + thing.foo = '_look over there!_' + thing.foo_html = '<em>look over there!</em>' + thing.save + end + end + context 'a non-markdown field changed' do shared_examples 'with cache version' do |cache_version| let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) } @@ -242,6 +251,30 @@ describe CacheMarkdownField do end end + describe '#legacy_markdown?' do + subject { thing.legacy_markdown? } + + it 'returns true for redcarpet versions' do + thing.cached_markdown_version = CacheMarkdownField::CACHE_COMMONMARK_VERSION_START - 1 + is_expected.to be_truthy + end + + it 'returns false for commonmark versions' do + thing.cached_markdown_version = CacheMarkdownField::CACHE_COMMONMARK_VERSION_START + is_expected.to be_falsey + end + + it 'returns false if nil' do + thing.cached_markdown_version = nil + is_expected.to be_falsey + end + + it 'returns false if 0' do + thing.cached_markdown_version = 0 + is_expected.to be_falsey + end + end + describe '#refresh_markdown_cache' do before do thing.foo = updated_markdown diff --git a/spec/models/error_tracking/project_error_tracking_setting_spec.rb b/spec/models/error_tracking/project_error_tracking_setting_spec.rb index 2f8ab21d4b2..d30228b863c 100644 --- a/spec/models/error_tracking/project_error_tracking_setting_spec.rb +++ b/spec/models/error_tracking/project_error_tracking_setting_spec.rb @@ -15,9 +15,11 @@ describe ErrorTracking::ProjectErrorTrackingSetting do describe 'Validations' do context 'when api_url is over 255 chars' do - it 'fails validation' do + before do subject.api_url = 'https://' + 'a' * 250 + end + it 'fails validation' do expect(subject).not_to be_valid expect(subject.errors.messages[:api_url]).to include('is too long (maximum is 255 characters)') end @@ -31,6 +33,34 @@ describe ErrorTracking::ProjectErrorTrackingSetting do end end + context 'presence validations' do + using RSpec::Parameterized::TableSyntax + + valid_api_url = 'http://example.com/api/0/projects/org-slug/proj-slug/' + valid_token = 'token' + + where(:enabled, :token, :api_url, :valid?) do + true | nil | nil | false + true | nil | valid_api_url | false + true | valid_token | nil | false + true | valid_token | valid_api_url | true + false | nil | nil | true + false | nil | valid_api_url | true + false | valid_token | nil | true + false | valid_token | valid_api_url | true + end + + with_them do + before do + subject.enabled = enabled + subject.token = token + subject.api_url = api_url + end + + it { expect(subject.valid?).to eq(valid?) } + end + end + context 'URL path' do it 'fails validation with wrong path' do subject.api_url = 'http://gitlab.com/project1/something' @@ -45,6 +75,16 @@ describe ErrorTracking::ProjectErrorTrackingSetting do expect(subject).to be_valid end end + + context 'non ascii chars in api_url' do + before do + subject.api_url = 'http://gitlab.com/api/0/projects/project1/something€' + end + + it 'fails validation' do + expect(subject).not_to be_valid + end + end end describe '#sentry_external_url' do @@ -106,4 +146,138 @@ describe ErrorTracking::ProjectErrorTrackingSetting do end end end + + describe '#list_sentry_projects' do + let(:projects) { [:list, :of, :projects] } + let(:sentry_client) { spy(:sentry_client) } + + it 'calls sentry client' do + expect(subject).to receive(:sentry_client).and_return(sentry_client) + expect(sentry_client).to receive(:list_projects).and_return(projects) + + result = subject.list_sentry_projects + + expect(result).to eq(projects: projects) + end + end + + context 'slugs' do + shared_examples_for 'slug from api_url' do |method, slug| + context 'when api_url is correct' do + before do + subject.api_url = 'http://gitlab.com/api/0/projects/org-slug/project-slug/' + end + + it 'returns slug' do + expect(subject.public_send(method)).to eq(slug) + end + end + + context 'when api_url is blank' do + before do + subject.api_url = nil + end + + it 'returns nil' do + expect(subject.public_send(method)).to be_nil + end + end + end + + it_behaves_like 'slug from api_url', :project_slug, 'project-slug' + it_behaves_like 'slug from api_url', :organization_slug, 'org-slug' + end + + context 'names from api_url' do + shared_examples_for 'name from api_url' do |name, titleized_slug| + context 'name is present in DB' do + it 'returns name from DB' do + subject[name] = 'Sentry name' + subject.api_url = 'http://gitlab.com/api/0/projects/org-slug/project-slug' + + expect(subject.public_send(name)).to eq('Sentry name') + end + end + + context 'name is null in DB' do + it 'titleizes and returns slug from api_url' do + subject[name] = nil + subject.api_url = 'http://gitlab.com/api/0/projects/org-slug/project-slug' + + expect(subject.public_send(name)).to eq(titleized_slug) + end + + it 'returns nil when api_url is incorrect' do + subject[name] = nil + subject.api_url = 'http://gitlab.com/api/0/projects/' + + expect(subject.public_send(name)).to be_nil + end + + it 'returns nil when api_url is blank' do + subject[name] = nil + subject.api_url = nil + + expect(subject.public_send(name)).to be_nil + end + end + end + + it_behaves_like 'name from api_url', :organization_name, 'Org Slug' + it_behaves_like 'name from api_url', :project_name, 'Project Slug' + end + + describe '.build_api_url_from' do + it 'correctly builds api_url with slugs' do + api_url = described_class.build_api_url_from( + api_host: 'http://sentry.com/', + organization_slug: 'org-slug', + project_slug: 'proj-slug' + ) + + expect(api_url).to eq('http://sentry.com/api/0/projects/org-slug/proj-slug/') + end + + it 'correctly builds api_url without slugs' do + api_url = described_class.build_api_url_from( + api_host: 'http://sentry.com/', + organization_slug: nil, + project_slug: nil + ) + + expect(api_url).to eq('http://sentry.com/api/0/projects/') + end + + it 'does not raise exception with invalid url' do + api_url = described_class.build_api_url_from( + api_host: ':::', + organization_slug: 'org-slug', + project_slug: 'proj-slug' + ) + + expect(api_url).to eq(':::') + end + end + + describe '#api_host' do + context 'when api_url exists' do + before do + subject.api_url = 'https://example.com/api/0/projects/org-slug/proj-slug/' + end + + it 'extracts the api_host from api_url' do + expect(subject.api_host).to eq('https://example.com/') + end + end + + context 'when api_url is nil' do + before do + subject.api_url = nil + end + + it 'returns nil' do + expect(subject.api_url).to eq(nil) + end + end + end end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index ce20bf2bef6..ef76e2311b1 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -543,6 +543,76 @@ describe Issues::UpdateService, :mailer do end end + context 'when updating a single task' do + before do + update_issue(description: "- [ ] Task 1\n- [ ] Task 2") + end + + it { expect(issue.tasks?).to eq(true) } + + context 'when a task is marked as completed' do + before do + update_issue(update_task: { index: 1, checked: true, line_source: '- [ ] Task 1', line_number: 1 }) + end + + it 'creates system note about task status change' do + note1 = find_note('marked the task **Task 1** as completed') + + expect(note1).not_to be_nil + + description_notes = find_notes('description') + expect(description_notes.length).to eq(1) + end + end + + context 'when a task is marked as incomplete' do + before do + update_issue(description: "- [x] Task 1\n- [X] Task 2") + update_issue(update_task: { index: 2, checked: false, line_source: '- [X] Task 2', line_number: 2 }) + end + + it 'creates system note about task status change' do + note1 = find_note('marked the task **Task 2** as incomplete') + + expect(note1).not_to be_nil + + description_notes = find_notes('description') + expect(description_notes.length).to eq(1) + end + end + + context 'when the task position has been modified' do + before do + update_issue(description: "- [ ] Task 1\n- [ ] Task 3\n- [ ] Task 2") + end + + it 'raises an exception' do + expect(Note.count).to eq(2) + expect do + update_issue(update_task: { index: 2, checked: true, line_source: '- [ ] Task 2', line_number: 2 }) + end.to raise_error(ActiveRecord::StaleObjectError) + expect(Note.count).to eq(2) + end + end + + context 'when the content changes but not task line number' do + before do + update_issue(description: "Paragraph\n\n- [ ] Task 1\n- [x] Task 2") + update_issue(description: "Paragraph with more words\n\n- [ ] Task 1\n- [x] Task 2") + update_issue(update_task: { index: 2, checked: false, line_source: '- [x] Task 2', line_number: 4 }) + end + + it 'creates system note about task status change' do + note1 = find_note('marked the task **Task 2** as incomplete') + + expect(note1).not_to be_nil + + description_notes = find_notes('description') + expect(description_notes.length).to eq(2) + end + end + end + context 'updating labels' do let(:label3) { create(:label, project: project) } let(:result) { described_class.new(project, user, params).execute(issue).reload } diff --git a/spec/services/members/create_service_spec.rb b/spec/services/members/create_service_spec.rb index 5c01463d757..3bc05182932 100644 --- a/spec/services/members/create_service_spec.rb +++ b/spec/services/members/create_service_spec.rb @@ -36,4 +36,13 @@ describe Members::CreateService do expect(result[:message]).to be_present expect(project.users).not_to include project_user end + + it 'does not add an invalid member' do + params = { user_ids: project_user.id.to_s, access_level: -1 } + result = described_class.new(user, params).execute(project) + + expect(result[:status]).to eq(:error) + expect(result[:message]).to include(project_user.username) + expect(project.users).not_to include project_user + end end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index f71e2b4bc24..54ce33dd103 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -16,7 +16,11 @@ describe Projects::CreateService, '#execute' do Label.create(title: "bug", template: true) project = create_project(user, opts) - expect(project.labels).not_to be_empty + created_label = project.reload.labels.last + + expect(created_label.type).to eq('ProjectLabel') + expect(created_label.project_id).to eq(project.id) + expect(created_label.title).to eq('bug') end context 'user namespace' do diff --git a/spec/services/task_list_toggle_service_spec.rb b/spec/services/task_list_toggle_service_spec.rb new file mode 100644 index 00000000000..750ac4c40ba --- /dev/null +++ b/spec/services/task_list_toggle_service_spec.rb @@ -0,0 +1,126 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe TaskListToggleService do + let(:sourcepos) { true } + let(:markdown) do + <<-EOT.strip_heredoc + * [ ] Task 1 + * [x] Task 2 + + A paragraph + + 1. [X] Item 1 + - [ ] Sub-item 1 + EOT + end + + let(:markdown_html) do + <<-EOT.strip_heredoc + <ul data-sourcepos="1:1-3:0" class="task-list" dir="auto"> + <li data-sourcepos="1:1-1:12" class="task-list-item"> + <input type="checkbox" class="task-list-item-checkbox" disabled> Task 1 + </li> + <li data-sourcepos="2:1-3:0" class="task-list-item"> + <input type="checkbox" class="task-list-item-checkbox" disabled checked> Task 2 + </li> + </ul> + <p data-sourcepos="4:1-4:11" dir="auto">A paragraph</p> + <ol data-sourcepos="6:1-7:19" class="task-list" dir="auto"> + <li data-sourcepos="6:1-7:19" class="task-list-item"> + <input type="checkbox" class="task-list-item-checkbox" disabled checked> Item 1 + <ul data-sourcepos="7:4-7:19" class="task-list"> + <li data-sourcepos="7:4-7:19" class="task-list-item"> + <input type="checkbox" class="task-list-item-checkbox" disabled> Sub-item 1 + </li> + </ul> + </li> + </ol> + EOT + end + + shared_examples 'task lists' do + it 'checks Task 1' do + toggler = described_class.new(markdown, markdown_html, + index: 1, toggle_as_checked: true, + line_source: '* [ ] Task 1', line_number: 1, + sourcepos: sourcepos) + + expect(toggler.execute).to be_truthy + expect(toggler.updated_markdown.lines[0]).to eq "* [x] Task 1\n" + expect(toggler.updated_markdown_html).to include('disabled checked> Task 1') + end + + it 'unchecks Item 1' do + toggler = described_class.new(markdown, markdown_html, + index: 3, toggle_as_checked: false, + line_source: '1. [X] Item 1', line_number: 6, + sourcepos: sourcepos) + + expect(toggler.execute).to be_truthy + expect(toggler.updated_markdown.lines[5]).to eq "1. [ ] Item 1\n" + expect(toggler.updated_markdown_html).to include('disabled> Item 1') + end + + it 'returns false if line_source does not match the text' do + toggler = described_class.new(markdown, markdown_html, + index: 2, toggle_as_checked: false, + line_source: '* [x] Task Added', line_number: 2, + sourcepos: sourcepos) + + expect(toggler.execute).to be_falsey + end + + it 'returns false if markdown is nil' do + toggler = described_class.new(nil, markdown_html, + index: 2, toggle_as_checked: false, + line_source: '* [x] Task Added', line_number: 2, + sourcepos: sourcepos) + + expect(toggler.execute).to be_falsey + end + + it 'returns false if markdown_html is nil' do + toggler = described_class.new(markdown, nil, + index: 2, toggle_as_checked: false, + line_source: '* [x] Task Added', line_number: 2, + sourcepos: sourcepos) + + expect(toggler.execute).to be_falsey + end + end + + context 'when using sourcepos' do + it_behaves_like 'task lists' + end + + context 'when using checkbox indexing' do + let(:sourcepos) { false } + let(:markdown_html) do + <<-EOT.strip_heredoc + <ul class="task-list" dir="auto"> + <li class="task-list-item"> + <input type="checkbox" class="task-list-item-checkbox" disabled> Task 1 + </li> + <li class="task-list-item"> + <input type="checkbox" class="task-list-item-checkbox" disabled checked> Task 2 + </li> + </ul> + <p dir="auto">A paragraph</p> + <ol class="task-list" dir="auto"> + <li class="task-list-item"> + <input type="checkbox" class="task-list-item-checkbox" disabled checked> Item 1 + <ul class="task-list"> + <li class="task-list-item"> + <input type="checkbox" class="task-list-item-checkbox" disabled> Sub-item 1 + </li> + </ul> + </li> + </ol> + EOT + end + + it_behaves_like 'task lists' + end +end diff --git a/yarn.lock b/yarn.lock index 5c9139fdbfa..a33d3a13e47 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3032,10 +3032,10 @@ decamelize@^2.0.0: dependencies: xregexp "4.0.0" -deckar01-task_list@^2.0.1: - version "2.0.1" - resolved "https://registry.yarnpkg.com/deckar01-task_list/-/deckar01-task_list-2.0.1.tgz#fdcfb6ab5717055a82f29e863a49990a043a06a9" - integrity sha512-i5fT8QxJ9iV6dfgy5U0NHW91O5cKsvDc4u8JNMnZ6efQc356bA9vKuXO3732agSry+bO6TolzTmuqSRi4tkkeA== +deckar01-task_list@^2.2.0: + version "2.2.0" + resolved "https://registry.yarnpkg.com/deckar01-task_list/-/deckar01-task_list-2.2.0.tgz#5cc3ea06f01d3d786b1a667064a462eb5d069bd3" + integrity sha512-NUfu5ARoD9SC2k+fBT5cBer59iKfEdawPrmfqp5+zAahTECb8z9dsuS1Xnx7jzFAmCCLnEs3z/aYucYXzNrKkQ== decode-uri-component@^0.2.0: version "0.2.0" @@ -3416,6 +3416,11 @@ elliptic@^6.0.0: minimalistic-assert "^1.0.0" minimalistic-crypto-utils "^1.0.0" +emoji-regex@^7.0.3: + version "7.0.3" + resolved "https://registry.yarnpkg.com/emoji-regex/-/emoji-regex-7.0.3.tgz#933a04052860c85e83c122479c4748a8e4c72156" + integrity sha512-CwBLREIQ7LvYFB0WyRvwhq5N5qPhc6PMjD6bYggFlI5YyDgl+0vxq5VHbMOFqLg7hfWzmu8T5Z1QofhmTIhItA== + emoji-unicode-version@^0.2.1: version "0.2.1" resolved "https://registry.yarnpkg.com/emoji-unicode-version/-/emoji-unicode-version-0.2.1.tgz#0ebf3666b5414097971d34994e299fce75cdbafc" |