diff options
126 files changed, 3289 insertions, 1358 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index d22a6de035d..422da05ba5f 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -318,6 +318,7 @@ review-docs-cleanup: cloud-native-image: image: ruby:2.4-alpine before_script: [] + dependencies: [] stage: test allow_failure: true variables: @@ -632,6 +633,7 @@ rails5_gemfile_lock_check: ee_compat_check: <<: *rake-exec + dependencies: [] except: - master - tags @@ -860,9 +862,7 @@ coverage: lint:javascript:report: <<: *dedicated-no-docs-and-no-qa-pull-cache-job stage: post-test - dependencies: - - compile-assets - - setup-test-env + dependencies: [] before_script: [] script: - date @@ -916,6 +916,7 @@ gitlab_git_test: variables: SETUP_DB: "false" before_script: [] + dependencies: [] cache: {} script: - spec/support/prepare-gitlab-git-test-for-commit --check-for-changes @@ -926,6 +927,7 @@ no_ee_check: variables: SETUP_DB: "false" before_script: [] + dependencies: [] cache: {} script: - scripts/no-ee-check diff --git a/.gitlab/CODEOWNERS b/.gitlab/CODEOWNERS.disabled index a4b773b15a9..a4b773b15a9 100644 --- a/.gitlab/CODEOWNERS +++ b/.gitlab/CODEOWNERS.disabled @@ -124,7 +124,7 @@ gem 'seed-fu', '~> 2.3.7' # Markdown and HTML processing gem 'html-pipeline', '~> 2.8' gem 'deckar01-task_list', '2.0.0' -gem 'gitlab-markup', '~> 1.6.4' +gem 'gitlab-markup', '~> 1.6.5' gem 'github-markup', '~> 1.7.0', require: 'github/markup' gem 'redcarpet', '~> 3.4' gem 'commonmarker', '~> 0.17' @@ -204,6 +204,9 @@ gem 'redis-rails', '~> 5.0.2' gem 'redis', '~> 3.2' gem 'connection_pool', '~> 2.0' +# Discord integration +gem 'discordrb-webhooks-blackst0ne', '~> 3.3', require: false + # HipChat integration gem 'hipchat', '~> 1.5.0' @@ -339,7 +342,7 @@ group :development, :test do gem 'minitest', '~> 5.7.0' # Generate Fake data - gem 'ffaker', '~> 2.4' + gem 'ffaker', '~> 2.10' gem 'capybara', '~> 2.15' gem 'capybara-screenshot', '~> 1.0.0' @@ -354,7 +357,7 @@ group :development, :test do gem 'rubocop-rspec', '~> 1.22.1' gem 'scss_lint', '~> 0.56.0', require: false - gem 'haml_lint', '~> 0.26.0', require: false + gem 'haml_lint', '~> 0.28.0', require: false gem 'simplecov', '~> 0.14.0', require: false gem 'bundler-audit', '~> 0.5.0', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 50e3ddef1e1..23261373c2b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -162,6 +162,8 @@ GEM rotp (~> 2.0) diff-lcs (1.3) diffy (3.1.0) + discordrb-webhooks-blackst0ne (3.3.0) + rest-client (~> 2.0) docile (1.1.5) domain_name (0.5.20180417) unf (>= 0.0.5, < 1.0.0) @@ -200,7 +202,7 @@ GEM multi_json fast_blank (1.0.0) fast_gettext (1.6.0) - ffaker (2.4.0) + ffaker (2.10.0) ffi (1.9.25) flipper (0.13.0) flipper-active_record (0.13.0) @@ -272,7 +274,7 @@ GEM gitaly-proto (0.123.0) grpc (~> 1.0) github-markup (1.7.0) - gitlab-markup (1.6.4) + gitlab-markup (1.6.5) gitlab-sidekiq-fetcher (0.3.0) sidekiq (~> 5) gitlab-styles (2.4.1) @@ -335,11 +337,11 @@ GEM haml (5.0.4) temple (>= 0.8.0) tilt - haml_lint (0.26.0) + haml_lint (0.28.0) haml (>= 4.0, < 5.1) rainbow rake (>= 10, < 13) - rubocop (>= 0.49.0) + rubocop (>= 0.50.0) sysexits (~> 1.1) hamlit (2.8.8) temple (>= 0.8.0) @@ -449,9 +451,9 @@ GEM memoizable (0.4.2) thread_safe (~> 0.3, >= 0.3.1) method_source (0.9.0) - mime-types (3.1) + mime-types (3.2.2) mime-types-data (~> 3.2015) - mime-types-data (3.2016.0521) + mime-types-data (3.2018.0812) mimemagic (0.3.0) mini_magick (4.8.0) mini_mime (1.0.1) @@ -596,7 +598,7 @@ GEM get_process_mem (~> 0.2) puma (>= 2.7, < 4) pyu-ruby-sasl (0.0.3.3) - rack (1.6.10) + rack (1.6.11) rack-accept (0.4.5) rack (>= 0.4) rack-attack (4.4.1) @@ -608,7 +610,7 @@ GEM httpclient (>= 2.4) multi_json (>= 1.3.6) rack (>= 1.1) - rack-protection (2.0.3) + rack-protection (2.0.4) rack rack-proxy (0.6.0) rack @@ -676,7 +678,7 @@ GEM redis-actionpack (>= 5.0, < 6) redis-activesupport (>= 5.0, < 6) redis-store (>= 1.2, < 2) - redis-store (1.4.1) + redis-store (1.6.0) redis (>= 2.2, < 5) regexp_parser (0.5.0) representable (3.0.4) @@ -802,7 +804,7 @@ GEM rack shoulda-matchers (3.1.2) activesupport (>= 4.0.0) - sidekiq (5.2.1) + sidekiq (5.2.3) connection_pool (~> 2.2, >= 2.2.2) rack-protection (>= 1.5.0) redis (>= 3.3.5, < 5) @@ -965,6 +967,7 @@ DEPENDENCIES devise (~> 4.4) devise-two-factor (~> 3.0.0) diffy (~> 3.1.0) + discordrb-webhooks-blackst0ne (~> 3.3) doorkeeper (~> 4.3) doorkeeper-openid_connect (~> 1.5) ed25519 (~> 1.2) @@ -974,7 +977,7 @@ DEPENDENCIES factory_bot_rails (~> 4.8.2) faraday (~> 0.12) fast_blank - ffaker (~> 2.4) + ffaker (~> 2.10) flipper (~> 0.13.0) flipper-active_record (~> 0.13.0) flipper-active_support_cache_store (~> 0.13.0) @@ -995,7 +998,7 @@ DEPENDENCIES gettext_i18n_rails_js (~> 1.3) gitaly-proto (~> 0.123.0) github-markup (~> 1.7.0) - gitlab-markup (~> 1.6.4) + gitlab-markup (~> 1.6.5) gitlab-sidekiq-fetcher gitlab-styles (~> 2.4) gitlab_omniauth-ldap (~> 2.0.4) @@ -1010,7 +1013,7 @@ DEPENDENCIES graphiql-rails (~> 1.4.10) graphql (~> 1.8.0) grpc (~> 1.15.0) - haml_lint (~> 0.26.0) + haml_lint (~> 0.28.0) hamlit (~> 2.8.8) hangouts-chat (~> 0.0.5) hashie-forbidden_attributes diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock index 181f2db95b0..8893088446b 100644 --- a/Gemfile.rails5.lock +++ b/Gemfile.rails5.lock @@ -165,6 +165,8 @@ GEM rotp (~> 2.0) diff-lcs (1.3) diffy (3.1.0) + discordrb-webhooks-blackst0ne (3.3.0) + rest-client (~> 2.0) docile (1.1.5) domain_name (0.5.20180417) unf (>= 0.0.5, < 1.0.0) @@ -203,7 +205,7 @@ GEM multi_json fast_blank (1.0.0) fast_gettext (1.6.0) - ffaker (2.4.0) + ffaker (2.10.0) ffi (1.9.25) flipper (0.13.0) flipper-active_record (0.13.0) @@ -275,7 +277,7 @@ GEM gitaly-proto (0.123.0) grpc (~> 1.0) github-markup (1.7.0) - gitlab-markup (1.6.4) + gitlab-markup (1.6.5) gitlab-sidekiq-fetcher (0.3.0) sidekiq (~> 5) gitlab-styles (2.4.1) @@ -338,11 +340,11 @@ GEM haml (5.0.4) temple (>= 0.8.0) tilt - haml_lint (0.26.0) + haml_lint (0.28.0) haml (>= 4.0, < 5.1) rainbow rake (>= 10, < 13) - rubocop (>= 0.49.0) + rubocop (>= 0.50.0) sysexits (~> 1.1) hamlit (2.8.8) temple (>= 0.8.0) @@ -544,7 +546,7 @@ GEM orm_adapter (0.5.0) os (1.0.0) parallel (1.12.1) - parser (2.5.1.2) + parser (2.5.3.0) ast (~> 2.4.0) parslet (1.8.2) peek (1.0.1) @@ -612,7 +614,7 @@ GEM httpclient (>= 2.4) multi_json (>= 1.3.6) rack (>= 1.1) - rack-protection (2.0.3) + rack-protection (2.0.4) rack rack-proxy (0.6.0) rack @@ -685,7 +687,7 @@ GEM redis-actionpack (>= 5.0, < 6) redis-activesupport (>= 5.0, < 6) redis-store (>= 1.2, < 2) - redis-store (1.4.1) + redis-store (1.6.0) redis (>= 2.2, < 5) regexp_parser (0.5.0) representable (3.0.4) @@ -810,7 +812,7 @@ GEM rack shoulda-matchers (3.1.2) activesupport (>= 4.0.0) - sidekiq (5.2.1) + sidekiq (5.2.3) connection_pool (~> 2.2, >= 2.2.2) rack-protection (>= 1.5.0) redis (>= 3.3.5, < 5) @@ -974,6 +976,7 @@ DEPENDENCIES devise (~> 4.4) devise-two-factor (~> 3.0.0) diffy (~> 3.1.0) + discordrb-webhooks-blackst0ne (~> 3.3) doorkeeper (~> 4.3) doorkeeper-openid_connect (~> 1.5) ed25519 (~> 1.2) @@ -983,7 +986,7 @@ DEPENDENCIES factory_bot_rails (~> 4.8.2) faraday (~> 0.12) fast_blank - ffaker (~> 2.4) + ffaker (~> 2.10) flipper (~> 0.13.0) flipper-active_record (~> 0.13.0) flipper-active_support_cache_store (~> 0.13.0) @@ -1004,7 +1007,7 @@ DEPENDENCIES gettext_i18n_rails_js (~> 1.3) gitaly-proto (~> 0.123.0) github-markup (~> 1.7.0) - gitlab-markup (~> 1.6.4) + gitlab-markup (~> 1.6.5) gitlab-sidekiq-fetcher gitlab-styles (~> 2.4) gitlab_omniauth-ldap (~> 2.0.4) @@ -1019,7 +1022,7 @@ DEPENDENCIES graphiql-rails (~> 1.4.10) graphql (~> 1.8.0) grpc (~> 1.15.0) - haml_lint (~> 0.26.0) + haml_lint (~> 0.28.0) hamlit (~> 2.8.8) hangouts-chat (~> 0.0.5) hashie-forbidden_attributes diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 7c60fb3da42..f80e20a4391 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -128,6 +128,7 @@ export default { eventHub.$once('fetchedNotesData', this.setDiscussions); }, methods: { + ...mapActions(['startTaskList']), ...mapActions('diffs', [ 'setBaseConfig', 'fetchDiffFiles', @@ -157,7 +158,13 @@ export default { if (this.isNotesFetched && !this.assignedDiscussions && !this.isLoading) { this.assignedDiscussions = true; - requestIdleCallback(() => this.assignDiscussionsToDiff(), { timeout: 1000 }); + requestIdleCallback( + () => + this.assignDiscussionsToDiff() + .then(this.$nextTick) + .then(this.startTaskList), + { timeout: 1000 }, + ); } }, adjustView() { diff --git a/app/assets/javascripts/diffs/components/tree_list.vue b/app/assets/javascripts/diffs/components/tree_list.vue index 91052b303a6..ff1eb23cea3 100644 --- a/app/assets/javascripts/diffs/components/tree_list.vue +++ b/app/assets/javascripts/diffs/components/tree_list.vue @@ -35,7 +35,7 @@ export default { if (search === '') return this.renderTreeList ? this.tree : this.allBlobs; - return this.allBlobs.filter(f => f.name.toLowerCase().indexOf(search) >= 0); + return this.allBlobs.filter(f => f.path.toLowerCase().indexOf(search) >= 0); }, rowDisplayTextKey() { if (this.renderTreeList && this.search.trim() === '') { diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index d3e9c7c88f0..41256fdd27a 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -190,6 +190,7 @@ export const saveDiffDiscussion = ({ dispatch }, { note, formData }) => { .then(result => dispatch('updateDiscussion', result.discussion, { root: true })) .then(discussion => dispatch('assignDiscussionsToDiff', [discussion])) .then(() => dispatch('closeDiffFileCommentForm', formData.diffFile.fileHash)) + .then(() => dispatch('startTaskList', null, { root: true })) .catch(() => createFlash(s__('MergeRequests|Saving the comment failed'))); }; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index a7eea2c1449..e651c197968 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -65,7 +65,13 @@ export default { const { highlightedDiffLines, parallelDiffLines } = diffFile; removeMatchLine(diffFile, lineNumbers, bottom); - const lines = addLineReferences(contextLines, lineNumbers, bottom); + + const lines = addLineReferences(contextLines, lineNumbers, bottom).map(line => ({ + ...line, + lineCode: line.lineCode || `${fileHash}_${line.oldLine}_${line.newLine}`, + discussions: line.discussions || [], + })); + addContextLines({ inlineLines: highlightedDiffLines, parallelLines: parallelDiffLines, diff --git a/app/assets/javascripts/jobs/store/actions.js b/app/assets/javascripts/jobs/store/actions.js index 54ed217572a..8045f6dc3ff 100644 --- a/app/assets/javascripts/jobs/store/actions.js +++ b/app/assets/javascripts/jobs/store/actions.js @@ -80,8 +80,8 @@ export const fetchJob = ({ state, dispatch }) => { export const receiveJobSuccess = ({ commit }, data = {}) => { commit(types.RECEIVE_JOB_SUCCESS, data); - if (data.favicon) { - setFaviconOverlay(data.favicon); + if (data.status && data.status.favicon) { + setFaviconOverlay(data.status.favicon); } else { resetFavicon(); } diff --git a/app/assets/javascripts/notes/components/comment_form.vue b/app/assets/javascripts/notes/components/comment_form.vue index 754c6e79ee4..10e80883c00 100644 --- a/app/assets/javascripts/notes/components/comment_form.vue +++ b/app/assets/javascripts/notes/components/comment_form.vue @@ -6,7 +6,6 @@ import Autosize from 'autosize'; import { __, sprintf } from '~/locale'; import Flash from '../../flash'; import Autosave from '../../autosave'; -import TaskList from '../../task_list'; import { capitalizeFirstCharacter, convertToCamelCase, @@ -146,7 +145,6 @@ export default { }); this.initAutoSave(); - this.initTaskList(); }, methods: { ...mapActions([ @@ -298,13 +296,6 @@ Please check your network connection and try again.`; ]); } }, - initTaskList() { - return new TaskList({ - dataType: 'note', - fieldName: 'note', - selector: '.notes', - }); - }, resizeTextarea() { this.$nextTick(() => { Autosize.update(this.$refs.textarea); diff --git a/app/assets/javascripts/notes/components/note_body.vue b/app/assets/javascripts/notes/components/note_body.vue index cf4c35de42c..9375627359c 100644 --- a/app/assets/javascripts/notes/components/note_body.vue +++ b/app/assets/javascripts/notes/components/note_body.vue @@ -4,7 +4,6 @@ import noteEditedText from './note_edited_text.vue'; import noteAwardsList from './note_awards_list.vue'; import noteAttachment from './note_attachment.vue'; import noteForm from './note_form.vue'; -import TaskList from '../../task_list'; import autosave from '../mixins/autosave'; export default { @@ -37,14 +36,12 @@ export default { }, mounted() { this.renderGFM(); - this.initTaskList(); if (this.isEditing) { this.initAutoSave(this.note); } }, updated() { - this.initTaskList(); this.renderGFM(); if (this.isEditing) { @@ -59,15 +56,6 @@ export default { renderGFM() { $(this.$refs['note-body']).renderGFM(); }, - initTaskList() { - if (this.canEdit) { - this.taskList = new TaskList({ - dataType: 'note', - fieldName: 'note', - selector: '.notes', - }); - } - }, handleFormUpdate(note, parentElement, callback) { this.$emit('handleFormUpdate', note, parentElement, callback); }, diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue index e302a89ee95..9ab91e2abe5 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/app/assets/javascripts/notes/components/noteable_note.vue @@ -46,6 +46,7 @@ export default { 'is-requesting being-posted': this.isRequesting, 'disabled-content': this.isDeleting, target: this.isTarget, + 'is-editable': this.note.current_user.can_edit, }; }, canResolve() { diff --git a/app/assets/javascripts/notes/components/notes_app.vue b/app/assets/javascripts/notes/components/notes_app.vue index e555279a6ac..69ddfd751e0 100644 --- a/app/assets/javascripts/notes/components/notes_app.vue +++ b/app/assets/javascripts/notes/components/notes_app.vue @@ -122,6 +122,7 @@ export default { setTargetNoteHash: 'setTargetNoteHash', toggleDiscussion: 'toggleDiscussion', setNotesFetchedState: 'setNotesFetchedState', + startTaskList: 'startTaskList', }), getComponentName(discussion) { if (discussion.isSkeletonNote) { @@ -157,6 +158,7 @@ export default { this.isFetching = false; }) .then(() => this.$nextTick()) + .then(() => this.startTaskList()) .then(() => this.checkLocationHash()) .catch(() => { this.setLoadingState(false); diff --git a/app/assets/javascripts/notes/stores/actions.js b/app/assets/javascripts/notes/stores/actions.js index 88739ffb083..a4ab079d258 100644 --- a/app/assets/javascripts/notes/stores/actions.js +++ b/app/assets/javascripts/notes/stores/actions.js @@ -1,6 +1,8 @@ +import Vue from 'vue'; import $ from 'jquery'; import axios from '~/lib/utils/axios_utils'; import Visibility from 'visibilityjs'; +import TaskList from '../../task_list'; import Flash from '../../flash'; import Poll from '../../lib/utils/poll'; import * as types from './mutation_types'; @@ -58,12 +60,13 @@ export const deleteNote = ({ commit, dispatch }, note) => dispatch('updateMergeRequestWidget'); }); -export const updateNote = ({ commit }, { endpoint, note }) => +export const updateNote = ({ commit, dispatch }, { endpoint, note }) => service .updateNote(endpoint, note) .then(res => res.json()) .then(res => { commit(types.UPDATE_NOTE, res); + dispatch('startTaskList'); }); export const replyToDiscussion = ({ commit }, { endpoint, data }) => @@ -85,6 +88,7 @@ export const createNewNote = ({ commit, dispatch }, { endpoint, data }) => commit(types.ADD_NEW_NOTE, res); dispatch('updateMergeRequestWidget'); + dispatch('startTaskList'); } return res; }); @@ -260,6 +264,8 @@ const pollSuccessCallBack = (resp, commit, state, getters, dispatch) => { commit(types.ADD_NEW_NOTE, note); } }); + + dispatch('startTaskList'); } commit(types.SET_LAST_FETCHED_AT, resp.last_fetched_at); @@ -368,5 +374,16 @@ export const setCommentsDisabled = ({ commit }, data) => { commit(types.DISABLE_COMMENTS, data); }; +export const startTaskList = ({ dispatch }) => + Vue.nextTick( + () => + new TaskList({ + dataType: 'note', + fieldName: 'note', + selector: '.notes .is-editable', + onSuccess: () => dispatch('startTaskList'), + }), + ); + // prevent babel-plugin-rewire from generating an invalid default during karma tests export default () => {}; diff --git a/app/assets/javascripts/vue_merge_request_widget/components/deployment.vue b/app/assets/javascripts/vue_merge_request_widget/components/deployment.vue index 2a8380f5f2b..4ec925aa8a6 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/deployment.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/deployment.vue @@ -40,10 +40,8 @@ export default { failed: __('Failed to deploy to'), }, data() { - const features = window.gon.features || {}; return { isStopping: false, - enableCiEnvironmentsStatusChanges: features.ciEnvironmentsStatusChanges, }; }, computed: { @@ -74,10 +72,7 @@ export default { : ''; }, shouldRenderDropdown() { - return ( - this.enableCiEnvironmentsStatusChanges && - (this.deployment.changes && this.deployment.changes.length > 0) - ); + return this.deployment.changes && this.deployment.changes.length > 0; }, }, methods: { diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index 00b06aea898..3aa79bf2466 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -47,12 +47,6 @@ @extend .fixed-width-container; } } - - .diffs { - .mr-version-controls { - @extend .fixed-width-container; - } - } } .issuable-details { diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index c57c1eee350..1b957f6cc69 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -1,6 +1,6 @@ $system-note-icon-size: 32px; $system-note-svg-size: 16px; -$note-form-margin-left: 70px; +$note-form-margin-left: 72px; @mixin vertical-line($left) { &::before { @@ -54,7 +54,7 @@ $note-form-margin-left: 70px; } .main-notes-list { - @include vertical-line(39px); + @include vertical-line(36px); } .notes { @@ -268,7 +268,7 @@ $note-form-margin-left: 70px; } .system-note { - padding: 6px $gl-padding-24; + padding: 6px 20px; margin: $gl-padding-24 0; background-color: transparent; diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index c02ec407262..0718658cd48 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -122,7 +122,7 @@ class Projects::BlobController < Projects::ApplicationController @lines.map! do |line| # These are marked as context lines but are loaded from blobs. # We also have context lines loaded from diffs in other places. - diff_line = Gitlab::Diff::Line.new(line, 'context', nil, nil, nil) + diff_line = Gitlab::Diff::Line.new(line, expanded_diff_line_type, nil, nil, nil) diff_line.rich_text = line diff_line end @@ -132,6 +132,11 @@ class Projects::BlobController < Projects::ApplicationController render json: DiffLineSerializer.new.represent(@lines) end + def expanded_diff_line_type + # Context lines can't receive comments. + Feature.enabled?(:comment_in_any_diff_line, @project) ? nil : 'context' + end + def add_match_line return unless @form.unfold? diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index 5307cd0720a..b3d77335c2a 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -22,6 +22,12 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic def render_diffs @environment = @merge_request.environments_for(current_user).last + notes_grouped_by_path = renderable_notes.group_by { |note| note.position.file_path } + + @diffs.diff_files.each do |diff_file| + notes = notes_grouped_by_path.fetch(diff_file.file_path, []) + notes.each { |note| diff_file.unfold_diff_lines(note.position) } + end @diffs.write_cache @@ -108,4 +114,10 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic @grouped_diff_discussions = @merge_request.grouped_diff_discussions(@compare.diff_refs) @notes = prepare_notes_for_rendering(@grouped_diff_discussions.values.flatten.flat_map(&:notes), @merge_request) end + + def renderable_notes + define_diff_comment_vars unless @notes + + @notes + end end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 4bdb857b2d9..23d16fed7b9 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -14,9 +14,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo before_action :set_issuables_index, only: [:index] before_action :authenticate_user!, only: [:assign_related_issues] before_action :check_user_can_push_to_source_branch!, only: [:rebase] - before_action do - push_frontend_feature_flag(:ci_environments_status_changes) - end def index @merge_requests = @issuables diff --git a/app/helpers/import_helper.rb b/app/helpers/import_helper.rb index 3d0eb3d0d51..49171df1433 100644 --- a/app/helpers/import_helper.rb +++ b/app/helpers/import_helper.rb @@ -83,7 +83,7 @@ module ImportHelper private def github_project_url(full_path) - URI.join(github_root_url, full_path).to_s + Gitlab::Utils.append_path(github_root_url, full_path) end def github_root_url @@ -95,6 +95,6 @@ module ImportHelper end def gitea_project_url(full_path) - URI.join(@gitea_host_url, full_path).to_s + Gitlab::Utils.append_path(@gitea_host_url, full_path) end end diff --git a/app/models/clusters/kubernetes_namespace.rb b/app/models/clusters/kubernetes_namespace.rb index ac7f9193b87..cbd52bfb48b 100644 --- a/app/models/clusters/kubernetes_namespace.rb +++ b/app/models/clusters/kubernetes_namespace.rb @@ -22,6 +22,8 @@ module Clusters key: Settings.attr_encrypted_db_key_base_truncated, algorithm: 'aes-256-cbc' + scope :has_service_account_token, -> { where.not(encrypted_service_account_token: nil) } + def token_name "#{namespace}-token" end diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb index ea02ae6c9d8..9860abeecf7 100644 --- a/app/models/clusters/platforms/kubernetes.rb +++ b/app/models/clusters/platforms/kubernetes.rb @@ -83,7 +83,7 @@ module Clusters .append(key: 'KUBE_CA_PEM_FILE', value: ca_pem, file: true) end - if kubernetes_namespace = cluster.kubernetes_namespaces.find_by(project: project) + if kubernetes_namespace = cluster.kubernetes_namespaces.has_service_account_token.find_by(project: project) variables.concat(kubernetes_namespace.predefined_variables) else # From 11.5, every Clusters::Project should have at least one diff --git a/app/models/concerns/deployable.rb b/app/models/concerns/deployable.rb index f4f1989f0a9..85db01af18d 100644 --- a/app/models/concerns/deployable.rb +++ b/app/models/concerns/deployable.rb @@ -13,17 +13,14 @@ module Deployable name: expanded_environment_name ) - environment.deployments.create!( + create_deployment!( project_id: environment.project_id, environment: environment, ref: ref, tag: tag, sha: sha, user: user, - deployable: self, - on_stop: on_stop).tap do |_| - self.reload # Reload relationships - end + on_stop: on_stop) end end end diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index 5f59e4832db..c32008aa9c7 100644 --- a/app/models/diff_note.rb +++ b/app/models/diff_note.rb @@ -66,6 +66,10 @@ class DiffNote < Note self.original_position.diff_refs == diff_refs end + def discussion_first_note? + self == discussion.first_note + end + private def enqueue_diff_file_creation_job @@ -78,26 +82,33 @@ class DiffNote < Note end def should_create_diff_file? - on_text? && note_diff_file.nil? && self == discussion.first_note + on_text? && note_diff_file.nil? && discussion_first_note? end def fetch_diff_file - if note_diff_file - diff = Gitlab::Git::Diff.new(note_diff_file.to_hash) - Gitlab::Diff::File.new(diff, - repository: project.repository, - diff_refs: original_position.diff_refs) - elsif created_at_diff?(noteable.diff_refs) - # We're able to use the already persisted diffs (Postgres) if we're - # presenting a "current version" of the MR discussion diff. - # So no need to make an extra Gitaly diff request for it. - # As an extra benefit, the returned `diff_file` already - # has `highlighted_diff_lines` data set from Redis on - # `Diff::FileCollection::MergeRequestDiff`. - noteable.diffs(original_position.diff_options).diff_files.first - else - original_position.diff_file(self.project.repository) - end + file = + if note_diff_file + diff = Gitlab::Git::Diff.new(note_diff_file.to_hash) + Gitlab::Diff::File.new(diff, + repository: project.repository, + diff_refs: original_position.diff_refs) + elsif created_at_diff?(noteable.diff_refs) + # We're able to use the already persisted diffs (Postgres) if we're + # presenting a "current version" of the MR discussion diff. + # So no need to make an extra Gitaly diff request for it. + # As an extra benefit, the returned `diff_file` already + # has `highlighted_diff_lines` data set from Redis on + # `Diff::FileCollection::MergeRequestDiff`. + noteable.diffs(original_position.diff_options).diff_files.first + else + original_position.diff_file(self.project.repository) + end + + # Since persisted diff files already have its content "unfolded" + # there's no need to make it pass through the unfolding process. + file&.unfold_diff_lines(position) unless note_diff_file + + file end def supported? diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 74583af1a29..6f1beede6f9 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -142,7 +142,7 @@ class MergeRequestDiff < ActiveRecord::Base end def commits_by_shas(shas) - return [] unless shas.present? + return MergeRequestDiffCommit.none unless shas.present? merge_request_diff_commits.where(sha: shas) end diff --git a/app/models/project.rb b/app/models/project.rb index 48905547ab4..d87fc1e4b86 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -135,6 +135,7 @@ class Project < ActiveRecord::Base # Project services has_one :campfire_service + has_one :discord_service has_one :drone_ci_service has_one :emails_on_push_service has_one :pipelines_email_service diff --git a/app/models/project_services/bamboo_service.rb b/app/models/project_services/bamboo_service.rb index d121d088ff6..a252052200a 100644 --- a/app/models/project_services/bamboo_service.rb +++ b/app/models/project_services/bamboo_service.rb @@ -86,14 +86,16 @@ class BambooService < CiService end def read_build_page(response) - if response.code != 200 || response.dig('results', 'results', 'size') == '0' - # If actual build link can't be determined, send user to build summary page. - URI.join("#{bamboo_url}/", "browse/#{build_key}").to_s - else - # If actual build link is available, go to build result page. - result_key = response.dig('results', 'results', 'result', get_build_result_index, 'planResultKey', 'key') - URI.join("#{bamboo_url}/", "browse/#{result_key}").to_s - end + key = + if response.code != 200 || response.dig('results', 'results', 'size') == '0' + # If actual build link can't be determined, send user to build summary page. + build_key + else + # If actual build link is available, go to build result page. + response.dig('results', 'results', 'result', get_build_result_index, 'planResultKey', 'key') + end + + build_url("browse/#{key}") end def read_commit_status(response) @@ -117,7 +119,7 @@ class BambooService < CiService end def build_url(path) - URI.join("#{bamboo_url}/", path).to_s + Gitlab::Utils.append_path(bamboo_url, path) end def get_path(path, query_params = {}) diff --git a/app/models/project_services/discord_service.rb b/app/models/project_services/discord_service.rb new file mode 100644 index 00000000000..21afd14dbff --- /dev/null +++ b/app/models/project_services/discord_service.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require "discordrb/webhooks" + +class DiscordService < ChatNotificationService + def title + "Discord Notifications" + end + + def description + "Receive event notifications in Discord" + end + + def self.to_param + "discord" + end + + def help + "This service sends notifications about project events to Discord channels.<br /> + To set up this service: + <ol> + <li><a href='https://support.discordapp.com/hc/en-us/articles/228383668-Intro-to-Webhooks'>Setup a custom Incoming Webhook</a>.</li> + <li>Paste the <strong>Webhook URL</strong> into the field below.</li> + <li>Select events below to enable notifications.</li> + </ol>" + end + + def event_field(event) + # No-op. + end + + def default_channel_placeholder + # No-op. + end + + def default_fields + [ + { type: "text", name: "webhook", placeholder: "e.g. https://discordapp.com/api/webhooks/…" }, + { type: "checkbox", name: "notify_only_broken_pipelines" }, + { type: "checkbox", name: "notify_only_default_branch" } + ] + end + + private + + def notify(message, opts) + client = Discordrb::Webhooks::Client.new(url: webhook) + + client.execute do |builder| + builder.content = message.pretext + end + end + + def custom_data(data) + super(data).merge(markdown: true) + end +end diff --git a/app/models/project_services/drone_ci_service.rb b/app/models/project_services/drone_ci_service.rb index 158ae0bf255..5ccc2f019cb 100644 --- a/app/models/project_services/drone_ci_service.rb +++ b/app/models/project_services/drone_ci_service.rb @@ -39,11 +39,9 @@ class DroneCiService < CiService end def commit_status_path(sha, ref) - url = [drone_url, - "gitlab/#{project.full_path}/commits/#{sha}", - "?branch=#{URI.encode(ref.to_s)}&access_token=#{token}"] - - URI.join(*url).to_s + Gitlab::Utils.append_path( + drone_url, + "gitlab/#{project.full_path}/commits/#{sha}?branch=#{URI.encode(ref.to_s)}&access_token=#{token}") end def commit_status(sha, ref) @@ -74,11 +72,9 @@ class DroneCiService < CiService end def build_page(sha, ref) - url = [drone_url, - "gitlab/#{project.full_path}/redirect/commits/#{sha}", - "?branch=#{URI.encode(ref.to_s)}"] - - URI.join(*url).to_s + Gitlab::Utils.append_path( + drone_url, + "gitlab/#{project.full_path}/redirect/commits/#{sha}?branch=#{URI.encode(ref.to_s)}") end def title diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb index 5a38f48c542..9066a0b7f1d 100644 --- a/app/models/project_services/jira_service.rb +++ b/app/models/project_services/jira_service.rb @@ -54,7 +54,7 @@ class JiraService < IssueTrackerService { username: self.username, password: self.password, - site: URI.join(url, '/').to_s, + site: URI.join(url, '/').to_s, # Intended to find the root context_path: url.path.chomp('/'), auth_type: :basic, read_timeout: 120, diff --git a/app/models/project_services/mock_ci_service.rb b/app/models/project_services/mock_ci_service.rb index 6883976f0c8..d8bba58dcbf 100644 --- a/app/models/project_services/mock_ci_service.rb +++ b/app/models/project_services/mock_ci_service.rb @@ -34,10 +34,9 @@ class MockCiService < CiService # http://jenkins.example.com:8888/job/test1/scm/bySHA1/12d65c # def build_page(sha, ref) - url = [mock_service_url, - "#{project.namespace.path}/#{project.path}/status/#{sha}"] - - URI.join(*url).to_s + Gitlab::Utils.append_path( + mock_service_url, + "#{project.namespace.path}/#{project.path}/status/#{sha}") end # Return string with build status or :error symbol @@ -61,10 +60,9 @@ class MockCiService < CiService end def commit_status_path(sha) - url = [mock_service_url, - "#{project.namespace.path}/#{project.path}/status/#{sha}.json"] - - URI.join(*url).to_s + Gitlab::Utils.append_path( + mock_service_url, + "#{project.namespace.path}/#{project.path}/status/#{sha}.json") end def read_commit_status(response) diff --git a/app/models/project_services/teamcity_service.rb b/app/models/project_services/teamcity_service.rb index eeeff5e802a..b8e17087db5 100644 --- a/app/models/project_services/teamcity_service.rb +++ b/app/models/project_services/teamcity_service.rb @@ -132,7 +132,7 @@ class TeamcityService < CiService end def build_url(path) - URI.join("#{teamcity_url}/", path).to_s + Gitlab::Utils.append_path(teamcity_url, path) end def get_path(path) diff --git a/app/models/service.rb b/app/models/service.rb index 4dbda7acab6..5b8bf6e7cf0 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -253,6 +253,7 @@ class Service < ActiveRecord::Base bugzilla campfire custom_issue_tracker + discord drone_ci emails_on_push external_wiki diff --git a/app/serializers/environment_status_entity.rb b/app/serializers/environment_status_entity.rb index f87cc894d2f..4c6664e9e25 100644 --- a/app/serializers/environment_status_entity.rb +++ b/app/serializers/environment_status_entity.rb @@ -37,7 +37,7 @@ class EnvironmentStatusEntity < Grape::Entity es.deployment.try(:formatted_deployment_time) end - expose :changes, if: ->(*) { Feature.enabled?(:ci_environments_status_changes, project) } + expose :changes private diff --git a/app/services/issuable/clone/attributes_rewriter.rb b/app/services/issuable/clone/attributes_rewriter.rb new file mode 100644 index 00000000000..0300cc0d8d3 --- /dev/null +++ b/app/services/issuable/clone/attributes_rewriter.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +module Issuable + module Clone + class AttributesRewriter < ::Issuable::Clone::BaseService + def initialize(current_user, original_entity, new_entity) + @current_user = current_user + @original_entity = original_entity + @new_entity = new_entity + end + + def execute + new_entity.update(milestone: cloneable_milestone, labels: cloneable_labels) + copy_resource_label_events + end + + private + + def cloneable_milestone + title = original_entity.milestone&.title + return unless title + + params = { title: title, project_ids: new_entity.project&.id, group_ids: group&.id } + + milestones = MilestonesFinder.new(params).execute + milestones.first + end + + def cloneable_labels + params = { + project_id: new_entity.project&.id, + group_id: group&.id, + title: original_entity.labels.select(:title), + include_ancestor_groups: true + } + + params[:only_group_labels] = true if new_parent.is_a?(Group) + + LabelsFinder.new(current_user, params).execute + end + + def copy_resource_label_events + original_entity.resource_label_events.find_in_batches do |batch| + events = batch.map do |event| + entity_key = new_entity.is_a?(Issue) ? 'issue_id' : 'epic_id' + # rubocop: disable CodeReuse/ActiveRecord + event.attributes + .except('id', 'reference', 'reference_html') + .merge(entity_key => new_entity.id, 'action' => ResourceLabelEvent.actions[event.action]) + # rubocop: enable CodeReuse/ActiveRecord + end + + Gitlab::Database.bulk_insert(ResourceLabelEvent.table_name, events) + end + end + + def entity_key + new_entity.class.name.parameterize('_').foreign_key + end + end + end +end diff --git a/app/services/issuable/clone/base_service.rb b/app/services/issuable/clone/base_service.rb new file mode 100644 index 00000000000..42dd9c666f5 --- /dev/null +++ b/app/services/issuable/clone/base_service.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module Issuable + module Clone + class BaseService < IssuableBaseService + attr_reader :original_entity, :new_entity + + alias_method :old_project, :project + + def execute(original_entity, new_project = nil) + @original_entity = original_entity + + # Using transaction because of a high resources footprint + # on rewriting notes (unfolding references) + # + ActiveRecord::Base.transaction do + @new_entity = create_new_entity + + update_new_entity + update_old_entity + create_notes + end + end + + private + + def update_new_entity + rewriters = [ContentRewriter, AttributesRewriter] + + rewriters.each do |rewriter| + rewriter.new(current_user, original_entity, new_entity).execute + end + end + + def update_old_entity + close_issue + end + + def create_notes + add_note_from + add_note_to + end + + def close_issue + close_service = Issues::CloseService.new(old_project, current_user) + close_service.execute(original_entity, notifications: false, system_note: false) + end + + def new_parent + new_entity.project ? new_entity.project : new_entity.group + end + + def group + if new_entity.project&.group && current_user.can?(:read_group, new_entity.project.group) + new_entity.project.group + end + end + end + end +end diff --git a/app/services/issuable/clone/content_rewriter.rb b/app/services/issuable/clone/content_rewriter.rb new file mode 100644 index 00000000000..e1e0b75085d --- /dev/null +++ b/app/services/issuable/clone/content_rewriter.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +module Issuable + module Clone + class ContentRewriter < ::Issuable::Clone::BaseService + def initialize(current_user, original_entity, new_entity) + @current_user = current_user + @original_entity = original_entity + @new_entity = new_entity + @project = original_entity.project + end + + def execute + rewrite_description + rewrite_award_emoji(original_entity, new_entity) + rewrite_notes + end + + private + + def rewrite_description + new_entity.update(description: rewrite_content(original_entity.description)) + end + + def rewrite_notes + original_entity.notes_with_associations.find_each do |note| + new_note = note.dup + new_params = { + project: new_entity.project, noteable: new_entity, + note: rewrite_content(new_note.note), + created_at: note.created_at, + updated_at: note.updated_at + } + + if note.system_note_metadata + new_params[:system_note_metadata] = note.system_note_metadata.dup + end + + new_note.update(new_params) + + rewrite_award_emoji(note, new_note) + end + end + + def rewrite_content(content) + return unless content + + rewriters = [Gitlab::Gfm::ReferenceRewriter, Gitlab::Gfm::UploadsRewriter] + + rewriters.inject(content) do |text, klass| + rewriter = klass.new(text, old_project, current_user) + rewriter.rewrite(new_parent) + end + end + + def rewrite_award_emoji(old_awardable, new_awardable) + old_awardable.award_emoji.each do |award| + new_award = award.dup + new_award.awardable = new_awardable + new_award.save + end + end + end + end +end diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb index d2bdba1e627..41b6a96b005 100644 --- a/app/services/issues/move_service.rb +++ b/app/services/issues/move_service.rb @@ -1,165 +1,66 @@ # frozen_string_literal: true module Issues - class MoveService < Issues::BaseService + class MoveService < Issuable::Clone::BaseService MoveError = Class.new(StandardError) - def execute(issue, new_project) - @old_issue = issue - @old_project = @project - @new_project = new_project + def execute(issue, target_project) + @target_project = target_project - unless issue.can_move?(current_user, new_project) + unless issue.can_move?(current_user, @target_project) raise MoveError, 'Cannot move issue due to insufficient permissions!' end - if @project == new_project + if @project == @target_project raise MoveError, 'Cannot move issue to project it originates from!' end - # Using transaction because of a high resources footprint - # on rewriting notes (unfolding references) - # - ActiveRecord::Base.transaction do - @new_issue = create_new_issue - - update_new_issue - update_old_issue - end + super notify_participants - @new_issue + new_entity end private - def update_new_issue - rewrite_notes - copy_resource_label_events - rewrite_issue_award_emoji - add_note_moved_from - end + def update_old_entity + super - def update_old_issue - add_note_moved_to - close_issue mark_as_moved end - def create_new_issue - new_params = { id: nil, iid: nil, label_ids: cloneable_label_ids, - milestone_id: cloneable_milestone_id, - project: @new_project, author: @old_issue.author, - description: rewrite_content(@old_issue.description), - assignee_ids: @old_issue.assignee_ids } - - new_params = @old_issue.serializable_hash.symbolize_keys.merge(new_params) - CreateService.new(@new_project, @current_user, new_params).execute - end - - # rubocop: disable CodeReuse/ActiveRecord - def cloneable_label_ids - params = { - project_id: @new_project.id, - title: @old_issue.labels.pluck(:title), - include_ancestor_groups: true - } + def create_new_entity + new_params = { + id: nil, + iid: nil, + project: @target_project, + author: original_entity.author, + assignee_ids: original_entity.assignee_ids + } - LabelsFinder.new(current_user, params).execute.pluck(:id) + new_params = original_entity.serializable_hash.symbolize_keys.merge(new_params) + CreateService.new(@target_project, @current_user, new_params).execute end - # rubocop: enable CodeReuse/ActiveRecord - - def cloneable_milestone_id - title = @old_issue.milestone&.title - return unless title - - if @new_project.group && can?(current_user, :read_group, @new_project.group) - group_id = @new_project.group.id - end - - params = - { title: title, project_ids: @new_project.id, group_ids: group_id } - milestones = MilestonesFinder.new(params).execute - milestones.first&.id - end - - def rewrite_notes - @old_issue.notes_with_associations.find_each do |note| - new_note = note.dup - new_params = { project: @new_project, noteable: @new_issue, - note: rewrite_content(new_note.note), - created_at: note.created_at, - updated_at: note.updated_at } - - new_note.update(new_params) - - rewrite_award_emoji(note, new_note) - end - end - - # rubocop: disable CodeReuse/ActiveRecord - def copy_resource_label_events - @old_issue.resource_label_events.find_in_batches do |batch| - events = batch.map do |event| - event.attributes - .except('id', 'reference', 'reference_html') - .merge('issue_id' => @new_issue.id, 'action' => ResourceLabelEvent.actions[event.action]) - end - - Gitlab::Database.bulk_insert(ResourceLabelEvent.table_name, events) - end - end - # rubocop: enable CodeReuse/ActiveRecord - - def rewrite_issue_award_emoji - rewrite_award_emoji(@old_issue, @new_issue) - end - - def rewrite_award_emoji(old_awardable, new_awardable) - old_awardable.award_emoji.each do |award| - new_award = award.dup - new_award.awardable = new_awardable - new_award.save - end - end - - def rewrite_content(content) - return unless content - - rewriters = [Gitlab::Gfm::ReferenceRewriter, - Gitlab::Gfm::UploadsRewriter] - - rewriters.inject(content) do |text, klass| - rewriter = klass.new(text, @old_project, @current_user) - rewriter.rewrite(@new_project) - end + def mark_as_moved + original_entity.update(moved_to: new_entity) end - def close_issue - close_service = CloseService.new(@old_project, @current_user) - close_service.execute(@old_issue, notifications: false, system_note: false) + def notify_participants + notification_service.async.issue_moved(original_entity, new_entity, @current_user) end - def add_note_moved_from - SystemNoteService.noteable_moved(@new_issue, @new_project, - @old_issue, @current_user, + def add_note_from + SystemNoteService.noteable_moved(new_entity, @target_project, + original_entity, current_user, direction: :from) end - def add_note_moved_to - SystemNoteService.noteable_moved(@old_issue, @old_project, - @new_issue, @current_user, + def add_note_to + SystemNoteService.noteable_moved(original_entity, old_project, + new_entity, current_user, direction: :to) end - - def mark_as_moved - @old_issue.update(moved_to: @new_issue) - end - - def notify_participants - notification_service.async.issue_moved(@old_issue, @new_issue, @current_user) - end end end diff --git a/app/services/merge_requests/reload_diffs_service.rb b/app/services/merge_requests/reload_diffs_service.rb index b47d8f3f63a..c64b2e99b52 100644 --- a/app/services/merge_requests/reload_diffs_service.rb +++ b/app/services/merge_requests/reload_diffs_service.rb @@ -29,10 +29,6 @@ module MergeRequests # rubocop: disable CodeReuse/ActiveRecord def clear_cache(new_diff) - # Executing the iteration we cache highlighted diffs for each diff file of - # MergeRequestDiff. - cacheable_collection(new_diff).write_cache - # Remove cache for all diffs on this MR. Do not use the association on the # model, as that will interfere with other actions happening when # reloading the diff. diff --git a/app/services/notes/base_service.rb b/app/services/notes/base_service.rb new file mode 100644 index 00000000000..c1260837c12 --- /dev/null +++ b/app/services/notes/base_service.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Notes + class BaseService < ::BaseService + def clear_noteable_diffs_cache(note) + if note.is_a?(DiffNote) && + note.discussion_first_note? && + note.position.unfolded_diff?(project.repository) + note.noteable.diffs.clear_cache + end + end + end +end diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 049e6c5a871..e03789e3ca9 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Notes - class CreateService < ::BaseService + class CreateService < ::Notes::BaseService def execute merge_request_diff_head_sha = params.delete(:merge_request_diff_head_sha) @@ -35,6 +35,7 @@ module Notes if !only_commands && note.save todo_service.new_note(note, current_user) + clear_noteable_diffs_cache(note) end if command_params.present? diff --git a/app/services/notes/destroy_service.rb b/app/services/notes/destroy_service.rb index 64e9accd97f..fa0c2c5c86b 100644 --- a/app/services/notes/destroy_service.rb +++ b/app/services/notes/destroy_service.rb @@ -1,11 +1,13 @@ # frozen_string_literal: true module Notes - class DestroyService < BaseService + class DestroyService < ::Notes::BaseService def execute(note) TodoService.new.destroy_target(note) do |note| note.destroy end + + clear_noteable_diffs_cache(note) end end end diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb index ffc1e5f75ca..e90599f2505 100644 --- a/app/uploaders/file_uploader.rb +++ b/app/uploaders/file_uploader.rb @@ -149,9 +149,9 @@ class FileUploader < GitlabUploader # return a new uploader with a file copy on another project def self.copy_to(uploader, to_project) - moved = uploader.dup.tap do |u| - u.model = to_project - end + moved = self.new(to_project) + moved.object_store = uploader.object_store + moved.filename = uploader.filename moved.copy_file(uploader.file) moved diff --git a/app/views/admin/background_jobs/show.html.haml b/app/views/admin/background_jobs/show.html.haml index faa5854bb40..9aa705d9fa6 100644 --- a/app/views/admin/background_jobs/show.html.haml +++ b/app/views/admin/background_jobs/show.html.haml @@ -34,10 +34,13 @@ .clearfix %p %i.fa.fa-exclamation-circle - If '[#{@concurrency} of #{@concurrency} busy]' is shown, restart GitLab with 'sudo service gitlab reload'. + If '[#{@concurrency} of #{@concurrency} busy]' is shown, restart GitLab. + = link_to sprite_icon('question', size: 16), help_page_path('administration/restart_gitlab') + %p %i.fa.fa-exclamation-circle If more than one sidekiq process is listed, stop GitLab, kill the remaining sidekiq processes (sudo pkill -u #{gitlab_config.user} -f sidekiq) and restart GitLab. + = link_to sprite_icon('question', size: 16), help_page_path('administration/restart_gitlab') diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index 3aff5538813..de768696fe9 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -165,7 +165,7 @@ .input-group .input-group-prepend .input-group-text - #{URI.join(root_url, @project.namespace.full_path)}/ + #{Gitlab::Utils.append_path(root_url, @project.namespace.full_path)}/ = f.text_field :path, class: 'form-control' %ul %li Be careful. Renaming a project's repository can have unintended side effects. diff --git a/changelogs/unreleased/53289-update-haml_lint-to-0-28-0.yml b/changelogs/unreleased/53289-update-haml_lint-to-0-28-0.yml new file mode 100644 index 00000000000..9a16666c416 --- /dev/null +++ b/changelogs/unreleased/53289-update-haml_lint-to-0-28-0.yml @@ -0,0 +1,5 @@ +--- +title: Update haml_lint to 0.28.0 +merge_request: 22660 +author: Takuya Noguchi +type: other diff --git a/changelogs/unreleased/53291-update-ffaker-to-2-10-0.yml b/changelogs/unreleased/53291-update-ffaker-to-2-10-0.yml new file mode 100644 index 00000000000..a1b95df5e32 --- /dev/null +++ b/changelogs/unreleased/53291-update-ffaker-to-2-10-0.yml @@ -0,0 +1,5 @@ +--- +title: Update ffaker to 2.10.0 +merge_request: 22661 +author: Takuya Noguchi +type: other diff --git a/changelogs/unreleased/53879-kube-token-nil.yml b/changelogs/unreleased/53879-kube-token-nil.yml new file mode 100644 index 00000000000..61a0db15d84 --- /dev/null +++ b/changelogs/unreleased/53879-kube-token-nil.yml @@ -0,0 +1,5 @@ +--- +title: Fix deployment jobs using nil KUBE_TOKEN due to migration issue +merge_request: 23009 +author: +type: fixed diff --git a/changelogs/unreleased/53888-missing-favicon.yml b/changelogs/unreleased/53888-missing-favicon.yml new file mode 100644 index 00000000000..ba6f26c6b9f --- /dev/null +++ b/changelogs/unreleased/53888-missing-favicon.yml @@ -0,0 +1,5 @@ +--- +title: Adds CI favicon back to jobs page +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/ashmckenzie-hmac-token-decode-and-tests.yml b/changelogs/unreleased/ashmckenzie-hmac-token-decode-and-tests.yml new file mode 100644 index 00000000000..d15c5654d99 --- /dev/null +++ b/changelogs/unreleased/ashmckenzie-hmac-token-decode-and-tests.yml @@ -0,0 +1,5 @@ +--- +title: Relocate JSONWebToken::HMACToken from EE +merge_request: 22906 +author: +type: changed diff --git a/changelogs/unreleased/blackst0ne-add-discord-service.yml b/changelogs/unreleased/blackst0ne-add-discord-service.yml new file mode 100644 index 00000000000..85dedf6d81f --- /dev/null +++ b/changelogs/unreleased/blackst0ne-add-discord-service.yml @@ -0,0 +1,5 @@ +--- +title: Add Discord integration +merge_request: 22684 +author: "@blackst0ne" +type: added diff --git a/changelogs/unreleased/discussion-perf-improvement.yml b/changelogs/unreleased/discussion-perf-improvement.yml new file mode 100644 index 00000000000..defff8a55f5 --- /dev/null +++ b/changelogs/unreleased/discussion-perf-improvement.yml @@ -0,0 +1,5 @@ +--- +title: Improve initial discussion rendering performance +merge_request: 22607 +author: +type: changed diff --git a/changelogs/unreleased/fix-tags-for-envs.yml b/changelogs/unreleased/fix-tags-for-envs.yml new file mode 100644 index 00000000000..633788ff6d8 --- /dev/null +++ b/changelogs/unreleased/fix-tags-for-envs.yml @@ -0,0 +1,5 @@ +--- +title: Do not reload self on hooks when creating deployment +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/mr-tree-filter-path-name.yml b/changelogs/unreleased/mr-tree-filter-path-name.yml new file mode 100644 index 00000000000..152f8a67337 --- /dev/null +++ b/changelogs/unreleased/mr-tree-filter-path-name.yml @@ -0,0 +1,5 @@ +--- +title: Changed merge request filtering to be by path instead of name +merge_request: +author: +type: changed diff --git a/changelogs/unreleased/osw-comment-on-any-line-on-diffs.yml b/changelogs/unreleased/osw-comment-on-any-line-on-diffs.yml new file mode 100644 index 00000000000..e25d64a89d7 --- /dev/null +++ b/changelogs/unreleased/osw-comment-on-any-line-on-diffs.yml @@ -0,0 +1,5 @@ +--- +title: Allow commenting on any diff line in Merge Requests +merge_request: 22914 +author: +type: added diff --git a/changelogs/unreleased/sh-53180-append-path.yml b/changelogs/unreleased/sh-53180-append-path.yml new file mode 100644 index 00000000000..64fae5522d8 --- /dev/null +++ b/changelogs/unreleased/sh-53180-append-path.yml @@ -0,0 +1,5 @@ +--- +title: Make sure there's only one slash as path separator +merge_request: 22954 +author: +type: other diff --git a/changelogs/unreleased/zj-remove-broken-storage.yml b/changelogs/unreleased/zj-remove-broken-storage.yml new file mode 100644 index 00000000000..9df87b40e09 --- /dev/null +++ b/changelogs/unreleased/zj-remove-broken-storage.yml @@ -0,0 +1,5 @@ +--- +title: Remove obsolete gitlab_shell rake tasks +merge_request: 22417 +author: +type: removed diff --git a/danger/documentation/Dangerfile b/danger/documentation/Dangerfile index fe819ee250c..1ee025f0972 100644 --- a/danger/documentation/Dangerfile +++ b/danger/documentation/Dangerfile @@ -24,9 +24,32 @@ The following files require a review from the Documentation team: * #{docs_paths_to_review.map { |path| "`#{path}`" }.join("\n* ")} -To make sure these changes are reviewed, mention `@gl-docsteam` in a separate -comment, and explain what needs to be reviewed by the team. Please don't mention -the team until your changes are ready for review. +When your content is ready for review, mention a technical writer in a separate +comment and explain what needs to be reviewed. + +You are welcome to mention them sooner if you have questions about writing or updating +the documentation. GitLabbers are also welcome to use the [#docs](https://gitlab.slack.com/archives/C16HYA2P5) channel on Slack. + +Who to ping [based on DevOps stages](https://about.gitlab.com/handbook/product/categories/#devops-stages): + +| Stage | Tech writer | +| ------------- | ----------- | +| ~Create | `@marcia` | +| ~Configure | `@eread` | +| ~Distribution | `@axil` | +| ~Geo | `@eread` | +| ~Gitaly | `@axil` | +| ~Gitter | `@axil` | +| ~Manage | `@eread` | +| ~Monitoring | `@axil` | +| ~Packaging | `@axil` | +| ~Plan | `@mikelewis`| +| ~Release | `marcia` | +| ~Secure | `@axil` | +| ~Verify | `@eread` | + +If you are not sure which category the change falls within, or the change is not +part of one of these categories, you can mention the whole team with `@gl-docsteam`. MARKDOWN unless gitlab.mr_labels.include?('Documentation') diff --git a/doc/administration/git_protocol.md b/doc/administration/git_protocol.md index 6b82771baf9..b1be078d672 100644 --- a/doc/administration/git_protocol.md +++ b/doc/administration/git_protocol.md @@ -4,9 +4,7 @@ description: "Set and configure Git protocol v2" # Configuring Git Protocol v2 -> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/46555) in GitLab 11.4. - ---- +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/46555) in GitLab 11.4. Git protocol v2 improves the v1 wire protocol in several ways and is enabled by default in GitLab for HTTP requests. In order to enable SSH, diff --git a/doc/api/services.md b/doc/api/services.md index 741ea83070f..f122bac6f1f 100644 --- a/doc/api/services.md +++ b/doc/api/services.md @@ -10,7 +10,7 @@ Asana - Teamwork without email Set Asana service for a project. -> This service adds commit messages as comments to Asana tasks. Once enabled, commit messages are checked for Asana task URLs (for example, `https://app.asana.com/0/123456/987654`) or task IDs starting with # (for example, `#987654`). Every task ID found will get the commit comment added to it. You can also close a task with a message containing: `fix #123456`. You can find your Api Keys here: https://asana.com/developers/documentation/getting-started/auth#api-key +> This service adds commit messages as comments to Asana tasks. Once enabled, commit messages are checked for Asana task URLs (for example, `https://app.asana.com/0/123456/987654`) or task IDs starting with # (for example, `#987654`). Every task ID found will get the commit comment added to it. You can also close a task with a message containing: `fix #123456`. You can find your Api Keys here: <https://asana.com/developers/documentation/getting-started/auth#api-key>. ``` PUT /projects/:id/services/asana @@ -92,7 +92,7 @@ Parameters: | Parameter | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `bamboo_url` | string | true | Bamboo root URL like https://bamboo.example.com | +| `bamboo_url` | string | true | Bamboo root URL. For example, `https://bamboo.example.com`. | | `build_key` | string | true | Bamboo build plan key like KEY | | `username` | string | true | A user with API access, if applicable | | `password` | string | true | Password of the user | @@ -117,7 +117,7 @@ GET /projects/:id/services/bamboo Bugzilla Issue Tracker -### Create/Edit Buildkite service +### Create/Edit Bugzilla service Set Bugzilla service for a project. @@ -168,7 +168,7 @@ Parameters: | Parameter | Type | Required | Description | | --------- | ---- | -------- | ----------- | | `token` | string | true | Buildkite project GitLab token | -| `project_url` | string | true | https://buildkite.com/example/project | +| `project_url` | string | true | `https://buildkite.com/example/project` | | `enable_ssl_verification` | boolean | false | Enable SSL verification | ### Delete Buildkite service @@ -278,7 +278,7 @@ Parameters: | Parameter | Type | Required | Description | | --------- | ---- | -------- | ----------- | | `token` | string | true | Drone CI project specific token | -| `drone_url` | string | true | http://drone.example.com | +| `drone_url` | string | true | `http://drone.example.com` | | `enable_ssl_verification` | boolean | false | Enable SSL verification | ### Delete Drone CI service @@ -421,7 +421,7 @@ Parameters: | Parameter | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `webhook` | string | true | The Hangouts Chat webhook. e.g. https://chat.googleapis.com/v1/spaces... | +| `webhook` | string | true | The Hangouts Chat webhook. For example, `https://chat.googleapis.com/v1/spaces...`. | | `notify_only_broken_pipelines` | boolean | false | Send notifications for broken pipelines | | `notify_only_default_branch` | boolean | false | Send notifications only for the default branch | | `push_events` | boolean | false | Enable notifications for push events | @@ -470,7 +470,7 @@ Parameters: | `notify` | boolean | false | Enable notifications | | `room` | string | false |Room name or ID | | `api_version` | string | false | Leave blank for default (v2) | -| `server` | string | false | Leave blank for default. https://hipchat.example.com | +| `server` | string | false | Leave blank for default. For example, `https://hipchat.example.com`. | ### Delete HipChat service @@ -496,7 +496,7 @@ Send IRC messages, on update, to a list of recipients through an Irker gateway. Set Irker (IRC gateway) service for a project. -> NOTE: Irker does NOT have built-in authentication, which makes it vulnerable to spamming IRC channels if it is hosted outside of a firewall. Please make sure you run the daemon within a secured network to prevent abuse. For more details, read: http://www.catb.org/~esr/irker/security.html. +> NOTE: Irker does NOT have built-in authentication, which makes it vulnerable to spamming IRC channels if it is hosted outside of a firewall. Please make sure you run the daemon within a secured network to prevent abuse. For more details, read: <http://www.catb.org/~esr/irker/security.html>. ``` PUT /projects/:id/services/irker @@ -546,7 +546,7 @@ Set JIRA service for a project. > **Notes:** > - Starting with GitLab 8.14, `api_url`, `issues_url`, `new_issue_url` and -> `project_url` are replaced by `project_key`, `url`. If you are using an +> `project_url` are replaced by `project_key`, `url`. If you are using an > older version, [follow this documentation][old-jira-api]. ``` @@ -557,7 +557,7 @@ Parameters: | Parameter | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `url` | string | yes | The URL to the JIRA project which is being linked to this GitLab project, e.g., `https://jira.example.com`. | +| `url` | string | yes | The URL to the JIRA project which is being linked to this GitLab project. For example, `https://jira.example.com`. | | `project_key` | string | yes | The short identifier for your JIRA project, all uppercase, e.g., `PROJ`. | | `username` | string | yes | The username of the user created to be used with GitLab/JIRA. | | `password` | string | yes | The password of the user created to be used with GitLab/JIRA. | @@ -589,7 +589,7 @@ PUT /projects/:id/services/kubernetes Parameters: - `namespace` (**required**) - The Kubernetes namespace to use -- `api_url` (**required**) - The URL to the Kubernetes cluster API, e.g., https://kubernetes.example.com +- `api_url` (**required**) - The URL to the Kubernetes cluster API. For example, `https://kubernetes.example.com` - `token` (**required**) - The service token to authenticate against the Kubernetes cluster with - `ca_pem` (optional) - A custom certificate authority bundle to verify the Kubernetes cluster with (PEM format) @@ -658,7 +658,6 @@ Parameters: | --------- | ---- | -------- | ----------- | | `token` | string | yes | The Slack token | - ### Delete Slack slash command service Delete Slack slash command service for a project. @@ -823,7 +822,7 @@ Parameters: | Parameter | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `api_url` | string | true | Prometheus API Base URL, like http://prometheus.example.com/ | +| `api_url` | string | true | Prometheus API Base URL. For example, `http://prometheus.example.com/`. | ### Delete Prometheus service @@ -934,7 +933,7 @@ Parameters: | Parameter | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `webhook` | string | true | https://hooks.slack.com/services/... | +| `webhook` | string | true | `https://hooks.slack.com/services/...` | | `username` | string | false | username | | `channel` | string | false | Default channel to use if others are not configured | | `notify_only_broken_pipelines` | boolean | false | Send notifications for broken pipelines | @@ -988,7 +987,7 @@ Parameters: | Parameter | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `webhook` | string | true | The Microsoft Teams webhook. e.g. https://outlook.office.com/webhook/... | +| `webhook` | string | true | The Microsoft Teams webhook. For example, `https://outlook.office.com/webhook/...` | ### Delete Microsoft Teams service @@ -1024,7 +1023,7 @@ Parameters: | Parameter | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `webhook` | string | true | The Mattermost webhook. e.g. http://mattermost_host/hooks/... | +| `webhook` | string | true | The Mattermost webhook. For example, `http://mattermost_host/hooks/...` | | `username` | string | false | username | | `channel` | string | false | Default channel to use if others are not configured | | `notify_only_broken_pipelines` | boolean | false | Send notifications for broken pipelines | @@ -1080,7 +1079,7 @@ Parameters: | Parameter | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `teamcity_url` | string | true | TeamCity root URL like https://teamcity.example.com | +| `teamcity_url` | string | true | TeamCity root URL. For example, `https://teamcity.example.com` | | `build_type` | string | true | Build configuration ID | | `username` | string | true | A user with permissions to trigger a manual build | | `password` | string | true | The password of the user | @@ -1104,7 +1103,6 @@ GET /projects/:id/services/teamcity [jira-doc]: ../user/project/integrations/jira.md [old-jira-api]: https://gitlab.com/gitlab-org/gitlab-ce/blob/8-13-stable/doc/api/services.md#jira - ## MockCI Mock an external CI. See [`gitlab-org/gitlab-mock-ci-service`](https://gitlab.com/gitlab-org/gitlab-mock-ci-service) for an example of a companion mock service. @@ -1123,7 +1121,7 @@ Parameters: | Parameter | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `mock_service_url` | string | true | http://localhost:4004 | +| `mock_service_url` | string | true | `http://localhost:4004` | ### Delete MockCI service diff --git a/doc/development/documentation/index.md b/doc/development/documentation/index.md index 154ede087cc..b8b86ac1bf5 100644 --- a/doc/development/documentation/index.md +++ b/doc/development/documentation/index.md @@ -41,9 +41,11 @@ how to structure GitLab docs. ## Markdown and styles -Currently GitLab docs use [Kramdown](https://gitlab.com/gitlab-com/gitlab-docs/issues/50) as the [markdown](../../user/markdown.md) engine. +[GitLab docs](https://gitlab.com/gitlab-com/gitlab-docs) uses [GitLab Kramdown](https://gitlab.com/gitlab-org/gitlab_kramdown) +as markdown engine. Check the [GitLab Markdown Kramdown Guide](https://about.gitlab.com/handbook/product/technical-writing/markdown-guide/) +for a complete Kramdown reference. -All the docs follow the [documentation style guidelines](styleguide.md). See [Linting](#linting) for help to follow the guidelines. +Follow the [documentation style guidelines](styleguide.md) strictly. ## Documentation directory structure @@ -198,6 +200,11 @@ redirect_to: '../path/to/file/README.md' It supports both full and relative URLs, e.g. `https://docs.gitlab.com/ee/path/to/file.html`, `../path/to/file.html`, `path/to/file.md`. Note that any `*.md` paths will be compiled to `*.html`. +NOTE: **Note:** +This redirection method will not provide a redirect fallback on GitLab `/help`. When using +it, make sure to add a link to the new page on the doc, otherwise it's a dead end for users that +land on the doc via `/help`. + ### Redirections for pages with Disqus comments If the documentation page being relocated already has any Disqus comments, @@ -223,145 +230,6 @@ redirect_from: 'https://docs.gitlab.com/my-old-location/README.html' Note: it is necessary to include the file name in the `redirect_from` URL, even if it's `index.html` or `README.html`. -## Linting - -To help adhere to the [documentation style guidelines](styleguide.md), and to improve the content -added to documentation, consider locally installing and running documentation linters. This will -help you catch common issues before raising merge requests for review of documentation. - -The following are some suggested linters you can install locally and sample configuration: - -- [`proselint`](#proselint) -- [`markdownlint`](#markdownlint) - -NOTE: **Note:** -This list does not limit what other linters you can add to your local documentation writing toolchain. - -### `proselint` - -`proselint` checks for common problems with English prose. It provides a - [plethora of checks](http://proselint.com/checks/) that are helpful for technical writing. - -`proselint` can be used [on the command line](http://proselint.com/utility/), either on a single - Markdown file or on all Markdown files in a project. For example, to run `proselint` on all - documentation in the [`gitlab-ce` project](https://gitlab.com/gitlab-org/gitlab-ce), run the - following commands from within the `gitlab-ce` project: - -```sh -cd doc -proselint **/*.md -``` - -`proselint` can also be run from within editors using plugins. For example, the following plugins - are available: - -- [Sublime Text](https://packagecontrol.io/packages/SublimeLinter-contrib-proselint) -- [Visual Studio Code](https://marketplace.visualstudio.com/items?itemName=PatrykPeszko.vscode-proselint) -- [Others](https://github.com/amperser/proselint#plugins-for-other-software) - -#### Sample `proselint` configuration - -All of the checks are good to use. However, excluding the `typography.symbols` and `misc.phrasal_adjectives` checks will reduce -noise. The following sample `proselint` configuration disables these checks: - -```json -{ - "checks": { - "typography.symbols": false, - "misc.phrasal_adjectives": false - } -} -``` - -A file with `proselint` configuration must be placed in a -[valid location](https://github.com/amperser/proselint#checks). For example, `~/.config/proselint/config`. - -### `markdownlint` - -`markdownlint` checks that certain rules ([example](https://github.com/DavidAnson/markdownlint/blob/master/README.md#rules--aliases)) - are followed for Markdown syntax. Our [style guidelines](styleguide.md) elaborate on which choices - must be made when selecting Markdown syntax for GitLab documentation and this tool helps - catch deviations from those guidelines. - -`markdownlint` can be used [on the command line](https://github.com/igorshubovych/markdownlint-cli#markdownlint-cli--), - either on a single Markdown file or on all Markdown files in a project. For example, to run - `markdownlint` on all documentation in the [`gitlab-ce` project](https://gitlab.com/gitlab-org/gitlab-ce), - run the following commands from within the `gitlab-ce` project: - -```sh -cd doc -markdownlint **/*.md -``` - -`markdownlint` can also be run from within editors using plugins. For example, the following plugins - are available: - -- [Sublime Text](https://packagecontrol.io/packages/SublimeLinter-contrib-markdownlint) -- [Visual Studio Code](https://marketplace.visualstudio.com/items?itemName=DavidAnson.vscode-markdownlint) -- [Others](https://github.com/DavidAnson/markdownlint#related) - -#### Sample `markdownlint` configuration - -The following sample `markdownlint` configuration modifies the available default rules to: - -- Adhere to the [style guidelines](styleguide.md). -- Apply conventions found in the GitLab documentation. -- Allow the flexibility of using some inline HTML. - -```json -{ - "default": true, - "header-style": { "style": "atx" }, - "ul-style": { "style": "dash" }, - "line-length": false, - "no-trailing-punctuation": false, - "ol-prefix": { "style": "one" }, - "blanks-around-fences": false, - "no-inline-html": { - "allowed_elements": [ - "table", - "tbody", - "tr", - "td", - "ul", - "ol", - "li", - "br", - "img", - "a", - "strong", - "i", - "div" - ] - }, - "hr-style": { "style": "---" }, - "fenced-code-language": false -} -``` - -For [`markdownlint`](https://github.com/DavidAnson/markdownlint/), this configuration must be -placed in a [valid location](https://github.com/igorshubovych/markdownlint-cli#configuration). For -example, `~/.markdownlintrc`. - -## Testing - -We treat documentation as code, thus have implemented some testing. -Currently, the following tests are in place: - -1. `docs lint`: Check that all internal (relative) links work correctly and - that all cURL examples in API docs use the full switches. It's recommended - to [check locally](#previewing-locally) before pushing to GitLab by executing the command - `bundle exec nanoc check internal_links` on your local - [`gitlab-docs`](https://gitlab.com/gitlab-com/gitlab-docs) directory. -1. [`ee_compat_check`](../automatic_ce_ee_merge.md#avoiding-ce-gt-ee-merge-conflicts-beforehand) (runs on CE only): - When you submit a merge request to GitLab Community Edition (CE), - there is this additional job that runs against Enterprise Edition (EE) - and checks if your changes can apply cleanly to the EE codebase. - If that job fails, read the instructions in the job log for what to do next. - As CE is merged into EE once a day, it's important to avoid merge conflicts. - Submitting an EE-equivalent merge request cherry-picking all commits from CE to EE is - essential to avoid them. - ## Branch naming If your contribution contains **only** documentation changes, you can speed up @@ -377,15 +245,6 @@ choices: If your branch name matches any of the above, it will run only the docs tests. If it doesn't, the whole test suite will run (including docs). -## Danger bot - -GitLab uses [danger bot](https://github.com/danger/danger) for some elements in -code review. For docs changes in merge requests, the following actions are taken: - -1. Whenever a change under `/doc` is made, the bot leaves a comment for the - author to mention `@gl-docsteam`, so that the docs can be properly - reviewed. - ## Merge requests for GitLab documentation Before getting started, make sure you read the introductory section @@ -428,105 +287,6 @@ Follow this [method for cherry-picking from CE to EE](../automatic_ce_ee_merge.m additionally to the CE MR. If there are many EE-only changes though, start a new MR to EE only. -## Previewing the changes live - -NOTE: **Note:** -To preview your changes to documentation locally, follow this -[development guide](https://gitlab.com/gitlab-com/gitlab-docs/blob/master/README.md#development-when-contributing-to-gitlab-documentation) or [these instructions for GDK](https://gitlab.com/gitlab-org/gitlab-development-kit/blob/master/doc/howto/gitlab_docs.md). - -The live preview is currently enabled for the following projects: - -- <https://gitlab.com/gitlab-org/gitlab-ce> -- <https://gitlab.com/gitlab-org/gitlab-ee> -- <https://gitlab.com/gitlab-org/gitlab-runner> - -If your branch contains only documentation changes, you can use -[special branch names](#branch-naming) to avoid long running pipelines. - -For [docs-only changes](#branch-naming), the review app is run automatically. -For all other branches, you can use the manual `review-docs-deploy-manual` job -in your merge request. You will need at least Maintainer permissions to be able -to run it. In the mini pipeline graph, you should see an `>>` icon. Clicking on it will -reveal the `review-docs-deploy-manual` job. Hit the play button for the job to start. - - - -NOTE: **Note:** -You will need to push a branch to those repositories, it doesn't work for forks. - -The `review-docs-deploy*` job will: - -1. Create a new branch in the [gitlab-docs](https://gitlab.com/gitlab-com/gitlab-docs) - project named after the scheme: `$DOCS_GITLAB_REPO_SUFFIX-$CI_ENVIRONMENT_SLUG`, - where `DOCS_GITLAB_REPO_SUFFIX` is the suffix for each product, e.g, `ce` for - CE, etc. -1. Trigger a cross project pipeline and build the docs site with your changes - -After a few minutes, the Review App will be deployed and you will be able to -preview the changes. The docs URL can be found in two places: - -- In the merge request widget -- In the output of the `review-docs-deploy*` job, which also includes the - triggered pipeline so that you can investigate whether something went wrong - -TIP: **Tip:** -Someone that has no merge rights to the CE/EE projects (think of forks from -contributors) will not be able to run the manual job. In that case, you can -ask someone from the GitLab team who has the permissions to do that for you. - -NOTE: **Note:** -Make sure that you always delete the branch of the merge request you were -working on. If you don't, the remote docs branch won't be removed either, -and the server where the Review Apps are hosted will eventually be out of -disk space. - -### Troubleshooting review apps - -In case the review app URL returns 404, follow these steps to debug: - -1. **Did you follow the URL from the merge request widget?** If yes, then check if - the link is the same as the one in the job output. -1. **Did you follow the URL from the job output?** If yes, then it means that - either the site is not yet deployed or something went wrong with the remote - pipeline. Give it a few minutes and it should appear online, otherwise you - can check the status of the remote pipeline from the link in the job output. - If the pipeline failed or got stuck, drop a line in the `#docs` chat channel. - -### Technical aspects - -If you want to know the hot details, here's what's really happening: - -1. You manually run the `review-docs-deploy` job in a CE/EE merge request. -1. The job runs the [`scripts/trigger-build-docs`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/scripts/trigger-build-docs) - script with the `deploy` flag, which in turn: - 1. Takes your branch name and applies the following: - - The slug of the branch name is used to avoid special characters since - ultimately this will be used by NGINX. - - The `preview-` prefix is added to avoid conflicts if there's a remote branch - with the same name that you created in the merge request. - - The final branch name is truncated to 42 characters to avoid filesystem - limitations with long branch names (> 63 chars). - 1. The remote branch is then created if it doesn't exist (meaning you can - re-run the manual job as many times as you want and this step will be skipped). - 1. A new cross-project pipeline is triggered in the docs project. - 1. The preview URL is shown both at the job output and in the merge request - widget. You also get the link to the remote pipeline. -1. In the docs project, the pipeline is created and it - [skips the test jobs](https://gitlab.com/gitlab-com/gitlab-docs/blob/8d5d5c750c602a835614b02f9db42ead1c4b2f5e/.gitlab-ci.yml#L50-55) - to lower the build time. -1. Once the docs site is built, the HTML files are uploaded as artifacts. -1. A specific Runner tied only to the docs project, runs the Review App job - that downloads the artifacts and uses `rsync` to transfer the files over - to a location where NGINX serves them. - -The following GitLab features are used among others: - -- [Manual actions](../../ci/yaml/README.md#manual-actions) -- [Multi project pipelines](https://docs.gitlab.com/ee/ci/multi_project_pipeline_graphs.html) -- [Review Apps](../../ci/review_apps/index.md) -- [Artifacts](../../ci/yaml/README.md#artifacts) -- [Specific Runner](../../ci/runners/README.md#locking-a-specific-runner-from-being-enabled-for-other-projects) - ## GitLab `/help` Every GitLab instance includes the documentation, which is available from `/help` @@ -678,5 +438,250 @@ date: 2017-02-01 Use the [writing method](https://about.gitlab.com/handbook/product/technical-writing/#writing-method) defined by the Technical Writing team. +## Previewing the changes live + +NOTE: **Note:** +To preview your changes to documentation locally, follow this +[development guide](https://gitlab.com/gitlab-com/gitlab-docs/blob/master/README.md#development-when-contributing-to-gitlab-documentation) or [these instructions for GDK](https://gitlab.com/gitlab-org/gitlab-development-kit/blob/master/doc/howto/gitlab_docs.md). + +The live preview is currently enabled for the following projects: + +- <https://gitlab.com/gitlab-org/gitlab-ce> +- <https://gitlab.com/gitlab-org/gitlab-ee> +- <https://gitlab.com/gitlab-org/gitlab-runner> + +If your branch contains only documentation changes, you can use +[special branch names](#branch-naming) to avoid long running pipelines. + +For [docs-only changes](#branch-naming), the review app is run automatically. +For all other branches, you can use the manual `review-docs-deploy-manual` job +in your merge request. You will need at least Maintainer permissions to be able +to run it. In the mini pipeline graph, you should see an `>>` icon. Clicking on it will +reveal the `review-docs-deploy-manual` job. Hit the play button for the job to start. + + + +NOTE: **Note:** +You will need to push a branch to those repositories, it doesn't work for forks. + +The `review-docs-deploy*` job will: + +1. Create a new branch in the [gitlab-docs](https://gitlab.com/gitlab-com/gitlab-docs) + project named after the scheme: `$DOCS_GITLAB_REPO_SUFFIX-$CI_ENVIRONMENT_SLUG`, + where `DOCS_GITLAB_REPO_SUFFIX` is the suffix for each product, e.g, `ce` for + CE, etc. +1. Trigger a cross project pipeline and build the docs site with your changes + +After a few minutes, the Review App will be deployed and you will be able to +preview the changes. The docs URL can be found in two places: + +- In the merge request widget +- In the output of the `review-docs-deploy*` job, which also includes the + triggered pipeline so that you can investigate whether something went wrong + +TIP: **Tip:** +Someone that has no merge rights to the CE/EE projects (think of forks from +contributors) will not be able to run the manual job. In that case, you can +ask someone from the GitLab team who has the permissions to do that for you. + +NOTE: **Note:** +Make sure that you always delete the branch of the merge request you were +working on. If you don't, the remote docs branch won't be removed either, +and the server where the Review Apps are hosted will eventually be out of +disk space. + +### Troubleshooting review apps + +In case the review app URL returns 404, follow these steps to debug: + +1. **Did you follow the URL from the merge request widget?** If yes, then check if + the link is the same as the one in the job output. +1. **Did you follow the URL from the job output?** If yes, then it means that + either the site is not yet deployed or something went wrong with the remote + pipeline. Give it a few minutes and it should appear online, otherwise you + can check the status of the remote pipeline from the link in the job output. + If the pipeline failed or got stuck, drop a line in the `#docs` chat channel. + +### Technical aspects + +If you want to know the in-depth details, here's what's really happening: + +1. You manually run the `review-docs-deploy` job in a CE/EE merge request. +1. The job runs the [`scripts/trigger-build-docs`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/scripts/trigger-build-docs) + script with the `deploy` flag, which in turn: + 1. Takes your branch name and applies the following: + - The slug of the branch name is used to avoid special characters since + ultimately this will be used by NGINX. + - The `preview-` prefix is added to avoid conflicts if there's a remote branch + with the same name that you created in the merge request. + - The final branch name is truncated to 42 characters to avoid filesystem + limitations with long branch names (> 63 chars). + 1. The remote branch is then created if it doesn't exist (meaning you can + re-run the manual job as many times as you want and this step will be skipped). + 1. A new cross-project pipeline is triggered in the docs project. + 1. The preview URL is shown both at the job output and in the merge request + widget. You also get the link to the remote pipeline. +1. In the docs project, the pipeline is created and it + [skips the test jobs](https://gitlab.com/gitlab-com/gitlab-docs/blob/8d5d5c750c602a835614b02f9db42ead1c4b2f5e/.gitlab-ci.yml#L50-55) + to lower the build time. +1. Once the docs site is built, the HTML files are uploaded as artifacts. +1. A specific Runner tied only to the docs project, runs the Review App job + that downloads the artifacts and uses `rsync` to transfer the files over + to a location where NGINX serves them. + +The following GitLab features are used among others: + +- [Manual actions](../../ci/yaml/README.md#manual-actions) +- [Multi project pipelines](https://docs.gitlab.com/ee/ci/multi_project_pipeline_graphs.html) +- [Review Apps](../../ci/review_apps/index.md) +- [Artifacts](../../ci/yaml/README.md#artifacts) +- [Specific Runner](../../ci/runners/README.md#locking-a-specific-runner-from-being-enabled-for-other-projects) + +## Testing + +We treat documentation as code, thus have implemented some testing. +Currently, the following tests are in place: + +1. `docs lint`: Check that all internal (relative) links work correctly and + that all cURL examples in API docs use the full switches. It's recommended + to [check locally](#previewing-locally) before pushing to GitLab by executing the command + `bundle exec nanoc check internal_links` on your local + [`gitlab-docs`](https://gitlab.com/gitlab-com/gitlab-docs) directory. +1. [`ee_compat_check`](../automatic_ce_ee_merge.md#avoiding-ce-gt-ee-merge-conflicts-beforehand) (runs on CE only): + When you submit a merge request to GitLab Community Edition (CE), + there is this additional job that runs against Enterprise Edition (EE) + and checks if your changes can apply cleanly to the EE codebase. + If that job fails, read the instructions in the job log for what to do next. + As CE is merged into EE once a day, it's important to avoid merge conflicts. + Submitting an EE-equivalent merge request cherry-picking all commits from CE to EE is + essential to avoid them. + +### Linting + +To help adhere to the [documentation style guidelines](styleguide.md), and to improve the content +added to documentation, consider locally installing and running documentation linters. This will +help you catch common issues before raising merge requests for review of documentation. + +The following are some suggested linters you can install locally and sample configuration: + +- [`proselint`](#proselint) +- [`markdownlint`](#markdownlint) + +NOTE: **Note:** +This list does not limit what other linters you can add to your local documentation writing toolchain. + +#### `proselint` + +`proselint` checks for common problems with English prose. It provides a + [plethora of checks](http://proselint.com/checks/) that are helpful for technical writing. + +`proselint` can be used [on the command line](http://proselint.com/utility/), either on a single + Markdown file or on all Markdown files in a project. For example, to run `proselint` on all + documentation in the [`gitlab-ce` project](https://gitlab.com/gitlab-org/gitlab-ce), run the + following commands from within the `gitlab-ce` project: + +```sh +cd doc +proselint **/*.md +``` + +`proselint` can also be run from within editors using plugins. For example, the following plugins + are available: + +- [Sublime Text](https://packagecontrol.io/packages/SublimeLinter-contrib-proselint) +- [Visual Studio Code](https://marketplace.visualstudio.com/items?itemName=PatrykPeszko.vscode-proselint) +- [Others](https://github.com/amperser/proselint#plugins-for-other-software) + +##### Sample `proselint` configuration + +All of the checks are good to use. However, excluding the `typography.symbols` and `misc.phrasal_adjectives` checks will reduce +noise. The following sample `proselint` configuration disables these checks: + +```json +{ + "checks": { + "typography.symbols": false, + "misc.phrasal_adjectives": false + } +} +``` + +A file with `proselint` configuration must be placed in a +[valid location](https://github.com/amperser/proselint#checks). For example, `~/.config/proselint/config`. + +#### `markdownlint` + +`markdownlint` checks that certain rules ([example](https://github.com/DavidAnson/markdownlint/blob/master/README.md#rules--aliases)) + are followed for Markdown syntax. Our [style guidelines](styleguide.md) elaborate on which choices + must be made when selecting Markdown syntax for GitLab documentation and this tool helps + catch deviations from those guidelines. + +`markdownlint` can be used [on the command line](https://github.com/igorshubovych/markdownlint-cli#markdownlint-cli--), + either on a single Markdown file or on all Markdown files in a project. For example, to run + `markdownlint` on all documentation in the [`gitlab-ce` project](https://gitlab.com/gitlab-org/gitlab-ce), + run the following commands from within the `gitlab-ce` project: + +```sh +cd doc +markdownlint **/*.md +``` + +`markdownlint` can also be run from within editors using plugins. For example, the following plugins + are available: + +- [Sublime Text](https://packagecontrol.io/packages/SublimeLinter-contrib-markdownlint) +- [Visual Studio Code](https://marketplace.visualstudio.com/items?itemName=DavidAnson.vscode-markdownlint) +- [Others](https://github.com/DavidAnson/markdownlint#related) + +##### Sample `markdownlint` configuration + +The following sample `markdownlint` configuration modifies the available default rules to: + +- Adhere to the [style guidelines](styleguide.md). +- Apply conventions found in the GitLab documentation. +- Allow the flexibility of using some inline HTML. + +```json +{ + "default": true, + "header-style": { "style": "atx" }, + "ul-style": { "style": "dash" }, + "line-length": false, + "no-trailing-punctuation": false, + "ol-prefix": { "style": "one" }, + "blanks-around-fences": false, + "no-inline-html": { + "allowed_elements": [ + "table", + "tbody", + "tr", + "td", + "ul", + "ol", + "li", + "br", + "img", + "a", + "strong", + "i", + "div" + ] + }, + "hr-style": { "style": "---" }, + "fenced-code-language": false +} +``` + +For [`markdownlint`](https://github.com/DavidAnson/markdownlint/), this configuration must be +placed in a [valid location](https://github.com/igorshubovych/markdownlint-cli#configuration). For +example, `~/.markdownlintrc`. + +## Danger bot + +GitLab uses [danger bot](https://github.com/danger/danger) for some elements in +code review. For docs changes in merge requests, whenever a change under `/doc` +is made, the bot leaves a comment for the author to mention `@gl-docsteam`, so +that the docs can be properly reviewed. + [gitlab-map]: https://gitlab.com/gitlab-org/gitlab-design/raw/master/production/resources/gitlab-map.png [graffle]: https://gitlab.com/gitlab-org/gitlab-design/blob/d8d39f4a87b90fb9ae89ca12dc565347b4900d5e/production/resources/gitlab-map.graffle diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md index 97b1b890836..8309ba9a72c 100644 --- a/doc/development/documentation/styleguide.md +++ b/doc/development/documentation/styleguide.md @@ -10,17 +10,15 @@ GitLab documentation. Check the Check the GitLab handbook for the [writing styles guidelines](https://about.gitlab.com/handbook/communication/#writing-style-guidelines). -For help adhering to the guidelines, see [Linting](index.md#linting). +For help adhering to the guidelines, see [linting](index.md#linting). ## Files - [Directory structure](index.md#location-and-naming-documents): place the docs - in the correct location. +in the correct location. - [Documentation files](index.md#documentation-files): name the files accordingly. -- [Markdown](../../user/markdown.md): use the GitLab Flavored Markdown in the - documentation. -NOTE: **Note:** +DANGER: **Attention:** **Do not** use capital letters, spaces, or special chars in file names, branch names, directory names, headings, or in anything that generates a path. @@ -28,65 +26,144 @@ NOTE: **Note:** **Do not** create new `README.md` files, name them `index.md` instead. There's a test that will fail if it spots a new `README.md` file. -## Text +### Markdown + +The [documentation website](https://docs.gitlab.com) had its markdown engine migrated from [Redcarpet to GitLab Kramdown](https://gitlab.com/gitlab-com/gitlab-docs/merge_requests/108) +in October, 2018. + +The [`gitlab-kramdown`](https://gitlab.com/gitlab-org/gitlab_kramdown) +gem will support all [GFM markup](../../user/markdown.md) in the future. For now, +use regular markdown markup, following the rules on this style guide. For a complete +Kramdown reference, check the [GiLab Markdown Kramdown Guide](https://about.gitlab.com/handbook/product/technical-writing/markdown-guide/). +Use Kramdown markup wisely: do not overuse its specific markup (e.g., `{:.class}`) as it will not render properly in +[`/help`](#gitlab-help). + +## Content -- Split up long lines (wrap text), this makes it much easier to review and edit. Only - double line breaks are shown as a full line break in [GitLab markdown][gfm]. - 80-100 characters is a good line length. - Make sure that the documentation is added in the correct - [directory](index.md#documentation-directory-structure) and that - there's a link to it somewhere useful. + [directory](index.md#documentation-directory-structure), linked from its + higher-level index, and linked from other related pages. - Do not duplicate information. - Be brief and clear. -- Unless there's a logical reason not to, add documents in alphabetical order. +- Unless there's a logical reason not to, structure the document in alphabetical order +(headings, tables, and lists). - Write in US English. -- Use [single spaces][] instead of double spaces. -- Jump a line between different markups (e.g., after every paragraph, header, list, etc) - Capitalize "G" and "L" in GitLab. -- Use sentence case for titles, headings, labels, menu items, and buttons. - Use title case when referring to [features](https://about.gitlab.com/features/) or - [products](https://about.gitlab.com/pricing/) (e.g., GitLab Runner, Geo, - Issue Boards, GitLab Core, Git, Prometheus, Kubernetes, etc), and methods or methodologies - (e.g., Continuous Integration, Continuous Deployment, Scrum, Agile, etc). Note that - some features are also objects (e.g. "Merge Requests" and "merge requests"). +[products](https://about.gitlab.com/pricing/) (e.g., GitLab Runner, Geo, +Issue Boards, GitLab Core, Git, Prometheus, Kubernetes, etc), and methods or methodologies +(e.g., Continuous Integration, Continuous Deployment, Scrum, Agile, etc). Note that +some features are also objects (e.g. "GitLab's Merge Requests support X." and "Create a new merge request for Z."). -## Formatting +## Text + +- Split up long lines (wrap text), this makes it much easier to review and edit. Only + double line breaks are shown as a full line break by creating new paragraphs. + 80-100 characters is the recommended line length. +- Use sentence case for titles, headings, labels, menu items, and buttons. +- Jump a line between different markups (e.g., after every paragraph, header, list, etc). Example: -- Use double asterisks (`**`) to mark a word or text in bold (`**bold**`). -- Use undescore (`_`) for text in italics (`_italic_`). -- Put an empty line between different markups. For example: ```md ## Header Paragraph. - - List item - - List item + - List item 1 + - List item 2 ``` -### Punctuation +## Emphasis -For punctuation rules, please refer to the [GitLab UX guide](https://design.gitlab.com/content/punctuation/). +- Use double asterisks (`**`) to mark a word or text in bold (`**bold**`). +- Use undescore (`_`) for text in italics (`_italic_`). +- Use greater than (`>`) for blockquotes. + +## Punctuation + +Check the general punctuation rules for the GitLab documentation on the table below. +Check specific punctuation rules for [list items](#list-items) below. + +| Rule | Example | +| ---- | ------- | +| Always end full sentences with a period. | _For a complete overview, read through this document._| +| Always add a space after a period when beginning a new sentence | _For a complete overview, check this doc. For other references, check out this guide._ | +| Do not use double spaces. | --- | +| Do not use tabs for indentation. Use spaces instead. You can configure your code editor to output spaces instead of tabs when pressing the tab key. | --- | +| Use serial commas ("Oxford commas") before the final 'and/or' in a list. | _You can create new issues, merge requests, and milestones._ | +| Always add a space before and after dashes when using it in a sentence (for replacing a comma, for example). | _You should try this - or not._ | +| Always use lowercase after a colon. | _Related Issues: a way to create a relationship between issues._ | + +## List items + +- Always start list items with a capital letter. +- Always leave a blank line before and after a list. +- Begin a line with spaces (not tabs) to denote a subitem. +- To nest subitems, indent them with two spaces. +- To nest code blocks, indent them with four spaces. +- Only use ordered lists when their items describe a sequence of steps to follow. -### Ordered and unordered lists +**Markup:** -- Use dashes (`-`) for unordered lists instead of asterisks (`*`). -- Use the number one (`1`) for ordered lists. +- Use dashes (`- `) for unordered lists instead of asterisks (`* `). +- Use the number one (`1`) for each item in an ordered list. +When rendered, the list items will appear with sequential numbering. + +**Punctuation:** + +- Do not add commas (`,`) or semicolons (`;`) to the end of a list item. +- Only add periods to the end of a list item if the item consists of a complete sentence. The [definition of full sentence](https://www2.le.ac.uk/offices/ld/resources/writing/grammar/grammar-guides/sentence) is: _"a complete sentence always contains a verb, expresses a complete idea, and makes sense standing alone"_. +- Be consistent throughout the list: if the majority of the items do not end in a period, do not end any of the items in a period, even if they consist of a complete sentence. The opposite is also valid: if the majority of the items end with a period, end all with a period. - Separate list items from explanatory text with a colon (`:`). For example: + ```md The list is as follows: - - First item: This explains the first item. - - Second item: This explains the second item. + - First item: this explains the first item. + - Second item: this explains the second item. ``` -- For further guidance on punctuation in bullet lists, please refer to the [GitLab UX guide](https://design.gitlab.com/content/punctuation/). + +**Examples:** + +Do: + +- First list item +- Second list item +- Third list item + +Don't: + +- First list item +- Second list item +- Third list item. + +Do: + +- Let's say this is a complete sentence. +- Let's say this is also a complete sentence. +- Not a complete sentence. + +Don't: + +- Let's say this is a complete sentence. +- Let's say this is also a complete sentence. +- Not a complete sentence + +## Quotes + +Valid for markdown content only, not for frontmatter entries: + +- Standard quotes: double quotes (`"`). Example: "This is wrapped in double quotes". +- Quote within a quote: double quotes (`"`) wrap single quotes (`'`). Example: "I am 'quoting' something within a quote". + +For other punctuation rules, please refer to the +[GitLab UX guide](https://design.gitlab.com/content/punctuation/). ## Headings - Add **only one H1** in each document, by adding `#` at the beginning of it (when using markdown). The `h1` will be the document `<title>`. -- Start with an h2 (`##`), and respect the order h2 > h3 > h4 > h5 > h6. - Never skip the hierarchy level, such as h2 > h4 +- Start with an `h2` (`##`), and respect the order `h2` > `h3` > `h4` > `h5` > `h6`. + Never skip the hierarchy level, such as `h2` > `h4` - Avoid putting numbers in headings. Numbers shift, hence documentation anchor links shift too, which eventually leads to dead links. If you think it is compelling to add numbers in headings, make sure to at least discuss it with @@ -96,21 +173,19 @@ For punctuation rules, please refer to the [GitLab UX guide](https://design.gitl - Avoid adding things that show ephemeral statuses. For example, if a feature is considered beta or experimental, put this info in a note, not in the heading. - When introducing a new document, be careful for the headings to be - grammatically and syntactically correct. Mention one or all - of the following GitLab members for a review: `@axil` or `@marcia`. + grammatically and syntactically correct. Mention an [assigned technical writer (TW)](https://about.gitlab.com/handbook/product/categories/) + for review. This is to ensure that no document with wrong heading is going live without an audit, thus preventing dead links and redirection issues when corrected. - Leave exactly one new line after a heading. +- Do not use links in headings. +- Add the corresponding [product badge](#product-badges) according to the tier the feature belongs. ## Links -- Use the regular inline link markdown markup `[Text](https://example.com)`. - It's easier to read, review, and maintain. -- If there's a link that repeats several times through the same document, - you can use `[Text][identifier]` and at the bottom of the section or the - document add: `[identifier]: https://example.com`, in which case, we do - encourage you to also add an alternative text: `[identifier]: https://example.com "Alternative text"` that appears when hovering your mouse on a link. +- Use inline link markdown markup `[Text](https://example.com)`. + It's easier to read, review, and maintain. **Do not** use `[Text][identifier]`. - To link to internal documentation, use relative links, not full URLs. Use `../` to navigate tp high-level directories, and always add the file name `file.md` at the end of the link with the `.md` extension, not `.html`. @@ -128,11 +203,12 @@ For punctuation rules, please refer to the [GitLab UX guide](https://design.gitl To indicate the steps of navigation through the UI: + - Use the exact word as shown in the UI, including any capital letters as-is. -- Use bold text for navigation items and the char `>` as separator - (e.g., `Navigate to your project's **Settings > CI/CD**` ). +- Use bold text for navigation items and the char "greater than" (`>`) as separator +(e.g., `Navigate to your project's **Settings > CI/CD**` ). - If there are any expandable menus, make sure to mention that the user - needs to expand the tab to find the settings you're referring to. +needs to expand the tab to find the settings you're referring to (e.g., `Navigate to your project's **Settings > CI/CD** and expand **General pipelines**`). ## Images @@ -141,13 +217,13 @@ To indicate the steps of navigation through the UI: names with the name of the document that they will be included in. For example, if there is a document called `twitter.md`, then a valid image name could be `twitter_login_screen.png`. -- Images should have a specific, non-generic name that will differentiate them. +- Images should have a specific, non-generic name that will differentiate and describe them properly. - Keep all file names in lower case. - Consider using PNG images instead of JPEG. - Compress all images with <https://tinypng.com/> or similar tool. - Compress gifs with <https://ezgif.com/optimize> or similar tool. - Images should be used (only when necessary) to _illustrate_ the description - of a process, not to _replace_ it. +of a process, not to _replace_ it. - Max image size: 100KB (gifs included). - The GitLab docs do not support videos yet. @@ -164,13 +240,39 @@ Inside the document: - If a heading is placed right after an image, always add three dashes (`---`) between the image and the heading. +## Code blocks + +- Always wrap code added to a sentence in inline code blocks (``` ` ```). +E.g., `.gitlab-ci.yml`, `git add .`, `CODEOWNERS`, `only: master`. +File names, commands, entries, and anything that refers to code should be added to code blocks. +To make things easier for the user, always add a full code block for things that can be +useful to copy and paste, as they can easily do it with the button on code blocks. +- For regular code blocks, always use a highlighting class corresponding to the +language for better readability. Examples: + + ```md + ```ruby + Ruby code + ``` + + ```js + JavaScript code + ``` + + ```md + Markdown code + ``` + ``` + +- For a complete reference on code blocks, check the [Kramdown guide](https://about.gitlab.com/handbook/product/technical-writing/markdown-guide/#code-blocks). + ## Alert boxes Whenever you want to call the attention to a particular sentence, use the following markup for highlighting. _Note that the alert boxes only work for one paragraph only. Multiple paragraphs, -lists, headers, etc will not render correctly._ +lists, headers, etc will not render correctly. For multiple lines, use blockquotes instead._ ### Note @@ -234,6 +336,31 @@ which renders in docs.gitlab.com to: If the text spans across multiple lines it's OK to split the line. +For multiple paragraphs, use the symbol `>` before every line: + +```md +> This is the first paragraph. +> +> This is the second paragraph. +> +> - This is a list item +> - Second item in the list +> +> ### This is an `h3` +``` + +Which renders to: + +> This is the first paragraph. +> +> This is the second paragraph. +> +> - This is a list item +> - Second item in the list +> +> ### This is an `h3` +>{:.no_toc} + ## Specific sections and terms To mention and/or reference specific terms in GitLab, please follow the styles @@ -290,18 +417,18 @@ feature availability: To exclude GitLab.com tiers (when the feature is not available in GitLab.com), add the keyword "only": +- For GitLab Core: `**[CORE ONLY]**`. - For GitLab Starter: `**[STARTER ONLY]**`. - For GitLab Premium: `**[PREMIUM ONLY]**`. - For GitLab Ultimate: `**[ULTIMATE ONLY]**`. -- For GitLab Core: `**[CORE ONLY]**`. The tier should be ideally added to headers, so that the full badge will be displayed. However, it can be also mentioned from paragraphs, list items, and table cells. For these cases, -the tier mention will be represented by an orange question mark. +the tier mention will be represented by an orange question mark that will show the tiers on hover. E.g., `**[STARTER]**` renders **[STARTER]**, `**[STARTER ONLY]**` renders **[STARTER ONLY]**. The absence of tiers' mentions mean that the feature is available in GitLab Core, -GitLab.com Free, and higher tiers. +GitLab.com Free, and all higher tiers. #### How it works @@ -348,8 +475,8 @@ prefer to document it in the CE docs to avoid duplication. Configuration settings include: -- Settings that touch configuration files in `config/`. -- NGINX settings and settings in `lib/support/` in general. +1. Settings that touch configuration files in `config/`. +1. NGINX settings and settings in `lib/support/` in general. When there is a list of steps to perform, usually that entails editing the configuration file and reconfiguring/restarting GitLab. In such case, follow @@ -386,13 +513,13 @@ the style below as a guide: In this case: -- before each step list the installation method is declared in bold -- three dashes (`---`) are used to create a horizontal line and separate the +- Before each step list the installation method is declared in bold +- Three dashes (`---`) are used to create a horizontal line and separate the two methods -- the code blocks are indented one or more spaces under the list item to render +- The code blocks are indented one or more spaces under the list item to render correctly -- different highlighting languages are used for each config in the code block -- the [references](#references) guide is used for reconfigure/restart +- Different highlighting languages are used for each config in the code block +- The [references](#references) guide is used for reconfigure/restart ### Fake tokens diff --git a/doc/development/fe_guide/vue.md b/doc/development/fe_guide/vue.md index f6cbd11042c..ccfd465531a 100644 --- a/doc/development/fe_guide/vue.md +++ b/doc/development/fe_guide/vue.md @@ -221,6 +221,14 @@ const vm = mountComponent(Component, data); The main return value of a Vue component is the rendered output. In order to test the component we need to test the rendered output. [Vue][vue-test] guide's to unit test show us exactly that: +## Vue.js Expert Role +One should apply to be a Vue.js expert by opening an MR when the Merge Request's they create and review show: +- Deep understanding of Vue and Vuex reactivy +- Vue and Vuex code are structured according to both official and our guidelines +- Full understanding of testing a Vue and Vuex application +- Vuex code follows the [documented pattern](./vuex.md#actions-pattern-request-and-receive-namespaces) +- Knowledge about the existing Vue and Vuex applications and existing reusable components + [vue-docs]: http://vuejs.org/guide/index.html [issue-boards]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/app/assets/javascripts/boards diff --git a/doc/development/testing_guide/end_to_end_tests.md b/doc/development/testing_guide/end_to_end_tests.md index 21ec926414d..e9f236c6b3a 100644 --- a/doc/development/testing_guide/end_to_end_tests.md +++ b/doc/development/testing_guide/end_to_end_tests.md @@ -1,32 +1,37 @@ -# End-to-End Testing +# End-to-end Testing -## What is End-to-End testing? +## What is end-to-end testing? -End-to-End testing is a strategy used to check whether your application works -as expected across entire software stack and architecture, including -integration of all microservices and components that are supposed to work +End-to-end testing is a strategy used to check whether your application works +as expected across the entire software stack and architecture, including +integration of all micro-services and components that are supposed to work together. ## How do we test GitLab? We use [Omnibus GitLab][omnibus-gitlab] to build GitLab packages and then we -test these packages using [GitLab QA][gitlab-qa] project, which is entirely -black-box, click-driven testing framework. +test these packages using the [GitLab QA orchestrator][gitlab-qa] tool, which is +a black-box testing framework for the API and the UI. ### Testing nightly builds We run scheduled pipeline each night to test nightly builds created by Omnibus. -You can find these nightly pipelines at [GitLab QA pipelines page][gitlab-qa-pipelines]. +You can find these nightly pipelines at [gitlab-org/quality/nightly/pipelines][quality-nightly-pipelines]. + +### Testing staging + +We run scheduled pipeline each night to test staging. +You can find these nightly pipelines at [gitlab-org/quality/staging/pipelines][quality-staging-pipelines]. ### Testing code in merge requests It is possible to run end-to-end tests (eventually being run within a [GitLab QA pipeline][gitlab-qa-pipelines]) for a merge request by triggering -the `package-and-qa` manual action, that should be present in a merge request -widget. +the `package-and-qa` manual action in the `test` stage, that should be present +in a merge request widget (unless the merge request is from a fork). Manual action that starts end-to-end tests is also available in merge requests -in Omnibus GitLab project. +in [Omnibus GitLab][omnibus-gitlab]. Below you can read more about how to use it and how does it work. @@ -35,46 +40,56 @@ Below you can read more about how to use it and how does it work. Currently, we are using _multi-project pipeline_-like approach to run QA pipelines. -1. Developer triggers a manual action, that can be found in CE and EE merge +1. Developer triggers a manual action, that can be found in CE / EE merge requests. This starts a chain of pipelines in multiple projects. -1. The script being executed triggers a pipeline in GitLab Omnibus and waits -for the resulting status. We call this a _status attribution_. +1. The script being executed triggers a pipeline in [Omnibus GitLab][omnibus-gitlab] +and waits for the resulting status. We call this a _status attribution_. -1. GitLab packages are being built in Omnibus pipeline. Packages are going to be -pushed to Container Registry. +1. GitLab packages are being built in the [Omnibus GitLab][omnibus-gitlab] +pipeline. Packages are then pushed to its Container Registry. 1. When packages are ready, and available in the registry, a final step in the -pipeline, that is now running in Omnibus, triggers a new pipeline in the GitLab -QA project. It also waits for a resulting status. +[Omnibus GitLab][omnibus-gitlab] pipeline, triggers a new +[GitLab QA pipeline][gitlab-qa-pipelines]. It also waits for a resulting status. 1. GitLab QA pulls images from the registry, spins-up containers and runs tests against a test environment that has been just orchestrated by the `gitlab-qa` tool. -1. The result of the GitLab QA pipeline is being propagated upstream, through -Omnibus, back to CE / EE merge request. +1. The result of the [GitLab QA pipeline][gitlab-qa-pipelines] is being +propagated upstream, through Omnibus, back to the CE / EE merge request. #### How do I write tests? In order to write new tests, you first need to learn more about GitLab QA -architecture. See the [documentation about it][gitlab-qa-architecture] in -GitLab QA project. +architecture. See the [documentation about it][gitlab-qa-architecture]. -Once you decided where to put test environment orchestration scenarios and -instance specs, take a look at the [relevant documentation][instance-qa-readme] -and examples in [the `qa/` directory][instance-qa-examples]. +Once you decided where to put [test environment orchestration scenarios] and +[instance-level scenarios], take a look at the [GitLab QA README][instance-qa-readme], +the [GitLab QA orchestrator README][gitlab-qa-readme], and [the already existing +instance-level scenarios][instance-level scenarios]. ## Where can I ask for help? You can ask question in the `#quality` channel on Slack (GitLab internal) or you can find an issue you would like to work on in -[the issue tracker][gitlab-qa-issues] and start a new discussion there. +[the `gitlab-ce` issue tracker][gitlab-ce-issues], +[the `gitlab-ee` issue tracker][gitlab-ce-issues], or +[the `gitlab-qa` issue tracker][gitlab-qa-issues]. [omnibus-gitlab]: https://gitlab.com/gitlab-org/omnibus-gitlab [gitlab-qa]: https://gitlab.com/gitlab-org/gitlab-qa +[gitlab-qa-readme]: https://gitlab.com/gitlab-org/gitlab-qa/tree/master/README.md [gitlab-qa-pipelines]: https://gitlab.com/gitlab-org/gitlab-qa/pipelines +[quality-nightly-pipelines]: https://gitlab.com/gitlab-org/quality/nightly/pipelines +[quality-staging-pipelines]: https://gitlab.com/gitlab-org/quality/staging/pipelines [gitlab-qa-architecture]: https://gitlab.com/gitlab-org/gitlab-qa/blob/master/docs/architecture.md -[gitlab-qa-issues]: https://gitlab.com/gitlab-org/gitlab-qa/issues +[gitlab-qa-issues]: https://gitlab.com/gitlab-org/gitlab-qa/issues?label_name%5B%5D=new+scenario +[gitlab-ce-issues]: https://gitlab.com/gitlab-org/gitlab-ce/issues?label_name[]=QA&label_name[]=test +[gitlab-ee-issues]: https://gitlab.com/gitlab-org/gitlab-ee/issues?label_name[]=QA&label_name[]=test +[test environment orchestration scenarios]: https://gitlab.com/gitlab-org/gitlab-qa/tree/master/lib/gitlab/qa/scenario +[instance-level scenarios]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/qa/qa/specs/features +[Page objects documentation]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/qa/qa/page/README.md [instance-qa-readme]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/qa/README.md [instance-qa-examples]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/qa/qa diff --git a/doc/development/testing_guide/smoke.md b/doc/development/testing_guide/smoke.md index 3f2843cba6e..3360031c220 100644 --- a/doc/development/testing_guide/smoke.md +++ b/doc/development/testing_guide/smoke.md @@ -1,8 +1,9 @@ # Smoke Tests -It is imperative in any testing suite that we have Smoke Tests. In short, smoke tests are will run quick sanity -end-to-end functional tests from GitLab QA and are designed to run against the specified environment to ensure that -basic functionality is working. +It is imperative in any testing suite that we have Smoke Tests. In short, smoke +tests will run quick sanity end-to-end functional tests from GitLab QA and are +designed to run against the specified environment to ensure that basic +functionality is working. Currently, our suite consists of this basic functionality coverage: @@ -11,6 +12,8 @@ Currently, our suite consists of this basic functionality coverage: - Issue Creation - Merge Request Creation +Smoke tests have the `:smoke` RSpec metadata. + --- [Return to Testing documentation](index.md) diff --git a/doc/development/testing_guide/testing_levels.md b/doc/development/testing_guide/testing_levels.md index 32ed22ca3ed..a8671fc3aa3 100644 --- a/doc/development/testing_guide/testing_levels.md +++ b/doc/development/testing_guide/testing_levels.md @@ -34,7 +34,11 @@ records should use stubs/doubles as much as possible. Formal definition: https://en.wikipedia.org/wiki/Integration_testing -These kind of tests ensure that individual parts of the application work well together, without the overhead of the actual app environment (i.e. the browser). These tests should assert at the request/response level: status code, headers, body. They're useful to test permissions, redirections, what view is rendered etc. +These kind of tests ensure that individual parts of the application work well +together, without the overhead of the actual app environment (i.e. the browser). +These tests should assert at the request/response level: status code, headers, +body. +They're useful to test permissions, redirections, what view is rendered etc. | Code path | Tests path | Testing engine | Notes | | --------- | ---------- | -------------- | ----- | @@ -67,20 +71,40 @@ run JavaScript tests, so you can either run unit tests (e.g. test a single JavaScript method), or integration tests (e.g. test a component that is composed of multiple components). -## System tests or feature tests +## White-box tests at the system level (formerly known as System / Feature tests) -Formal definition: https://en.wikipedia.org/wiki/System_testing. +Formal definitions: -These kind of tests ensure the application works as expected from a user point -of view (aka black-box testing). These tests should test a happy path for a -given page or set of pages, and a test case should be added for any regression +- https://en.wikipedia.org/wiki/System_testing +- https://en.wikipedia.org/wiki/White-box_testing + +These kind of tests ensure the GitLab *Rails* application (i.e. +`gitlab-ce`/`gitlab-ee`) works as expected from a *browser* point of view. + +Note that: + +- knowledge of the internals of the application are still required +- data needed for the tests are usually created directly using RSpec factories +- expectations are often set on the database or objects state + +These tests should only be used when: + +- the functionality/component being tested is small +- the internal state of the objects/database *needs* to be tested +- it cannot be tested at a lower level + +For instance, to test the breadcrumbs on a given page, writing a system test +makes sense since it's a small component, which cannot be tested at the unit or +controller level. + +Only test the happy path, but make sure to add a test case for any regression that couldn't have been caught at lower levels with better tests (i.e. if a regression is found, regression tests should be added at the lowest-level possible). | Tests path | Testing engine | Notes | | ---------- | -------------- | ----- | -| `spec/features/` | [Capybara] + [RSpec] | If your spec has the `:js` metadata, the browser driver will be [Poltergeist], otherwise it's using [RackTest]. | +| `spec/features/` | [Capybara] + [RSpec] | If your test has the `:js` metadata, the browser driver will be [Poltergeist], otherwise it's using [RackTest]. | ### Consider **not** writing a system test! @@ -89,7 +113,7 @@ we have enough Unit & Integration tests), we shouldn't need to duplicate their thorough testing at the System test level. It's very easy to add tests, but a lot harder to remove or improve tests, so one -should take care of not introducing too many (slow and duplicated) specs. +should take care of not introducing too many (slow and duplicated) tests. The reasons why we should follow these best practices are as follows: @@ -107,29 +131,33 @@ The reasons why we should follow these best practices are as follows: [Poltergeist]: https://github.com/teamcapybara/capybara#poltergeist [RackTest]: https://github.com/teamcapybara/capybara#racktest -## Black-box tests or end-to-end tests +## Black-box tests at the system level, aka end-to-end tests + +Formal definitions: + +- https://en.wikipedia.org/wiki/System_testing +- https://en.wikipedia.org/wiki/Black-box_testing GitLab consists of [multiple pieces] such as [GitLab Shell], [GitLab Workhorse], [Gitaly], [GitLab Pages], [GitLab Runner], and GitLab Rails. All theses pieces are configured and packaged by [GitLab Omnibus]. -[GitLab QA] is a tool that allows to test that all these pieces integrate well -together by building a Docker image for a given version of GitLab Rails and -running feature tests (i.e. using Capybara) against it. +The QA framework and instance-level scenarios are [part of GitLab Rails] so that +they're always in-sync with the codebase (especially the views). -The actual test scenarios and steps are [part of GitLab Rails] so that they're -always in-sync with the codebase. +Note that: -### Smoke tests - -Smoke tests are quick tests that may be run at any time (especially after the pre-deployment migrations). +- knowledge of the internals of the application are not required +- data needed for the tests can only be created using the GUI or the API +- expectations can only be made against the browser page and API responses -Much like feature tests - these tests run against the UI and ensure that basic functionality is working. +Every new feature should come with a [test plan]. -> See [Smoke Tests](smoke.md) for more information. +| Tests path | Testing engine | Notes | +| ---------- | -------------- | ----- | +| `qa/qa/specs/features/` | [Capybara] + [RSpec] + Custom QA framework | Tests should be placed under their corresponding [Product category] | -Read a separate document about [end-to-end tests](end_to_end_tests.md) to -learn more. +> See [end-to-end tests](end_to_end_tests.md) for more information. [multiple pieces]: ../architecture.md#components [GitLab Shell]: https://gitlab.com/gitlab-org/gitlab-shell @@ -138,8 +166,29 @@ learn more. [GitLab Pages]: https://gitlab.com/gitlab-org/gitlab-pages [GitLab Runner]: https://gitlab.com/gitlab-org/gitlab-runner [GitLab Omnibus]: https://gitlab.com/gitlab-org/omnibus-gitlab -[GitLab QA]: https://gitlab.com/gitlab-org/gitlab-qa [part of GitLab Rails]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/qa +[test plan]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/.gitlab/issue_templates/Test%20plan.md +[Product category]: https://about.gitlab.com/handbook/product/categories/ + +### Smoke tests + +Smoke tests are quick tests that may be run at any time (especially after the +pre-deployment migrations). + +These tests run against the UI and ensure that basic functionality is working. + +> See [Smoke Tests](smoke.md) for more information. + +### GitLab QA orchestrator + +[GitLab QA orchestrator] is a tool that allows to test that all these pieces +integrate well together by building a Docker image for a given version of GitLab +Rails and running end-to-end tests (i.e. using Capybara) against it. + +Learn more in the [GitLab QA orchestrator README][gitlab-qa-readme]. + +[GitLab QA orchestrator]: https://gitlab.com/gitlab-org/gitlab-qa +[gitlab-qa-readme]: https://gitlab.com/gitlab-org/gitlab-qa/tree/master/README.md ## EE-specific tests diff --git a/doc/integration/bitbucket.md b/doc/integration/bitbucket.md index bf587b5b296..a69db1d1a6e 100644 --- a/doc/integration/bitbucket.md +++ b/doc/integration/bitbucket.md @@ -54,6 +54,7 @@ you to use. ``` Account: Email, Read + Projects: Read Repositories: Read Pull Requests: Read Issues: Read diff --git a/doc/integration/google.md b/doc/integration/google.md index 73e2f5826ff..b91d40d4bd4 100644 --- a/doc/integration/google.md +++ b/doc/integration/google.md @@ -35,7 +35,7 @@ In Google's side: 1. You should now be able to see a Client ID and Client secret. Note them down or keep this page open as you will need them later. -1. From the **Dashboard** select **ENABLE APIS AND SERVICES > Compute > Google+ API > Enable** +1. From the **Dashboard** select **ENABLE APIS AND SERVICES > Social > Google+ API > Enable** 1. To enable projects to access [Google Kubernetes Engine](../user/project/clusters/index.md), you must also enable these APIs: - Google Kubernetes Engine API diff --git a/doc/user/markdown.md b/doc/user/markdown.md index 93aa41e9a98..6c6119a2691 100644 --- a/doc/user/markdown.md +++ b/doc/user/markdown.md @@ -1,6 +1,11 @@ -# Markdown +# GitLab Markdown -This markdown guide is valid for GitLab's system markdown entries and files. +This markdown guide is **valid for GitLab's system markdown entries and files**. +It is not valid for the [GitLab documentation website](https://docs.gitlab.com) +nor [GitLab's main website](https://about.gitlab.com), as they both use +[Kramdown](https://kramdown.gettalong.org) as their markdown engine. +The documentation website uses an extended Kramdown gem, [GitLab Kramdown](https://gitlab.com/gitlab-org/gitlab_kramdown). +Consult the [GitLab Kramdown Guide](https://about.gitlab.com/handbook/product/technical-writing/markdown-guide/) for a complete Kramdown reference._ ## GitLab Flavored Markdown (GFM) @@ -8,21 +13,21 @@ GitLab uses "GitLab Flavored Markdown" (GFM). It extends the [CommonMark specifi You can use GFM in the following areas: -- comments -- issues -- merge requests -- milestones -- snippets (the snippet must be named with a `.md` extension) -- wiki pages -- markdown documents inside the repository +- Comments +- Issues +- Merge requests +- Milestones +- Snippets (the snippet must be named with a `.md` extension) +- Wiki pages +- Markdown documents inside repositories You can also use other rich text files in GitLab. You might have to install a dependency to do so. Please see the [`github-markup` gem readme](https://github.com/gitlabhq/markup#markups) for more information. > **Notes:** > -> For the best result, we encourage you to check this document out as rendered -> by GitLab itself: [markdown.md] +> For the best result, we encourage you to check this document out as [rendered +> by GitLab itself](markdown.md). > > As of 11.1, GitLab uses the [CommonMark Ruby Library][commonmarker] for Markdown processing of all new issues, merge requests, comments, and other Markdown content @@ -30,6 +35,9 @@ in the GitLab system. As of 11.3, wiki pages and Markdown files (`.md`) in the repositories are also processed with CommonMark. Older content in issues/comments are still processed using the [Redcarpet Ruby library][redcarpet]. > +> The documentation website had its [markdown engine migrated from Redcarpet to Kramdown](https://gitlab.com/gitlab-com/gitlab-docs/merge_requests/108) +in October 2018. +> > _Where there are significant differences, we will try to call them out in this document._ ### Transitioning to CommonMark diff --git a/doc/user/project/import/manifest.md b/doc/user/project/import/manifest.md index 24bf6541a9d..baf410d9c9e 100644 --- a/doc/user/project/import/manifest.md +++ b/doc/user/project/import/manifest.md @@ -29,7 +29,7 @@ Below is a valid example of a manifest file: ```xml <manifest> - <remote review="https://android-review.googlesource.com/" /> + <remote review="https://android.googlesource.com/" /> <project path="build/make" name="platform/build" /> <project path="build/blueprint" name="platform/build/blueprint" /> @@ -38,10 +38,10 @@ Below is a valid example of a manifest file: As a result, the following projects will be created: -| GitLab | Import URL | -|---|---| -| https://gitlab.com/YOUR_GROUP/build/make | https://android-review.googlesource.com/platform/build | -| https://gitlab.com/YOUR_GROUP/build/blueprint | https://android-review.googlesource.com/platform/build/blueprint | +| GitLab | Import URL | +|:------------------------------------------------|:------------------------------------------------------------| +| `https://gitlab.com/YOUR_GROUP/build/make` | <https://android.googlesource.com/platform/build> | +| `https://gitlab.com/YOUR_GROUP/build/blueprint` | <https://android.googlesource.com/platform/build/blueprint> | ## Importing the repositories diff --git a/doc/user/project/integrations/discord_notifications.md b/doc/user/project/integrations/discord_notifications.md new file mode 100644 index 00000000000..e157f5cc106 --- /dev/null +++ b/doc/user/project/integrations/discord_notifications.md @@ -0,0 +1,29 @@ +# Discord Notifications service + +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/22684) in GitLab 11.5. + +The Discord Notifications service sends event notifications from GitLab to the channel for which the webhook was created. + +To send GitLab event notifications to a Discord channel, create a webhook in Discord and configure it in GitLab. + +## Create webhook + +1. Open the Discord channel you want to receive GitLab event notifications. +1. From the channel menu, select **Edit channel**. +1. Click on **Webhooks** menu item. +1. Click the **Create Webhook** button and fill in the name of the bot that will post the messages. Optionally, edit the avatar. +1. Note the URL from the **WEBHOOK URL** field. +1. Click the **Save** button. + +## Configure created webhook in GitLab + +With the webhook URL created in the Discord channel, you can set up the Discord Notifications service in GitLab. + +1. Navigate to the [Integrations page](project_services.md#accessing-the-project-services) in your project's settings. That is, **Project > Settings > Integrations**. +1. Select the **Discord Notifications** project service to configure it. +1. Check the **Active** checkbox to turn on the service. +1. Check the checkboxes corresponding to the GitLab events for which you want to send notifications to Discord. +1. Paste the webhook URL that you copied from the create Discord webhook step. +1. Configure the remaining options and click the **Save changes** button. + +The Discord channel you created the webhook for will now receive notification of the GitLab events that were configured. diff --git a/doc/user/project/integrations/project_services.md b/doc/user/project/integrations/project_services.md index efb0381d7aa..be45ce46dfd 100644 --- a/doc/user/project/integrations/project_services.md +++ b/doc/user/project/integrations/project_services.md @@ -30,6 +30,7 @@ Click on the service links to see further configuration instructions and details | [Bugzilla](bugzilla.md) | Bugzilla issue tracker | | Campfire | Simple web-based real-time group chat | | Custom Issue Tracker | Custom issue tracker | +| [Discord Notifications](discord_notifications.md) | Receive event notifications in Discord | | Drone CI | Continuous Integration platform built on Docker, written in Go | | [Emails on push](emails_on_push.md) | Email the commits and diff of each push to a list of recipients | | External Wiki | Replaces the link to the internal wiki with a link to an external wiki | diff --git a/lib/api/services.rb b/lib/api/services.rb index 0ae05ce08f1..1cb3b8a7277 100644 --- a/lib/api/services.rb +++ b/lib/api/services.rb @@ -298,6 +298,14 @@ module API desc: 'Title' } ], + 'discord' => [ + { + required: true, + name: :webhook, + type: String, + desc: 'Discord webhook. e.g. https://discordapp.com/api/webhooks/…' + } + ], 'drone-ci' => [ { required: true, @@ -677,6 +685,7 @@ module API BuildkiteService, CampfireService, CustomIssueTrackerService, + DiscordService, DroneCiService, EmailsOnPushService, ExternalWikiService, diff --git a/lib/banzai/filter/absolute_link_filter.rb b/lib/banzai/filter/absolute_link_filter.rb index 04ec568eee3..a9bdb004c4b 100644 --- a/lib/banzai/filter/absolute_link_filter.rb +++ b/lib/banzai/filter/absolute_link_filter.rb @@ -29,6 +29,7 @@ module Banzai end def absolute_link_attr(uri) + # Here we really want to expand relative path to absolute path URI.join(Gitlab.config.gitlab.url, uri).to_s end end diff --git a/lib/bitbucket_server/connection.rb b/lib/bitbucket_server/connection.rb index 45a437844bd..7efcdcf8619 100644 --- a/lib/bitbucket_server/connection.rb +++ b/lib/bitbucket_server/connection.rb @@ -88,35 +88,19 @@ module BitbucketServer def build_url(path) return path if path.starts_with?(root_url) - url_join_paths(root_url, path) + Gitlab::Utils.append_path(root_url, path) end def root_url - url_join_paths(base_uri, "/rest/api/#{api_version}") + Gitlab::Utils.append_path(base_uri, "rest/api/#{api_version}") end def delete_url(resource, path) if resource == :branches - url_join_paths(base_uri, "/rest/branch-utils/#{api_version}#{path}") + Gitlab::Utils.append_path(base_uri, "rest/branch-utils/#{api_version}#{path}") else build_url(path) end end - - # URI.join is stupid in that slashes are important: - # - # # URI.join('http://example.com/subpath', 'hello') - # => http://example.com/hello - # - # We really want http://example.com/subpath/hello - # - def url_join_paths(*paths) - paths.map { |path| strip_slashes(path) }.join(SEPARATOR) - end - - def strip_slashes(path) - path = path[1..-1] if path.starts_with?(SEPARATOR) - path.chomp(SEPARATOR) - end end end diff --git a/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml b/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml index c759bb7098e..ec949c290bd 100644 --- a/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml @@ -116,7 +116,9 @@ code_quality: license_management: stage: test - image: "registry.gitlab.com/gitlab-org/security-products/license-management:$CI_SERVER_VERSION_MAJOR-$CI_SERVER_VERSION_MINOR-stable" + image: + name: "registry.gitlab.com/gitlab-org/security-products/license-management:$CI_SERVER_VERSION_MAJOR-$CI_SERVER_VERSION_MINOR-stable" + entrypoint: [""] allow_failure: true script: - license_management diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb index f3bd8b69869..8ba44dff06f 100644 --- a/lib/gitlab/diff/file.rb +++ b/lib/gitlab/diff/file.rb @@ -28,6 +28,7 @@ module Gitlab @repository = repository @diff_refs = diff_refs @fallback_diff_refs = fallback_diff_refs + @unfolded = false # Ensure items are collected in the the batch new_blob_lazy @@ -137,6 +138,24 @@ module Gitlab Gitlab::Diff::Parser.new.parse(raw_diff.each_line, diff_file: self).to_a end + # Changes diff_lines according to the given position. That is, + # it checks whether the position requires blob lines into the diff + # in order to be presented. + def unfold_diff_lines(position) + return unless position + + unfolder = Gitlab::Diff::LinesUnfolder.new(self, position) + + if unfolder.unfold_required? + @diff_lines = unfolder.unfolded_diff_lines + @unfolded = true + end + end + + def unfolded? + @unfolded + end + def highlighted_diff_lines @highlighted_diff_lines ||= Gitlab::Diff::Highlight.new(self, repository: self.repository).highlight diff --git a/lib/gitlab/diff/line.rb b/lib/gitlab/diff/line.rb index 74fed7c4b1b..f0c4977fc50 100644 --- a/lib/gitlab/diff/line.rb +++ b/lib/gitlab/diff/line.rb @@ -5,9 +5,9 @@ module Gitlab class Line SERIALIZE_KEYS = %i(line_code rich_text text type index old_pos new_pos).freeze - attr_reader :line_code, :type, :index, :old_pos, :new_pos + attr_reader :line_code, :type, :old_pos, :new_pos attr_writer :rich_text - attr_accessor :text + attr_accessor :text, :index def initialize(text, type, index, old_pos, new_pos, parent_file: nil, line_code: nil, rich_text: nil) @text, @type, @index = text, type, index @@ -21,7 +21,14 @@ module Gitlab end def self.init_from_hash(hash) - new(hash[:text], hash[:type], hash[:index], hash[:old_pos], hash[:new_pos], line_code: hash[:line_code], rich_text: hash[:rich_text]) + new(hash[:text], + hash[:type], + hash[:index], + hash[:old_pos], + hash[:new_pos], + parent_file: hash[:parent_file], + line_code: hash[:line_code], + rich_text: hash[:rich_text]) end def to_hash diff --git a/lib/gitlab/diff/lines_unfolder.rb b/lib/gitlab/diff/lines_unfolder.rb new file mode 100644 index 00000000000..9306b7e16a2 --- /dev/null +++ b/lib/gitlab/diff/lines_unfolder.rb @@ -0,0 +1,235 @@ +# frozen_string_literal: true + +# Given a position, calculates which Blob lines should be extracted, treated and +# injected in the current diff file lines in order to present a "unfolded" diff. +module Gitlab + module Diff + class LinesUnfolder + include Gitlab::Utils::StrongMemoize + + UNFOLD_CONTEXT_SIZE = 3 + + def initialize(diff_file, position) + @diff_file = diff_file + @blob = diff_file.old_blob + @position = position + @generate_top_match_line = true + @generate_bottom_match_line = true + + # These methods update `@generate_top_match_line` and + # `@generate_bottom_match_line`. + @from_blob_line = calculate_from_blob_line! + @to_blob_line = calculate_to_blob_line! + end + + # Returns merged diff lines with required blob lines with correct + # positions. + def unfolded_diff_lines + strong_memoize(:unfolded_diff_lines) do + next unless unfold_required? + + merged_diff_with_blob_lines + end + end + + # Returns the extracted lines from the old blob which should be merged + # with the current diff lines. + def blob_lines + strong_memoize(:blob_lines) do + # Blob lines, unlike diffs, doesn't start with an empty space for + # unchanged line, so the parsing and highlighting step can get fuzzy + # without the following change. + line_prefix = ' ' + blob_as_diff_lines = @blob.data.each_line.map { |line| "#{line_prefix}#{line}" } + + lines = Gitlab::Diff::Parser.new.parse(blob_as_diff_lines, diff_file: @diff_file).to_a + + from = from_blob_line - 1 + to = to_blob_line - 1 + + lines[from..to] + end + end + + def unfold_required? + strong_memoize(:unfold_required) do + next false unless @diff_file.text? + next false unless @position.unchanged? + next false if @diff_file.new_file? || @diff_file.deleted_file? + next false unless @position.old_line + # Invalid position (MR import scenario) + next false if @position.old_line > @blob.lines.size + next false if @diff_file.diff_lines.empty? + next false if @diff_file.line_for_position(@position) + next false unless unfold_line + + true + end + end + + private + + attr_reader :from_blob_line, :to_blob_line + + def merged_diff_with_blob_lines + lines = @diff_file.diff_lines + match_line = unfold_line + insert_index = bottom? ? -1 : match_line.index + + lines -= [match_line] unless bottom? + + lines.insert(insert_index, *blob_lines_with_matches) + + # The inserted blob lines have invalid indexes, so we need + # to reindex them. + reindex(lines) + + lines + end + + # Returns 'unchanged' blob lines with recalculated `old_pos` and + # `new_pos` and the recalculated new match line (needed if we for instance + # we unfolded once, but there are still folded lines). + def blob_lines_with_matches + old_pos = from_blob_line + new_pos = from_blob_line + offset + + new_blob_lines = [] + + new_blob_lines.push(top_blob_match_line) if top_blob_match_line + + blob_lines.each do |line| + new_blob_lines << Gitlab::Diff::Line.new(line.text, line.type, nil, old_pos, new_pos, + parent_file: @diff_file) + + old_pos += 1 + new_pos += 1 + end + + new_blob_lines.push(bottom_blob_match_line) if bottom_blob_match_line + + new_blob_lines + end + + def reindex(lines) + lines.each_with_index { |line, i| line.index = i } + end + + def top_blob_match_line + strong_memoize(:top_blob_match_line) do + next unless @generate_top_match_line + + old_pos = from_blob_line + new_pos = from_blob_line + offset + + build_match_line(old_pos, new_pos) + end + end + + def bottom_blob_match_line + strong_memoize(:bottom_blob_match_line) do + # The bottom line match addition is already handled on + # Diff::File#diff_lines_for_serializer + next if bottom? + next unless @generate_bottom_match_line + + position = line_after_unfold_position.old_pos + + old_pos = position + new_pos = position + offset + + build_match_line(old_pos, new_pos) + end + end + + def build_match_line(old_pos, new_pos) + blob_lines_length = blob_lines.length + old_line_ref = [old_pos, blob_lines_length].join(',') + new_line_ref = [new_pos, blob_lines_length].join(',') + new_match_line_str = "@@ -#{old_line_ref}+#{new_line_ref} @@" + + Gitlab::Diff::Line.new(new_match_line_str, 'match', nil, old_pos, new_pos) + end + + # Returns the first line position that should be extracted + # from `blob_lines`. + def calculate_from_blob_line! + return unless unfold_required? + + from = comment_position - UNFOLD_CONTEXT_SIZE + + # There's no line before the match if it's in the top-most + # position. + prev_line_number = line_before_unfold_position&.old_pos || 0 + + if from <= prev_line_number + 1 + @generate_top_match_line = false + from = prev_line_number + 1 + end + + from + end + + # Returns the last line position that should be extracted + # from `blob_lines`. + def calculate_to_blob_line! + return unless unfold_required? + + to = comment_position + UNFOLD_CONTEXT_SIZE + + return to if bottom? + + next_line_number = line_after_unfold_position.old_pos + + if to >= next_line_number - 1 + @generate_bottom_match_line = false + to = next_line_number - 1 + end + + to + end + + def offset + unfold_line.new_pos - unfold_line.old_pos + end + + def line_before_unfold_position + return unless index = unfold_line&.index + + @diff_file.diff_lines[index - 1] if index > 0 + end + + def line_after_unfold_position + return unless index = unfold_line&.index + + @diff_file.diff_lines[index + 1] if index >= 0 + end + + def bottom? + strong_memoize(:bottom) do + @position.old_line > last_line.old_pos + end + end + + # Returns the line which needed to be expanded in order to send a comment + # in `@position`. + def unfold_line + strong_memoize(:unfold_line) do + next last_line if bottom? + + @diff_file.diff_lines.find do |line| + line.old_pos > comment_position && line.type == 'match' + end + end + end + + def comment_position + @position.old_line + end + + def last_line + @diff_file.diff_lines.last + end + end + end +end diff --git a/lib/gitlab/diff/position.rb b/lib/gitlab/diff/position.rb index 9c4d9377593..e8f98f52111 100644 --- a/lib/gitlab/diff/position.rb +++ b/lib/gitlab/diff/position.rb @@ -103,6 +103,10 @@ module Gitlab @diff_refs ||= DiffRefs.new(base_sha: base_sha, start_sha: start_sha, head_sha: head_sha) end + def unfolded_diff?(repository) + diff_file(repository)&.unfolded? + end + def diff_file(repository) return @diff_file if defined?(@diff_file) @@ -136,7 +140,13 @@ module Gitlab return unless diff_refs.complete? return unless comparison = diff_refs.compare_in(repository.project) - comparison.diffs(diff_options).diff_files.first + file = comparison.diffs(diff_options).diff_files.first + + # We need to unfold diff lines according to the position in order + # to correctly calculate the line code and trace position changes. + file&.unfold_diff_lines(self) + + file end def get_formatter_class(type) diff --git a/lib/gitlab/gfm/reference_rewriter.rb b/lib/gitlab/gfm/reference_rewriter.rb index 455814a9159..641446b52a5 100644 --- a/lib/gitlab/gfm/reference_rewriter.rb +++ b/lib/gitlab/gfm/reference_rewriter.rb @@ -31,19 +31,19 @@ module Gitlab class ReferenceRewriter RewriteError = Class.new(StandardError) - def initialize(text, source_project, current_user) + def initialize(text, source_parent, current_user) @text = text - @source_project = source_project + @source_parent = source_parent @current_user = current_user @original_html = markdown(text) @pattern = Gitlab::ReferenceExtractor.references_pattern end - def rewrite(target_project) + def rewrite(target_parent) return @text unless needs_rewrite? @text.gsub(@pattern) do |reference| - unfold_reference(reference, Regexp.last_match, target_project) + unfold_reference(reference, Regexp.last_match, target_parent) end end @@ -53,14 +53,14 @@ module Gitlab private - def unfold_reference(reference, match, target_project) + def unfold_reference(reference, match, target_parent) before = @text[0...match.begin(0)] after = @text[match.end(0)..-1] referable = find_referable(reference) return reference unless referable - cross_reference = build_cross_reference(referable, target_project) + cross_reference = build_cross_reference(referable, target_parent) return reference if reference == cross_reference if cross_reference.nil? @@ -72,17 +72,17 @@ module Gitlab end def find_referable(reference) - extractor = Gitlab::ReferenceExtractor.new(@source_project, + extractor = Gitlab::ReferenceExtractor.new(@source_parent, @current_user) extractor.analyze(reference) extractor.all.first end - def build_cross_reference(referable, target_project) + def build_cross_reference(referable, target_parent) if referable.respond_to?(:project) - referable.to_reference(target_project) + referable.to_reference(target_parent) else - referable.to_reference(@source_project, target_project: target_project) + referable.to_reference(@source_parent, target_project: target_parent) end end @@ -91,7 +91,7 @@ module Gitlab end def markdown(text) - Banzai.render(text, project: @source_project, no_original_data: true) + Banzai.render(text, project: @source_parent, no_original_data: true) end end end diff --git a/lib/gitlab/gfm/uploads_rewriter.rb b/lib/gitlab/gfm/uploads_rewriter.rb index f7e66697da3..b767c8a278d 100644 --- a/lib/gitlab/gfm/uploads_rewriter.rb +++ b/lib/gitlab/gfm/uploads_rewriter.rb @@ -16,14 +16,15 @@ module Gitlab @pattern = FileUploader::MARKDOWN_PATTERN end - def rewrite(target_project) + def rewrite(target_parent) return @text unless needs_rewrite? @text.gsub(@pattern) do |markdown| file = find_file(@source_project, $~[:secret], $~[:file]) break markdown unless file.try(:exists?) - moved = FileUploader.copy_to(file, target_project) + klass = target_parent.is_a?(Namespace) ? NamespaceFileUploader : FileUploader + moved = klass.copy_to(file, target_parent) moved.markdown_link end end diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb index 860c39feb64..15137140639 100644 --- a/lib/gitlab/gon_helper.rb +++ b/lib/gitlab/gon_helper.rb @@ -8,7 +8,10 @@ module Gitlab def add_gon_variables gon.api_version = 'v4' - gon.default_avatar_url = URI.join(Gitlab.config.gitlab.url, ActionController::Base.helpers.image_path('no_avatar.png')).to_s + gon.default_avatar_url = + Gitlab::Utils.append_path( + Gitlab.config.gitlab.url, + ActionController::Base.helpers.image_path('no_avatar.png')) gon.max_file_size = Gitlab::CurrentSettings.max_attachment_size gon.asset_host = ActionController::Base.asset_host gon.webpack_public_path = webpack_public_path diff --git a/lib/gitlab/manifest_import/manifest.rb b/lib/gitlab/manifest_import/manifest.rb index 4d6034fb956..b69b9ac4b64 100644 --- a/lib/gitlab/manifest_import/manifest.rb +++ b/lib/gitlab/manifest_import/manifest.rb @@ -63,7 +63,7 @@ module Gitlab end def repository_url(name) - URI.join(remote, name).to_s + Gitlab::Utils.append_path(remote, name) end def remote diff --git a/lib/gitlab/middleware/go.rb b/lib/gitlab/middleware/go.rb index 1fd8f147b44..6943567fb6d 100644 --- a/lib/gitlab/middleware/go.rb +++ b/lib/gitlab/middleware/go.rb @@ -38,7 +38,7 @@ module Gitlab def go_body(path) config = Gitlab.config - project_url = URI.join(config.gitlab.url, path) + project_url = Gitlab::Utils.append_path(config.gitlab.url, path) import_prefix = strip_url(project_url.to_s) repository_url = if Gitlab::CurrentSettings.enabled_git_access_protocol == 'ssh' diff --git a/lib/gitlab/quick_actions/command_definition.rb b/lib/gitlab/quick_actions/command_definition.rb index 96415271316..c682eb22890 100644 --- a/lib/gitlab/quick_actions/command_definition.rb +++ b/lib/gitlab/quick_actions/command_definition.rb @@ -2,13 +2,14 @@ module Gitlab module QuickActions class CommandDefinition attr_accessor :name, :aliases, :description, :explanation, :params, - :condition_block, :parse_params_block, :action_block + :condition_block, :parse_params_block, :action_block, :warning def initialize(name, attributes = {}) @name = name @aliases = attributes[:aliases] || [] @description = attributes[:description] || '' + @warning = attributes[:warning] || '' @explanation = attributes[:explanation] || '' @params = attributes[:params] || [] @condition_block = attributes[:condition_block] @@ -33,11 +34,13 @@ module Gitlab def explain(context, arg) return unless available?(context) - if explanation.respond_to?(:call) - execute_block(explanation, context, arg) - else - explanation - end + message = if explanation.respond_to?(:call) + execute_block(explanation, context, arg) + else + explanation + end + + warning.empty? ? message : "#{message} (#{warning})" end def execute(context, arg) @@ -61,6 +64,7 @@ module Gitlab name: name, aliases: aliases, description: desc, + warning: warning, params: prms } end diff --git a/lib/gitlab/quick_actions/dsl.rb b/lib/gitlab/quick_actions/dsl.rb index d82dccd0db5..192c7ec2ff5 100644 --- a/lib/gitlab/quick_actions/dsl.rb +++ b/lib/gitlab/quick_actions/dsl.rb @@ -31,6 +31,10 @@ module Gitlab @description = block_given? ? block : text end + def warning(message = '') + @warning = message + end + # Allows to define params for the next quick action. # These params are shown in the autocomplete menu. # @@ -133,6 +137,7 @@ module Gitlab name, aliases: aliases, description: @description, + warning: @warning, explanation: @explanation, params: @params, condition_block: @condition_block, @@ -150,6 +155,7 @@ module Gitlab @explanation = nil @params = nil @condition_block = nil + @warning = nil @parse_params_block = nil end end diff --git a/lib/json_web_token/hmac_token.rb b/lib/json_web_token/hmac_token.rb new file mode 100644 index 00000000000..ceb1b9c913f --- /dev/null +++ b/lib/json_web_token/hmac_token.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require 'jwt' + +module JSONWebToken + class HMACToken < Token + IAT_LEEWAY = 60 + JWT_ALGORITHM = 'HS256' + + def initialize(secret) + super() + + @secret = secret + end + + def self.decode(token, secret, leeway: IAT_LEEWAY, verify_iat: true) + JWT.decode(token, secret, true, leeway: leeway, verify_iat: verify_iat, algorithm: JWT_ALGORITHM) + end + + def encoded + JWT.encode(payload, secret, JWT_ALGORITHM) + end + + private + + attr_reader :secret + end +end diff --git a/lib/json_web_token/token.rb b/lib/json_web_token/token.rb index ce5d6f248d0..c59beef02c9 100644 --- a/lib/json_web_token/token.rb +++ b/lib/json_web_token/token.rb @@ -1,17 +1,22 @@ # frozen_string_literal: true +require 'securerandom' + module JSONWebToken class Token attr_accessor :issuer, :subject, :audience, :id attr_accessor :issued_at, :not_before, :expire_time + DEFAULT_NOT_BEFORE_TIME = 5 + DEFAULT_EXPIRE_TIME = 60 + def initialize @id = SecureRandom.uuid @issued_at = Time.now # we give a few seconds for time shift - @not_before = issued_at - 5.seconds + @not_before = issued_at - DEFAULT_NOT_BEFORE_TIME # default 60 seconds should be more than enough for this authentication token - @expire_time = issued_at + 1.minute + @expire_time = issued_at + DEFAULT_EXPIRE_TIME @custom_payload = {} end diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index 663bebfe71a..a2c3e32948f 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -45,7 +45,6 @@ namespace :gitlab do start_checking "GitLab Shell" check_gitlab_shell - check_repos_hooks_directory_is_link check_gitlab_shell_self_test finished_checking "GitLab Shell" @@ -54,42 +53,6 @@ namespace :gitlab do # Checks ######################## - def check_repos_hooks_directory_is_link - print "hooks directories in repos are links: ... " - - gitlab_shell_hooks_path = Gitlab.config.gitlab_shell.hooks_path - - unless Project.count > 0 - puts "can't check, you have no projects".color(:magenta) - return - end - - puts "" - - Project.find_each(batch_size: 100) do |project| - print sanitized_message(project) - project_hook_directory = File.join(project.repository.path_to_repo, "hooks") - - if project.empty_repo? - puts "repository is empty".color(:magenta) - elsif File.directory?(project_hook_directory) && File.directory?(gitlab_shell_hooks_path) && - (File.realpath(project_hook_directory) == File.realpath(gitlab_shell_hooks_path)) - puts 'ok'.color(:green) - else - puts "wrong or missing hooks".color(:red) - try_fixing_it( - sudo_gitlab("#{File.join(gitlab_shell_path, 'bin/create-hooks')} #{repository_storage_paths_args.join(' ')}"), - 'Check the hooks_path in config/gitlab.yml', - 'Check your gitlab-shell installation' - ) - for_more_information( - see_installation_guide_section "GitLab Shell" - ) - fix_and_rerun - end - end - end - def check_gitlab_shell_self_test gitlab_shell_repo_base = gitlab_shell_path check_cmd = File.expand_path('bin/check', gitlab_shell_repo_base) diff --git a/scripts/review_apps/review-apps.sh b/scripts/review_apps/review-apps.sh index 016682e5a3d..b180860899a 100755 --- a/scripts/review_apps/review-apps.sh +++ b/scripts/review_apps/review-apps.sh @@ -136,6 +136,7 @@ HELM_CMD=$(cat << EOF helm upgrade --install \ --wait \ --timeout 600 \ + --set global.appConfig.enableUsagePing=false \ --set releaseOverride="$CI_ENVIRONMENT_SLUG" \ --set global.hosts.hostSuffix="$HOST_SUFFIX" \ --set global.hosts.domain="$REVIEW_APPS_DOMAIN" \ diff --git a/spec/controllers/concerns/send_file_upload_spec.rb b/spec/controllers/concerns/send_file_upload_spec.rb index 4f1f6bb31f3..379b2d6b935 100644 --- a/spec/controllers/concerns/send_file_upload_spec.rb +++ b/spec/controllers/concerns/send_file_upload_spec.rb @@ -76,7 +76,7 @@ describe SendFileUpload do it 'sends a file with a custom type' do headers = double - expected_headers = %r(response-content-disposition=attachment%3Bfilename%3D%22test.js%22&response-content-type=application/javascript) + expected_headers = %r(response-content-disposition=attachment%3Bfilename%3D%22test.js%22&response-content-type=application/ecmascript) expect(Gitlab::Workhorse).to receive(:send_url).with(expected_headers).and_call_original expect(headers).to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-url:/) diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index 28f7e4634a5..74771abde71 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -141,6 +141,28 @@ describe Projects::BlobController do expect(lines.first).to have_key('rich_text') end + context 'comment in any diff line feature flag' do + it 'renders context lines when feature disabled' do + stub_feature_flags(comment_in_any_diff_line: false) + + do_get(since: 1, to: 5, offset: 10, from_merge_request: true) + lines = JSON.parse(response.body) + all_context = lines.all? { |line| line['type'] == 'context' } + + expect(all_context).to be(true) + end + + it 'renders unchanged lines when feature enabled' do + stub_feature_flags(comment_in_any_diff_line: true) + + do_get(since: 1, to: 5, offset: 10, from_merge_request: true) + lines = JSON.parse(response.body) + all_unchanged = lines.all? { |line| line['type'].nil? } + + expect(all_unchanged).to be(true) + end + end + context 'when rendering match lines' do it 'adds top match line when "since" is less than 1' do do_get(since: 5, to: 10, offset: 10, from_merge_request: true) @@ -157,7 +179,7 @@ describe Projects::BlobController do match_line = JSON.parse(response.body).first - expect(match_line['type']).to eq('context') + expect(match_line['type']).to be_nil end it 'adds bottom match line when "t"o is less than blob size' do @@ -177,7 +199,7 @@ describe Projects::BlobController do match_line = JSON.parse(response.body).last - expect(match_line['type']).to eq('context') + expect(match_line['type']).to be_nil end end end diff --git a/spec/factories/clusters/kubernetes_namespaces.rb b/spec/factories/clusters/kubernetes_namespaces.rb index 3f10f0ecc74..3a4f5193550 100644 --- a/spec/factories/clusters/kubernetes_namespaces.rb +++ b/spec/factories/clusters/kubernetes_namespaces.rb @@ -13,7 +13,7 @@ FactoryBot.define do end trait :with_token do - service_account_token { Faker::Lorem.characters(10) } + service_account_token { FFaker::Lorem.characters(10) } end end end diff --git a/spec/features/issues/user_views_issue_spec.rb b/spec/features/issues/user_views_issue_spec.rb index 117e5986f29..330b6f0e77a 100644 --- a/spec/features/issues/user_views_issue_spec.rb +++ b/spec/features/issues/user_views_issue_spec.rb @@ -1,9 +1,9 @@ require "spec_helper" describe "User views issue" do - set(:project) { create(:project_empty_repo, :public) } - set(:user) { create(:user) } - set(:issue) { create(:issue, project: project, description: "# Description header", author: user) } + let(:project) { create(:project_empty_repo, :public) } + let(:user) { create(:user) } + let(:issue) { create(:issue, project: project, description: "# Description header", author: user) } before do project.add_developer(user) diff --git a/spec/features/merge_request/user_posts_diff_notes_spec.rb b/spec/features/merge_request/user_posts_diff_notes_spec.rb index fa148715855..51b78d3e7d1 100644 --- a/spec/features/merge_request/user_posts_diff_notes_spec.rb +++ b/spec/features/merge_request/user_posts_diff_notes_spec.rb @@ -85,12 +85,13 @@ describe 'Merge request > User posts diff notes', :js do # `.line_holder` will be an unfolded line. let(:line_holder) { first('#a5cc2925ca8258af241be7e5b0381edf30266302 .line_holder') } - it 'does not allow commenting on the left side' do - should_not_allow_commenting(line_holder, 'left') + it 'allows commenting on the left side' do + should_allow_commenting(line_holder, 'left') end - it 'does not allow commenting on the right side' do - should_not_allow_commenting(line_holder, 'right') + it 'allows commenting on the right side' do + # Automatically shifts comment box to left side. + should_allow_commenting(line_holder, 'right') end end end @@ -147,8 +148,8 @@ describe 'Merge request > User posts diff notes', :js do # `.line_holder` will be an unfolded line. let(:line_holder) { first('.line_holder[id="a5cc2925ca8258af241be7e5b0381edf30266302_1_1"]') } - it 'does not allow commenting' do - should_not_allow_commenting line_holder + it 'allows commenting' do + should_allow_commenting line_holder end end diff --git a/spec/javascripts/diffs/components/tree_list_spec.js b/spec/javascripts/diffs/components/tree_list_spec.js index fc94d0bab5b..a0b380adfd6 100644 --- a/spec/javascripts/diffs/components/tree_list_spec.js +++ b/spec/javascripts/diffs/components/tree_list_spec.js @@ -81,7 +81,7 @@ describe('Diffs tree list component', () => { }); it('filters tree list to blobs matching search', done => { - vm.search = 'index'; + vm.search = 'app/index'; vm.$nextTick(() => { expect(vm.$el.querySelectorAll('.file-row').length).toBe(1); diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js index fed04cbaed8..8821cde76f4 100644 --- a/spec/javascripts/diffs/store/mutations_spec.js +++ b/spec/javascripts/diffs/store/mutations_spec.js @@ -98,7 +98,7 @@ describe('DiffsStoreMutations', () => { it('should call utils.addContextLines with proper params', () => { const options = { lineNumbers: { oldLineNumber: 1, newLineNumber: 2 }, - contextLines: [{ oldLine: 1 }], + contextLines: [{ oldLine: 1, newLine: 1, lineCode: 'ff9200_1_1', discussions: [] }], fileHash: 'ff9200', params: { bottom: true, @@ -110,7 +110,7 @@ describe('DiffsStoreMutations', () => { parallelDiffLines: [], }; const state = { diffFiles: [diffFile] }; - const lines = [{ oldLine: 1 }]; + const lines = [{ oldLine: 1, newLine: 1 }]; const findDiffFileSpy = spyOnDependency(mutations, 'findDiffFile').and.returnValue(diffFile); const removeMatchLineSpy = spyOnDependency(mutations, 'removeMatchLine'); diff --git a/spec/javascripts/notes/stores/actions_spec.js b/spec/javascripts/notes/stores/actions_spec.js index 0c0bc45b201..fcdd834e4a0 100644 --- a/spec/javascripts/notes/stores/actions_spec.js +++ b/spec/javascripts/notes/stores/actions_spec.js @@ -396,6 +396,9 @@ describe('Actions Notes Store', () => { { type: 'updateMergeRequestWidget', }, + { + type: 'startTaskList', + }, ], done, ); diff --git a/spec/javascripts/vue_mr_widget/components/deployment_spec.js b/spec/javascripts/vue_mr_widget/components/deployment_spec.js index 2f1bd00fa10..ebbcaeb6f30 100644 --- a/spec/javascripts/vue_mr_widget/components/deployment_spec.js +++ b/spec/javascripts/vue_mr_widget/components/deployment_spec.js @@ -174,57 +174,13 @@ describe('Deployment component', () => { }); }); - describe('with `features.ciEnvironmentsStatusChanges` enabled', () => { - beforeEach(() => { - window.gon = window.gon || {}; - window.gon.features = window.gon.features || {}; - window.gon.features.ciEnvironmentsStatusChanges = true; - vm = mountComponent(Component, { deployment: { ...deploymentMockData } }); - }); - - afterEach(() => { - window.gon.features = {}; - }); - - it('renders dropdown with changes', () => { - expect(vm.$el.querySelector('.js-mr-wigdet-deployment-dropdown')).not.toBeNull(); - expect(vm.$el.querySelector('.js-deploy-url-feature-flag')).toBeNull(); - }); - }); - - describe('with `features.ciEnvironmentsStatusChanges` disabled', () => { - beforeEach(() => { - window.gon = window.gon || {}; - window.gon.features = window.gon.features || {}; - window.gon.features.ciEnvironmentsStatusChanges = false; - - vm = mountComponent(Component, { deployment: { ...deploymentMockData } }); - }); - - afterEach(() => { - delete window.gon.features.ciEnvironmentsStatusChanges; - }); - - it('renders the old link to the review app', () => { - expect(vm.$el.querySelector('.js-mr-wigdet-deployment-dropdown')).toBeNull(); - expect(vm.$el.querySelector('.js-deploy-url-feature-flag')).not.toBeNull(); - }); - }); - describe('without changes', () => { beforeEach(() => { - window.gon = window.gon || {}; - window.gon.features = window.gon.features || {}; - window.gon.features.ciEnvironmentsStatusChanges = true; delete deploymentMockData.changes; vm = mountComponent(Component, { deployment: { ...deploymentMockData } }); }); - afterEach(() => { - delete window.gon.features.ciEnvironmentsStatusChanges; - }); - it('renders the link to the review app without dropdown', () => { expect(vm.$el.querySelector('.js-mr-wigdet-deployment-dropdown')).toBeNull(); expect(vm.$el.querySelector('.js-deploy-url-feature-flag')).not.toBeNull(); diff --git a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js index 09fbe87b27e..f72bf627c10 100644 --- a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js +++ b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js @@ -458,10 +458,6 @@ describe('mrWidgetOptions', () => { }; beforeEach(done => { - window.gon = window.gon || {}; - window.gon.features = window.gon.features || {}; - window.gon.features.ciEnvironmentsStatusChanges = true; - vm.mr.deployments.push( { ...deploymentMockData, diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb index 2f51642b58e..3417896e259 100644 --- a/spec/lib/gitlab/diff/file_spec.rb +++ b/spec/lib/gitlab/diff/file_spec.rb @@ -41,6 +41,52 @@ describe Gitlab::Diff::File do end end + describe '#unfold_diff_lines' do + let(:unfolded_lines) { double('expanded-lines') } + let(:unfolder) { instance_double(Gitlab::Diff::LinesUnfolder) } + let(:position) { instance_double(Gitlab::Diff::Position, old_line: 10) } + + before do + allow(Gitlab::Diff::LinesUnfolder).to receive(:new) { unfolder } + end + + context 'when unfold required' do + before do + allow(unfolder).to receive(:unfold_required?) { true } + allow(unfolder).to receive(:unfolded_diff_lines) { unfolded_lines } + end + + it 'changes @unfolded to true' do + diff_file.unfold_diff_lines(position) + + expect(diff_file).to be_unfolded + end + + it 'updates @diff_lines' do + diff_file.unfold_diff_lines(position) + + expect(diff_file.diff_lines).to eq(unfolded_lines) + end + end + + context 'when unfold not required' do + before do + allow(unfolder).to receive(:unfold_required?) { false } + end + + it 'keeps @unfolded false' do + diff_file.unfold_diff_lines(position) + + expect(diff_file).not_to be_unfolded + end + + it 'does not update @diff_lines' do + expect { diff_file.unfold_diff_lines(position) } + .not_to change(diff_file, :diff_lines) + end + end + end + describe '#mode_changed?' do it { expect(diff_file.mode_changed?).to be_falsey } end diff --git a/spec/lib/gitlab/diff/lines_unfolder_spec.rb b/spec/lib/gitlab/diff/lines_unfolder_spec.rb new file mode 100644 index 00000000000..8e00c8e0e30 --- /dev/null +++ b/spec/lib/gitlab/diff/lines_unfolder_spec.rb @@ -0,0 +1,750 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Diff::LinesUnfolder do + let(:raw_diff) do + <<-DIFF.strip_heredoc + @@ -7,9 +7,6 @@ + "tags": ["devel", "development", "nightly"], + "desktop-file-name-prefix": "(Development) ", + "finish-args": [ + - "--share=ipc", "--socket=x11", + - "--socket=wayland", + - "--talk-name=org.gnome.OnlineAccounts", + "--talk-name=org.freedesktop.Tracker1", + "--filesystem=home", + "--talk-name=org.gtk.vfs", "--talk-name=org.gtk.vfs.*", + @@ -62,7 +59,7 @@ + }, + { + "name": "gnome-desktop", + - "config-opts": ["--disable-debug-tools", "--disable-udev"], + + "config-opts": ["--disable-debug-tools", "--disable-"], + "sources": [ + { + "type": "git", + @@ -83,11 +80,6 @@ + "buildsystem": "meson", + "builddir": true, + "name": "nautilus", + - "config-opts": [ + - "-Denable-desktop=false", + - "-Denable-selinux=false", + - "--libdir=/app/lib" + - ], + "sources": [ + { + "type": "git", + DIFF + end + + let(:raw_old_blob) do + <<-BLOB.strip_heredoc + { + "app-id": "org.gnome.Nautilus", + "runtime": "org.gnome.Platform", + "runtime-version": "master", + "sdk": "org.gnome.Sdk", + "command": "nautilus", + "tags": ["devel", "development", "nightly"], + "desktop-file-name-prefix": "(Development) ", + "finish-args": [ + "--share=ipc", "--socket=x11", + "--socket=wayland", + "--talk-name=org.gnome.OnlineAccounts", + "--talk-name=org.freedesktop.Tracker1", + "--filesystem=home", + "--talk-name=org.gtk.vfs", "--talk-name=org.gtk.vfs.*", + "--filesystem=xdg-run/dconf", "--filesystem=~/.config/dconf:ro", + "--talk-name=ca.desrt.dconf", "--env=DCONF_USER_CONFIG_DIR=.config/dconf" + ], + "cleanup": [ "/include", "/share/bash-completion" ], + "modules": [ + { + "name": "exiv2", + "sources": [ + { + "type": "archive", + "url": "http://exiv2.org/builds/exiv2-0.26-trunk.tar.gz", + "sha256": "c75e3c4a0811bf700d92c82319373b7a825a2331c12b8b37d41eb58e4f18eafb" + }, + { + "type": "shell", + "commands": [ + "cp -f /usr/share/gnu-config/config.sub ./config/", + "cp -f /usr/share/gnu-config/config.guess ./config/" + ] + } + ] + }, + { + "name": "gexiv2", + "config-opts": [ "--disable-introspection" ], + "sources": [ + { + "type": "git", + "url": "https://git.gnome.org/browse/gexiv2" + } + ] + }, + { + "name": "tracker", + "cleanup": [ "/bin", "/etc", "/libexec" ], + "config-opts": [ "--disable-miner-apps", "--disable-static", + "--disable-tracker-extract", "--disable-tracker-needle", + "--disable-tracker-preferences", "--disable-artwork", + "--disable-tracker-writeback", "--disable-miner-user-guides", + "--with-bash-completion-dir=no" ], + "sources": [ + { + "type": "git", + "url": "https://git.gnome.org/browse/tracker" + } + ] + }, + { + "name": "gnome-desktop", + "config-opts": ["--disable-debug-tools", "--disable-udev"], + "sources": [ + { + "type": "git", + "url": "https://git.gnome.org/browse/gnome-desktop" + } + ] + }, + { + "name": "gnome-autoar", + "sources": [ + { + "type": "git", + "url": "https://git.gnome.org/browse/gnome-autoar" + } + ] + }, + { + "buildsystem": "meson", + "builddir": true, + "name": "nautilus", + "config-opts": [ + "-Denable-desktop=false", + "-Denable-selinux=false", + "--libdir=/app/lib" + ], + "sources": [ + { + "type": "git", + "url": "https://gitlab.gnome.org/GNOME/nautilus.git" + } + ] + } + ] + }, + { + "app-id": "foo", + "runtime": "foo", + "runtime-version": "foo", + "sdk": "foo", + "command": "foo", + "tags": ["foo", "bar", "kux"], + "desktop-file-name-prefix": "(Foo) ", + { + "buildsystem": "meson", + "builddir": true, + "name": "nautilus", + "sources": [ + { + "type": "git", + "url": "https://gitlab.gnome.org/GNOME/nautilus.git" + } + ] + } + }, + { + "app-id": "foo", + "runtime": "foo", + "runtime-version": "foo", + "sdk": "foo", + "command": "foo", + "tags": ["foo", "bar", "kux"], + "desktop-file-name-prefix": "(Foo) ", + { + "buildsystem": "meson", + "builddir": true, + "name": "nautilus", + "sources": [ + { + "type": "git", + "url": "https://gitlab.gnome.org/GNOME/nautilus.git" + } + ] + } + } + BLOB + end + + let(:project) { create(:project) } + + let(:old_blob) { Gitlab::Git::Blob.new(data: raw_old_blob) } + + let(:diff) do + Gitlab::Git::Diff.new(diff: raw_diff, + new_path: "build-aux/flatpak/org.gnome.Nautilus.json", + old_path: "build-aux/flatpak/org.gnome.Nautilus.json", + a_mode: "100644", + b_mode: "100644", + new_file: false, + renamed_file: false, + deleted_file: false, + too_large: false) + end + + let(:diff_file) do + Gitlab::Diff::File.new(diff, repository: project.repository) + end + + before do + allow(old_blob).to receive(:load_all_data!) + allow(diff_file).to receive(:old_blob) { old_blob } + end + + subject { described_class.new(diff_file, position) } + + context 'position requires a middle expansion and new match lines' do + let(:position) do + Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + head_sha: "1487062132228de836236c522fe52fed4980a46c", + old_path: "build-aux/flatpak/org.gnome.Nautilus.json", + new_path: "build-aux/flatpak/org.gnome.Nautilus.json", + position_type: "text", + old_line: 43, + new_line: 40) + end + + context 'blob lines' do + let(:expected_blob_lines) do + [[40, 40, " \"config-opts\": [ \"--disable-introspection\" ],"], + [41, 41, " \"sources\": ["], + [42, 42, " {"], + [43, 43, " \"type\": \"git\","], + [44, 44, " \"url\": \"https://git.gnome.org/browse/gexiv2\""], + [45, 45, " }"], + [46, 46, " ]"]] + end + + it 'returns the extracted blob lines correctly' do + extracted_lines = subject.blob_lines + + expect(extracted_lines.size).to eq(7) + + extracted_lines.each_with_index do |line, i| + expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i]) + end + end + end + + context 'diff lines' do + let(:expected_diff_lines) do + [[7, 7, "@@ -7,9 +7,6 @@"], + [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"], + [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","], + [9, 9, " \"finish-args\": ["], + [10, 10, "- \"--share=ipc\", \"--socket=x11\","], + [11, 10, "- \"--socket=wayland\","], + [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","], + [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","], + [14, 11, " \"--filesystem=home\","], + [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","], + + # New match line + [40, 37, "@@ -40,7+37,7 @@"], + + # Injected blob lines + [40, 37, " \"config-opts\": [ \"--disable-introspection\" ],"], + [41, 38, " \"sources\": ["], + [42, 39, " {"], + [43, 40, " \"type\": \"git\","], # comment + [44, 41, " \"url\": \"https://git.gnome.org/browse/gexiv2\""], + [45, 42, " }"], + [46, 43, " ]"], + # end + + # Second match line + [62, 59, "@@ -62,7+59,7 @@"], + + [62, 59, " },"], + [63, 60, " {"], + [64, 61, " \"name\": \"gnome-desktop\","], + [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"], + [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"], + [66, 63, " \"sources\": ["], + [67, 64, " {"], + [68, 65, " \"type\": \"git\","], + [83, 80, "@@ -83,11 +80,6 @@"], + [83, 80, " \"buildsystem\": \"meson\","], + [84, 81, " \"builddir\": true,"], + [85, 82, " \"name\": \"nautilus\","], + [86, 83, "- \"config-opts\": ["], + [87, 83, "- \"-Denable-desktop=false\","], + [88, 83, "- \"-Denable-selinux=false\","], + [89, 83, "- \"--libdir=/app/lib\""], + [90, 83, "- ],"], + [91, 83, " \"sources\": ["], + [92, 84, " {"], + [93, 85, " \"type\": \"git\","]] + end + + it 'return merge of blob lines with diff lines correctly' do + new_diff_lines = subject.unfolded_diff_lines + + expected_diff_lines.each_with_index do |expected_line, i| + line = new_diff_lines[i] + + expect([line.old_pos, line.new_pos, line.text]) + .to eq([expected_line[0], expected_line[1], expected_line[2]]) + end + end + + it 'merged lines have correct line codes' do + new_diff_lines = subject.unfolded_diff_lines + + new_diff_lines.each_with_index do |line, i| + old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1] + + unless line.type == 'match' + expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos)) + end + end + end + end + end + + context 'position requires a middle expansion and no top match line' do + let(:position) do + Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + head_sha: "1487062132228de836236c522fe52fed4980a46c", + old_path: "build-aux/flatpak/org.gnome.Nautilus.json", + new_path: "build-aux/flatpak/org.gnome.Nautilus.json", + position_type: "text", + old_line: 16, + new_line: 17) + end + + context 'blob lines' do + let(:expected_blob_lines) do + [[16, 16, " \"--filesystem=xdg-run/dconf\", \"--filesystem=~/.config/dconf:ro\","], + [17, 17, " \"--talk-name=ca.desrt.dconf\", \"--env=DCONF_USER_CONFIG_DIR=.config/dconf\""], + [18, 18, " ],"], + [19, 19, " \"cleanup\": [ \"/include\", \"/share/bash-completion\" ],"]] + end + + it 'returns the extracted blob lines correctly' do + extracted_lines = subject.blob_lines + + expect(extracted_lines.size).to eq(4) + + extracted_lines.each_with_index do |line, i| + expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i]) + end + end + end + + context 'diff lines' do + let(:expected_diff_lines) do + [[7, 7, "@@ -7,9 +7,6 @@"], + [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"], + [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","], + [9, 9, " \"finish-args\": ["], + [10, 10, "- \"--share=ipc\", \"--socket=x11\","], + [11, 10, "- \"--socket=wayland\","], + [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","], + [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","], + [14, 11, " \"--filesystem=home\","], + [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","], + # No new match needed + + # Injected blob lines + [16, 13, " \"--filesystem=xdg-run/dconf\", \"--filesystem=~/.config/dconf:ro\","], + [17, 14, " \"--talk-name=ca.desrt.dconf\", \"--env=DCONF_USER_CONFIG_DIR=.config/dconf\""], + [18, 15, " ],"], + [19, 16, " \"cleanup\": [ \"/include\", \"/share/bash-completion\" ],"], + # end + + # Second match line + [62, 59, "@@ -62,4+59,4 @@"], + + [62, 59, " },"], + [63, 60, " {"], + [64, 61, " \"name\": \"gnome-desktop\","], + [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"], + [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"], + [66, 63, " \"sources\": ["], + [67, 64, " {"], + [68, 65, " \"type\": \"git\","], + [83, 80, "@@ -83,11 +80,6 @@"], + [83, 80, " \"buildsystem\": \"meson\","], + [84, 81, " \"builddir\": true,"], + [85, 82, " \"name\": \"nautilus\","], + [86, 83, "- \"config-opts\": ["], + [87, 83, "- \"-Denable-desktop=false\","], + [88, 83, "- \"-Denable-selinux=false\","], + [89, 83, "- \"--libdir=/app/lib\""], + [90, 83, "- ],"], + [91, 83, " \"sources\": ["], + [92, 84, " {"], + [93, 85, " \"type\": \"git\","]] + end + + it 'return merge of blob lines with diff lines correctly' do + new_diff_lines = subject.unfolded_diff_lines + + expected_diff_lines.each_with_index do |expected_line, i| + line = new_diff_lines[i] + + expect([line.old_pos, line.new_pos, line.text]) + .to eq([expected_line[0], expected_line[1], expected_line[2]]) + end + end + + it 'merged lines have correct line codes' do + new_diff_lines = subject.unfolded_diff_lines + + new_diff_lines.each_with_index do |line, i| + old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1] + + unless line.type == 'match' + expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos)) + end + end + end + end + end + + context 'position requires a middle expansion and no bottom match line' do + let(:position) do + Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + head_sha: "1487062132228de836236c522fe52fed4980a46c", + old_path: "build-aux/flatpak/org.gnome.Nautilus.json", + new_path: "build-aux/flatpak/org.gnome.Nautilus.json", + position_type: "text", + old_line: 82, + new_line: 79) + end + + context 'blob lines' do + let(:expected_blob_lines) do + [[79, 79, " }"], + [80, 80, " ]"], + [81, 81, " },"], + [82, 82, " {"]] + end + + it 'returns the extracted blob lines correctly' do + extracted_lines = subject.blob_lines + + expect(extracted_lines.size).to eq(4) + + extracted_lines.each_with_index do |line, i| + expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i]) + end + end + end + + context 'diff lines' do + let(:expected_diff_lines) do + [[7, 7, "@@ -7,9 +7,6 @@"], + [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"], + [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","], + [9, 9, " \"finish-args\": ["], + [10, 10, "- \"--share=ipc\", \"--socket=x11\","], + [11, 10, "- \"--socket=wayland\","], + [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","], + [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","], + [14, 11, " \"--filesystem=home\","], + [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","], + [62, 59, "@@ -62,7 +59,7 @@"], + [62, 59, " },"], + [63, 60, " {"], + [64, 61, " \"name\": \"gnome-desktop\","], + [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"], + [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"], + [66, 63, " \"sources\": ["], + [67, 64, " {"], + [68, 65, " \"type\": \"git\","], + + # New top match line + [79, 76, "@@ -79,4+76,4 @@"], + + # Injected blob lines + [79, 76, " }"], + [80, 77, " ]"], + [81, 78, " },"], + [82, 79, " {"], + # end + + # No new second match line + [83, 80, " \"buildsystem\": \"meson\","], + [84, 81, " \"builddir\": true,"], + [85, 82, " \"name\": \"nautilus\","], + [86, 83, "- \"config-opts\": ["], + [87, 83, "- \"-Denable-desktop=false\","], + [88, 83, "- \"-Denable-selinux=false\","], + [89, 83, "- \"--libdir=/app/lib\""], + [90, 83, "- ],"], + [91, 83, " \"sources\": ["], + [92, 84, " {"], + [93, 85, " \"type\": \"git\","]] + end + + it 'return merge of blob lines with diff lines correctly' do + new_diff_lines = subject.unfolded_diff_lines + + expected_diff_lines.each_with_index do |expected_line, i| + line = new_diff_lines[i] + + expect([line.old_pos, line.new_pos, line.text]) + .to eq([expected_line[0], expected_line[1], expected_line[2]]) + end + end + + it 'merged lines have correct line codes' do + new_diff_lines = subject.unfolded_diff_lines + + new_diff_lines.each_with_index do |line, i| + old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1] + + unless line.type == 'match' + expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos)) + end + end + end + end + end + + context 'position requires a short top expansion' do + let(:position) do + Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + head_sha: "1487062132228de836236c522fe52fed4980a46c", + old_path: "build-aux/flatpak/org.gnome.Nautilus.json", + new_path: "build-aux/flatpak/org.gnome.Nautilus.json", + position_type: "text", + old_line: 6, + new_line: 6) + end + + context 'blob lines' do + let(:expected_blob_lines) do + [[3, 3, " \"runtime\": \"org.gnome.Platform\","], + [4, 4, " \"runtime-version\": \"master\","], + [5, 5, " \"sdk\": \"org.gnome.Sdk\","], + [6, 6, " \"command\": \"nautilus\","]] + end + + it 'returns the extracted blob lines correctly' do + extracted_lines = subject.blob_lines + + expect(extracted_lines.size).to eq(4) + + extracted_lines.each_with_index do |line, i| + expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i]) + end + end + end + + context 'diff lines' do + let(:expected_diff_lines) do + # New match line + [[3, 3, "@@ -3,4+3,4 @@"], + + # Injected blob lines + [3, 3, " \"runtime\": \"org.gnome.Platform\","], + [4, 4, " \"runtime-version\": \"master\","], + [5, 5, " \"sdk\": \"org.gnome.Sdk\","], + [6, 6, " \"command\": \"nautilus\","], + # end + [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"], + [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","], + [9, 9, " \"finish-args\": ["], + [10, 10, "- \"--share=ipc\", \"--socket=x11\","], + [11, 10, "- \"--socket=wayland\","], + [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","], + [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","], + [14, 11, " \"--filesystem=home\","], + [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","], + [62, 59, "@@ -62,7 +59,7 @@"], + [62, 59, " },"], + [63, 60, " {"], + [64, 61, " \"name\": \"gnome-desktop\","], + [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"], + [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"], + [66, 63, " \"sources\": ["], + [67, 64, " {"], + [68, 65, " \"type\": \"git\","], + [83, 80, "@@ -83,11 +80,6 @@"], + [83, 80, " \"buildsystem\": \"meson\","], + [84, 81, " \"builddir\": true,"], + [85, 82, " \"name\": \"nautilus\","], + [86, 83, "- \"config-opts\": ["], + [87, 83, "- \"-Denable-desktop=false\","], + [88, 83, "- \"-Denable-selinux=false\","], + [89, 83, "- \"--libdir=/app/lib\""], + [90, 83, "- ],"], + [91, 83, " \"sources\": ["], + [92, 84, " {"], + [93, 85, " \"type\": \"git\","]] + end + + it 'return merge of blob lines with diff lines correctly' do + new_diff_lines = subject.unfolded_diff_lines + + expected_diff_lines.each_with_index do |expected_line, i| + line = new_diff_lines[i] + + expect([line.old_pos, line.new_pos, line.text]) + .to eq([expected_line[0], expected_line[1], expected_line[2]]) + end + end + + it 'merged lines have correct line codes' do + new_diff_lines = subject.unfolded_diff_lines + + new_diff_lines.each_with_index do |line, i| + old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1] + + unless line.type == 'match' + expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos)) + end + end + end + end + end + + context 'position sits between two match lines (no expasion needed)' do + let(:position) do + Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + head_sha: "1487062132228de836236c522fe52fed4980a46c", + old_path: "build-aux/flatpak/org.gnome.Nautilus.json", + new_path: "build-aux/flatpak/org.gnome.Nautilus.json", + position_type: "text", + old_line: 64, + new_line: 61) + end + + context 'diff lines' do + it 'returns nil' do + expect(subject.unfolded_diff_lines).to be_nil + end + end + end + + context 'position requires bottom expansion and new match lines' do + let(:position) do + Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + head_sha: "1487062132228de836236c522fe52fed4980a46c", + old_path: "build-aux/flatpak/org.gnome.Nautilus.json", + new_path: "build-aux/flatpak/org.gnome.Nautilus.json", + position_type: "text", + old_line: 107, + new_line: 99) + end + + context 'blob lines' do + let(:expected_blob_lines) do + [[104, 104, " \"sdk\": \"foo\","], + [105, 105, " \"command\": \"foo\","], + [106, 106, " \"tags\": [\"foo\", \"bar\", \"kux\"],"], + [107, 107, " \"desktop-file-name-prefix\": \"(Foo) \","], + [108, 108, " {"], + [109, 109, " \"buildsystem\": \"meson\","], + [110, 110, " \"builddir\": true,"]] + end + + it 'returns the extracted blob lines correctly' do + extracted_lines = subject.blob_lines + + expect(extracted_lines.size).to eq(7) + + extracted_lines.each_with_index do |line, i| + expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i]) + end + end + end + + context 'diff lines' do + let(:expected_diff_lines) do + [[7, 7, "@@ -7,9 +7,6 @@"], + [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"], + [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","], + [9, 9, " \"finish-args\": ["], + [10, 10, "- \"--share=ipc\", \"--socket=x11\","], + [11, 10, "- \"--socket=wayland\","], + [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","], + [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","], + [14, 11, " \"--filesystem=home\","], + [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","], + [62, 59, "@@ -62,7 +59,7 @@"], + [62, 59, " },"], + [63, 60, " {"], + [64, 61, " \"name\": \"gnome-desktop\","], + [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"], + [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"], + [66, 63, " \"sources\": ["], + [67, 64, " {"], + [68, 65, " \"type\": \"git\","], + [83, 80, "@@ -83,11 +80,6 @@"], + [83, 80, " \"buildsystem\": \"meson\","], + [84, 81, " \"builddir\": true,"], + [85, 82, " \"name\": \"nautilus\","], + [86, 83, "- \"config-opts\": ["], + [87, 83, "- \"-Denable-desktop=false\","], + [88, 83, "- \"-Denable-selinux=false\","], + [89, 83, "- \"--libdir=/app/lib\""], + [90, 83, "- ],"], + [91, 83, " \"sources\": ["], + [92, 84, " {"], + [93, 85, " \"type\": \"git\","], + # New match line + [104, 96, "@@ -104,7+96,7 @@"], + + # Injected blob lines + [104, 96, " \"sdk\": \"foo\","], + [105, 97, " \"command\": \"foo\","], + [106, 98, " \"tags\": [\"foo\", \"bar\", \"kux\"],"], + [107, 99, " \"desktop-file-name-prefix\": \"(Foo) \","], + [108, 100, " {"], + [109, 101, " \"buildsystem\": \"meson\","], + [110, 102, " \"builddir\": true,"]] + # end + end + + it 'return merge of blob lines with diff lines correctly' do + new_diff_lines = subject.unfolded_diff_lines + + expected_diff_lines.each_with_index do |expected_line, i| + line = new_diff_lines[i] + + expect([line.old_pos, line.new_pos, line.text]) + .to eq([expected_line[0], expected_line[1], expected_line[2]]) + end + end + + it 'merged lines have correct line codes' do + new_diff_lines = subject.unfolded_diff_lines + + new_diff_lines.each_with_index do |line, i| + old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1] + + unless line.type == 'match' + expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos)) + end + end + end + end + end +end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index f4efa450cca..1d184375a52 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -198,6 +198,7 @@ project: - last_event - services - campfire_service +- discord_service - drone_ci_service - emails_on_push_service - pipelines_email_service diff --git a/spec/lib/gitlab/quick_actions/command_definition_spec.rb b/spec/lib/gitlab/quick_actions/command_definition_spec.rb index b03c1e23ca3..5dae82a63b4 100644 --- a/spec/lib/gitlab/quick_actions/command_definition_spec.rb +++ b/spec/lib/gitlab/quick_actions/command_definition_spec.rb @@ -210,6 +210,19 @@ describe Gitlab::QuickActions::CommandDefinition do end end + context 'when warning is set' do + before do + subject.explanation = 'Explanation' + subject.warning = 'dangerous!' + end + + it 'returns this static string' do + result = subject.explain({}, nil) + + expect(result).to eq 'Explanation (dangerous!)' + end + end + context 'when the explanation is dynamic' do before do subject.explanation = proc { |arg| "Dynamic #{arg}" } diff --git a/spec/lib/gitlab/quick_actions/dsl_spec.rb b/spec/lib/gitlab/quick_actions/dsl_spec.rb index 067a30fd7e2..fd4df8694ba 100644 --- a/spec/lib/gitlab/quick_actions/dsl_spec.rb +++ b/spec/lib/gitlab/quick_actions/dsl_spec.rb @@ -12,6 +12,7 @@ describe Gitlab::QuickActions::Dsl do params 'The first argument' explanation 'Static explanation' + warning 'Possible problem!' command :explanation_with_aliases, :once, :first do |arg| arg end @@ -64,6 +65,7 @@ describe Gitlab::QuickActions::Dsl do expect(no_args_def.condition_block).to be_nil expect(no_args_def.action_block).to be_a_kind_of(Proc) expect(no_args_def.parse_params_block).to be_nil + expect(no_args_def.warning).to eq('') expect(explanation_with_aliases_def.name).to eq(:explanation_with_aliases) expect(explanation_with_aliases_def.aliases).to eq([:once, :first]) @@ -73,6 +75,7 @@ describe Gitlab::QuickActions::Dsl do expect(explanation_with_aliases_def.condition_block).to be_nil expect(explanation_with_aliases_def.action_block).to be_a_kind_of(Proc) expect(explanation_with_aliases_def.parse_params_block).to be_nil + expect(explanation_with_aliases_def.warning).to eq('Possible problem!') expect(dynamic_description_def.name).to eq(:dynamic_description) expect(dynamic_description_def.aliases).to eq([]) @@ -82,6 +85,7 @@ describe Gitlab::QuickActions::Dsl do expect(dynamic_description_def.condition_block).to be_nil expect(dynamic_description_def.action_block).to be_a_kind_of(Proc) expect(dynamic_description_def.parse_params_block).to be_nil + expect(dynamic_description_def.warning).to eq('') expect(cc_def.name).to eq(:cc) expect(cc_def.aliases).to eq([]) @@ -91,6 +95,7 @@ describe Gitlab::QuickActions::Dsl do expect(cc_def.condition_block).to be_nil expect(cc_def.action_block).to be_nil expect(cc_def.parse_params_block).to be_nil + expect(cc_def.warning).to eq('') expect(cond_action_def.name).to eq(:cond_action) expect(cond_action_def.aliases).to eq([]) @@ -100,6 +105,7 @@ describe Gitlab::QuickActions::Dsl do expect(cond_action_def.condition_block).to be_a_kind_of(Proc) expect(cond_action_def.action_block).to be_a_kind_of(Proc) expect(cond_action_def.parse_params_block).to be_nil + expect(cond_action_def.warning).to eq('') expect(with_params_parsing_def.name).to eq(:with_params_parsing) expect(with_params_parsing_def.aliases).to eq([]) @@ -109,6 +115,7 @@ describe Gitlab::QuickActions::Dsl do expect(with_params_parsing_def.condition_block).to be_nil expect(with_params_parsing_def.action_block).to be_a_kind_of(Proc) expect(with_params_parsing_def.parse_params_block).to be_a_kind_of(Proc) + expect(with_params_parsing_def.warning).to eq('') expect(substitution_def.name).to eq(:something) expect(substitution_def.aliases).to eq([]) @@ -118,6 +125,7 @@ describe Gitlab::QuickActions::Dsl do expect(substitution_def.condition_block).to be_nil expect(substitution_def.action_block.call('text')).to eq('text Some complicated thing you want in here') expect(substitution_def.parse_params_block).to be_nil + expect(substitution_def.warning).to eq('') end end end diff --git a/spec/lib/json_web_token/hmac_token_spec.rb b/spec/lib/json_web_token/hmac_token_spec.rb new file mode 100644 index 00000000000..f2cbc381967 --- /dev/null +++ b/spec/lib/json_web_token/hmac_token_spec.rb @@ -0,0 +1,133 @@ +# frozen_string_literal: true + +require 'json' +require 'timecop' + +describe JSONWebToken::HMACToken do + let(:secret) { 'shh secret squirrel' } + + shared_examples 'a valid, non-expired token' do + it 'is an Array with two elements' do + expect(decoded_token).to be_a(Array) + expect(decoded_token.count).to eq(2) + end + + it 'contains the following keys in the first Array element Hash - jti, iat, nbf, exp' do + expect(decoded_token[0].keys).to include('jti', 'iat', 'nbf', 'exp') + end + + it 'contains the following keys in the second Array element Hash - typ and alg' do + expect(decoded_token[1]['typ']).to eql('JWT') + expect(decoded_token[1]['alg']).to eql('HS256') + end + end + + describe '.decode' do + let(:leeway) { described_class::IAT_LEEWAY } + let(:decoded_token) { described_class.decode(encoded_token, secret, leeway: leeway) } + + context 'with an invalid token' do + context 'that is junk' do + let(:encoded_token) { 'junk' } + + it "raises exception saying 'Not enough or too many segments'" do + expect { decoded_token }.to raise_error(JWT::DecodeError, 'Not enough or too many segments') + end + end + + context 'that has been fiddled with' do + let(:encoded_token) do + described_class.new(secret).encoded.tap { |token| token[0] = 'E' } + end + + it "raises exception saying 'Invalid segment encoding'" do + expect { decoded_token }.to raise_error(JWT::DecodeError, 'Invalid segment encoding') + end + end + + context 'that was generated using a different secret' do + let(:encoded_token) { described_class.new('some other secret').encoded } + + it "raises exception saying 'Signature verification raised" do + expect { decoded_token }.to raise_error(JWT::VerificationError, 'Signature verification raised') + end + end + + context 'that is expired' do + # Needs the ! so Timecop.freeze() is effective + let!(:encoded_token) { described_class.new(secret).encoded } + + it "raises exception saying 'Signature has expired'" do + # Needs to be 120 seconds, because the default expiry is 60 seconds + # with an additional 60 second leeway. + Timecop.freeze(Time.now + 120) do + expect { decoded_token }.to raise_error(JWT::ExpiredSignature, 'Signature has expired') + end + end + end + end + + context 'with a valid token' do + let(:encoded_token) do + hmac_token = described_class.new(secret) + hmac_token.expire_time = Time.now + expire_time + hmac_token.encoded + end + + context 'that has expired' do + let(:expire_time) { 0 } + + context 'with the default leeway' do + Timecop.freeze(Time.now + 1) do + it_behaves_like 'a valid, non-expired token' + end + end + + context 'with a leeway of 0 seconds' do + let(:leeway) { 0 } + + it "raises exception saying 'Signature has expired'" do + Timecop.freeze(Time.now + 1) do + expect { decoded_token }.to raise_error(JWT::ExpiredSignature, 'Signature has expired') + end + end + end + end + + context 'that has not expired' do + let(:expire_time) { described_class::DEFAULT_EXPIRE_TIME } + + it_behaves_like 'a valid, non-expired token' + end + end + end + + describe '#encoded' do + let(:decoded_token) { described_class.decode(encoded_token, secret) } + + context 'without data' do + let(:encoded_token) { described_class.new(secret).encoded } + + it_behaves_like 'a valid, non-expired token' + end + + context 'with data' do + let(:data) { { secret_key: 'secret value' }.to_json } + let(:encoded_token) do + ec = described_class.new(secret) + ec[:data] = data + ec.encoded + end + + it_behaves_like 'a valid, non-expired token' + + it "contains the 'data' key in the first Array element Hash" do + expect(decoded_token[0]).to have_key('data') + end + + it 'can re-read back the data' do + expect(decoded_token[0]['data']).to eql(data) + end + end + end +end diff --git a/spec/models/clusters/kubernetes_namespace_spec.rb b/spec/models/clusters/kubernetes_namespace_spec.rb index 0dfeea5cd2f..c068c4d7739 100644 --- a/spec/models/clusters/kubernetes_namespace_spec.rb +++ b/spec/models/clusters/kubernetes_namespace_spec.rb @@ -8,6 +8,22 @@ RSpec.describe Clusters::KubernetesNamespace, type: :model do it { is_expected.to belong_to(:cluster) } it { is_expected.to have_one(:platform_kubernetes) } + describe 'has_service_account_token' do + subject { described_class.has_service_account_token } + + context 'namespace has service_account_token' do + let!(:namespace) { create(:cluster_kubernetes_namespace, :with_token) } + + it { is_expected.to include(namespace) } + end + + context 'namespace has no service_account_token' do + let!(:namespace) { create(:cluster_kubernetes_namespace) } + + it { is_expected.not_to include(namespace) } + end + end + describe 'namespace uniqueness validation' do let(:cluster_project) { create(:cluster_project) } let(:kubernetes_namespace) { build(:cluster_kubernetes_namespace, namespace: 'my-namespace') } diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index f5d261c4e9d..99fd6ccc4d8 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -210,9 +210,11 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching let(:api_url) { 'https://kube.domain.com' } let(:ca_pem) { 'CA PEM DATA' } + subject { kubernetes.predefined_variables(project: cluster.project) } + shared_examples 'setting variables' do it 'sets the variables' do - expect(kubernetes.predefined_variables(project: cluster.project)).to include( + expect(subject).to include( { key: 'KUBE_URL', value: api_url, public: true }, { key: 'KUBE_CA_PEM', value: ca_pem, public: true }, { key: 'KUBE_CA_PEM_FILE', value: ca_pem, public: true, file: true } @@ -220,6 +222,30 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching end end + context 'kubernetes namespace is created with no service account token' do + let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, cluster: cluster) } + + it_behaves_like 'setting variables' + + it 'sets KUBE_TOKEN' do + expect(subject).to include( + { key: 'KUBE_TOKEN', value: kubernetes.token, public: false } + ) + end + end + + context 'kubernetes namespace is created with no service account token' do + let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, :with_token, cluster: cluster) } + + it_behaves_like 'setting variables' + + it 'sets KUBE_TOKEN' do + expect(subject).to include( + { key: 'KUBE_TOKEN', value: kubernetes_namespace.service_account_token, public: false } + ) + end + end + context 'namespace is provided' do let(:namespace) { 'my-project' } @@ -228,12 +254,24 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching end it_behaves_like 'setting variables' + + it 'sets KUBE_TOKEN' do + expect(subject).to include( + { key: 'KUBE_TOKEN', value: kubernetes.token, public: false } + ) + end end context 'no namespace provided' do let(:namespace) { kubernetes.actual_namespace } it_behaves_like 'setting variables' + + it 'sets KUBE_TOKEN' do + expect(subject).to include( + { key: 'KUBE_TOKEN', value: kubernetes.token, public: false } + ) + end end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index c7202b481d3..131db6a5ff9 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -2651,6 +2651,10 @@ describe MergeRequest do describe '#includes_any_commits?' do it 'returns false' do + expect(subject.includes_any_commits?([])).to be_falsey + end + + it 'returns false' do expect(subject.includes_any_commits?([Gitlab::Git::BLANK_SHA])).to be_falsey end diff --git a/spec/models/project_services/discord_service_spec.rb b/spec/models/project_services/discord_service_spec.rb new file mode 100644 index 00000000000..be82f223478 --- /dev/null +++ b/spec/models/project_services/discord_service_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require "spec_helper" + +describe DiscordService do + it_behaves_like "chat service", "Discord notifications" do + let(:client) { Discordrb::Webhooks::Client } + let(:client_arguments) { { url: webhook_url } } + let(:content_key) { :content } + end +end diff --git a/spec/models/project_services/hangouts_chat_service_spec.rb b/spec/models/project_services/hangouts_chat_service_spec.rb index cfa55188a64..0505ac9b49c 100644 --- a/spec/models/project_services/hangouts_chat_service_spec.rb +++ b/spec/models/project_services/hangouts_chat_service_spec.rb @@ -1,246 +1,11 @@ -require 'spec_helper' +# frozen_string_literal: true -describe HangoutsChatService do - describe 'Associations' do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end - - describe 'Validations' do - context 'when service is active' do - before do - subject.active = true - end - - it { is_expected.to validate_presence_of(:webhook) } - it_behaves_like 'issue tracker service URL attribute', :webhook - end - - context 'when service is inactive' do - before do - subject.active = false - end - - it { is_expected.not_to validate_presence_of(:webhook) } - end - end - - describe '#execute' do - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } - let(:webhook_url) { 'https://example.gitlab.com/' } - - before do - allow(subject).to receive_messages( - project: project, - project_id: project.id, - service_hook: true, - webhook: webhook_url - ) - - WebMock.stub_request(:post, webhook_url) - end - - shared_examples 'Hangouts Chat service' do - it 'calls Hangouts Chat API' do - subject.execute(sample_data) - - expect(WebMock) - .to have_requested(:post, webhook_url) - .with { |req| req.body =~ /\A{"text":.+}\Z/ } - .once - end - end - - context 'with push events' do - let(:sample_data) do - Gitlab::DataBuilder::Push.build_sample(project, user) - end - - it_behaves_like 'Hangouts Chat service' - - it 'specifies the webhook when it is configured' do - expect(HangoutsChat::Sender).to receive(:new).with(webhook_url).and_return(double(:hangouts_chat_service).as_null_object) - - subject.execute(sample_data) - end - - context 'with not default branch' do - let(:sample_data) do - Gitlab::DataBuilder::Push.build(project, user, nil, nil, 'not-the-default-branch') - end - - context 'when notify_only_default_branch enabled' do - before do - subject.notify_only_default_branch = true - end - - it 'does not call the Hangouts Chat API' do - result = subject.execute(sample_data) - - expect(result).to be_falsy - end - end - - context 'when notify_only_default_branch disabled' do - before do - subject.notify_only_default_branch = false - end - - it_behaves_like 'Hangouts Chat service' - end - end - end - - context 'with issue events' do - let(:opts) { { title: 'Awesome issue', description: 'please fix' } } - let(:sample_data) do - service = Issues::CreateService.new(project, user, opts) - issue = service.execute - service.hook_data(issue, 'open') - end - - it_behaves_like 'Hangouts Chat service' - end - - context 'with merge events' do - let(:opts) do - { - title: 'Awesome merge_request', - description: 'please fix', - source_branch: 'feature', - target_branch: 'master' - } - end - - let(:sample_data) do - service = MergeRequests::CreateService.new(project, user, opts) - merge_request = service.execute - service.hook_data(merge_request, 'open') - end - - before do - project.add_developer(user) - end +require "spec_helper" - it_behaves_like 'Hangouts Chat service' - end - - context 'with wiki page events' do - let(:opts) do - { - title: 'Awesome wiki_page', - content: 'Some text describing some thing or another', - format: 'md', - message: 'user created page: Awesome wiki_page' - } - end - let(:wiki_page) { create(:wiki_page, wiki: project.wiki, attrs: opts) } - let(:sample_data) { Gitlab::DataBuilder::WikiPage.build(wiki_page, user, 'create') } - - it_behaves_like 'Hangouts Chat service' - end - - context 'with note events' do - let(:sample_data) { Gitlab::DataBuilder::Note.build(note, user) } - - context 'with commit comment' do - let(:note) do - create(:note_on_commit, author: user, - project: project, - commit_id: project.repository.commit.id, - note: 'a comment on a commit') - end - - it_behaves_like 'Hangouts Chat service' - end - - context 'with merge request comment' do - let(:note) do - create(:note_on_merge_request, project: project, - note: 'merge request note') - end - - it_behaves_like 'Hangouts Chat service' - end - - context 'with issue comment' do - let(:note) do - create(:note_on_issue, project: project, note: 'issue note') - end - - it_behaves_like 'Hangouts Chat service' - end - - context 'with snippet comment' do - let(:note) do - create(:note_on_project_snippet, project: project, - note: 'snippet note') - end - - it_behaves_like 'Hangouts Chat service' - end - end - - context 'with pipeline events' do - let(:pipeline) do - create(:ci_pipeline, - project: project, status: status, - sha: project.commit.sha, ref: project.default_branch) - end - let(:sample_data) { Gitlab::DataBuilder::Pipeline.build(pipeline) } - - context 'with failed pipeline' do - let(:status) { 'failed' } - - it_behaves_like 'Hangouts Chat service' - end - - context 'with succeeded pipeline' do - let(:status) { 'success' } - - context 'with default notify_only_broken_pipelines' do - it 'does not call Hangouts Chat API' do - result = subject.execute(sample_data) - - expect(result).to be_falsy - end - end - - context 'when notify_only_broken_pipelines is false' do - before do - subject.notify_only_broken_pipelines = false - end - - it_behaves_like 'Hangouts Chat service' - end - end - - context 'with not default branch' do - let(:pipeline) do - create(:ci_pipeline, project: project, status: 'failed', ref: 'not-the-default-branch') - end - - context 'when notify_only_default_branch enabled' do - before do - subject.notify_only_default_branch = true - end - - it 'does not call the Hangouts Chat API' do - result = subject.execute(sample_data) - - expect(result).to be_falsy - end - end - - context 'when notify_only_default_branch disabled' do - before do - subject.notify_only_default_branch = false - end - - it_behaves_like 'Hangouts Chat service' - end - end - end +describe HangoutsChatService do + it_behaves_like "chat service", "Hangouts Chat" do + let(:client) { HangoutsChat::Sender } + let(:client_arguments) { webhook_url } + let(:content_key) { :text } end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 471f19f9b7c..bdff68cee8b 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -33,6 +33,7 @@ describe Project do it { is_expected.to have_one(:asana_service) } it { is_expected.to have_many(:boards) } it { is_expected.to have_one(:campfire_service) } + it { is_expected.to have_one(:discord_service) } it { is_expected.to have_one(:drone_ci_service) } it { is_expected.to have_one(:emails_on_push_service) } it { is_expected.to have_one(:pipelines_email_service) } @@ -2414,7 +2415,7 @@ describe Project do end context 'when user configured kubernetes from CI/CD > Clusters and KubernetesNamespace migration has been executed' do - let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace) } + let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, :with_token) } let!(:cluster) { kubernetes_namespace.cluster } let(:project) { kubernetes_namespace.project } diff --git a/spec/serializers/environment_status_entity_spec.rb b/spec/serializers/environment_status_entity_spec.rb index 52bd40ecb5e..58e2e627410 100644 --- a/spec/serializers/environment_status_entity_spec.rb +++ b/spec/serializers/environment_status_entity_spec.rb @@ -33,14 +33,6 @@ describe EnvironmentStatusEntity do it { is_expected.not_to include(:metrics_url) } it { is_expected.not_to include(:metrics_monitoring_url) } - context 'when :ci_environments_status_changes feature flag is disabled' do - before do - stub_feature_flags(ci_environments_status_changes: false) - end - - it { is_expected.not_to include(:changes) } - end - context 'when the user is project maintainer' do before do project.add_maintainer(user) diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 5c87ed5c3c6..193148d403a 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -387,15 +387,24 @@ describe Ci::CreatePipelineService do context 'with environment' do before do - config = YAML.dump(deploy: { environment: { name: "review/$CI_COMMIT_REF_NAME" }, script: 'ls' }) + config = YAML.dump( + deploy: { + environment: { name: "review/$CI_COMMIT_REF_NAME" }, + script: 'ls', + tags: ['hello'] + }) + stub_ci_pipeline_yaml_file(config) end - it 'creates the environment' do + it 'creates the environment with tags' do result = execute_service expect(result).to be_persisted expect(Environment.find_by(name: "review/master")).to be_present + expect(result.builds.first.tag_list).to contain_exactly('hello') + expect(result.builds.first.deployment).to be_persisted + expect(result.builds.first.deployment.deployable).to be_a(Ci::Build) end end diff --git a/spec/services/issuable/clone/attributes_rewriter_spec.rb b/spec/services/issuable/clone/attributes_rewriter_spec.rb new file mode 100644 index 00000000000..20bda6984bd --- /dev/null +++ b/spec/services/issuable/clone/attributes_rewriter_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Issuable::Clone::AttributesRewriter do + let(:user) { create(:user) } + let(:group) { create(:group) } + let(:project1) { create(:project, :public, group: group) } + let(:project2) { create(:project, :public, group: group) } + let(:original_issue) { create(:issue, project: project1) } + let(:new_issue) { create(:issue, project: project2) } + + subject { described_class.new(user, original_issue, new_issue) } + + context 'setting labels' do + it 'sets labels present in the new project and group labels' do + project1_label_1 = create(:label, title: 'label1', project: project1) + project1_label_2 = create(:label, title: 'label2', project: project1) + project2_label_1 = create(:label, title: 'label1', project: project2) + group_label = create(:group_label, title: 'group_label', group: group) + create(:label, title: 'label3', project: project2) + + original_issue.update(labels: [project1_label_1, project1_label_2, group_label]) + + subject.execute + + expect(new_issue.reload.labels).to match_array([project2_label_1, group_label]) + end + + it 'does not set any labels when not used on the original issue' do + subject.execute + + expect(new_issue.reload.labels).to be_empty + end + + it 'copies the resource label events' do + resource_label_events = create_list(:resource_label_event, 2, issue: original_issue) + + subject.execute + + expected = resource_label_events.map(&:label_id) + + expect(new_issue.resource_label_events.map(&:label_id)).to match_array(expected) + end + end + + context 'setting milestones' do + it 'sets milestone to nil when old issue milestone is not in the new project' do + milestone = create(:milestone, title: 'milestone', project: project1) + + original_issue.update(milestone: milestone) + + subject.execute + + expect(new_issue.reload.milestone).to be_nil + end + + it 'copies the milestone when old issue milestone title is in the new project' do + milestone_project1 = create(:milestone, title: 'milestone', project: project1) + milestone_project2 = create(:milestone, title: 'milestone', project: project2) + + original_issue.update(milestone: milestone_project1) + + subject.execute + + expect(new_issue.reload.milestone).to eq(milestone_project2) + end + + it 'copies the milestone when old issue milestone is a group milestone' do + milestone = create(:milestone, title: 'milestone', group: group) + + original_issue.update(milestone: milestone) + + subject.execute + + expect(new_issue.reload.milestone).to eq(milestone) + end + end +end diff --git a/spec/services/issuable/clone/content_rewriter_spec.rb b/spec/services/issuable/clone/content_rewriter_spec.rb new file mode 100644 index 00000000000..4d3cb0bd254 --- /dev/null +++ b/spec/services/issuable/clone/content_rewriter_spec.rb @@ -0,0 +1,153 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Issuable::Clone::ContentRewriter do + let(:user) { create(:user) } + let(:group) { create(:group) } + let(:project1) { create(:project, :public, group: group) } + let(:project2) { create(:project, :public, group: group) } + + let(:other_issue) { create(:issue, project: project1) } + let(:merge_request) { create(:merge_request) } + + subject { described_class.new(user, original_issue, new_issue)} + + let(:description) { 'Simple text' } + let(:original_issue) { create(:issue, description: description, project: project1) } + let(:new_issue) { create(:issue, project: project2) } + + context 'rewriting award emojis' do + it 'copies the award emojis' do + create(:award_emoji, awardable: original_issue, name: 'thumbsup') + create(:award_emoji, awardable: original_issue, name: 'thumbsdown') + + expect { subject.execute }.to change { AwardEmoji.count }.by(2) + + expect(new_issue.award_emoji.map(&:name)).to match_array(%w(thumbsup thumbsdown)) + end + end + + context 'rewriting description' do + before do + subject.execute + end + + context 'when description is a simple text' do + it 'does not rewrite the description' do + expect(new_issue.reload.description).to eq(original_issue.description) + end + end + + context 'when description contains a local reference' do + let(:description) { "See ##{other_issue.iid}" } + + it 'rewrites the local reference correctly' do + expected_description = "See #{project1.path}##{other_issue.iid}" + + expect(new_issue.reload.description).to eq(expected_description) + end + end + + context 'when description contains a cross reference' do + let(:description) { "See #{merge_request.project.full_path}!#{merge_request.iid}" } + + it 'rewrites the cross reference correctly' do + expected_description = "See #{merge_request.project.full_path}!#{merge_request.iid}" + + expect(new_issue.reload.description).to eq(expected_description) + end + end + + context 'when description contains a user reference' do + let(:description) { "FYU #{user.to_reference}" } + + it 'works with a user reference' do + expect(new_issue.reload.description).to eq("FYU #{user.to_reference}") + end + end + + context 'when description contains uploads' do + let(:uploader) { build(:file_uploader, project: project1) } + let(:description) { "Text and #{uploader.markdown_link}" } + + it 'rewrites uploads in the description' do + upload = Upload.last + + expect(new_issue.description).not_to eq(description) + expect(new_issue.description).to match(/Text and #{FileUploader::MARKDOWN_PATTERN}/) + expect(upload.secret).not_to eq(uploader.secret) + expect(new_issue.description).to include(upload.secret) + expect(new_issue.description).to include(upload.path) + end + end + end + + context 'rewriting notes' do + context 'simple notes' do + let!(:notes) do + [ + create(:note, noteable: original_issue, project: project1, + created_at: 2.weeks.ago, updated_at: 1.week.ago), + create(:note, noteable: original_issue, project: project1), + create(:note, system: true, noteable: original_issue, project: project1) + ] + end + let!(:system_note_metadata) { create(:system_note_metadata, note: notes.last) } + let!(:award_emoji) { create(:award_emoji, awardable: notes.first, name: 'thumbsup')} + + before do + subject.execute + end + + it 'rewrites existing notes in valid order' do + expect(new_issue.notes.order('id ASC').pluck(:note).first(3)).to eq(notes.map(&:note)) + end + + it 'copies all the issue notes' do + expect(new_issue.notes.count).to eq(3) + end + + it 'does not change the note attributes' do + subject.execute + + new_note = new_issue.notes.first + + expect(new_note.note).to eq(notes.first.note) + expect(new_note.author).to eq(notes.first.author) + end + + it 'copies the award emojis' do + subject.execute + + new_note = new_issue.notes.first + new_note.award_emoji.first.name = 'thumbsup' + end + + it 'copies system_note_metadata for system note' do + new_note = new_issue.notes.last + + expect(new_note.system_note_metadata.action).to eq(system_note_metadata.action) + expect(new_note.system_note_metadata.id).not_to eq(system_note_metadata.id) + end + end + + context 'notes with reference' do + let(:text) do + "See ##{other_issue.iid} and #{merge_request.project.full_path}!#{merge_request.iid}" + end + let!(:note) { create(:note, noteable: original_issue, note: text, project: project1) } + + it 'rewrites the references correctly' do + subject.execute + + new_note = new_issue.notes.first + + expected_text = "See #{other_issue.project.path}##{other_issue.iid} and #{merge_request.project.full_path}!#{merge_request.iid}" + + expect(new_note.note).to eq(expected_text) + expect(new_note.author).to eq(note.author) + end + end + end +end diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb index b5767583952..1e088bc7d9b 100644 --- a/spec/services/issues/move_service_spec.rb +++ b/spec/services/issues/move_service_spec.rb @@ -10,11 +10,9 @@ describe Issues::MoveService do let(:sub_group_2) { create(:group, :private, parent: group) } let(:old_project) { create(:project, namespace: sub_group_1) } let(:new_project) { create(:project, namespace: sub_group_2) } - let(:milestone1) { create(:milestone, project_id: old_project.id, title: 'v9.0') } let(:old_issue) do - create(:issue, title: title, description: description, - project: old_project, author: author, milestone: milestone1) + create(:issue, title: title, description: description, project: old_project, author: author) end subject(:move_service) do @@ -25,16 +23,6 @@ describe Issues::MoveService do before do old_project.add_reporter(user) new_project.add_reporter(user) - - labels = Array.new(2) { |x| "label%d" % (x + 1) } - - labels.each do |label| - old_issue.labels << create(:label, - project_id: old_project.id, - title: label) - - new_project.labels << create(:label, title: label) - end end end @@ -48,91 +36,6 @@ describe Issues::MoveService do context 'issue movable' do include_context 'user can move issue' - context 'move to new milestone' do - let(:new_issue) { move_service.execute(old_issue, new_project) } - - context 'project milestone' do - let!(:milestone2) do - create(:milestone, project_id: new_project.id, title: 'v9.0') - end - - it 'assigns milestone to new issue' do - expect(new_issue.reload.milestone.title).to eq 'v9.0' - expect(new_issue.reload.milestone).to eq(milestone2) - end - end - - context 'group milestones' do - let!(:group) { create(:group, :private) } - let!(:group_milestone_1) do - create(:milestone, group_id: group.id, title: 'v9.0_group') - end - - before do - old_issue.update(milestone: group_milestone_1) - old_project.update(namespace: group) - new_project.update(namespace: group) - - group.add_users([user], GroupMember::DEVELOPER) - end - - context 'when moving to a project of the same group' do - it 'keeps the same group milestone' do - expect(new_issue.reload.project).to eq(new_project) - expect(new_issue.reload.milestone).to eq(group_milestone_1) - end - end - - context 'when moving to a project of a different group' do - let!(:group_2) { create(:group, :private) } - - let!(:group_milestone_2) do - create(:milestone, group_id: group_2.id, title: 'v9.0_group') - end - - before do - old_issue.update(milestone: group_milestone_1) - new_project.update(namespace: group_2) - - group_2.add_users([user], GroupMember::DEVELOPER) - end - - it 'assigns to new group milestone of same title' do - expect(new_issue.reload.project).to eq(new_project) - expect(new_issue.reload.milestone).to eq(group_milestone_2) - end - end - end - end - - context 'issue with group labels', :nested_groups do - it 'assigns group labels to new issue' do - label = create(:group_label, group: group) - label_issue = create(:labeled_issue, description: description, project: old_project, - milestone: milestone1, labels: [label]) - old_project.add_reporter(user) - new_project.add_reporter(user) - - new_issue = move_service.execute(label_issue, new_project) - - expect(new_issue).to have_attributes( - project: new_project, - labels: include(label) - ) - end - end - - context 'issue with resource label events' do - it 'assigns resource label events to new issue' do - old_issue.resource_label_events = create_list(:resource_label_event, 2, issue: old_issue) - - new_issue = move_service.execute(old_issue, new_project) - - expected = old_issue.resource_label_events.map(&:label_id) - expect(new_issue.resource_label_events.map(&:label_id)).to match_array(expected) - end - end - context 'generic issue' do include_context 'issue move executed' @@ -140,18 +43,6 @@ describe Issues::MoveService do expect(new_issue.project).to eq new_project end - it 'assign labels to new issue' do - expected_label_titles = new_issue.reload.labels.map(&:title) - expect(expected_label_titles).to include 'label1' - expect(expected_label_titles).to include 'label2' - expect(expected_label_titles.size).to eq 2 - - new_issue.labels.each do |label| - expect(new_project.labels).to include(label) - expect(old_project.labels).not_to include(label) - end - end - it 'rewrites issue title' do expect(new_issue.title).to eq title end @@ -203,140 +94,25 @@ describe Issues::MoveService do end end - context 'issue with notes' do - context 'notes without references' do - let(:notes_params) do - [{ system: false, note: 'Some comment 1' }, - { system: true, note: 'Some system note' }, - { system: false, note: 'Some comment 2' }] - end - let(:award_names) { %w(thumbsup thumbsdown facepalm) } - let(:notes_contents) { notes_params.map { |n| n[:note] } } - - before do - note_params = { noteable: old_issue, project: old_project, author: author } - notes_params.each_with_index do |note, index| - new_note = create(:note, note_params.merge(note)) - award_emoji_params = { awardable: new_note, name: award_names[index] } - create(:award_emoji, award_emoji_params) - end - end - - include_context 'issue move executed' - - let(:all_notes) { new_issue.notes.order('id ASC') } - let(:system_notes) { all_notes.system } - let(:user_notes) { all_notes.user } - - it 'rewrites existing notes in valid order' do - expect(all_notes.pluck(:note).first(3)).to eq notes_contents - end - - it 'creates new emojis for the new notes' do - expect(all_notes.map(&:award_emoji).to_a.flatten.map(&:name)).to eq award_names - end - - it 'adds a system note about move after rewritten notes' do - expect(system_notes.last.note).to match /^moved from/ - end - - it 'preserves orignal author of comment' do - expect(user_notes.pluck(:author_id)).to all(eq(author.id)) - end - end - - context 'note that has been updated' do - let!(:note) do - create(:note, noteable: old_issue, project: old_project, - author: author, updated_at: Date.yesterday, - created_at: Date.yesterday) - end - - include_context 'issue move executed' - - it 'preserves time when note has been created at' do - expect(new_issue.notes.first.created_at).to eq note.created_at - end + context 'issue with assignee' do + let(:assignee) { create(:user) } - it 'preserves time when note has been updated at' do - expect(new_issue.notes.first.updated_at).to eq note.updated_at - end - end - - context 'issue with assignee' do - let(:assignee) { create(:user) } - - before do - old_issue.assignees = [assignee] - end - - it 'preserves assignee with access to the new issue' do - new_project.add_reporter(assignee) - - new_issue = move_service.execute(old_issue, new_project) - - expect(new_issue.assignees).to eq([assignee]) - end - - it 'ignores assignee without access to the new issue' do - new_issue = move_service.execute(old_issue, new_project) - - expect(new_issue.assignees).to be_empty - end - end - - context 'notes with references' do - before do - create(:merge_request, source_project: old_project) - create(:note, noteable: old_issue, project: old_project, author: author, - note: 'Note with reference to merge request !1') - end - - include_context 'issue move executed' - let(:new_note) { new_issue.notes.first } - - it 'rewrites references using a cross reference to old project' do - expect(new_note.note) - .to eq "Note with reference to merge request #{old_project.to_reference(new_project)}!1" - end - end - - context 'issue description with uploads' do - let(:uploader) { build(:file_uploader, project: old_project) } - let(:description) { "Text and #{uploader.markdown_link}" } - - include_context 'issue move executed' - - it 'rewrites uploads in description' do - expect(new_issue.description).not_to eq description - expect(new_issue.description) - .to match(/Text and #{FileUploader::MARKDOWN_PATTERN}/) - expect(new_issue.description).not_to include uploader.secret - end + before do + old_issue.assignees = [assignee] end - end - describe 'rewriting references' do - include_context 'issue move executed' + it 'preserves assignee with access to the new issue' do + new_project.add_reporter(assignee) - context 'issue references' do - let(:another_issue) { create(:issue, project: old_project) } - let(:description) { "Some description #{another_issue.to_reference}" } + new_issue = move_service.execute(old_issue, new_project) - it 'rewrites referenced issues creating cross project reference' do - expect(new_issue.description) - .to eq "Some description #{another_issue.to_reference(new_project)}" - end + expect(new_issue.assignees).to eq([assignee]) end - context "user references" do - let(:another_issue) { create(:issue, project: old_project) } - let(:description) { "Some description #{user.to_reference}" } + it 'ignores assignee without access to the new issue' do + new_issue = move_service.execute(old_issue, new_project) - it "doesn't throw any errors for issues containing user references" do - expect(new_issue.description) - .to eq "Some description #{user.to_reference}" - end + expect(new_issue.assignees).to be_empty end end @@ -416,25 +192,5 @@ describe Issues::MoveService do it { expect { move }.to raise_error(StandardError, /permissions/) } end end - - context 'movable issue with no assigned labels' do - before do - old_project.add_reporter(user) - new_project.add_reporter(user) - - labels = Array.new(2) { |x| "label%d" % (x + 1) } - - labels.each do |label| - new_project.labels << create(:label, title: label) - end - end - - include_context 'issue move executed' - - it 'does not assign labels to new issue' do - expected_label_titles = new_issue.reload.labels.map(&:title) - expect(expected_label_titles.size).to eq 0 - end - end end end diff --git a/spec/services/merge_requests/reload_diffs_service_spec.rb b/spec/services/merge_requests/reload_diffs_service_spec.rb index 546c9f277c5..5acd01828cb 100644 --- a/spec/services/merge_requests/reload_diffs_service_spec.rb +++ b/spec/services/merge_requests/reload_diffs_service_spec.rb @@ -31,32 +31,11 @@ describe MergeRequests::ReloadDiffsService, :use_clean_rails_memory_store_cachin end context 'cache clearing' do - before do - allow_any_instance_of(Gitlab::Diff::File).to receive(:text?).and_return(true) - allow_any_instance_of(Gitlab::Diff::File).to receive(:diffable?).and_return(true) - end - - it 'retrieves the diff files to cache the highlighted result' do - new_diff = merge_request.create_merge_request_diff - cache_key = new_diff.diffs_collection.cache_key - - expect(merge_request).to receive(:create_merge_request_diff).and_return(new_diff) - expect(Rails.cache).to receive(:read).with(cache_key).and_call_original - expect(Rails.cache).to receive(:write).with(cache_key, anything, anything).and_call_original - - subject.execute - end - it 'clears the cache for older diffs on the merge request' do old_diff = merge_request.merge_request_diff old_cache_key = old_diff.diffs_collection.cache_key - new_diff = merge_request.create_merge_request_diff - new_cache_key = new_diff.diffs_collection.cache_key - expect(merge_request).to receive(:create_merge_request_diff).and_return(new_diff) expect(Rails.cache).to receive(:delete).with(old_cache_key).and_call_original - expect(Rails.cache).to receive(:read).with(new_cache_key).and_call_original - expect(Rails.cache).to receive(:write).with(new_cache_key, anything, anything).and_call_original subject.execute end diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index b1290fd0d47..80b015d4cd0 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -57,6 +57,57 @@ describe Notes::CreateService do end end + context 'noteable highlight cache clearing' do + let(:project_with_repo) { create(:project, :repository) } + let(:merge_request) do + create(:merge_request, source_project: project_with_repo, + target_project: project_with_repo) + end + + let(:position) do + Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 14, + diff_refs: merge_request.diff_refs) + end + + let(:new_opts) do + opts.merge(in_reply_to_discussion_id: nil, + type: 'DiffNote', + noteable_type: 'MergeRequest', + noteable_id: merge_request.id, + position: position.to_h) + end + + before do + allow_any_instance_of(Gitlab::Diff::Position) + .to receive(:unfolded_diff?) { true } + end + + it 'clears noteable diff cache when it was unfolded for the note position' do + expect_any_instance_of(Gitlab::Diff::HighlightCache).to receive(:clear) + + described_class.new(project_with_repo, user, new_opts).execute + end + + it 'does not clear cache when note is not the first of the discussion' do + prev_note = + create(:diff_note_on_merge_request, noteable: merge_request, + project: project_with_repo) + reply_opts = + opts.merge(in_reply_to_discussion_id: prev_note.discussion_id, + type: 'DiffNote', + noteable_type: 'MergeRequest', + noteable_id: merge_request.id, + position: position.to_h) + + expect(merge_request).not_to receive(:diffs) + + described_class.new(project_with_repo, user, reply_opts).execute + end + end + context 'note diff file' do let(:project_with_repo) { create(:project, :repository) } let(:merge_request) do diff --git a/spec/services/notes/destroy_service_spec.rb b/spec/services/notes/destroy_service_spec.rb index 64445be560e..b1f4e87e8ea 100644 --- a/spec/services/notes/destroy_service_spec.rb +++ b/spec/services/notes/destroy_service_spec.rb @@ -21,5 +21,38 @@ describe Notes::DestroyService do expect { described_class.new(project, user).execute(note) } .to change { user.todos_pending_count }.from(1).to(0) end + + context 'noteable highlight cache clearing' do + let(:repo_project) { create(:project, :repository) } + let(:merge_request) do + create(:merge_request, source_project: repo_project, + target_project: repo_project) + end + + let(:note) do + create(:diff_note_on_merge_request, project: repo_project, + noteable: merge_request) + end + + before do + allow(note.position).to receive(:unfolded_diff?) { true } + end + + it 'clears noteable diff cache when it was unfolded for the note position' do + expect(merge_request).to receive_message_chain(:diffs, :clear_cache) + + described_class.new(repo_project, user).execute(note) + end + + it 'does not clear cache when note is not the first of the discussion' do + reply_note = create(:diff_note_on_merge_request, in_reply_to: note, + project: repo_project, + noteable: merge_request) + + expect(merge_request).not_to receive(:diffs) + + described_class.new(repo_project, user).execute(reply_note) + end + end end end diff --git a/spec/support/shared_examples/models/chat_service_spec.rb b/spec/support/shared_examples/models/chat_service_spec.rb new file mode 100644 index 00000000000..cf1d52a9616 --- /dev/null +++ b/spec/support/shared_examples/models/chat_service_spec.rb @@ -0,0 +1,242 @@ +require "spec_helper" + +shared_examples_for "chat service" do |service_name| + describe "Associations" do + it { is_expected.to belong_to :project } + it { is_expected.to have_one :service_hook } + end + + describe "Validations" do + context "when service is active" do + before do + subject.active = true + end + + it { is_expected.to validate_presence_of(:webhook) } + it_behaves_like "issue tracker service URL attribute", :webhook + end + + context "when service is inactive" do + before do + subject.active = false + end + + it { is_expected.not_to validate_presence_of(:webhook) } + end + end + + describe "#execute" do + let(:user) { create(:user) } + let(:project) { create(:project, :repository) } + let(:webhook_url) { "https://example.gitlab.com/" } + + before do + allow(subject).to receive_messages( + project: project, + project_id: project.id, + service_hook: true, + webhook: webhook_url + ) + + WebMock.stub_request(:post, webhook_url) + end + + shared_examples "#{service_name} service" do + it "calls #{service_name} API" do + subject.execute(sample_data) + + expect(WebMock).to have_requested(:post, webhook_url).with { |req| req.body =~ /\A{"#{content_key}":.+}\Z/ }.once + end + end + + context "with push events" do + let(:sample_data) do + Gitlab::DataBuilder::Push.build_sample(project, user) + end + + it_behaves_like "#{service_name} service" + + it "specifies the webhook when it is configured" do + expect(client).to receive(:new).with(client_arguments).and_return(double(:chat_service).as_null_object) + + subject.execute(sample_data) + end + + context "with not default branch" do + let(:sample_data) do + Gitlab::DataBuilder::Push.build(project, user, nil, nil, "not-the-default-branch") + end + + context "when notify_only_default_branch enabled" do + before do + subject.notify_only_default_branch = true + end + + it "does not call the Discord Webhooks API" do + result = subject.execute(sample_data) + + expect(result).to be_falsy + end + end + + context "when notify_only_default_branch disabled" do + before do + subject.notify_only_default_branch = false + end + + it_behaves_like "#{service_name} service" + end + end + end + + context "with issue events" do + let(:opts) { { title: "Awesome issue", description: "please fix" } } + let(:sample_data) do + service = Issues::CreateService.new(project, user, opts) + issue = service.execute + service.hook_data(issue, "open") + end + + it_behaves_like "#{service_name} service" + end + + context "with merge events" do + let(:opts) do + { + title: "Awesome merge_request", + description: "please fix", + source_branch: "feature", + target_branch: "master" + } + end + + let(:sample_data) do + service = MergeRequests::CreateService.new(project, user, opts) + merge_request = service.execute + service.hook_data(merge_request, "open") + end + + before do + project.add_developer(user) + end + + it_behaves_like "#{service_name} service" + end + + context "with wiki page events" do + let(:opts) do + { + title: "Awesome wiki_page", + content: "Some text describing some thing or another", + format: "md", + message: "user created page: Awesome wiki_page" + } + end + let(:wiki_page) { create(:wiki_page, wiki: project.wiki, attrs: opts) } + let(:sample_data) { Gitlab::DataBuilder::WikiPage.build(wiki_page, user, "create") } + + it_behaves_like "#{service_name} service" + end + + context "with note events" do + let(:sample_data) { Gitlab::DataBuilder::Note.build(note, user) } + + context "with commit comment" do + let(:note) do + create(:note_on_commit, + author: user, + project: project, + commit_id: project.repository.commit.id, + note: "a comment on a commit") + end + + it_behaves_like "#{service_name} service" + end + + context "with merge request comment" do + let(:note) do + create(:note_on_merge_request, project: project, note: "merge request note") + end + + it_behaves_like "#{service_name} service" + end + + context "with issue comment" do + let(:note) do + create(:note_on_issue, project: project, note: "issue note") + end + + it_behaves_like "#{service_name} service" + end + + context "with snippet comment" do + let(:note) do + create(:note_on_project_snippet, project: project, note: "snippet note") + end + + it_behaves_like "#{service_name} service" + end + end + + context "with pipeline events" do + let(:pipeline) do + create(:ci_pipeline, + project: project, status: status, + sha: project.commit.sha, ref: project.default_branch) + end + let(:sample_data) { Gitlab::DataBuilder::Pipeline.build(pipeline) } + + context "with failed pipeline" do + let(:status) { "failed" } + + it_behaves_like "#{service_name} service" + end + + context "with succeeded pipeline" do + let(:status) { "success" } + + context "with default notify_only_broken_pipelines" do + it "does not call Discord Webhooks API" do + result = subject.execute(sample_data) + + expect(result).to be_falsy + end + end + + context "when notify_only_broken_pipelines is false" do + before do + subject.notify_only_broken_pipelines = false + end + + it_behaves_like "#{service_name} service" + end + end + + context "with not default branch" do + let(:pipeline) do + create(:ci_pipeline, project: project, status: "failed", ref: "not-the-default-branch") + end + + context "when notify_only_default_branch enabled" do + before do + subject.notify_only_default_branch = true + end + + it "does not call the Discord Webhooks API" do + result = subject.execute(sample_data) + + expect(result).to be_falsy + end + end + + context "when notify_only_default_branch disabled" do + before do + subject.notify_only_default_branch = false + end + + it_behaves_like "#{service_name} service" + end + end + end + end +end diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb index 7e24efda5dd..c74e0bf1955 100644 --- a/spec/uploaders/file_uploader_spec.rb +++ b/spec/uploaders/file_uploader_spec.rb @@ -81,19 +81,24 @@ describe FileUploader do end describe 'copy_to' do + let(:new_project) { create(:project) } + let(:moved) { described_class.copy_to(subject, new_project) } + shared_examples 'returns a valid uploader' do describe 'returned uploader' do - let(:new_project) { create(:project) } - let(:moved) { described_class.copy_to(subject, new_project) } - it 'generates a new secret' do expect(subject).to be expect(described_class).to receive(:generate_secret).once.and_call_original expect(moved).to be end - it 'create new upload' do - expect(moved.upload).not_to eq(subject.upload) + it 'creates new upload correctly' do + upload = moved.upload + + expect(upload).not_to eq(subject.upload) + expect(upload.model).to eq(new_project) + expect(upload.uploader).to eq('FileUploader') + expect(upload.secret).not_to eq(subject.upload.secret) end it 'copies the file' do @@ -111,6 +116,12 @@ describe FileUploader do end include_examples 'returns a valid uploader' + + it 'copies the file to the correct location' do + expect(moved.upload.path).to eq("#{moved.upload.secret}/dk.png") + expect(moved.file.path).to end_with("public/uploads/#{new_project.disk_path}/#{moved.upload.secret}/dk.png") + expect(moved.filename).to eq('dk.png') + end end context 'files are stored remotely' do @@ -121,6 +132,12 @@ describe FileUploader do end include_examples 'returns a valid uploader' + + it 'copies the file to the correct location' do + expect(moved.upload.path).to eq("#{new_project.disk_path}/#{moved.upload.secret}/dk.png") + expect(moved.file.path).to eq("#{new_project.disk_path}/#{moved.upload.secret}/dk.png") + expect(moved.filename).to eq('dk.png') + end end end diff --git a/spec/uploaders/namespace_file_uploader_spec.rb b/spec/uploaders/namespace_file_uploader_spec.rb index 799c6db57fa..d09725ee4be 100644 --- a/spec/uploaders/namespace_file_uploader_spec.rb +++ b/spec/uploaders/namespace_file_uploader_spec.rb @@ -55,4 +55,62 @@ describe NamespaceFileUploader do it_behaves_like "migrates", to_store: described_class::Store::REMOTE it_behaves_like "migrates", from_store: described_class::Store::REMOTE, to_store: described_class::Store::LOCAL end + + describe 'copy_to' do + let(:group) { create(:group) } + let(:moved) { described_class.copy_to(subject, group) } + + shared_examples 'returns a valid uploader' do + it 'generates a new secret' do + expect(subject).to be + expect(described_class).to receive(:generate_secret).once.and_call_original + expect(moved).to be + end + + it 'creates new upload correctly' do + upload = moved.upload + + expect(upload).not_to eq(subject.upload) + expect(upload.model).to eq(group) + expect(upload.uploader).to eq('NamespaceFileUploader') + expect(upload.secret).not_to eq(subject.upload.secret) + end + + it 'copies the file' do + expect(subject.file).to exist + expect(moved.file).to exist + expect(subject.file).not_to eq(moved.file) + expect(subject.object_store).to eq(moved.object_store) + end + end + + context 'files are stored locally' do + before do + subject.store!(fixture_file_upload('spec/fixtures/dk.png')) + end + + include_examples 'returns a valid uploader' + + it 'copies the file to the correct location' do + expect(moved.upload.path).to eq("#{moved.upload.secret}/dk.png") + expect(moved.file.path).to end_with("system/namespace/#{group.id}/#{moved.upload.secret}/dk.png") + expect(moved.filename).to eq('dk.png') + end + end + + context 'files are stored remotely' do + before do + stub_uploads_object_storage + subject.store!(fixture_file_upload('spec/fixtures/dk.png')) + subject.migrate!(ObjectStorage::Store::REMOTE) + end + + include_examples 'returns a valid uploader' + + it 'copies the file to the correct location' do + expect(moved.file.path).to eq("namespace/#{group.id}/#{moved.upload.secret}/dk.png") + expect(moved.filename).to eq('dk.png') + end + end + end end |