diff options
| author | Filipa Lacerda <filipa@gitlab.com> | 2017-12-20 09:47:43 +0000 |
|---|---|---|
| committer | Filipa Lacerda <filipa@gitlab.com> | 2017-12-20 09:47:43 +0000 |
| commit | 800ab47a53ea1dfaabdb67501360a14322268315 (patch) | |
| tree | f6bb2327510806a066cc3776120a2d92b23cc205 | |
| parent | 572de0c1c2788d8d434a967f6ef0cb144f7f668f (diff) | |
| parent | febb0b9a8014f5b480ff7baab1d189fce49210a5 (diff) | |
| download | gitlab-ce-800ab47a53ea1dfaabdb67501360a14322268315.tar.gz | |
Merge branch 'master' into 22643-manual-job-page
* master: (50 commits)
Prevent some specs from mangling the gitlab-shell checkout
Line up search dropdown with other nav dropdowns
Fix onion-skin re-entering state
Remove related links in MR widget when empty state
Show inline edit button for issues
Fix tags in the Activity tab not being clickable
Fix shortcut links on help page
Don't link LFS-objects multiple times.
[CE->EE] Fix spec/lib/gitlab/git/gitlab_projects_spec.rb
Tidy up the documentation of Gitlab HA/Gitlab Application
Make sure two except won't overwrite each other
Update axios.md
Remove transitionend event from GL dropdown
Preserve gem path so that we use the same gems
Load commit in batches for pipelines#index
BlobViewer::PackageJson - if private link to homepage
Do not generate links for private NPM modules in blob view
Fix missing WHERE clause in 20171106135924_issues_milestone_id_foreign_key migration
Inverse the has_multiple_clusters? helper usage
Remove block styling from search dropdown
...
109 files changed, 1672 insertions, 796 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index d35fd28c766..c26e7f0aeba 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,5 +1,10 @@ image: "dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.3.5-golang-1.8-git-2.14-chrome-63.0-node-8.x-yarn-1.2-postgresql-9.6" +.dedicated-runner: &dedicated-runner + retry: 1 + tags: + - gitlab-org + .default-cache: &default-cache key: "ruby-235-with-yarn" paths: @@ -42,11 +47,6 @@ stages: - post-cleanup # Predefined scopes -.dedicated-runner: &dedicated-runner - retry: 1 - tags: - - gitlab-org - .tests-metadata-state: &tests-metadata-state <<: *dedicated-runner variables: @@ -80,11 +80,15 @@ stages: except: - /(^qa[\/-].*|.*-qa$)/ +.except-docs-and-qa: &except-docs-and-qa + except: + - /(^docs[\/-].*|.*-docs$)/ + - /(^qa[\/-].*|.*-qa$)/ + .rspec-metadata: &rspec-metadata <<: *dedicated-runner + <<: *except-docs-and-qa <<: *pull-cache - <<: *except-docs - <<: *except-qa stage: test script: - JOB_NAME=( $CI_JOB_NAME ) @@ -121,9 +125,8 @@ stages: .spinach-metadata: &spinach-metadata <<: *dedicated-runner + <<: *except-docs-and-qa <<: *pull-cache - <<: *except-docs - <<: *except-qa stage: test script: - JOB_NAME=( $CI_JOB_NAME ) @@ -162,6 +165,7 @@ stages: # Trigger a package build in omnibus-gitlab repository # package-qa: + <<: *dedicated-runner image: ruby:2.4-alpine before_script: [] stage: build @@ -175,6 +179,7 @@ package-qa: # Review docs base .review-docs: &review-docs + <<: *dedicated-runner <<: *except-qa image: ruby:2.4-alpine before_script: @@ -220,8 +225,7 @@ review-docs-cleanup: # Retrieve knapsack and rspec_flaky reports retrieve-tests-metadata: <<: *tests-metadata-state - <<: *except-docs - <<: *except-qa + <<: *except-docs-and-qa stage: prepare cache: key: tests_metadata @@ -284,9 +288,9 @@ flaky-examples-check: - scripts/detect-new-flaky-examples $NEW_FLAKY_SPECS_REPORT setup-test-env: - <<: *use-pg <<: *dedicated-runner <<: *except-docs + <<: *use-pg stage: prepare cache: <<: *default-cache @@ -375,19 +379,18 @@ spinach-mysql 3 4: *spinach-metadata-mysql SETUP_DB: "false" .rake-exec: &rake-exec - <<: *ruby-static-analysis <<: *dedicated-runner - <<: *except-docs - <<: *except-qa + <<: *except-docs-and-qa <<: *pull-cache + <<: *ruby-static-analysis stage: test script: - bundle exec rake $CI_JOB_NAME static-analysis: - <<: *ruby-static-analysis <<: *dedicated-runner <<: *except-docs + <<: *ruby-static-analysis stage: test script: - scripts/static-analysis @@ -441,8 +444,7 @@ ee_compat_check: # DB migration, rollback, and seed jobs .db-migrate-reset: &db-migrate-reset <<: *dedicated-runner - <<: *except-docs - <<: *except-qa + <<: *except-docs-and-qa <<: *pull-cache stage: test script: @@ -456,11 +458,16 @@ db:migrate:reset-mysql: <<: *db-migrate-reset <<: *use-mysql +db:check-schema-pg: + <<: *db-migrate-reset + <<: *use-pg + script: + - source scripts/schema_changed.sh + .migration-paths: &migration-paths <<: *dedicated-runner + <<: *except-docs-and-qa <<: *pull-cache - <<: *except-docs - <<: *except-qa stage: test variables: SETUP_DB: "false" @@ -486,8 +493,7 @@ migration:path-mysql: .db-rollback: &db-rollback <<: *dedicated-runner - <<: *except-docs - <<: *except-qa + <<: *except-docs-and-qa <<: *pull-cache stage: test script: @@ -504,8 +510,7 @@ db:rollback-mysql: .db-seed_fu: &db-seed_fu <<: *dedicated-runner - <<: *except-docs - <<: *except-qa + <<: *except-docs-and-qa <<: *pull-cache stage: test variables: @@ -530,17 +535,10 @@ db:seed_fu-mysql: <<: *db-seed_fu <<: *use-mysql -db:check-schema-pg: - <<: *db-migrate-reset - <<: *use-pg - script: - - source scripts/schema_changed.sh - # Frontend-related jobs gitlab:assets:compile: <<: *dedicated-runner - <<: *except-docs - <<: *except-qa + <<: *except-docs-and-qa <<: *pull-cache stage: test dependencies: [] @@ -561,11 +559,10 @@ gitlab:assets:compile: - webpack-report/ karma: - <<: *use-pg <<: *dedicated-runner - <<: *except-docs - <<: *except-qa + <<: *except-docs-and-qa <<: *pull-cache + <<: *use-pg stage: test variables: BABEL_ENV: "coverage" @@ -604,6 +601,7 @@ codequality: paths: [codeclimate.json] qa:internal: + <<: *dedicated-runner <<: *except-docs stage: test variables: @@ -616,8 +614,7 @@ qa:internal: coverage: <<: *dedicated-runner - <<: *except-docs - <<: *except-qa + <<: *except-docs-and-qa <<: *pull-cache stage: post-test services: [] @@ -636,8 +633,7 @@ coverage: lint:javascript:report: <<: *dedicated-runner - <<: *except-docs - <<: *except-qa + <<: *except-docs-and-qa <<: *pull-cache stage: post-test dependencies: @@ -695,9 +691,9 @@ cache gems: - master@gitlab-org/gitlab-ee gitlab_git_test: + <<: *dedicated-runner + <<: *except-docs-and-qa <<: *pull-cache - <<: *except-docs - <<: *except-qa variables: SETUP_DB: "false" script: @@ -263,7 +263,7 @@ gem 'gettext_i18n_rails', '~> 1.8.0' gem 'gettext_i18n_rails_js', '~> 1.2.0' gem 'gettext', '~> 3.2.2', require: false, group: :development -gem 'batch-loader' +gem 'batch-loader', '~> 1.2.1' # Perf bar gem 'peek', '~> 1.0.1' diff --git a/Gemfile.lock b/Gemfile.lock index 11040fab805..a6e3c9e27cc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -78,7 +78,7 @@ GEM thread_safe (~> 0.3, >= 0.3.1) babosa (1.0.2) base32 (0.3.2) - batch-loader (1.1.1) + batch-loader (1.2.1) bcrypt (3.1.11) bcrypt_pbkdf (1.0.0) benchmark-ips (2.3.0) @@ -988,7 +988,7 @@ DEPENDENCIES awesome_print (~> 1.2.0) babosa (~> 1.0.2) base32 (~> 0.3.0) - batch-loader + batch-loader (~> 1.2.1) bcrypt_pbkdf (~> 1.0) benchmark-ips (~> 2.3.0) better_errors (~> 2.1.0) diff --git a/app/assets/javascripts/commit/image_file.js b/app/assets/javascripts/commit/image_file.js index 5662802525e..b6a0ece7907 100644 --- a/app/assets/javascripts/commit/image_file.js +++ b/app/assets/javascripts/commit/image_file.js @@ -176,6 +176,7 @@ export default class ImageFile { left: dragTrackWidth }); + $frameAdded.css('opacity', 1); framePadding = parseInt($frameAdded.css('right').replace('px', ''), 10); _this.initDraggable($dragger, framePadding, function(e, left) { diff --git a/app/assets/javascripts/diff_notes/components/diff_note_avatars.js b/app/assets/javascripts/diff_notes/components/diff_note_avatars.js index 06ce84d7599..300b02da663 100644 --- a/app/assets/javascripts/diff_notes/components/diff_note_avatars.js +++ b/app/assets/javascripts/diff_notes/components/diff_note_avatars.js @@ -1,8 +1,8 @@ /* global CommentsStore */ -/* global notes */ import Vue from 'vue'; import collapseIcon from '../icons/collapse_icon.svg'; +import Notes from '../../notes'; import userAvatarImage from '../../vue_shared/components/user_avatar/user_avatar_image.vue'; const DiffNoteAvatars = Vue.extend({ @@ -129,7 +129,7 @@ const DiffNoteAvatars = Vue.extend({ }, methods: { clickedAvatar(e) { - notes.onAddDiffNote(e); + Notes.instance.onAddDiffNote(e); // Toggle the active state of the toggle all button this.toggleDiscussionsToggleState(); diff --git a/app/assets/javascripts/docs/docs_bundle.js b/app/assets/javascripts/docs/docs_bundle.js new file mode 100644 index 00000000000..a32bd6d0fc7 --- /dev/null +++ b/app/assets/javascripts/docs/docs_bundle.js @@ -0,0 +1,13 @@ +import Mousetrap from 'mousetrap'; + +function addMousetrapClick(el, key) { + el.addEventListener('click', () => Mousetrap.trigger(key)); +} + +function domContentLoaded() { + addMousetrapClick(document.querySelector('.js-trigger-shortcut'), '?'); + addMousetrapClick(document.querySelector('.js-trigger-search-bar'), 's'); +} + +document.addEventListener('DOMContentLoaded', domContentLoaded); + diff --git a/app/assets/javascripts/gl_dropdown.js b/app/assets/javascripts/gl_dropdown.js index cf4a70e321e..64f258aed64 100644 --- a/app/assets/javascripts/gl_dropdown.js +++ b/app/assets/javascripts/gl_dropdown.js @@ -300,7 +300,7 @@ GitLabDropdown = (function() { return function(data) { _this.fullData = data; _this.parseData(_this.fullData); - _this.focusTextInput(true); + _this.focusTextInput(); if (_this.options.filterable && _this.filter && _this.filter.input && _this.filter.input.val() && _this.filter.input.val().trim() !== '') { return _this.filter.input.trigger('input'); } @@ -790,24 +790,16 @@ GitLabDropdown = (function() { return [selectedObject, isMarking]; }; - GitLabDropdown.prototype.focusTextInput = function(triggerFocus = false) { + GitLabDropdown.prototype.focusTextInput = function() { if (this.options.filterable) { - this.dropdown.one('transitionend', () => { - const initialScrollTop = $(window).scrollTop(); + const initialScrollTop = $(window).scrollTop(); - if (this.dropdown.is('.open')) { - this.filterInput.focus(); - } - - if ($(window).scrollTop() < initialScrollTop) { - $(window).scrollTop(initialScrollTop); - } - }); + if (this.dropdown.is('.open')) { + this.filterInput.focus(); + } - if (triggerFocus) { - // This triggers after a ajax request - // in case of slow requests, the dropdown transition could already be finished - this.dropdown.trigger('transitionend'); + if ($(window).scrollTop() < initialScrollTop) { + $(window).scrollTop(initialScrollTop); } } }; diff --git a/app/assets/javascripts/init_notes.js b/app/assets/javascripts/init_notes.js index 3a8b4360cb6..882aedfcc76 100644 --- a/app/assets/javascripts/init_notes.js +++ b/app/assets/javascripts/init_notes.js @@ -1,4 +1,4 @@ -/* global Notes */ +import Notes from './notes'; export default () => { const dataEl = document.querySelector('.js-notes-data'); @@ -10,5 +10,7 @@ export default () => { autocomplete, } = JSON.parse(dataEl.innerHTML); - window.notes = new Notes(notesUrl, notesIds, now, diffView, autocomplete); + // Create a singleton so that we don't need to assign + // into the window object, we can just access the current isntance with Notes.instance + Notes.initialize(notesUrl, notesIds, now, diffView, autocomplete); }; diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue index 25ebe5314e0..952f49d522e 100644 --- a/app/assets/javascripts/issue_show/components/app.vue +++ b/app/assets/javascripts/issue_show/components/app.vue @@ -32,7 +32,7 @@ export default { showInlineEditButton: { type: Boolean, required: false, - default: false, + default: true, }, showDeleteButton: { type: Boolean, diff --git a/app/assets/javascripts/issue_show/components/title.vue b/app/assets/javascripts/issue_show/components/title.vue index a363d06d950..b7e6eadd440 100644 --- a/app/assets/javascripts/issue_show/components/title.vue +++ b/app/assets/javascripts/issue_show/components/title.vue @@ -79,7 +79,7 @@ v-tooltip v-if="showInlineEditButton && canUpdate" type="button" - class="btn btn-default btn-edit btn-svg" + class="btn btn-default btn-edit btn-svg js-issuable-edit" v-html="pencilIcon" title="Edit title and description" data-placement="bottom" diff --git a/app/assets/javascripts/issue_show/index.js b/app/assets/javascripts/issue_show/index.js index 7b762496ba5..75dfdedcf1b 100644 --- a/app/assets/javascripts/issue_show/index.js +++ b/app/assets/javascripts/issue_show/index.js @@ -1,5 +1,4 @@ import Vue from 'vue'; -import eventHub from './event_hub'; import issuableApp from './components/app.vue'; import '../vue_shared/vue_resource_interceptor'; @@ -7,12 +6,6 @@ document.addEventListener('DOMContentLoaded', () => { const initialDataEl = document.getElementById('js-issuable-app-initial-data'); const props = JSON.parse(initialDataEl.innerHTML.replace(/"/g, '"')); - $('.js-issuable-edit').on('click', (e) => { - e.preventDefault(); - - eventHub.$emit('open.form'); - }); - return new Vue({ el: document.getElementById('js-issuable-app'), components: { diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index b984914ad68..15a3a91c5f5 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -45,9 +45,7 @@ import './layout_nav'; import LazyLoader from './lazy_loader'; import './line_highlighter'; import initLogoAnimation from './logo'; -import './merge_request_tabs'; import './milestone_select'; -import './notes'; import './preview_markdown'; import './projects_dropdown'; import './render_gfm'; diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index cacca35ca98..acfc62fe5cb 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -1,5 +1,4 @@ /* eslint-disable no-new, class-methods-use-this */ -/* global notes */ import Cookies from 'js-cookie'; import Flash from './flash'; @@ -16,6 +15,7 @@ import initDiscussionTab from './image_diff/init_discussion_tab'; import Diff from './diff'; import { localTimeAgo } from './lib/utils/datetime_utility'; import syntaxHighlight from './syntax_highlight'; +import Notes from './notes'; /* eslint-disable max-len */ // MergeRequestTabs @@ -324,7 +324,7 @@ export default class MergeRequestTabs { if (anchor && anchor.length > 0) { const notesContent = anchor.closest('.notes_content'); const lineType = notesContent.hasClass('new') ? 'new' : 'old'; - notes.toggleDiffNote({ + Notes.instance.toggleDiffNote({ target: anchor, lineType, forceShow: true, diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 042fe44e1c6..a2b8e6f6495 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -37,6 +37,12 @@ const MAX_VISIBLE_COMMIT_LIST_COUNT = 3; const REGEX_QUICK_ACTIONS = /^\/\w+.*$/gm; export default class Notes { + static initialize(notes_url, note_ids, last_fetched_at, view, enableGFM = true) { + if (!this.instance) { + this.instance = new Notes(notes_url, note_ids, last_fetched_at, view, enableGFM); + } + } + constructor(notes_url, note_ids, last_fetched_at, view, enableGFM = true) { this.updateTargetButtons = this.updateTargetButtons.bind(this); this.updateComment = this.updateComment.bind(this); diff --git a/app/assets/javascripts/shortcuts.js b/app/assets/javascripts/shortcuts.js index 130730b1700..d2f0d7410da 100644 --- a/app/assets/javascripts/shortcuts.js +++ b/app/assets/javascripts/shortcuts.js @@ -51,7 +51,10 @@ export default class Shortcuts { } onToggleHelp(e) { - e.preventDefault(); + if (e.preventDefault) { + e.preventDefault(); + } + Shortcuts.toggleHelp(this.enabledHelp); } @@ -112,6 +115,9 @@ export default class Shortcuts { static focusSearch(e) { $('#search').focus(); - e.preventDefault(); + + if (e.preventDefault) { + e.preventDefault(); + } } } diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js index 9cb3edead86..8a9129c385b 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js +++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js @@ -62,7 +62,7 @@ export default { return this.mr.hasCI; }, shouldRenderRelatedLinks() { - return !!this.mr.relatedLinks; + return !!this.mr.relatedLinks && !this.mr.isNothingToMergeState; }, shouldRenderDeployments() { return this.mr.deployments.length; diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js b/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js index 7c15abfff10..2bace3311c8 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js @@ -1,30 +1,32 @@ +import { stateKey } from './state_maps'; + export default function deviseState(data) { if (data.project_archived) { - return 'archived'; + return stateKey.archived; } else if (data.branch_missing) { - return 'missingBranch'; + return stateKey.missingBranch; } else if (!data.commits_count) { - return 'nothingToMerge'; + return stateKey.nothingToMerge; } else if (this.mergeStatus === 'unchecked') { - return 'checking'; + return stateKey.checking; } else if (data.has_conflicts) { - return 'conflicts'; + return stateKey.conflicts; } else if (data.work_in_progress) { - return 'workInProgress'; + return stateKey.workInProgress; } else if (this.onlyAllowMergeIfPipelineSucceeds && this.isPipelineFailed) { - return 'pipelineFailed'; + return stateKey.pipelineFailed; } else if (this.hasMergeableDiscussionsState) { - return 'unresolvedDiscussions'; + return stateKey.unresolvedDiscussions; } else if (this.isPipelineBlocked) { - return 'pipelineBlocked'; + return stateKey.pipelineBlocked; } else if (this.hasSHAChanged) { - return 'shaMismatch'; + return stateKey.shaMismatch; } else if (this.mergeWhenPipelineSucceeds) { - return this.mergeError ? 'autoMergeFailed' : 'mergeWhenPipelineSucceeds'; + return this.mergeError ? stateKey.autoMergeFailed : stateKey.mergeWhenPipelineSucceeds; } else if (!this.canMerge) { - return 'notAllowedToMerge'; + return stateKey.notAllowedToMerge; } else if (this.canBeMerged) { - return 'readyToMerge'; + return stateKey.readyToMerge; } return null; } diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js index 707766e08e4..93d31a2a684 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js @@ -1,5 +1,6 @@ import Timeago from 'timeago.js'; import { getStateKey } from '../dependencies'; +import { stateKey } from './state_maps'; import { formatDate } from '../../lib/utils/datetime_utility'; export default class MergeRequestStore { @@ -120,6 +121,10 @@ export default class MergeRequestStore { } } + get isNothingToMergeState() { + return this.state === stateKey.nothingToMerge; + } + static getEventObject(event) { return { author: MergeRequestStore.getAuthorObject(event), diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js b/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js index 9074a064a6d..de980c175fb 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js @@ -31,6 +31,23 @@ const statesToShowHelpWidget = [ 'autoMergeFailed', ]; +export const stateKey = { + archived: 'archived', + missingBranch: 'missingBranch', + nothingToMerge: 'nothingToMerge', + checking: 'checking', + conflicts: 'conflicts', + workInProgress: 'workInProgress', + pipelineFailed: 'pipelineFailed', + unresolvedDiscussions: 'unresolvedDiscussions', + pipelineBlocked: 'pipelineBlocked', + shaMismatch: 'shaMismatch', + autoMergeFailed: 'autoMergeFailed', + mergeWhenPipelineSucceeds: 'mergeWhenPipelineSucceeds', + notAllowedToMerge: 'notAllowedToMerge', + readyToMerge: 'readyToMerge', +}; + export default { stateToComponentMap, statesToShowHelpWidget, diff --git a/app/assets/stylesheets/framework/contextual-sidebar.scss b/app/assets/stylesheets/framework/contextual-sidebar.scss index 8baf7ca23a4..2e417315ed7 100644 --- a/app/assets/stylesheets/framework/contextual-sidebar.scss +++ b/app/assets/stylesheets/framework/contextual-sidebar.scss @@ -9,12 +9,6 @@ padding-left: $contextual-sidebar-width; } - // Override position: absolute - .right-sidebar { - position: fixed; - height: calc(100% - #{$header-height}); - } - .issues-bulk-update.right-sidebar.right-sidebar-expanded .issuable-sidebar-header { padding: 10px 0 15px; } diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index 478269f3fcf..bc907a390d8 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -16,27 +16,18 @@ @mixin set-visible { transform: translateY(0); - visibility: visible; - opacity: 1; - transition-duration: 100ms, 150ms, 25ms; - transition-delay: 35ms, 50ms, 25ms; + display: block; } @mixin set-invisible { transform: translateY(-10px); - visibility: hidden; - opacity: 0; - transition-property: opacity, transform, visibility; - transition-duration: 70ms, 250ms, 250ms; - transition-timing-function: linear, $dropdown-animation-timing; - transition-delay: 25ms, 50ms, 0ms; + display: none; } .open { .dropdown-menu, .dropdown-menu-nav { @include set-visible; - display: block; min-height: 40px; @media (max-width: $screen-xs-max) { @@ -55,6 +46,11 @@ } } +// Get search dropdown to line up with other nav dropdowns +.search-input-container .dropdown-menu { + margin-top: 11px; +} + .dropdown-toggle { padding: 6px 8px 6px 10px; background-color: $white-light; @@ -214,7 +210,6 @@ .dropdown-menu, .dropdown-menu-nav { @include set-invisible; - display: block; position: absolute; width: auto; top: 100%; diff --git a/app/assets/stylesheets/framework/sidebar.scss b/app/assets/stylesheets/framework/sidebar.scss index 0742c0a2a09..d61809cb0a4 100644 --- a/app/assets/stylesheets/framework/sidebar.scss +++ b/app/assets/stylesheets/framework/sidebar.scss @@ -90,11 +90,6 @@ .right-sidebar { border-left: 1px solid $border-color; height: calc(100% - #{$header-height}); - - &.affix { - position: fixed; - top: $header-height; - } } .with-performance-bar .right-sidebar.affix { diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index e19196e0c41..e1637618ab2 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -122,7 +122,7 @@ } .right-sidebar { - position: absolute; + position: fixed; top: $header-height; bottom: 0; right: 0; @@ -502,7 +502,7 @@ top: $header-height + $performance-bar-height; .issuable-sidebar { - height: calc(100% - #{$header-height} - #{$performance-bar-height}); + height: calc(100% - #{$performance-bar-height}); } } diff --git a/app/assets/stylesheets/pages/search.scss b/app/assets/stylesheets/pages/search.scss index 49c8e546bf2..c9363188505 100644 --- a/app/assets/stylesheets/pages/search.scss +++ b/app/assets/stylesheets/pages/search.scss @@ -108,13 +108,6 @@ input[type="checkbox"]:hover { // Custom dropdown positioning .dropdown-menu { - transition-property: opacity, transform; - transition-duration: 250ms, 250ms; - transition-delay: 0ms, 25ms; - transition-timing-function: $dropdown-animation-timing; - transform: translateY(0); - opacity: 0; - display: block; left: -5px; } @@ -152,13 +145,6 @@ input[type="checkbox"]:hover { background-color: $nav-badge-bg; border-color: $border-color; } - - .dropdown-menu { - transition-duration: 100ms, 75ms; - transition-delay: 75ms, 100ms; - transform: translateY(7px); - opacity: 1; - } } &.has-value { diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index c3013884369..74a4f437dc8 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/app/controllers/concerns/issuable_actions.rb @@ -55,7 +55,6 @@ module IssuableActions def destroy Issuable::DestroyService.new(issuable.project, current_user).execute(issuable) - TodoService.new.destroy_issuable(issuable, current_user) name = issuable.human_class_name flash[:notice] = "The #{name} was successfully deleted." diff --git a/app/controllers/projects/clusters/gcp_controller.rb b/app/controllers/projects/clusters/gcp_controller.rb index b64f7a2a6bd..d3b9d8a9bbc 100644 --- a/app/controllers/projects/clusters/gcp_controller.rb +++ b/app/controllers/projects/clusters/gcp_controller.rb @@ -39,6 +39,7 @@ class Projects::Clusters::GcpController < Projects::ApplicationController params.require(:cluster).permit( :enabled, :name, + :environment_scope, provider_gcp_attributes: [ :gcp_project_id, :zone, diff --git a/app/controllers/projects/clusters/user_controller.rb b/app/controllers/projects/clusters/user_controller.rb index d7678512073..d0db64b2fa9 100644 --- a/app/controllers/projects/clusters/user_controller.rb +++ b/app/controllers/projects/clusters/user_controller.rb @@ -26,6 +26,7 @@ class Projects::Clusters::UserController < Projects::ApplicationController params.require(:cluster).permit( :enabled, :name, + :environment_scope, platform_kubernetes_attributes: [ :namespace, :api_url, diff --git a/app/controllers/projects/clusters_controller.rb b/app/controllers/projects/clusters_controller.rb index 4a7879db313..1dc7f1b3a7f 100644 --- a/app/controllers/projects/clusters_controller.rb +++ b/app/controllers/projects/clusters_controller.rb @@ -87,6 +87,7 @@ class Projects::ClustersController < Projects::ApplicationController if cluster.managed? params.require(:cluster).permit( :enabled, + :environment_scope, platform_kubernetes_attributes: [ :namespace ] @@ -95,6 +96,7 @@ class Projects::ClustersController < Projects::ApplicationController params.require(:cluster).permit( :enabled, :name, + :environment_scope, platform_kubernetes_attributes: [ :api_url, :token, diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb index ec7c645df5a..b478e7b5e05 100644 --- a/app/controllers/projects/pipeline_schedules_controller.rb +++ b/app/controllers/projects/pipeline_schedules_controller.rb @@ -1,9 +1,11 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController before_action :schedule, except: [:index, :new, :create] + before_action :play_rate_limit, only: [:play] + before_action :authorize_play_pipeline_schedule!, only: [:play] before_action :authorize_read_pipeline_schedule! before_action :authorize_create_pipeline_schedule!, only: [:new, :create] - before_action :authorize_update_pipeline_schedule!, except: [:index, :new, :create] + before_action :authorize_update_pipeline_schedule!, except: [:index, :new, :create, :play] before_action :authorize_admin_pipeline_schedule!, only: [:destroy] def index @@ -40,6 +42,18 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController end end + def play + job_id = RunPipelineScheduleWorker.perform_async(schedule.id, current_user.id) + + if job_id + flash[:notice] = "Successfully scheduled a pipeline to run. Go to the <a href=\"#{project_pipelines_path(@project)}\">Pipelines page</a> for details.".html_safe + else + flash[:alert] = 'Unable to schedule a pipeline to run immediately' + end + + redirect_to pipeline_schedules_path(@project) + end + def take_ownership if schedule.update(owner: current_user) redirect_to pipeline_schedules_path(@project) @@ -60,6 +74,17 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController private + def play_rate_limit + return unless current_user + + limiter = ::Gitlab::ActionRateLimiter.new(action: :play_pipeline_schedule) + + return unless limiter.throttled?([current_user, schedule], 1) + + flash[:alert] = 'You cannot play this scheduled pipeline at the moment. Please wait a minute.' + redirect_to pipeline_schedules_path(@project) + end + def schedule @schedule ||= project.pipeline_schedules.find(params[:id]) end @@ -70,6 +95,10 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController variables_attributes: [:id, :key, :value, :_destroy] ) end + def authorize_play_pipeline_schedule! + return access_denied! unless can?(current_user, :play_pipeline_schedule, schedule) + end + def authorize_update_pipeline_schedule! return access_denied! unless can?(current_user, :update_pipeline_schedule, schedule) end diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 7ad7b3003af..e146d0d3cd5 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -29,6 +29,8 @@ class Projects::PipelinesController < Projects::ApplicationController @pipelines_count = PipelinesFinder .new(project).execute.count + @pipelines.map(&:commit) # List commits for batch loading + respond_to do |format| format.html format.json do diff --git a/app/helpers/clusters_helper.rb b/app/helpers/clusters_helper.rb new file mode 100644 index 00000000000..7e4eb06b99d --- /dev/null +++ b/app/helpers/clusters_helper.rb @@ -0,0 +1,5 @@ +module ClustersHelper + def has_multiple_clusters?(project) + false + end +end diff --git a/app/helpers/gitlab_routing_helper.rb b/app/helpers/gitlab_routing_helper.rb index a77aa0ad2cc..7f3c118c7ab 100644 --- a/app/helpers/gitlab_routing_helper.rb +++ b/app/helpers/gitlab_routing_helper.rb @@ -182,6 +182,11 @@ module GitlabRoutingHelper edit_project_pipeline_schedule_path(project, schedule) end + def play_pipeline_schedule_path(schedule, *args) + project = schedule.project + play_project_pipeline_schedule_path(project, schedule, *args) + end + def take_ownership_pipeline_schedule_path(schedule, *args) project = schedule.project take_ownership_project_pipeline_schedule_path(project, schedule, *args) diff --git a/app/models/blob_viewer/dependency_manager.rb b/app/models/blob_viewer/dependency_manager.rb index a8d9be945dc..cc4950240af 100644 --- a/app/models/blob_viewer/dependency_manager.rb +++ b/app/models/blob_viewer/dependency_manager.rb @@ -27,10 +27,17 @@ module BlobViewer private - def package_name_from_json(key) - prepare! + def json_data + @json_data ||= begin + prepare! + JSON.parse(blob.data) + rescue + {} + end + end - JSON.parse(blob.data)[key] rescue nil + def package_name_from_json(key) + json_data[key] end def package_name_from_method_call(name) diff --git a/app/models/blob_viewer/package_json.rb b/app/models/blob_viewer/package_json.rb index 09221efb56c..46cd2f04f4d 100644 --- a/app/models/blob_viewer/package_json.rb +++ b/app/models/blob_viewer/package_json.rb @@ -16,7 +16,25 @@ module BlobViewer @package_name ||= package_name_from_json('name') end + def package_type + private? ? 'private package' : super + end + def package_url + private? ? homepage : npm_url + end + + private + + def private? + !!json_data['private'] + end + + def homepage + json_data['homepage'] + end + + def npm_url "https://www.npmjs.com/package/#{package_name}" end end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 28f154581a9..d4690da3be6 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -287,8 +287,12 @@ module Ci Ci::Pipeline.truncate_sha(sha) end + # NOTE: This is loaded lazily and will never be nil, even if the commit + # cannot be found. + # + # Use constructs like: `pipeline.commit.present?` def commit - @commit ||= project.commit_by(oid: sha) + @commit ||= Commit.lazy(project, sha) end def branch? @@ -338,12 +342,9 @@ module Ci end def latest? - return false unless ref - - commit = project.commit(ref) - return false unless commit + return false unless ref && commit.present? - commit.sha == sha + project.commit(ref) == commit end def retried diff --git a/app/models/commit.rb b/app/models/commit.rb index 13c31111134..2be07ca7d3c 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -86,6 +86,20 @@ class Commit def valid_hash?(key) !!(/\A#{COMMIT_SHA_PATTERN}\z/ =~ key) end + + def lazy(project, oid) + BatchLoader.for({ project: project, oid: oid }).batch do |items, loader| + items_by_project = items.group_by { |i| i[:project] } + + items_by_project.each do |project, commit_ids| + oids = commit_ids.map { |i| i[:oid] } + + project.repository.commits_by(oids: oids).each do |commit| + loader.call({ project: commit.project, oid: commit.id }, commit) if commit + end + end + end + end end attr_accessor :raw @@ -103,7 +117,7 @@ class Commit end def ==(other) - (self.class === other) && (raw == other.raw) + other.is_a?(self.class) && raw == other.raw end def self.reference_prefix @@ -224,8 +238,8 @@ class Commit notes.includes(:author) end - def method_missing(m, *args, &block) - @raw.__send__(m, *args, &block) # rubocop:disable GitlabSecurity/PublicSend + def method_missing(method, *args, &block) + @raw.__send__(method, *args, &block) # rubocop:disable GitlabSecurity/PublicSend end def respond_to_missing?(method, include_private = false) diff --git a/app/models/diff_discussion.rb b/app/models/diff_discussion.rb index 4a65738214b..d67b16584a4 100644 --- a/app/models/diff_discussion.rb +++ b/app/models/diff_discussion.rb @@ -22,12 +22,9 @@ class DiffDiscussion < Discussion def merge_request_version_params return unless for_merge_request? - return {} if active? - if on_merge_request_commit? - { commit_id: commit_id } - else - noteable.version_params_for(position.diff_refs) + version_params.tap do |params| + params[:commit_id] = commit_id if on_merge_request_commit? end end @@ -37,4 +34,12 @@ class DiffDiscussion < Discussion position: position.to_json ) end + + private + + def version_params + return {} if active? + + noteable.version_params_for(position.diff_refs) + end end diff --git a/app/models/repository.rb b/app/models/repository.rb index 552a354d1ce..387428d90a6 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -118,6 +118,18 @@ class Repository @commit_cache[oid] = find_commit(oid) end + def commits_by(oids:) + return [] unless oids.present? + + commits = Gitlab::Git::Commit.batch_by_oid(raw_repository, oids) + + if commits.present? + Commit.decorate(commits, @project) + else + [] + end + end + def commits(ref, path: nil, limit: nil, offset: nil, skip_merges: false, after: nil, before: nil) options = { repo: raw_repository, @@ -221,6 +233,12 @@ class Repository branch_names.include?(branch_name) end + def tag_exists?(tag_name) + return false unless raw_repository + + tag_names.include?(tag_name) + end + def ref_exists?(ref) !!raw_repository&.ref_exists?(ref) rescue ArgumentError diff --git a/app/policies/ci/pipeline_policy.rb b/app/policies/ci/pipeline_policy.rb index 4e689a9efd5..6363c382ff8 100644 --- a/app/policies/ci/pipeline_policy.rb +++ b/app/policies/ci/pipeline_policy.rb @@ -2,16 +2,18 @@ module Ci class PipelinePolicy < BasePolicy delegate { @subject.project } - condition(:protected_ref) do - access = ::Gitlab::UserAccess.new(@user, project: @subject.project) + condition(:protected_ref) { ref_protected?(@user, @subject.project, @subject.tag?, @subject.ref) } - if @subject.tag? - !access.can_create_tag?(@subject.ref) + rule { protected_ref }.prevent :update_pipeline + + def ref_protected?(user, project, tag, ref) + access = ::Gitlab::UserAccess.new(user, project: project) + + if tag + !access.can_create_tag?(ref) else - !access.can_update_branch?(@subject.ref) + !access.can_update_branch?(ref) end end - - rule { protected_ref }.prevent :update_pipeline end end diff --git a/app/policies/ci/pipeline_schedule_policy.rb b/app/policies/ci/pipeline_schedule_policy.rb index 6b7598e1821..abcf536b2f7 100644 --- a/app/policies/ci/pipeline_schedule_policy.rb +++ b/app/policies/ci/pipeline_schedule_policy.rb @@ -2,13 +2,23 @@ module Ci class PipelineSchedulePolicy < PipelinePolicy alias_method :pipeline_schedule, :subject + condition(:protected_ref) do + ref_protected?(@user, @subject.project, @subject.project.repository.tag_exists?(@subject.ref), @subject.ref) + end + condition(:owner_of_schedule) do can?(:developer_access) && pipeline_schedule.owned_by?(@user) end + rule { can?(:developer_access) }.policy do + enable :play_pipeline_schedule + end + rule { can?(:master_access) | owner_of_schedule }.policy do enable :update_pipeline_schedule enable :admin_pipeline_schedule end + + rule { protected_ref }.prevent :play_pipeline_schedule end end diff --git a/app/services/issuable/destroy_service.rb b/app/services/issuable/destroy_service.rb index 0610b401213..7197a426a72 100644 --- a/app/services/issuable/destroy_service.rb +++ b/app/services/issuable/destroy_service.rb @@ -1,8 +1,10 @@ module Issuable class DestroyService < IssuableBaseService def execute(issuable) - if issuable.destroy - issuable.update_project_counter_caches + TodoService.new.destroy_target(issuable) do |issuable| + if issuable.destroy + issuable.update_project_counter_caches + end end end end diff --git a/app/services/notes/destroy_service.rb b/app/services/notes/destroy_service.rb index b819bd17039..fb78420d324 100644 --- a/app/services/notes/destroy_service.rb +++ b/app/services/notes/destroy_service.rb @@ -1,7 +1,9 @@ module Notes class DestroyService < BaseService def execute(note) - note.destroy + TodoService.new.destroy_target(note) do |note| + note.destroy + end end end end diff --git a/app/services/projects/unlink_fork_service.rb b/app/services/projects/unlink_fork_service.rb index c499f384426..842fe4e09c4 100644 --- a/app/services/projects/unlink_fork_service.rb +++ b/app/services/projects/unlink_fork_service.rb @@ -5,7 +5,7 @@ module Projects if fork_source = @project.fork_source fork_source.lfs_objects.find_each do |lfs_object| - lfs_object.projects << @project + lfs_object.projects << @project unless lfs_object.projects.include?(@project) end refresh_forks_count(fork_source) diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index 575853fd66b..c2ca404b179 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -31,12 +31,20 @@ class TodoService mark_pending_todos_as_done(issue, current_user) end - # When we destroy an issuable we should: + # When we destroy a todo target we should: # - # * refresh the todos count cache for the current user + # * refresh the todos count cache for all users with todos on the target # - def destroy_issuable(issuable, user) - user.update_todos_count_cache + # This needs to yield back to the caller to destroy the target, because it + # collects the todo users before the todos themselves are deleted, then + # updates the todo counts for those users. + # + def destroy_target(target) + todo_users = User.where(id: target.todos.pending.select(:user_id)).to_a + + yield target + + todo_users.each(&:update_todos_count_cache) end # When we reassign an issue we should: diff --git a/app/views/events/event/_push.html.haml b/app/views/events/event/_push.html.haml index 9a763887b30..f85f5c5be88 100644 --- a/app/views/events/event/_push.html.haml +++ b/app/views/events/event/_push.html.haml @@ -7,7 +7,8 @@ %span.pushed #{event.action_name} #{event.ref_type} %strong - commits_link = project_commits_path(project, event.ref_name) - = link_to_if project.repository.branch_exists?(event.ref_name), event.ref_name, commits_link, class: 'ref-name' + - should_link = event.tag? ? project.repository.tag_exists?(event.ref_name) : project.repository.branch_exists?(event.ref_name) + = link_to_if should_link, event.ref_name, commits_link, class: 'ref-name' = render "events/event_scope", event: event diff --git a/app/views/help/index.html.haml b/app/views/help/index.html.haml index 021de4f0caf..b8692009225 100644 --- a/app/views/help/index.html.haml +++ b/app/views/help/index.html.haml @@ -1,3 +1,5 @@ += webpack_bundle_tag 'docs' + %div - if current_application_settings.help_page_text.present? = markdown_field(current_application_settings, :help_page_text) @@ -37,8 +39,12 @@ Quick help %ul.well-list %li= link_to 'See our website for getting help', support_url - %li= link_to 'Use the search bar on the top of this page', '#', onclick: 'Shortcuts.focusSearch(event)' - %li= link_to 'Use shortcuts', '#', onclick: 'Shortcuts.toggleHelp()' + %li + %button.btn-blank.btn-link.js-trigger-search-bar{ type: 'button' } + Use the search bar on the top of this page + %li + %button.btn-blank.btn-link.js-trigger-shortcut{ type: 'button' } + Use shortcuts - unless current_application_settings.help_page_hide_commercial_content? %li= link_to 'Get a support subscription', 'https://about.gitlab.com/pricing/' %li= link_to 'Compare GitLab editions', 'https://about.gitlab.com/features/#compare' diff --git a/app/views/projects/blob/viewers/_dependency_manager.html.haml b/app/views/projects/blob/viewers/_dependency_manager.html.haml index a0f0215a5ff..87aa7c1dbf8 100644 --- a/app/views/projects/blob/viewers/_dependency_manager.html.haml +++ b/app/views/projects/blob/viewers/_dependency_manager.html.haml @@ -6,6 +6,6 @@ - if viewer.package_name and defines a #{viewer.package_type} named %strong< - = link_to viewer.package_name, viewer.package_url, target: '_blank', rel: 'noopener noreferrer' + = link_to_if viewer.package_url.present?, viewer.package_name, viewer.package_url, target: '_blank', rel: 'noopener noreferrer' = link_to 'Learn more', viewer.manager_url, target: '_blank', rel: 'noopener noreferrer' diff --git a/app/views/projects/clusters/gcp/_form.html.haml b/app/views/projects/clusters/gcp/_form.html.haml index 0f6bae97571..e384b60d8d9 100644 --- a/app/views/projects/clusters/gcp/_form.html.haml +++ b/app/views/projects/clusters/gcp/_form.html.haml @@ -7,6 +7,9 @@ .form-group = field.label :name, s_('ClusterIntegration|Cluster name') = field.text_field :name, class: 'form-control', placeholder: s_('ClusterIntegration|Cluster name') + .form-group + = field.label :environment_scope, s_('ClusterIntegration|Environment scope') + = field.text_field :environment_scope, class: 'form-control', readonly: !has_multiple_clusters?(@project), placeholder: s_('ClusterIntegration|Environment scope') = field.fields_for :provider_gcp, @cluster.provider_gcp do |provider_gcp_field| .form-group diff --git a/app/views/projects/clusters/gcp/_show.html.haml b/app/views/projects/clusters/gcp/_show.html.haml index 3fa9f69708a..bde85aed341 100644 --- a/app/views/projects/clusters/gcp/_show.html.haml +++ b/app/views/projects/clusters/gcp/_show.html.haml @@ -8,6 +8,11 @@ = form_for @cluster, url: namespace_project_cluster_path(@project.namespace, @project, @cluster), as: :cluster do |field| = form_errors(@cluster) + + .form-group + = field.label :environment_scope, s_('ClusterIntegration|Environment scope') + = field.text_field :environment_scope, class: 'form-control js-select-on-focus', readonly: !has_multiple_clusters?(@project), placeholder: s_('ClusterIntegration|Environment scope') + = field.fields_for :platform_kubernetes, @cluster.platform_kubernetes do |platform_kubernetes_field| .form-group = platform_kubernetes_field.label :api_url, s_('ClusterIntegration|API URL') diff --git a/app/views/projects/clusters/user/_form.html.haml b/app/views/projects/clusters/user/_form.html.haml index 4a9bd5186c6..babfca0c567 100644 --- a/app/views/projects/clusters/user/_form.html.haml +++ b/app/views/projects/clusters/user/_form.html.haml @@ -3,6 +3,9 @@ .form-group = field.label :name, s_('ClusterIntegration|Cluster name') = field.text_field :name, class: 'form-control', placeholder: s_('ClusterIntegration|Cluster name') + .form-group + = field.label :environment_scope, s_('ClusterIntegration|Environment scope') + = field.text_field :environment_scope, class: 'form-control', readonly: !has_multiple_clusters?(@project), placeholder: s_('ClusterIntegration|Environment scope') = field.fields_for :platform_kubernetes, @cluster.platform_kubernetes do |platform_kubernetes_field| .form-group diff --git a/app/views/projects/clusters/user/_show.html.haml b/app/views/projects/clusters/user/_show.html.haml index 5931e0b7f17..89595bca007 100644 --- a/app/views/projects/clusters/user/_show.html.haml +++ b/app/views/projects/clusters/user/_show.html.haml @@ -4,6 +4,10 @@ = field.label :name, s_('ClusterIntegration|Cluster name') = field.text_field :name, class: 'form-control', placeholder: s_('ClusterIntegration|Cluster name') + .form-group + = field.label :environment_scope, s_('ClusterIntegration|Environment scope') + = field.text_field :environment_scope, class: 'form-control js-select-on-focus', readonly: !has_multiple_clusters?(@project), placeholder: s_('ClusterIntegration|Environment scope') + = field.fields_for :platform_kubernetes, @cluster.platform_kubernetes do |platform_kubernetes_field| .form-group = platform_kubernetes_field.label :api_url, s_('ClusterIntegration|API URL') diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index eab7879c7bf..1f28d8acff6 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -39,8 +39,6 @@ = icon('caret-down') .dropdown-menu.dropdown-menu-align-right.hidden-lg %ul - - if can_update_issue - %li= link_to 'Edit', edit_project_issue_path(@project, @issue), class: 'js-issuable-edit' - unless current_user == @issue.author %li= link_to 'Report abuse', new_abuse_report_path(user_id: @issue.author.id, ref_url: issue_url(@issue)) - if can_update_issue @@ -52,9 +50,6 @@ %li.divider %li= link_to 'New issue', new_project_issue_path(@project), title: 'New issue', id: 'new_issue_link' - - if can_update_issue - = link_to 'Edit', edit_project_issue_path(@project, @issue), class: 'hidden-xs hidden-sm btn btn-grouped js-issuable-edit' - = render 'shared/issuable/close_reopen_button', issuable: @issue, can_update: can_update_issue - if can_report_spam diff --git a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml index bd8c38292d6..f8c4005a9e0 100644 --- a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml +++ b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml @@ -26,10 +26,12 @@ = pipeline_schedule.owner&.name %td .pull-right.btn-group + - if can?(current_user, :play_pipeline_schedule, pipeline_schedule) + = link_to play_pipeline_schedule_path(pipeline_schedule), method: :post, title: s_('Play'), class: 'btn' do + = icon('play') - if can?(current_user, :update_pipeline_schedule, pipeline_schedule) = link_to take_ownership_pipeline_schedule_path(pipeline_schedule), method: :post, title: s_('PipelineSchedules|Take ownership'), class: 'btn' do = s_('PipelineSchedules|Take ownership') - - if can?(current_user, :update_pipeline_schedule, pipeline_schedule) = link_to edit_pipeline_schedule_path(pipeline_schedule), title: _('Edit'), class: 'btn' do = icon('pencil') - if can?(current_user, :admin_pipeline_schedule, pipeline_schedule) diff --git a/app/views/projects/pipelines/_info.html.haml b/app/views/projects/pipelines/_info.html.haml index 01ea9356af5..85946aec1f2 100644 --- a/app/views/projects/pipelines/_info.html.haml +++ b/app/views/projects/pipelines/_info.html.haml @@ -1,6 +1,6 @@ #js-pipeline-header-vue.pipeline-header-container -- if @commit +- if @commit.present? .commit-box %h3.commit-title = markdown(@commit.title, pipeline: :single_line) @@ -8,28 +8,28 @@ %pre.commit-description = preserve(markdown(@commit.description, pipeline: :single_line)) -.info-well - - if @commit.status - .well-segment.pipeline-info - .icon-container - = icon('clock-o') - = pluralize @pipeline.total_size, "job" - - if @pipeline.ref - from - = link_to @pipeline.ref, project_ref_path(@project, @pipeline.ref), class: "ref-name" - - if @pipeline.duration - in - = time_interval_in_words(@pipeline.duration) - - if @pipeline.queued_duration - = "(queued for #{time_interval_in_words(@pipeline.queued_duration)})" + .info-well + - if @commit.status + .well-segment.pipeline-info + .icon-container + = icon('clock-o') + = pluralize @pipeline.total_size, "job" + - if @pipeline.ref + from + = link_to @pipeline.ref, project_ref_path(@project, @pipeline.ref), class: "ref-name" + - if @pipeline.duration + in + = time_interval_in_words(@pipeline.duration) + - if @pipeline.queued_duration + = "(queued for #{time_interval_in_words(@pipeline.queued_duration)})" - .well-segment.branch-info - .icon-container.commit-icon - = custom_icon("icon_commit") - = link_to @commit.short_id, project_commit_path(@project, @pipeline.sha), class: "commit-sha js-details-short" - = link_to("#", class: "js-details-expand hidden-xs hidden-sm") do - %span.text-expander - \... - %span.js-details-content.hide - = link_to @pipeline.sha, project_commit_path(@project, @pipeline.sha), class: "commit-sha commit-hash-full" - = clipboard_button(text: @pipeline.sha, title: "Copy commit SHA to clipboard") + .well-segment.branch-info + .icon-container.commit-icon + = custom_icon("icon_commit") + = link_to @commit.short_id, project_commit_path(@project, @pipeline.sha), class: "commit-sha js-details-short" + = link_to("#", class: "js-details-expand hidden-xs hidden-sm") do + %span.text-expander + \... + %span.js-details-content.hide + = link_to @pipeline.sha, project_commit_path(@project, @pipeline.sha), class: "commit-sha commit-hash-full" + = clipboard_button(text: @pipeline.sha, title: "Copy commit SHA to clipboard") diff --git a/app/views/shared/empty_states/_issues.html.haml b/app/views/shared/empty_states/_issues.html.haml index e039a73cd3b..62437f5fc9d 100644 --- a/app/views/shared/empty_states/_issues.html.haml +++ b/app/views/shared/empty_states/_issues.html.haml @@ -8,16 +8,17 @@ = image_tag 'illustrations/issues.svg' .col-xs-12 .text-content - - if has_button && current_user + - if current_user %h4 = _("The Issue Tracker is the place to add things that need to be improved or solved in a project") %p = _("Issues can be bugs, tasks or ideas to be discussed. Also, issues are searchable and filterable.") - .text-center - - if project_select_button - = render 'shared/new_project_item_select', path: 'issues/new', label: 'New issue', type: :issues - - else - = link_to 'New issue', button_path, class: 'btn btn-success', title: 'New issue', id: 'new_issue_link' + - if has_button + .text-center + - if project_select_button + = render 'shared/new_project_item_select', path: 'issues/new', label: 'New issue', type: :issues + - else + = link_to 'New issue', button_path, class: 'btn btn-success', title: 'New issue', id: 'new_issue_link' - else %h4.text-center= _("There are no issues to show") %p diff --git a/app/views/shared/issuable/_assignees.html.haml b/app/views/shared/issuable/_assignees.html.haml index 217af7c9fac..fc86f855865 100644 --- a/app/views/shared/issuable/_assignees.html.haml +++ b/app/views/shared/issuable/_assignees.html.haml @@ -1,14 +1,10 @@ -- max_render = 3 -- max = [max_render, issue.assignees.length].min +- max_render = 4 +- assignees_rendering_overflow = issue.assignees.size > max_render +- render_count = assignees_rendering_overflow ? max_render - 1 : max_render +- more_assignees_count = issue.assignees.size - render_count -- issue.assignees.take(max).each do |assignee| +- issue.assignees.take(render_count).each do |assignee| = link_to_member(@project, assignee, name: false, title: "Assigned to :name") -- if issue.assignees.length > max_render - - counter = issue.assignees.length - max_render - - %span{ class: 'avatar-counter has-tooltip', data: { container: 'body', placement: 'bottom', 'line-type' => 'old', 'original-title' => "+#{counter} more assignees" } } - - if counter < 99 - = "+#{counter}" - - else - 99+ +- if more_assignees_count.positive? + %span{ class: 'avatar-counter has-tooltip', data: { container: 'body', placement: 'bottom', 'line-type' => 'old', 'original-title' => "+#{more_assignees_count} more assignees" } } +#{more_assignees_count} diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index ba31a5aa9c2..268b7028fd9 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -39,6 +39,7 @@ - pipeline_cache:expire_job_cache - pipeline_cache:expire_pipeline_cache - pipeline_creation:create_pipeline +- pipeline_creation:run_pipeline_schedule - pipeline_default:build_coverage - pipeline_default:build_trace_sections - pipeline_default:pipeline_metrics diff --git a/app/workers/expire_pipeline_cache_worker.rb b/app/workers/expire_pipeline_cache_worker.rb index 3e34de22c19..db73d37868a 100644 --- a/app/workers/expire_pipeline_cache_worker.rb +++ b/app/workers/expire_pipeline_cache_worker.rb @@ -13,7 +13,7 @@ class ExpirePipelineCacheWorker store.touch(project_pipelines_path(project)) store.touch(project_pipeline_path(project, pipeline)) - store.touch(commit_pipelines_path(project, pipeline.commit)) if pipeline.commit + store.touch(commit_pipelines_path(project, pipeline.commit)) unless pipeline.commit.nil? store.touch(new_merge_request_pipelines_path(project)) each_pipelines_merge_request_path(project, pipeline) do |path| store.touch(path) diff --git a/app/workers/run_pipeline_schedule_worker.rb b/app/workers/run_pipeline_schedule_worker.rb new file mode 100644 index 00000000000..8f5138fc873 --- /dev/null +++ b/app/workers/run_pipeline_schedule_worker.rb @@ -0,0 +1,22 @@ +class RunPipelineScheduleWorker + include ApplicationWorker + include PipelineQueue + + queue_namespace :pipeline_creation + + def perform(schedule_id, user_id) + schedule = Ci::PipelineSchedule.find_by(id: schedule_id) + user = User.find_by(id: user_id) + + return unless schedule && user + + run_pipeline_schedule(schedule, user) + end + + def run_pipeline_schedule(schedule, user) + Ci::CreatePipelineService.new(schedule.project, + user, + ref: schedule.ref) + .execute(:schedule, ignore_skip_ci: true, save_on_errors: false, schedule: schedule) + end +end diff --git a/changelogs/unreleased/33028-event-tag-links.yml b/changelogs/unreleased/33028-event-tag-links.yml new file mode 100644 index 00000000000..1d674200dcd --- /dev/null +++ b/changelogs/unreleased/33028-event-tag-links.yml @@ -0,0 +1,5 @@ +--- +title: Fix tags in the Activity tab not being clickable +merge_request: 15996 +author: Mario de la Ossa +type: fixed diff --git a/changelogs/unreleased/36020-private-npm-modules.yml b/changelogs/unreleased/36020-private-npm-modules.yml new file mode 100644 index 00000000000..5c2585a602e --- /dev/null +++ b/changelogs/unreleased/36020-private-npm-modules.yml @@ -0,0 +1,5 @@ +--- +title: Do not generate NPM links for private NPM modules in blob view +merge_request: 16002 +author: Mario de la Ossa +type: added diff --git a/changelogs/unreleased/39298-list-of-avatars-2.yml b/changelogs/unreleased/39298-list-of-avatars-2.yml new file mode 100644 index 00000000000..e2095561c0e --- /dev/null +++ b/changelogs/unreleased/39298-list-of-avatars-2.yml @@ -0,0 +1,5 @@ +--- +title: List of avatars should never show +1 +merge_request: 15972 +author: Jacopo Beschi @jacopo-beschi +type: added diff --git a/changelogs/unreleased/40871-todo-notification-count-shows-notification-without-having-a-todo.yml b/changelogs/unreleased/40871-todo-notification-count-shows-notification-without-having-a-todo.yml new file mode 100644 index 00000000000..ee196629def --- /dev/null +++ b/changelogs/unreleased/40871-todo-notification-count-shows-notification-without-having-a-todo.yml @@ -0,0 +1,5 @@ +--- +title: Reset todo counters when the target is deleted +merge_request: 15807 +author: +type: fixed diff --git a/changelogs/unreleased/bvl-fix-unlinking-with-lfs-objects.yml b/changelogs/unreleased/bvl-fix-unlinking-with-lfs-objects.yml new file mode 100644 index 00000000000..058d686e74c --- /dev/null +++ b/changelogs/unreleased/bvl-fix-unlinking-with-lfs-objects.yml @@ -0,0 +1,6 @@ +--- +title: Don't link LFS objects to a project when unlinking forks when they were already + linked +merge_request: 16006 +author: +type: fixed diff --git a/changelogs/unreleased/fix-docs-help-shortcut.yml b/changelogs/unreleased/fix-docs-help-shortcut.yml new file mode 100644 index 00000000000..8c172e44160 --- /dev/null +++ b/changelogs/unreleased/fix-docs-help-shortcut.yml @@ -0,0 +1,5 @@ +--- +title: Fix shortcut links on help page +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/fix-onion-skin-reenter.yml b/changelogs/unreleased/fix-onion-skin-reenter.yml new file mode 100644 index 00000000000..66b12c037b0 --- /dev/null +++ b/changelogs/unreleased/fix-onion-skin-reenter.yml @@ -0,0 +1,5 @@ +--- +title: Fix onion-skin re-entering state +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/remove-links-mr-empty-state.yml b/changelogs/unreleased/remove-links-mr-empty-state.yml new file mode 100644 index 00000000000..c666bc2c81d --- /dev/null +++ b/changelogs/unreleased/remove-links-mr-empty-state.yml @@ -0,0 +1,5 @@ +--- +title: Remove related links in MR widget when empty state +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/sh-add-schedule-pipeline-run-now.yml b/changelogs/unreleased/sh-add-schedule-pipeline-run-now.yml new file mode 100644 index 00000000000..6d06f695f10 --- /dev/null +++ b/changelogs/unreleased/sh-add-schedule-pipeline-run-now.yml @@ -0,0 +1,5 @@ +--- +title: Add button to run scheduled pipeline immediately +merge_request: +author: +type: added diff --git a/changelogs/unreleased/show-inline-edit-btn.yml b/changelogs/unreleased/show-inline-edit-btn.yml new file mode 100644 index 00000000000..8cfe9b7d75a --- /dev/null +++ b/changelogs/unreleased/show-inline-edit-btn.yml @@ -0,0 +1,5 @@ +--- +title: Move edit button to second row on issue page (and change it to a pencil icon) +merge_request: +author: +type: changed diff --git a/config/routes/project.rb b/config/routes/project.rb index 093da10f57f..239b5480321 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -179,6 +179,7 @@ constraints(ProjectUrlConstrainer.new) do resources :pipeline_schedules, except: [:show] do member do + post :play post :take_ownership end end diff --git a/config/webpack.config.js b/config/webpack.config.js index 78ced4c3e8c..f02fcda827a 100644 --- a/config/webpack.config.js +++ b/config/webpack.config.js @@ -36,6 +36,7 @@ var config = { cycle_analytics: './cycle_analytics/cycle_analytics_bundle.js', commit_pipelines: './commit/pipelines/pipelines_bundle.js', deploy_keys: './deploy_keys/index.js', + docs: './docs/docs_bundle.js', diff_notes: './diff_notes/diff_notes_bundle.js', environments: './environments/environments_bundle.js', environments_folder: './environments/folder/environments_folder_bundle.js', diff --git a/db/migrate/20171106135924_issues_milestone_id_foreign_key.rb b/db/migrate/20171106135924_issues_milestone_id_foreign_key.rb index e6a780d0964..bfb3dcae511 100644 --- a/db/migrate/20171106135924_issues_milestone_id_foreign_key.rb +++ b/db/migrate/20171106135924_issues_milestone_id_foreign_key.rb @@ -16,6 +16,7 @@ class IssuesMilestoneIdForeignKey < ActiveRecord::Migration def self.with_orphaned_milestones where('NOT EXISTS (SELECT true FROM milestones WHERE milestones.id = issues.milestone_id)') + .where('milestone_id IS NOT NULL') end end diff --git a/doc/administration/high_availability/gitlab.md b/doc/administration/high_availability/gitlab.md index 42666357faf..b85a166089d 100644 --- a/doc/administration/high_availability/gitlab.md +++ b/doc/administration/high_availability/gitlab.md @@ -1,6 +1,6 @@ # Configuring GitLab for HA -Assuming you have already configured a database, Redis, and NFS, you can +Assuming you have already configured a [database](database.md), [Redis](redis.md), and [NFS](nfs.md), you can configure the GitLab application server(s) now. Complete the steps below for each GitLab application server in your environment. @@ -48,34 +48,33 @@ for each GitLab application server in your environment. data locations. See [NFS documentation](nfs.md) for `/etc/gitlab/gitlab.rb` configuration values for various scenarios. The example below assumes you've added NFS mounts in the default data locations. - + ```ruby external_url 'https://gitlab.example.com' # Prevent GitLab from starting if NFS data mounts are not available high_availability['mountpoint'] = '/var/opt/gitlab/git-data' - + # Disable components that will not be on the GitLab application server - postgresql['enable'] = false - redis['enable'] = false - + roles ['application_role'] + # PostgreSQL connection details gitlab_rails['db_adapter'] = 'postgresql' gitlab_rails['db_encoding'] = 'unicode' gitlab_rails['db_host'] = '10.1.0.5' # IP/hostname of database server gitlab_rails['db_password'] = 'DB password' - + # Redis connection details gitlab_rails['redis_port'] = '6379' gitlab_rails['redis_host'] = '10.1.0.6' # IP/hostname of Redis server gitlab_rails['redis_password'] = 'Redis Password' ``` - - > **Note:** To maintain uniformity of links across HA clusters, the `external_url` - on the first application server as well as the additional application - servers should point to the external url that users will use to access GitLab. + + > **Note:** To maintain uniformity of links across HA clusters, the `external_url` + on the first application server as well as the additional application + servers should point to the external url that users will use to access GitLab. In a typical HA setup, this will be the url of the load balancer which will - route traffic to all GitLab application servers in the HA cluster. + route traffic to all GitLab application servers in the HA cluster. 1. Run `sudo gitlab-ctl reconfigure` to compile the configuration. diff --git a/doc/development/fe_guide/axios.md b/doc/development/fe_guide/axios.md index 962fe3dcec9..1daa6758171 100644 --- a/doc/development/fe_guide/axios.md +++ b/doc/development/fe_guide/axios.md @@ -11,7 +11,7 @@ This exported module should be used instead of directly using `axios` to ensure ## Usage ```javascript - import axios from '~/lib/utils/axios_utils'; + import axios from './lib/utils/axios_utils'; axios.get(url) .then((response) => { diff --git a/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md b/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md index 6adde447975..195285f9157 100644 --- a/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md +++ b/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md @@ -163,3 +163,11 @@ For Windows, you can use `wincred` or Microsoft's [Git Credential Manager for Wi More details about various methods of storing the user credentials can be found on [Git Credential Storage documentation](https://git-scm.com/book/en/v2/Git-Tools-Credential-Storage). + +### LFS objects are missing on push + +GitLab checks files to detect LFS pointers on push. If LFS pointers are detected, GitLab tries to verify that those files already exist in LFS on GitLab. + +Verify that LFS in installed locally and consider a manual push with `git lfs push --all`. + +If you are storing LFS files outside of GitLab you can disable LFS on the project by settting `lfs_enabled: false` with the [projets api](../../api/projects.md#edit-project). diff --git a/lib/gitlab/action_rate_limiter.rb b/lib/gitlab/action_rate_limiter.rb new file mode 100644 index 00000000000..4cd3bdefda3 --- /dev/null +++ b/lib/gitlab/action_rate_limiter.rb @@ -0,0 +1,47 @@ +module Gitlab + # This class implements a simple rate limiter that can be used to throttle + # certain actions. Unlike Rack Attack and Rack::Throttle, which operate at + # the middleware level, this can be used at the controller level. + class ActionRateLimiter + TIME_TO_EXPIRE = 60 # 1 min + + attr_accessor :action, :expiry_time + + def initialize(action:, expiry_time: TIME_TO_EXPIRE) + @action = action + @expiry_time = expiry_time + end + + # Increments the given cache key and increments the value by 1 with the + # given expiration time. Returns the incremented value. + # + # key - An array of ActiveRecord instances + def increment(key) + value = 0 + + Gitlab::Redis::Cache.with do |redis| + cache_key = action_key(key) + value = redis.incr(cache_key) + redis.expire(cache_key, expiry_time) if value == 1 + end + + value + end + + # Increments the given key and returns true if the action should + # be throttled. + # + # key - An array of ActiveRecord instances + # threshold_value - The maximum number of times this action should occur in the given time interval + def throttled?(key, threshold_value) + self.increment(key) > threshold_value + end + + private + + def action_key(key) + serialized = key.map { |obj| "#{obj.class.model_name.to_s.underscore}:#{obj.id}" }.join(":") + "action_rate_limiter:#{action}:#{serialized}" + end + end +end diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index e90b158fb34..145721dea76 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -228,6 +228,19 @@ module Gitlab end end end + + # Only to be used when the object ids will not necessarily have a + # relation to each other. The last 10 commits for a branch for example, + # should go through .where + def batch_by_oid(repo, oids) + repo.gitaly_migrate(:list_commits_by_oid) do |is_enabled| + if is_enabled + repo.gitaly_commit_client.list_commits_by_oid(oids) + else + oids.map { |oid| find(repo, oid) }.compact + end + end + end end def initialize(repository, raw_commit, head = nil) diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index 7985f5b5457..fb3e27770b4 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -169,6 +169,15 @@ module Gitlab consume_commits_response(response) end + def list_commits_by_oid(oids) + request = Gitaly::ListCommitsByOidRequest.new(repository: @gitaly_repo, oid: oids) + + response = GitalyClient.call(@repository.storage, :commit_service, :list_commits_by_oid, request, timeout: GitalyClient.medium_timeout) + consume_commits_response(response) + rescue GRPC::Unknown # If no repository is found, happens mainly during testing + [] + end + def commits_by_message(query, revision: '', path: '', limit: 1000, offset: 0) request = Gitaly::CommitsByMessageRequest.new( repository: @gitaly_repo, diff --git a/scripts/gitaly-test-spawn b/scripts/gitaly-test-spawn index 8e05eca8d7e..ecb68c6acc6 100755 --- a/scripts/gitaly-test-spawn +++ b/scripts/gitaly-test-spawn @@ -1,7 +1,8 @@ #!/usr/bin/env ruby gitaly_dir = 'tmp/tests/gitaly' -env = { 'HOME' => File.expand_path('tmp/tests') } +env = { 'HOME' => File.expand_path('tmp/tests'), + 'GEM_PATH' => Gem.path.join(':') } args = %W[#{gitaly_dir}/gitaly #{gitaly_dir}/config.toml] # Print the PID of the spawned process diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index c5d08cb0b9d..a2ef937609b 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -874,7 +874,7 @@ describe Projects::IssuesController do end it 'delegates the update of the todos count cache to TodoService' do - expect_any_instance_of(TodoService).to receive(:destroy_issuable).with(issue, owner).once + expect_any_instance_of(TodoService).to receive(:destroy_target).with(issue).once delete :destroy, namespace_id: project.namespace, project_id: project, id: issue.iid end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index d03ecefe47f..58116e6e0fe 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -468,7 +468,7 @@ describe Projects::MergeRequestsController do end it 'delegates the update of the todos count cache to TodoService' do - expect_any_instance_of(TodoService).to receive(:destroy_issuable).with(merge_request, owner).once + expect_any_instance_of(TodoService).to receive(:destroy_target).with(merge_request).once delete :destroy, namespace_id: project.namespace, project_id: project, id: merge_request.iid end diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb index 4e52e261920..966ffdf6996 100644 --- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb +++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb @@ -3,10 +3,12 @@ require 'spec_helper' describe Projects::PipelineSchedulesController do include AccessMatchersForController - set(:project) { create(:project, :public) } - let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project) } + set(:project) { create(:project, :public, :repository) } + set(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project) } describe 'GET #index' do + render_views + let(:scope) { nil } let!(:inactive_pipeline_schedule) do create(:ci_pipeline_schedule, :inactive, project: project) @@ -96,7 +98,7 @@ describe Projects::PipelineSchedulesController do end end - context 'when variables_attributes has two variables and duplicted' do + context 'when variables_attributes has two variables and duplicated' do let(:schedule) do basic_param.merge({ variables_attributes: [{ key: 'AAA', value: 'AAA123' }, { key: 'AAA', value: 'BBB123' }] @@ -364,6 +366,65 @@ describe Projects::PipelineSchedulesController do end end + describe 'POST #play', :clean_gitlab_redis_cache do + set(:user) { create(:user) } + let(:ref) { 'master' } + + before do + project.add_developer(user) + + sign_in(user) + end + + context 'when an anonymous user makes the request' do + before do + sign_out(user) + end + + it 'does not allow pipeline to be executed' do + expect(RunPipelineScheduleWorker).not_to receive(:perform_async) + + post :play, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when a developer makes the request' do + it 'executes a new pipeline' do + expect(RunPipelineScheduleWorker).to receive(:perform_async).with(pipeline_schedule.id, user.id).and_return('job-123') + + post :play, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id + + expect(flash[:notice]).to start_with 'Successfully scheduled a pipeline to run' + expect(response).to have_gitlab_http_status(302) + end + + it 'prevents users from scheduling the same pipeline repeatedly' do + 2.times do + post :play, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id + end + + expect(flash.to_a.size).to eq(2) + expect(flash[:alert]).to eq 'You cannot play this scheduled pipeline at the moment. Please wait a minute.' + expect(response).to have_gitlab_http_status(302) + end + end + + context 'when a developer attempts to schedule a protected ref' do + it 'does not allow pipeline to be executed' do + create(:protected_branch, project: project, name: ref) + protected_schedule = create(:ci_pipeline_schedule, project: project, ref: ref) + + expect(RunPipelineScheduleWorker).not_to receive(:perform_async) + + post :play, namespace_id: project.namespace.to_param, project_id: project, id: protected_schedule.id + + expect(response).to have_gitlab_http_status(404) + end + end + end + describe 'DELETE #destroy' do set(:user) { create(:user) } diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 1604a2da485..35ac999cc65 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -17,13 +17,10 @@ describe Projects::PipelinesController do describe 'GET index.json' do before do - branch_head = project.commit - parent = branch_head.parent - - create(:ci_empty_pipeline, status: 'pending', project: project, sha: branch_head.id) - create(:ci_empty_pipeline, status: 'running', project: project, sha: branch_head.id) - create(:ci_empty_pipeline, status: 'created', project: project, sha: parent.id) - create(:ci_empty_pipeline, status: 'success', project: project, sha: parent.id) + %w(pending running created success).each_with_index do |status, index| + sha = project.commit("HEAD~#{index}") + create(:ci_empty_pipeline, status: status, project: project, sha: sha) + end end subject do @@ -46,7 +43,7 @@ describe Projects::PipelinesController do context 'when performing gitaly calls', :request_store do it 'limits the Gitaly requests' do - expect { subject }.to change { Gitlab::GitalyClient.get_request_count }.by(8) + expect { subject }.to change { Gitlab::GitalyClient.get_request_count }.by(3) end end end diff --git a/spec/features/help_pages_spec.rb b/spec/features/help_pages_spec.rb index ab896a310be..0d04ed612c2 100644 --- a/spec/features/help_pages_spec.rb +++ b/spec/features/help_pages_spec.rb @@ -32,6 +32,24 @@ describe 'Help Pages' do it_behaves_like 'help page', prefix: '/gitlab' end + + context 'quick link shortcuts', :js do + before do + visit help_path + end + + it 'focuses search bar' do + find('.js-trigger-search-bar').click + + expect(page).to have_selector('#search:focus') + end + + it 'opens shortcuts help dialog' do + find('.js-trigger-shortcut').click + + expect(page).to have_selector('#modal-shortcuts') + end + end end context 'in a production environment with version check enabled', :js do diff --git a/spec/features/issues/issue_detail_spec.rb b/spec/features/issues/issue_detail_spec.rb index 4224a8fe5d4..babb0285590 100644 --- a/spec/features/issues/issue_detail_spec.rb +++ b/spec/features/issues/issue_detail_spec.rb @@ -24,7 +24,7 @@ feature 'Issue Detail', :js do visit project_issue_path(project, issue) wait_for_requests - click_link 'Edit' + page.find('.js-issuable-edit').click fill_in 'issuable-title', with: 'issue title' click_button 'Save' wait_for_requests diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index 852d9e368aa..d1ff057a0c6 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -8,729 +8,753 @@ describe 'Issues' do let(:user) { create(:user) } let(:project) { create(:project, :public) } - before do - sign_in(user) - user2 = create(:user) - - project.team << [[user, user2], :developer] - end + describe 'while user is signed out' do + describe 'empty state' do + it 'user sees empty state' do + visit project_issues_path(project) - describe 'Edit issue' do - let!(:issue) do - create(:issue, - author: user, - assignees: [user], - project: project) + expect(page).to have_content('Register / Sign In') + expect(page).to have_content('The Issue Tracker is the place to add things that need to be improved or solved in a project.') + expect(page).to have_content('You can register or sign in to create issues for this project.') + end end + end + describe 'while user is signed in' do before do - visit edit_project_issue_path(project, issue) - find('.js-zen-enter').click - end - - it 'opens new issue popup' do - expect(page).to have_content("Issue ##{issue.iid}") - end - end + sign_in(user) + user2 = create(:user) - describe 'Editing issue assignee' do - let!(:issue) do - create(:issue, - author: user, - assignees: [user], - project: project) + project.team << [[user, user2], :developer] end - it 'allows user to select unassigned', :js do - visit edit_project_issue_path(project, issue) - - expect(page).to have_content "Assignee #{user.name}" + describe 'empty state' do + it 'user sees empty state' do + visit project_issues_path(project) - first('.js-user-search').click - click_link 'Unassigned' + expect(page).to have_content('The Issue Tracker is the place to add things that need to be improved or solved in a project') + expect(page).to have_content('Issues can be bugs, tasks or ideas to be discussed. Also, issues are searchable and filterable.') + expect(page).to have_content('New issue') + end + end - click_button 'Save changes' + describe 'Edit issue' do + let!(:issue) do + create(:issue, + author: user, + assignees: [user], + project: project) + end - page.within('.assignee') do - expect(page).to have_content 'No assignee - assign yourself' + before do + visit edit_project_issue_path(project, issue) + find('.js-zen-enter').click end - expect(issue.reload.assignees).to be_empty + it 'opens new issue popup' do + expect(page).to have_content("Issue ##{issue.iid}") + end end - end - describe 'due date', :js do - context 'on new form' do - before do - visit new_project_issue_path(project) + describe 'Editing issue assignee' do + let!(:issue) do + create(:issue, + author: user, + assignees: [user], + project: project) end - it 'saves with due date' do - date = Date.today.at_beginning_of_month - - fill_in 'issue_title', with: 'bug 345' - fill_in 'issue_description', with: 'bug description' - find('#issuable-due-date').click + it 'allows user to select unassigned', :js do + visit edit_project_issue_path(project, issue) - page.within '.pika-single' do - click_button date.day - end + expect(page).to have_content "Assignee #{user.name}" - expect(find('#issuable-due-date').value).to eq date.to_s + first('.js-user-search').click + click_link 'Unassigned' - click_button 'Submit issue' + click_button 'Save changes' - page.within '.issuable-sidebar' do - expect(page).to have_content date.to_s(:medium) + page.within('.assignee') do + expect(page).to have_content 'No assignee - assign yourself' end + + expect(issue.reload.assignees).to be_empty end end - context 'on edit form' do - let(:issue) { create(:issue, author: user, project: project, due_date: Date.today.at_beginning_of_month.to_s) } + describe 'due date', :js do + context 'on new form' do + before do + visit new_project_issue_path(project) + end - before do - visit edit_project_issue_path(project, issue) - end + it 'saves with due date' do + date = Date.today.at_beginning_of_month - it 'saves with due date' do - date = Date.today.at_beginning_of_month + fill_in 'issue_title', with: 'bug 345' + fill_in 'issue_description', with: 'bug description' + find('#issuable-due-date').click - expect(find('#issuable-due-date').value).to eq date.to_s + page.within '.pika-single' do + click_button date.day + end - date = date.tomorrow + expect(find('#issuable-due-date').value).to eq date.to_s - fill_in 'issue_title', with: 'bug 345' - fill_in 'issue_description', with: 'bug description' - find('#issuable-due-date').click + click_button 'Submit issue' - page.within '.pika-single' do - click_button date.day + page.within '.issuable-sidebar' do + expect(page).to have_content date.to_s(:medium) + end end + end - expect(find('#issuable-due-date').value).to eq date.to_s + context 'on edit form' do + let(:issue) { create(:issue, author: user, project: project, due_date: Date.today.at_beginning_of_month.to_s) } - click_button 'Save changes' - - page.within '.issuable-sidebar' do - expect(page).to have_content date.to_s(:medium) + before do + visit edit_project_issue_path(project, issue) end - end - it 'warns about version conflict' do - issue.update(title: "New title") + it 'saves with due date' do + date = Date.today.at_beginning_of_month - fill_in 'issue_title', with: 'bug 345' - fill_in 'issue_description', with: 'bug description' + expect(find('#issuable-due-date').value).to eq date.to_s - click_button 'Save changes' + date = date.tomorrow - expect(page).to have_content 'Someone edited the issue the same time you did' - end - end - end + fill_in 'issue_title', with: 'bug 345' + fill_in 'issue_description', with: 'bug description' + find('#issuable-due-date').click + + page.within '.pika-single' do + click_button date.day + end - describe 'Issue info' do - it 'links to current issue in breadcrubs' do - issue = create(:issue, project: project) + expect(find('#issuable-due-date').value).to eq date.to_s - visit project_issue_path(project, issue) + click_button 'Save changes' - expect(find('.breadcrumbs-sub-title a')[:href]).to end_with(issue_path(issue)) - end + page.within '.issuable-sidebar' do + expect(page).to have_content date.to_s(:medium) + end + end - it 'excludes award_emoji from comment count' do - issue = create(:issue, author: user, assignees: [user], project: project, title: 'foobar') - create(:award_emoji, awardable: issue) + it 'warns about version conflict' do + issue.update(title: "New title") - visit project_issues_path(project, assignee_id: user.id) + fill_in 'issue_title', with: 'bug 345' + fill_in 'issue_description', with: 'bug description' - expect(page).to have_content 'foobar' - expect(page.all('.no-comments').first.text).to eq "0" - end - end + click_button 'Save changes' - describe 'Filter issue' do - before do - %w(foobar barbaz gitlab).each do |title| - create(:issue, - author: user, - assignees: [user], - project: project, - title: title) + expect(page).to have_content 'Someone edited the issue the same time you did' + end end - - @issue = Issue.find_by(title: 'foobar') - @issue.milestone = create(:milestone, project: project) - @issue.assignees = [] - @issue.save end - let(:issue) { @issue } + describe 'Issue info' do + it 'links to current issue in breadcrubs' do + issue = create(:issue, project: project) - it 'allows filtering by issues with no specified assignee' do - visit project_issues_path(project, assignee_id: IssuableFinder::NONE) + visit project_issue_path(project, issue) - expect(page).to have_content 'foobar' - expect(page).not_to have_content 'barbaz' - expect(page).not_to have_content 'gitlab' - end + expect(find('.breadcrumbs-sub-title a')[:href]).to end_with(issue_path(issue)) + end - it 'allows filtering by a specified assignee' do - visit project_issues_path(project, assignee_id: user.id) + it 'excludes award_emoji from comment count' do + issue = create(:issue, author: user, assignees: [user], project: project, title: 'foobar') + create(:award_emoji, awardable: issue) - expect(page).not_to have_content 'foobar' - expect(page).to have_content 'barbaz' - expect(page).to have_content 'gitlab' - end - end + visit project_issues_path(project, assignee_id: user.id) - describe 'filter issue' do - titles = %w[foo bar baz] - titles.each_with_index do |title, index| - let!(title.to_sym) do - create(:issue, title: title, - project: project, - created_at: Time.now - (index * 60)) + expect(page).to have_content 'foobar' + expect(page.all('.no-comments').first.text).to eq "0" end end - let(:newer_due_milestone) { create(:milestone, due_date: '2013-12-11') } - let(:later_due_milestone) { create(:milestone, due_date: '2013-12-12') } - it 'sorts by newest' do - visit project_issues_path(project, sort: sort_value_created_date) + describe 'Filter issue' do + before do + %w(foobar barbaz gitlab).each do |title| + create(:issue, + author: user, + assignees: [user], + project: project, + title: title) + end - expect(first_issue).to include('foo') - expect(last_issue).to include('baz') - end + @issue = Issue.find_by(title: 'foobar') + @issue.milestone = create(:milestone, project: project) + @issue.assignees = [] + @issue.save + end - it 'sorts by most recently updated' do - baz.updated_at = Time.now + 100 - baz.save - visit project_issues_path(project, sort: sort_value_recently_updated) + let(:issue) { @issue } - expect(first_issue).to include('baz') - end + it 'allows filtering by issues with no specified assignee' do + visit project_issues_path(project, assignee_id: IssuableFinder::NONE) - describe 'sorting by due date' do - before do - foo.update(due_date: 1.day.from_now) - bar.update(due_date: 6.days.from_now) + expect(page).to have_content 'foobar' + expect(page).not_to have_content 'barbaz' + expect(page).not_to have_content 'gitlab' end - it 'sorts by due date' do - visit project_issues_path(project, sort: sort_value_due_date) + it 'allows filtering by a specified assignee' do + visit project_issues_path(project, assignee_id: user.id) - expect(first_issue).to include('foo') + expect(page).not_to have_content 'foobar' + expect(page).to have_content 'barbaz' + expect(page).to have_content 'gitlab' end + end - it 'sorts by due date by excluding nil due dates' do - bar.update(due_date: nil) + describe 'filter issue' do + titles = %w[foo bar baz] + titles.each_with_index do |title, index| + let!(title.to_sym) do + create(:issue, title: title, + project: project, + created_at: Time.now - (index * 60)) + end + end + let(:newer_due_milestone) { create(:milestone, due_date: '2013-12-11') } + let(:later_due_milestone) { create(:milestone, due_date: '2013-12-12') } - visit project_issues_path(project, sort: sort_value_due_date) + it 'sorts by newest' do + visit project_issues_path(project, sort: sort_value_created_date) expect(first_issue).to include('foo') + expect(last_issue).to include('baz') end - context 'with a filter on labels' do - let(:label) { create(:label, project: project) } + it 'sorts by most recently updated' do + baz.updated_at = Time.now + 100 + baz.save + visit project_issues_path(project, sort: sort_value_recently_updated) + expect(first_issue).to include('baz') + end + + describe 'sorting by due date' do before do - create(:label_link, label: label, target: foo) + foo.update(due_date: 1.day.from_now) + bar.update(due_date: 6.days.from_now) + end + + it 'sorts by due date' do + visit project_issues_path(project, sort: sort_value_due_date) + + expect(first_issue).to include('foo') end - it 'sorts by least recently due date by excluding nil due dates' do + it 'sorts by due date by excluding nil due dates' do bar.update(due_date: nil) - visit project_issues_path(project, label_names: [label.name], sort: sort_value_due_date_later) + visit project_issues_path(project, sort: sort_value_due_date) expect(first_issue).to include('foo') end - end - end - describe 'filtering by due date' do - before do - foo.update(due_date: 1.day.from_now) - bar.update(due_date: 6.days.from_now) - end + context 'with a filter on labels' do + let(:label) { create(:label, project: project) } + + before do + create(:label_link, label: label, target: foo) + end + + it 'sorts by least recently due date by excluding nil due dates' do + bar.update(due_date: nil) - it 'filters by none' do - visit project_issues_path(project, due_date: Issue::NoDueDate.name) + visit project_issues_path(project, label_names: [label.name], sort: sort_value_due_date_later) - page.within '.issues-holder' do - expect(page).not_to have_content('foo') - expect(page).not_to have_content('bar') - expect(page).to have_content('baz') + expect(first_issue).to include('foo') + end end end - it 'filters by any' do - visit project_issues_path(project, due_date: Issue::AnyDueDate.name) + describe 'filtering by due date' do + before do + foo.update(due_date: 1.day.from_now) + bar.update(due_date: 6.days.from_now) + end + + it 'filters by none' do + visit project_issues_path(project, due_date: Issue::NoDueDate.name) - page.within '.issues-holder' do - expect(page).to have_content('foo') - expect(page).to have_content('bar') - expect(page).to have_content('baz') + page.within '.issues-holder' do + expect(page).not_to have_content('foo') + expect(page).not_to have_content('bar') + expect(page).to have_content('baz') + end + end + + it 'filters by any' do + visit project_issues_path(project, due_date: Issue::AnyDueDate.name) + + page.within '.issues-holder' do + expect(page).to have_content('foo') + expect(page).to have_content('bar') + expect(page).to have_content('baz') + end end - end - it 'filters by due this week' do - foo.update(due_date: Date.today.beginning_of_week + 2.days) - bar.update(due_date: Date.today.end_of_week) - baz.update(due_date: Date.today - 8.days) + it 'filters by due this week' do + foo.update(due_date: Date.today.beginning_of_week + 2.days) + bar.update(due_date: Date.today.end_of_week) + baz.update(due_date: Date.today - 8.days) - visit project_issues_path(project, due_date: Issue::DueThisWeek.name) + visit project_issues_path(project, due_date: Issue::DueThisWeek.name) - page.within '.issues-holder' do - expect(page).to have_content('foo') - expect(page).to have_content('bar') - expect(page).not_to have_content('baz') + page.within '.issues-holder' do + expect(page).to have_content('foo') + expect(page).to have_content('bar') + expect(page).not_to have_content('baz') + end end - end - it 'filters by due this month' do - foo.update(due_date: Date.today.beginning_of_month + 2.days) - bar.update(due_date: Date.today.end_of_month) - baz.update(due_date: Date.today - 50.days) + it 'filters by due this month' do + foo.update(due_date: Date.today.beginning_of_month + 2.days) + bar.update(due_date: Date.today.end_of_month) + baz.update(due_date: Date.today - 50.days) - visit project_issues_path(project, due_date: Issue::DueThisMonth.name) + visit project_issues_path(project, due_date: Issue::DueThisMonth.name) - page.within '.issues-holder' do - expect(page).to have_content('foo') - expect(page).to have_content('bar') - expect(page).not_to have_content('baz') + page.within '.issues-holder' do + expect(page).to have_content('foo') + expect(page).to have_content('bar') + expect(page).not_to have_content('baz') + end end - end - it 'filters by overdue' do - foo.update(due_date: Date.today + 2.days) - bar.update(due_date: Date.today + 20.days) - baz.update(due_date: Date.yesterday) + it 'filters by overdue' do + foo.update(due_date: Date.today + 2.days) + bar.update(due_date: Date.today + 20.days) + baz.update(due_date: Date.yesterday) - visit project_issues_path(project, due_date: Issue::Overdue.name) + visit project_issues_path(project, due_date: Issue::Overdue.name) - page.within '.issues-holder' do - expect(page).not_to have_content('foo') - expect(page).not_to have_content('bar') - expect(page).to have_content('baz') + page.within '.issues-holder' do + expect(page).not_to have_content('foo') + expect(page).not_to have_content('bar') + expect(page).to have_content('baz') + end end end - end - describe 'sorting by milestone' do - before do - foo.milestone = newer_due_milestone - foo.save - bar.milestone = later_due_milestone - bar.save - end + describe 'sorting by milestone' do + before do + foo.milestone = newer_due_milestone + foo.save + bar.milestone = later_due_milestone + bar.save + end - it 'sorts by milestone' do - visit project_issues_path(project, sort: sort_value_milestone) + it 'sorts by milestone' do + visit project_issues_path(project, sort: sort_value_milestone) - expect(first_issue).to include('foo') - expect(last_issue).to include('baz') + expect(first_issue).to include('foo') + expect(last_issue).to include('baz') + end end - end - describe 'combine filter and sort' do - let(:user2) { create(:user) } + describe 'combine filter and sort' do + let(:user2) { create(:user) } - before do - foo.assignees << user2 - foo.save - bar.assignees << user2 - bar.save - end + before do + foo.assignees << user2 + foo.save + bar.assignees << user2 + bar.save + end - it 'sorts with a filter applied' do - visit project_issues_path(project, sort: sort_value_created_date, assignee_id: user2.id) + it 'sorts with a filter applied' do + visit project_issues_path(project, sort: sort_value_created_date, assignee_id: user2.id) - expect(first_issue).to include('foo') - expect(last_issue).to include('bar') - expect(page).not_to have_content('baz') + expect(first_issue).to include('foo') + expect(last_issue).to include('bar') + expect(page).not_to have_content('baz') + end end end - end - describe 'when I want to reset my incoming email token' do - let(:project1) { create(:project, namespace: user.namespace) } - let!(:issue) { create(:issue, project: project1) } + describe 'when I want to reset my incoming email token' do + let(:project1) { create(:project, namespace: user.namespace) } + let!(:issue) { create(:issue, project: project1) } - before do - stub_incoming_email_setting(enabled: true, address: "p+%{key}@gl.ab") - project1.team << [user, :master] - visit namespace_project_issues_path(user.namespace, project1) - end + before do + stub_incoming_email_setting(enabled: true, address: "p+%{key}@gl.ab") + project1.team << [user, :master] + visit namespace_project_issues_path(user.namespace, project1) + end - it 'changes incoming email address token', :js do - find('.issuable-email-modal-btn').click - previous_token = find('input#issuable_email').value - find('.incoming-email-token-reset').click + it 'changes incoming email address token', :js do + find('.issuable-email-modal-btn').click + previous_token = find('input#issuable_email').value + find('.incoming-email-token-reset').click - wait_for_requests + wait_for_requests - expect(page).to have_no_field('issuable_email', with: previous_token) - new_token = project1.new_issuable_address(user.reload, 'issue') - expect(page).to have_field( - 'issuable_email', - with: new_token - ) + expect(page).to have_no_field('issuable_email', with: previous_token) + new_token = project1.new_issuable_address(user.reload, 'issue') + expect(page).to have_field( + 'issuable_email', + with: new_token + ) + end end - end - describe 'update labels from issue#show', :js do - let(:issue) { create(:issue, project: project, author: user, assignees: [user]) } - let!(:label) { create(:label, project: project) } + describe 'update labels from issue#show', :js do + let(:issue) { create(:issue, project: project, author: user, assignees: [user]) } + let!(:label) { create(:label, project: project) } - before do - visit project_issue_path(project, issue) - end + before do + visit project_issue_path(project, issue) + end - it 'will not send ajax request when no data is changed' do - page.within '.labels' do - click_link 'Edit' + it 'will not send ajax request when no data is changed' do + page.within '.labels' do + click_link 'Edit' - find('.dropdown-menu-close', match: :first).click + find('.dropdown-menu-close', match: :first).click - expect(page).not_to have_selector('.block-loading') + expect(page).not_to have_selector('.block-loading') + end end end - end - describe 'update assignee from issue#show' do - let(:issue) { create(:issue, project: project, author: user, assignees: [user]) } + describe 'update assignee from issue#show' do + let(:issue) { create(:issue, project: project, author: user, assignees: [user]) } - context 'by authorized user' do - it 'allows user to select unassigned', :js do - visit project_issue_path(project, issue) + context 'by authorized user' do + it 'allows user to select unassigned', :js do + visit project_issue_path(project, issue) - page.within('.assignee') do - expect(page).to have_content "#{user.name}" + page.within('.assignee') do + expect(page).to have_content "#{user.name}" - click_link 'Edit' - click_link 'Unassigned' - first('.title').click - expect(page).to have_content 'No assignee' - end + click_link 'Edit' + click_link 'Unassigned' + first('.title').click + expect(page).to have_content 'No assignee' + end - # wait_for_requests does not work with vue-resource at the moment - sleep 1 + # wait_for_requests does not work with vue-resource at the moment + sleep 1 - expect(issue.reload.assignees).to be_empty - end + expect(issue.reload.assignees).to be_empty + end - it 'allows user to select an assignee', :js do - issue2 = create(:issue, project: project, author: user) - visit project_issue_path(project, issue2) + it 'allows user to select an assignee', :js do + issue2 = create(:issue, project: project, author: user) + visit project_issue_path(project, issue2) - page.within('.assignee') do - expect(page).to have_content "No assignee" - end + page.within('.assignee') do + expect(page).to have_content "No assignee" + end - page.within '.assignee' do - click_link 'Edit' - end + page.within '.assignee' do + click_link 'Edit' + end - page.within '.dropdown-menu-user' do - click_link user.name - end + page.within '.dropdown-menu-user' do + click_link user.name + end - page.within('.assignee') do - expect(page).to have_content user.name + page.within('.assignee') do + expect(page).to have_content user.name + end end - end - it 'allows user to unselect themselves', :js do - issue2 = create(:issue, project: project, author: user) - visit project_issue_path(project, issue2) + it 'allows user to unselect themselves', :js do + issue2 = create(:issue, project: project, author: user) + visit project_issue_path(project, issue2) - page.within '.assignee' do - click_link 'Edit' - click_link user.name + page.within '.assignee' do + click_link 'Edit' + click_link user.name - page.within '.value .author' do - expect(page).to have_content user.name - end + page.within '.value .author' do + expect(page).to have_content user.name + end - click_link 'Edit' - click_link user.name + click_link 'Edit' + click_link user.name - page.within '.value .assign-yourself' do - expect(page).to have_content "No assignee" + page.within '.value .assign-yourself' do + expect(page).to have_content "No assignee" + end end end end - end - context 'by unauthorized user' do - let(:guest) { create(:user) } + context 'by unauthorized user' do + let(:guest) { create(:user) } - before do - project.team << [[guest], :guest] - end + before do + project.team << [[guest], :guest] + end - it 'shows assignee text', :js do - sign_out(:user) - sign_in(guest) + it 'shows assignee text', :js do + sign_out(:user) + sign_in(guest) - visit project_issue_path(project, issue) - expect(page).to have_content issue.assignees.first.name + visit project_issue_path(project, issue) + expect(page).to have_content issue.assignees.first.name + end end end - end - describe 'update milestone from issue#show' do - let!(:issue) { create(:issue, project: project, author: user) } - let!(:milestone) { create(:milestone, project: project) } + describe 'update milestone from issue#show' do + let!(:issue) { create(:issue, project: project, author: user) } + let!(:milestone) { create(:milestone, project: project) } - context 'by authorized user' do - it 'allows user to select unassigned', :js do - visit project_issue_path(project, issue) + context 'by authorized user' do + it 'allows user to select unassigned', :js do + visit project_issue_path(project, issue) - page.within('.milestone') do - expect(page).to have_content "None" - end + page.within('.milestone') do + expect(page).to have_content "None" + end - find('.block.milestone .edit-link').click - sleep 2 # wait for ajax stuff to complete - first('.dropdown-content li').click - sleep 2 - page.within('.milestone') do - expect(page).to have_content 'None' + find('.block.milestone .edit-link').click + sleep 2 # wait for ajax stuff to complete + first('.dropdown-content li').click + sleep 2 + page.within('.milestone') do + expect(page).to have_content 'None' + end + + expect(issue.reload.milestone).to be_nil end - expect(issue.reload.milestone).to be_nil - end + it 'allows user to de-select milestone', :js do + visit project_issue_path(project, issue) - it 'allows user to de-select milestone', :js do - visit project_issue_path(project, issue) + page.within('.milestone') do + click_link 'Edit' + click_link milestone.title - page.within('.milestone') do - click_link 'Edit' - click_link milestone.title - - page.within '.value' do - expect(page).to have_content milestone.title - end + page.within '.value' do + expect(page).to have_content milestone.title + end - click_link 'Edit' - click_link milestone.title + click_link 'Edit' + click_link milestone.title - page.within '.value' do - expect(page).to have_content 'None' + page.within '.value' do + expect(page).to have_content 'None' + end end end end - end - context 'by unauthorized user' do - let(:guest) { create(:user) } + context 'by unauthorized user' do + let(:guest) { create(:user) } - before do - project.team << [guest, :guest] - issue.milestone = milestone - issue.save - end + before do + project.team << [guest, :guest] + issue.milestone = milestone + issue.save + end - it 'shows milestone text', :js do - sign_out(:user) - sign_in(guest) + it 'shows milestone text', :js do + sign_out(:user) + sign_in(guest) - visit project_issue_path(project, issue) - expect(page).to have_content milestone.title + visit project_issue_path(project, issue) + expect(page).to have_content milestone.title + end end end - end - describe 'new issue' do - let!(:issue) { create(:issue, project: project) } + describe 'new issue' do + let!(:issue) { create(:issue, project: project) } - context 'by unauthenticated user' do - before do - sign_out(:user) - end + context 'by unauthenticated user' do + before do + sign_out(:user) + end - it 'redirects to signin then back to new issue after signin' do - visit project_issues_path(project) + it 'redirects to signin then back to new issue after signin' do + visit project_issues_path(project) - page.within '.nav-controls' do - click_link 'New issue' - end + page.within '.nav-controls' do + click_link 'New issue' + end - expect(current_path).to eq new_user_session_path + expect(current_path).to eq new_user_session_path - gitlab_sign_in(create(:user)) + gitlab_sign_in(create(:user)) - expect(current_path).to eq new_project_issue_path(project) + expect(current_path).to eq new_project_issue_path(project) + end end - end - context 'dropzone upload file', :js do - before do - visit new_project_issue_path(project) - end + context 'dropzone upload file', :js do + before do + visit new_project_issue_path(project) + end - it 'uploads file when dragging into textarea' do - dropzone_file Rails.root.join('spec', 'fixtures', 'banana_sample.gif') + it 'uploads file when dragging into textarea' do + dropzone_file Rails.root.join('spec', 'fixtures', 'banana_sample.gif') - expect(page.find_field("issue_description").value).to have_content 'banana_sample' - end + expect(page.find_field("issue_description").value).to have_content 'banana_sample' + end - it "doesn't add double newline to end of a single attachment markdown" do - dropzone_file Rails.root.join('spec', 'fixtures', 'banana_sample.gif') + it "doesn't add double newline to end of a single attachment markdown" do + dropzone_file Rails.root.join('spec', 'fixtures', 'banana_sample.gif') - expect(page.find_field("issue_description").value).not_to match /\n\n$/ - end + expect(page.find_field("issue_description").value).not_to match /\n\n$/ + end - it "cancels a file upload correctly" do - slow_requests do - dropzone_file([Rails.root.join('spec', 'fixtures', 'dk.png')], 0, false) + it "cancels a file upload correctly" do + slow_requests do + dropzone_file([Rails.root.join('spec', 'fixtures', 'dk.png')], 0, false) - click_button 'Cancel' - end + click_button 'Cancel' + end - expect(page).to have_button('Attach a file') - expect(page).not_to have_button('Cancel') - expect(page).not_to have_selector('.uploading-progress-container', visible: true) + expect(page).to have_button('Attach a file') + expect(page).not_to have_button('Cancel') + expect(page).not_to have_selector('.uploading-progress-container', visible: true) + end end - end - context 'form filled by URL parameters' do - let(:project) { create(:project, :public, :repository) } + context 'form filled by URL parameters' do + let(:project) { create(:project, :public, :repository) } - before do - project.repository.create_file( - user, - '.gitlab/issue_templates/bug.md', - 'this is a test "bug" template', - message: 'added issue template', - branch_name: 'master') - - visit new_project_issue_path(project, issuable_template: 'bug') - end + before do + project.repository.create_file( + user, + '.gitlab/issue_templates/bug.md', + 'this is a test "bug" template', + message: 'added issue template', + branch_name: 'master') + + visit new_project_issue_path(project, issuable_template: 'bug') + end - it 'fills in template' do - expect(find('.js-issuable-selector .dropdown-toggle-text')).to have_content('bug') + it 'fills in template' do + expect(find('.js-issuable-selector .dropdown-toggle-text')).to have_content('bug') + end end end - end - describe 'new issue by email' do - shared_examples 'show the email in the modal' do - let(:issue) { create(:issue, project: project) } + describe 'new issue by email' do + shared_examples 'show the email in the modal' do + let(:issue) { create(:issue, project: project) } - before do - project.issues << issue - stub_incoming_email_setting(enabled: true, address: "p+%{key}@gl.ab") + before do + project.issues << issue + stub_incoming_email_setting(enabled: true, address: "p+%{key}@gl.ab") - visit project_issues_path(project) - click_button('Email a new issue') - end + visit project_issues_path(project) + click_button('Email a new issue') + end - it 'click the button to show modal for the new email' do - page.within '#issuable-email-modal' do - email = project.new_issuable_address(user, 'issue') + it 'click the button to show modal for the new email' do + page.within '#issuable-email-modal' do + email = project.new_issuable_address(user, 'issue') - expect(page).to have_selector("input[value='#{email}']") + expect(page).to have_selector("input[value='#{email}']") + end end end - end - context 'with existing issues' do - let!(:issue) { create(:issue, project: project, author: user) } + context 'with existing issues' do + let!(:issue) { create(:issue, project: project, author: user) } - it_behaves_like 'show the email in the modal' - end + it_behaves_like 'show the email in the modal' + end - context 'without existing issues' do - it_behaves_like 'show the email in the modal' + context 'without existing issues' do + it_behaves_like 'show the email in the modal' + end end - end - describe 'due date' do - context 'update due on issue#show', :js do - let(:issue) { create(:issue, project: project, author: user, assignees: [user]) } + describe 'due date' do + context 'update due on issue#show', :js do + let(:issue) { create(:issue, project: project, author: user, assignees: [user]) } - before do - visit project_issue_path(project, issue) - end + before do + visit project_issue_path(project, issue) + end - it 'adds due date to issue' do - date = Date.today.at_beginning_of_month + 2.days + it 'adds due date to issue' do + date = Date.today.at_beginning_of_month + 2.days - page.within '.due_date' do - click_link 'Edit' + page.within '.due_date' do + click_link 'Edit' - page.within '.pika-single' do - click_button date.day - end + page.within '.pika-single' do + click_button date.day + end - wait_for_requests + wait_for_requests - expect(find('.value').text).to have_content date.strftime('%b %-d, %Y') + expect(find('.value').text).to have_content date.strftime('%b %-d, %Y') + end end - end - it 'removes due date from issue' do - date = Date.today.at_beginning_of_month + 2.days + it 'removes due date from issue' do + date = Date.today.at_beginning_of_month + 2.days - page.within '.due_date' do - click_link 'Edit' + page.within '.due_date' do + click_link 'Edit' - page.within '.pika-single' do - click_button date.day - end + page.within '.pika-single' do + click_button date.day + end - wait_for_requests + wait_for_requests - expect(page).to have_no_content 'No due date' + expect(page).to have_no_content 'No due date' - click_link 'remove due date' - expect(page).to have_content 'No due date' + click_link 'remove due date' + expect(page).to have_content 'No due date' + end end end end - end - describe 'title issue#show', :js do - it 'updates the title', :js do - issue = create(:issue, author: user, assignees: [user], project: project, title: 'new title') + describe 'title issue#show', :js do + it 'updates the title', :js do + issue = create(:issue, author: user, assignees: [user], project: project, title: 'new title') - visit project_issue_path(project, issue) + visit project_issue_path(project, issue) - expect(page).to have_text("new title") + expect(page).to have_text("new title") - issue.update(title: "updated title") + issue.update(title: "updated title") - wait_for_requests - expect(page).to have_text("updated title") + wait_for_requests + expect(page).to have_text("updated title") + end end - end - describe 'confidential issue#show', :js do - it 'shows confidential sibebar information as confidential and can be turned off' do - issue = create(:issue, :confidential, project: project) + describe 'confidential issue#show', :js do + it 'shows confidential sibebar information as confidential and can be turned off' do + issue = create(:issue, :confidential, project: project) - visit project_issue_path(project, issue) + visit project_issue_path(project, issue) - expect(page).to have_css('.issuable-note-warning') - expect(find('.issuable-sidebar-item.confidentiality')).to have_css('.is-active') - expect(find('.issuable-sidebar-item.confidentiality')).not_to have_css('.not-active') + expect(page).to have_css('.issuable-note-warning') + expect(find('.issuable-sidebar-item.confidentiality')).to have_css('.is-active') + expect(find('.issuable-sidebar-item.confidentiality')).not_to have_css('.not-active') - find('.confidential-edit').click - expect(page).to have_css('.sidebar-item-warning-message') + find('.confidential-edit').click + expect(page).to have_css('.sidebar-item-warning-message') - within('.sidebar-item-warning-message') do - find('.btn-close').click - end + within('.sidebar-item-warning-message') do + find('.btn-close').click + end - wait_for_requests + wait_for_requests - visit project_issue_path(project, issue) + visit project_issue_path(project, issue) - expect(page).not_to have_css('.is-active') + expect(page).not_to have_css('.is-active') + end end end end diff --git a/spec/features/merge_requests/image_diff_notes.rb b/spec/features/merge_requests/image_diff_notes_spec.rb index 021c4e03428..ddc73437917 100644 --- a/spec/features/merge_requests/image_diff_notes.rb +++ b/spec/features/merge_requests/image_diff_notes_spec.rb @@ -10,8 +10,6 @@ feature 'image diff notes', :js do project.team << [user, :master] sign_in user - page.driver.set_cookie('sidebar_collapsed', 'true') - # Stub helper to return any blob file as image from public app folder. # This is necessary to run this specs since we don't display repo images in capybara. allow_any_instance_of(DiffHelper).to receive(:diff_file_blob_raw_path).and_return('/apple-touch-icon.png') @@ -141,13 +139,13 @@ feature 'image diff notes', :js do end it 'allows expanding/collapsing the discussion notes' do - page.all('.js-diff-notes-toggle')[0].trigger('click') - page.all('.js-diff-notes-toggle')[1].trigger('click') + page.all('.js-diff-notes-toggle')[0].click + page.all('.js-diff-notes-toggle')[1].click expect(page).not_to have_content('image diff test comment') - page.all('.js-diff-notes-toggle')[0].trigger('click') - page.all('.js-diff-notes-toggle')[1].trigger('click') + page.all('.js-diff-notes-toggle')[0].click + page.all('.js-diff-notes-toggle')[1].click expect(page).to have_content('image diff test comment') end @@ -196,13 +194,31 @@ feature 'image diff notes', :js do expect(find('.onion-skin-frame')['style']).to match('width: 228px; height: 240px;') end + + it 'resets onion skin view mode opacity when toggling between view modes' do + find('.view-modes-menu .onion-skin').click + + # Simulate dragging onion-skin slider + drag_and_drop_by(find('.dragger'), -30, 0) + + expect(find('.onion-skin-frame .frame.added', visible: false)['style']).not_to match('opacity: 1;') + + find('.view-modes-menu .swipe').click + find('.view-modes-menu .onion-skin').click + + expect(find('.onion-skin-frame .frame.added', visible: false)['style']).to match('opacity: 1;') + end end -end -def create_image_diff_note - find('.js-add-image-diff-note-button', match: :first).click - page.all('.js-add-image-diff-note-button')[0].trigger('click') - find('.diff-content .note-textarea').native.send_keys('image diff test comment') - click_button 'Comment' - wait_for_requests + def drag_and_drop_by(element, right_by, down_by) + page.driver.browser.action.drag_and_drop_by(element.native, right_by, down_by).perform + end + + def create_image_diff_note + find('.js-add-image-diff-note-button', match: :first).click + page.all('.js-add-image-diff-note-button')[0].click + find('.diff-content .note-textarea').native.send_keys('image diff test comment') + click_button 'Comment' + wait_for_requests + end end diff --git a/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb b/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb index 6c616bf0456..8ac9821b879 100644 --- a/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb +++ b/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb @@ -2,15 +2,15 @@ require 'spec_helper' feature 'project owner sees a link to create a license file in empty project', :js do let(:project_master) { create(:user) } - let(:project) { create(:project) } + let(:project) { create(:project_empty_repo) } + background do - project.team << [project_master, :master] + project.add_master(project_master) sign_in(project_master) end scenario 'project master creates a license file from a template' do visit project_path(project) - click_link 'Create empty bare repository' click_on 'LICENSE' expect(page).to have_content('New file') @@ -26,8 +26,6 @@ feature 'project owner sees a link to create a license file in empty project', : expect(file_content).to have_content("Copyright (c) #{Time.now.year} #{project.namespace.human_name}") fill_in :commit_message, with: 'Add a LICENSE file', visible: true - # Remove pre-receive hook so we can push without auth - FileUtils.rm_f(File.join(project.repository.path, 'hooks', 'pre-receive')) click_button 'Commit changes' expect(current_path).to eq( diff --git a/spec/features/projects/issuable_templates_spec.rb b/spec/features/projects/issuable_templates_spec.rb index 0257cd157c9..4319fc2746c 100644 --- a/spec/features/projects/issuable_templates_spec.rb +++ b/spec/features/projects/issuable_templates_spec.rb @@ -32,9 +32,7 @@ feature 'issuable templates', :js do message: 'added issue template', branch_name: 'master') visit project_issue_path project, issue - page.within('.js-issuable-actions') do - click_on 'Edit' - end + page.find('.js-issuable-edit').click fill_in :'issuable-title', with: 'test issue title' end @@ -77,9 +75,7 @@ feature 'issuable templates', :js do message: 'added issue template', branch_name: 'master') visit project_issue_path project, issue - page.within('.js-issuable-actions') do - click_on 'Edit' - end + page.find('.js-issuable-edit').click fill_in :'issuable-title', with: 'test issue title' fill_in :'issue-description', with: prior_description end diff --git a/spec/features/tags/master_views_tags_spec.rb b/spec/features/tags/master_views_tags_spec.rb index 9edc7ced163..4662367d843 100644 --- a/spec/features/tags/master_views_tags_spec.rb +++ b/spec/features/tags/master_views_tags_spec.rb @@ -4,18 +4,17 @@ feature 'Master views tags' do let(:user) { create(:user) } before do - project.team << [user, :master] + project.add_master(user) sign_in(user) end context 'when project has no tags' do let(:project) { create(:project_empty_repo) } + before do visit project_path(project) click_on 'README' fill_in :commit_message, with: 'Add a README file', visible: true - # Remove pre-receive hook so we can push without auth - FileUtils.rm_f(File.join(project.repository.path, 'hooks', 'pre-receive')) click_button 'Commit changes' visit project_tags_path(project) end diff --git a/spec/helpers/notes_helper_spec.rb b/spec/helpers/notes_helper_spec.rb index cd15e27b497..36a44f8567a 100644 --- a/spec/helpers/notes_helper_spec.rb +++ b/spec/helpers/notes_helper_spec.rb @@ -41,6 +41,7 @@ describe NotesHelper do describe '#discussion_path' do let(:project) { create(:project, :repository) } + let(:anchor) { discussion.line_code } context 'for a merge request discusion' do let(:merge_request) { create(:merge_request, source_project: project, target_project: project, importing: true) } @@ -151,6 +152,15 @@ describe NotesHelper do expect(helper.discussion_path(discussion)).to be_nil end end + + context 'for a contextual commit discussion' do + let(:commit) { merge_request.commits.last } + let(:discussion) { create(:diff_note_on_merge_request, noteable: merge_request, project: project, commit_id: commit.id).to_discussion } + + it 'returns the merge request diff discussion scoped in the commit' do + expect(helper.discussion_path(discussion)).to eq(diffs_project_merge_request_path(project, merge_request, commit_id: commit.id, anchor: anchor)) + end + end end context 'for a commit discussion' do @@ -160,7 +170,7 @@ describe NotesHelper do let(:discussion) { create(:diff_note_on_commit, project: project).to_discussion } it 'returns the commit path with the line code' do - expect(helper.discussion_path(discussion)).to eq(project_commit_path(project, commit, anchor: discussion.line_code)) + expect(helper.discussion_path(discussion)).to eq(project_commit_path(project, commit, anchor: anchor)) end end @@ -168,7 +178,7 @@ describe NotesHelper do let(:discussion) { create(:legacy_diff_note_on_commit, project: project).to_discussion } it 'returns the commit path with the line code' do - expect(helper.discussion_path(discussion)).to eq(project_commit_path(project, commit, anchor: discussion.line_code)) + expect(helper.discussion_path(discussion)).to eq(project_commit_path(project, commit, anchor: anchor)) end end diff --git a/spec/javascripts/merge_request_notes_spec.js b/spec/javascripts/merge_request_notes_spec.js index 6054b75d0b8..e983e4de3fc 100644 --- a/spec/javascripts/merge_request_notes_spec.js +++ b/spec/javascripts/merge_request_notes_spec.js @@ -1,11 +1,9 @@ -/* global Notes */ - import 'autosize'; import '~/gl_form'; import '~/lib/utils/text_utility'; import '~/render_gfm'; import '~/render_math'; -import '~/notes'; +import Notes from '~/notes'; const upArrowKeyCode = 38; diff --git a/spec/javascripts/merge_request_tabs_spec.js b/spec/javascripts/merge_request_tabs_spec.js index 31426ceb110..050f0ea9ebd 100644 --- a/spec/javascripts/merge_request_tabs_spec.js +++ b/spec/javascripts/merge_request_tabs_spec.js @@ -1,5 +1,4 @@ /* eslint-disable no-var, comma-dangle, object-shorthand */ -/* global Notes */ import * as urlUtils from '~/lib/utils/url_utility'; import MergeRequestTabs from '~/merge_request_tabs'; @@ -7,7 +6,7 @@ import '~/commit/pipelines/pipelines_bundle'; import '~/breakpoints'; import '~/lib/utils/common_utils'; import Diff from '~/diff'; -import '~/notes'; +import Notes from '~/notes'; import 'vendor/jquery.scrollTo'; (function () { @@ -279,8 +278,8 @@ import 'vendor/jquery.scrollTo'; loadFixtures('merge_requests/diff_comment.html.raw'); $('body').attr('data-page', 'projects:merge_requests:show'); window.gl.ImageFile = () => {}; - window.notes = new Notes('', []); - spyOn(window.notes, 'toggleDiffNote').and.callThrough(); + Notes.initialize('', []); + spyOn(Notes.instance, 'toggleDiffNote').and.callThrough(); }); afterEach(() => { @@ -338,7 +337,7 @@ import 'vendor/jquery.scrollTo'; this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); expect(noteId.length).toBeGreaterThan(0); - expect(window.notes.toggleDiffNote).toHaveBeenCalledWith({ + expect(Notes.instance.toggleDiffNote).toHaveBeenCalledWith({ target: jasmine.any(Object), lineType: 'old', forceShow: true, @@ -349,7 +348,7 @@ import 'vendor/jquery.scrollTo'; spyOn(urlUtils, 'getLocationHash').and.returnValue('note_something-that-does-not-exist'); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); - expect(window.notes.toggleDiffNote).not.toHaveBeenCalled(); + expect(Notes.instance.toggleDiffNote).not.toHaveBeenCalled(); }); }); @@ -359,7 +358,7 @@ import 'vendor/jquery.scrollTo'; this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); expect(noteLineNumId.length).toBeGreaterThan(0); - expect(window.notes.toggleDiffNote).not.toHaveBeenCalled(); + expect(Notes.instance.toggleDiffNote).not.toHaveBeenCalled(); }); }); }); @@ -393,7 +392,7 @@ import 'vendor/jquery.scrollTo'; this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); expect(noteId.length).toBeGreaterThan(0); - expect(window.notes.toggleDiffNote).toHaveBeenCalledWith({ + expect(Notes.instance.toggleDiffNote).toHaveBeenCalledWith({ target: jasmine.any(Object), lineType: 'new', forceShow: true, @@ -404,7 +403,7 @@ import 'vendor/jquery.scrollTo'; spyOn(urlUtils, 'getLocationHash').and.returnValue('note_something-that-does-not-exist'); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); - expect(window.notes.toggleDiffNote).not.toHaveBeenCalled(); + expect(Notes.instance.toggleDiffNote).not.toHaveBeenCalled(); }); }); @@ -414,7 +413,7 @@ import 'vendor/jquery.scrollTo'; this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); expect(noteLineNumId.length).toBeGreaterThan(0); - expect(window.notes.toggleDiffNote).not.toHaveBeenCalled(); + expect(Notes.instance.toggleDiffNote).not.toHaveBeenCalled(); }); }); }); diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index e09b8dc7fc5..167f074fb9b 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -1,12 +1,10 @@ /* eslint-disable space-before-function-paren, no-unused-expressions, no-var, object-shorthand, comma-dangle, max-len */ -/* global Notes */ - import * as urlUtils from '~/lib/utils/url_utility'; import 'autosize'; import '~/gl_form'; import '~/lib/utils/text_utility'; import '~/render_gfm'; -import '~/notes'; +import Notes from '~/notes'; (function() { window.gon || (window.gon = {}); 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 9e6d0aa472c..74b343c573e 100644 --- a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js +++ b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js @@ -2,6 +2,7 @@ import Vue from 'vue'; import mrWidgetOptions from '~/vue_merge_request_widget/mr_widget_options'; import eventHub from '~/vue_merge_request_widget/event_hub'; import notify from '~/lib/utils/notify'; +import { stateKey } from '~/vue_merge_request_widget/stores/state_maps'; import mockData from './mock_data'; import mountComponent from '../helpers/vue_mount_component_helper'; @@ -344,4 +345,31 @@ describe('mrWidgetOptions', () => { expect(comps['mr-widget-merge-when-pipeline-succeeds']).toBeDefined(); }); }); + + describe('rendering relatedLinks', () => { + beforeEach((done) => { + vm.mr.relatedLinks = { + assignToMe: null, + closing: ` + <a class="close-related-link" href="#'> + Close + </a> + `, + mentioned: '', + }; + Vue.nextTick(done); + }); + + it('renders if there are relatedLinks', () => { + expect(vm.$el.querySelector('.close-related-link')).toBeDefined(); + }); + + it('does not render if state is nothingToMerge', (done) => { + vm.mr.state = stateKey.nothingToMerge; + Vue.nextTick(() => { + expect(vm.$el.querySelector('.close-related-link')).toBeNull(); + done(); + }); + }); + }); }); diff --git a/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js b/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js index 8e5614b20f0..33d052aceb2 100644 --- a/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js +++ b/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js @@ -1,4 +1,5 @@ import MergeRequestStore from '~/vue_merge_request_widget/stores/mr_widget_store'; +import { stateKey } from '~/vue_merge_request_widget/stores/state_maps'; import mockData from '../mock_data'; describe('MergeRequestStore', () => { @@ -52,5 +53,17 @@ describe('MergeRequestStore', () => { expect(store.isPipelineSkipped).toBe(false); }); }); + + describe('isNothingToMergeState', () => { + it('returns true when nothingToMerge', () => { + store.state = stateKey.nothingToMerge; + expect(store.isNothingToMergeState).toEqual(true); + }); + + it('returns false when not nothingToMerge', () => { + store.state = 'state'; + expect(store.isNothingToMergeState).toEqual(false); + }); + }); }); }); diff --git a/spec/lib/gitlab/action_rate_limiter_spec.rb b/spec/lib/gitlab/action_rate_limiter_spec.rb new file mode 100644 index 00000000000..542fc03e555 --- /dev/null +++ b/spec/lib/gitlab/action_rate_limiter_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe Gitlab::ActionRateLimiter do + let(:redis) { double('redis') } + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:key) { [user, project] } + let(:cache_key) { "action_rate_limiter:test_action:user:#{user.id}:project:#{project.id}" } + + subject { described_class.new(action: :test_action, expiry_time: 100) } + + before do + allow(Gitlab::Redis::Cache).to receive(:with).and_yield(redis) + end + + it 'increases the throttle count and sets the expire time' do + expect(redis).to receive(:incr).with(cache_key).and_return(1) + expect(redis).to receive(:expire).with(cache_key, 100) + + expect(subject.throttled?(key, 1)).to be false + end + + it 'returns true if the key is throttled' do + expect(redis).to receive(:incr).with(cache_key).and_return(2) + expect(redis).not_to receive(:expire) + + expect(subject.throttled?(key, 1)).to be true + end +end diff --git a/spec/lib/gitlab/git/gitlab_projects_spec.rb b/spec/lib/gitlab/git/gitlab_projects_spec.rb index 18906955df6..24da9589458 100644 --- a/spec/lib/gitlab/git/gitlab_projects_spec.rb +++ b/spec/lib/gitlab/git/gitlab_projects_spec.rb @@ -41,7 +41,8 @@ describe Gitlab::Git::GitlabProjects do end it "fails if the source path doesn't exist" do - expect(logger).to receive(:error).with("mv-project failed: source path <#{tmp_repos_path}/bad-src.git> does not exist.") + expected_source_path = File.join(tmp_repos_path, 'bad-src.git') + expect(logger).to receive(:error).with("mv-project failed: source path <#{expected_source_path}> does not exist.") result = build_gitlab_projects(tmp_repos_path, 'bad-src.git').mv_project('repo.git') expect(result).to be_falsy @@ -50,7 +51,8 @@ describe Gitlab::Git::GitlabProjects do it 'fails if the destination path already exists' do FileUtils.mkdir_p(File.join(tmp_repos_path, 'already-exists.git')) - message = "mv-project failed: destination path <#{tmp_repos_path}/already-exists.git> already exists." + expected_distination_path = File.join(tmp_repos_path, 'already-exists.git') + message = "mv-project failed: destination path <#{expected_distination_path}> already exists." expect(logger).to receive(:error).with(message) expect(gl_projects.mv_project('already-exists.git')).to be_falsy diff --git a/spec/models/blob_viewer/package_json_spec.rb b/spec/models/blob_viewer/package_json_spec.rb index 0f8330e91c1..5ed2f4400bc 100644 --- a/spec/models/blob_viewer/package_json_spec.rb +++ b/spec/models/blob_viewer/package_json_spec.rb @@ -22,4 +22,51 @@ describe BlobViewer::PackageJson do expect(subject.package_name).to eq('module-name') end end + + describe '#package_url' do + it 'returns the package URL' do + expect(subject).to receive(:prepare!) + + expect(subject.package_url).to eq("https://www.npmjs.com/package/#{subject.package_name}") + end + end + + describe '#package_type' do + it 'returns "package"' do + expect(subject).to receive(:prepare!) + + expect(subject.package_type).to eq('package') + end + end + + context 'when package.json has "private": true' do + let(:data) do + <<-SPEC.strip_heredoc + { + "name": "module-name", + "version": "10.3.1", + "private": true, + "homepage": "myawesomepackage.com" + } + SPEC + end + let(:blob) { fake_blob(path: 'package.json', data: data) } + subject { described_class.new(blob) } + + describe '#package_url' do + it 'returns homepage if any' do + expect(subject).to receive(:prepare!) + + expect(subject.package_url).to eq('myawesomepackage.com') + end + end + + describe '#package_type' do + it 'returns "private package"' do + expect(subject).to receive(:prepare!) + + expect(subject.package_type).to eq('private package') + end + end + end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index d18a5c9dfa6..cd955a5eb69 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -13,6 +13,45 @@ describe Commit do it { is_expected.to include_module(StaticModel) } end + describe '.lazy' do + set(:project) { create(:project, :repository) } + + context 'when the commits are found' do + let(:oids) do + %w( + 498214de67004b1da3d820901307bed2a68a8ef6 + c642fe9b8b9f28f9225d7ea953fe14e74748d53b + 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + 048721d90c449b244b7b4c53a9186b04330174ec + 281d3a76f31c812dbf48abce82ccf6860adedd81 + ) + end + + subject { oids.map { |oid| described_class.lazy(project, oid) } } + + it 'batches requests for commits' do + expect(project.repository).to receive(:commits_by).once.and_call_original + + subject.first.title + subject.last.title + end + + it 'maintains ordering' do + subject.each_with_index do |commit, i| + expect(commit.id).to eq(oids[i]) + end + end + end + + context 'when not found' do + it 'returns nil as commit' do + commit = described_class.lazy(project, 'deadbeef').__sync + + expect(commit).to be_nil + end + end + end + describe '#author' do it 'looks up the author in a case-insensitive way' do user = create(:user, email: commit.author_email.upcase) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 799d99c0369..1d7069feebd 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -239,6 +239,54 @@ describe Repository do end end + describe '#commits_by' do + set(:project) { create(:project, :repository) } + + shared_examples 'batch commits fetching' do + let(:oids) { TestEnv::BRANCH_SHA.values } + + subject { project.repository.commits_by(oids: oids) } + + it 'finds each commit' do + expect(subject).not_to include(nil) + expect(subject.size).to eq(oids.size) + end + + it 'returns only Commit instances' do + expect(subject).to all( be_a(Commit) ) + end + + context 'when some commits are not found ' do + let(:oids) do + ['deadbeef'] + TestEnv::BRANCH_SHA.values.first(10) + end + + it 'returns only found commits' do + expect(subject).not_to include(nil) + expect(subject.size).to eq(10) + end + end + + context 'when no oids are passed' do + let(:oids) { [] } + + it 'does not call #batch_by_oid' do + expect(Gitlab::Git::Commit).not_to receive(:batch_by_oid) + + subject + end + end + end + + context 'when Gitaly list_commits_by_oid is enabled' do + it_behaves_like 'batch commits fetching' + end + + context 'when Gitaly list_commits_by_oid is enabled', :disable_gitaly do + it_behaves_like 'batch commits fetching' + end + end + describe '#find_commits_by_message' do shared_examples 'finding commits by message' do it 'returns commits with messages containing a given string' do @@ -1163,6 +1211,15 @@ describe Repository do end end + describe '#tag_exists?' do + it 'uses tag_names' do + allow(repository).to receive(:tag_names).and_return(['foobar']) + + expect(repository.tag_exists?('foobar')).to eq(true) + expect(repository.tag_exists?('master')).to eq(false) + end + end + describe '#branch_names', :use_clean_rails_memory_store_caching do let(:fake_branch_names) { ['foobar'] } diff --git a/spec/policies/ci/pipeline_schedule_policy_spec.rb b/spec/policies/ci/pipeline_schedule_policy_spec.rb new file mode 100644 index 00000000000..1b0e9fac355 --- /dev/null +++ b/spec/policies/ci/pipeline_schedule_policy_spec.rb @@ -0,0 +1,92 @@ +require 'spec_helper' + +describe Ci::PipelineSchedulePolicy, :models do + set(:user) { create(:user) } + set(:project) { create(:project, :repository) } + set(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly, project: project) } + + let(:policy) do + described_class.new(user, pipeline_schedule) + end + + describe 'rules' do + describe 'rules for protected ref' do + before do + project.add_developer(user) + end + + context 'when no one can push or merge to the branch' do + before do + create(:protected_branch, :no_one_can_push, + name: pipeline_schedule.ref, project: project) + end + + it 'does not include ability to play pipeline schedule' do + expect(policy).to be_disallowed :play_pipeline_schedule + end + end + + context 'when developers can push to the branch' do + before do + create(:protected_branch, :developers_can_merge, + name: pipeline_schedule.ref, project: project) + end + + it 'includes ability to update pipeline' do + expect(policy).to be_allowed :play_pipeline_schedule + end + end + + context 'when no one can create the tag' do + let(:tag) { 'v1.0.0' } + + before do + pipeline_schedule.update(ref: tag) + + create(:protected_tag, :no_one_can_create, + name: pipeline_schedule.ref, project: project) + end + + it 'does not include ability to play pipeline schedule' do + expect(policy).to be_disallowed :play_pipeline_schedule + end + end + + context 'when no one can create the tag but it is not a tag' do + before do + create(:protected_tag, :no_one_can_create, + name: pipeline_schedule.ref, project: project) + end + + it 'includes ability to play pipeline schedule' do + expect(policy).to be_allowed :play_pipeline_schedule + end + end + end + + describe 'rules for owner of schedule' do + before do + project.add_developer(user) + pipeline_schedule.update(owner: user) + end + + it 'includes abilities to do do all operations on pipeline schedule' do + expect(policy).to be_allowed :play_pipeline_schedule + expect(policy).to be_allowed :update_pipeline_schedule + expect(policy).to be_allowed :admin_pipeline_schedule + end + end + + describe 'rules for a master' do + before do + project.add_master(user) + end + + it 'includes abilities to do do all operations on pipeline schedule' do + expect(policy).to be_allowed :play_pipeline_schedule + expect(policy).to be_allowed :update_pipeline_schedule + expect(policy).to be_allowed :admin_pipeline_schedule + end + end + end +end diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb index 88d347322a6..c38795ad1a1 100644 --- a/spec/serializers/pipeline_serializer_spec.rb +++ b/spec/serializers/pipeline_serializer_spec.rb @@ -1,6 +1,7 @@ require 'spec_helper' describe PipelineSerializer do + set(:project) { create(:project, :repository) } set(:user) { create(:user) } let(:serializer) do @@ -16,7 +17,7 @@ describe PipelineSerializer do end context 'when a single object is being serialized' do - let(:resource) { create(:ci_empty_pipeline) } + let(:resource) { create(:ci_empty_pipeline, project: project) } it 'serializers the pipeline object' do expect(subject[:id]).to eq resource.id @@ -24,7 +25,7 @@ describe PipelineSerializer do end context 'when multiple objects are being serialized' do - let(:resource) { create_list(:ci_pipeline, 2) } + let(:resource) { create_list(:ci_pipeline, 2, project: project) } it 'serializers the array of pipelines' do expect(subject).not_to be_empty @@ -100,7 +101,6 @@ describe PipelineSerializer do context 'number of queries' do let(:resource) { Ci::Pipeline.all } - let(:project) { create(:project) } before do # Since RequestStore.active? is true we have to allow the diff --git a/spec/services/issuable/destroy_service_spec.rb b/spec/services/issuable/destroy_service_spec.rb index d74d98c6079..0a3647a814f 100644 --- a/spec/services/issuable/destroy_service_spec.rb +++ b/spec/services/issuable/destroy_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Issuable::DestroyService do let(:user) { create(:user) } - let(:project) { create(:project) } + let(:project) { create(:project, :public) } subject(:service) { described_class.new(project, user) } @@ -19,6 +19,13 @@ describe Issuable::DestroyService do service.execute(issue) end + + it 'updates the todo caches for users with todos on the issue' do + create(:todo, target: issue, user: user, author: user, project: project) + + expect { service.execute(issue) } + .to change { user.todos_pending_count }.from(1).to(0) + end end context 'when issuable is a merge request' do @@ -33,6 +40,13 @@ describe Issuable::DestroyService do service.execute(merge_request) end + + it 'updates the todo caches for users with todos on the merge request' do + create(:todo, target: merge_request, user: user, author: user, project: project) + + expect { service.execute(merge_request) } + .to change { user.todos_pending_count }.from(1).to(0) + end end end end diff --git a/spec/services/notes/destroy_service_spec.rb b/spec/services/notes/destroy_service_spec.rb index c9a99a43edb..64445be560e 100644 --- a/spec/services/notes/destroy_service_spec.rb +++ b/spec/services/notes/destroy_service_spec.rb @@ -1,15 +1,25 @@ require 'spec_helper' describe Notes::DestroyService do + set(:project) { create(:project, :public) } + set(:issue) { create(:issue, project: project) } + let(:user) { issue.author } + describe '#execute' do it 'deletes a note' do - project = create(:project) - issue = create(:issue, project: project) note = create(:note, project: project, noteable: issue) - described_class.new(project, note.author).execute(note) + described_class.new(project, user).execute(note) expect(project.issues.find(issue.id).notes).not_to include(note) end + + it 'updates the todo counts for users with todos for the note' do + note = create(:note, project: project, noteable: issue) + create(:todo, note: note, target: issue, user: user, author: user, project: project) + + expect { described_class.new(project, user).execute(note) } + .to change { user.todos_pending_count }.from(1).to(0) + end end end diff --git a/spec/services/projects/unlink_fork_service_spec.rb b/spec/services/projects/unlink_fork_service_spec.rb index 2bba71fef4f..3ec6139bfa6 100644 --- a/spec/services/projects/unlink_fork_service_spec.rb +++ b/spec/services/projects/unlink_fork_service_spec.rb @@ -62,6 +62,26 @@ describe Projects::UnlinkForkService do expect(source.forks_count).to be_zero end + context 'when the source has LFS objects' do + let(:lfs_object) { create(:lfs_object) } + + before do + lfs_object.projects << project + end + + it 'links the fork to the lfs object before unlinking' do + subject.execute + + expect(lfs_object.projects).to include(forked_project) + end + + it 'does not fail if the lfs objects were already linked' do + lfs_object.projects << forked_project + + expect { subject.execute }.not_to raise_error + end + end + context 'when the original project was deleted' do it 'does not fail when the original project is deleted' do source = forked_project.forked_from_project diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index dc2673abc73..88013acae0a 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -248,11 +248,26 @@ describe TodoService do end end - describe '#destroy_issuable' do - it 'refresh the todos count cache for the user' do - expect(john_doe).to receive(:update_todos_count_cache).and_call_original + describe '#destroy_target' do + it 'refreshes the todos count cache for users with todos on the target' do + create(:todo, target: issue, user: john_doe, author: john_doe, project: issue.project) - service.destroy_issuable(issue, john_doe) + expect_any_instance_of(User).to receive(:update_todos_count_cache).and_call_original + + service.destroy_target(issue) { } + end + + it 'does not refresh the todos count cache for users with only done todos on the target' do + create(:todo, :done, target: issue, user: john_doe, author: john_doe, project: issue.project) + + expect_any_instance_of(User).not_to receive(:update_todos_count_cache) + + service.destroy_target(issue) { } + end + + it 'yields the target to the caller' do + expect { |b| service.destroy_target(issue, &b) } + .to yield_with_args(issue) end end diff --git a/spec/views/events/event/_push.html.haml_spec.rb b/spec/views/events/event/_push.html.haml_spec.rb new file mode 100644 index 00000000000..f5634de4916 --- /dev/null +++ b/spec/views/events/event/_push.html.haml_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +describe 'events/event/_push.html.haml' do + let(:event) { build_stubbed(:push_event) } + + context 'with a branch' do + let(:payload) { build_stubbed(:push_event_payload, event: event) } + + before do + allow(event).to receive(:push_event_payload).and_return(payload) + end + + it 'links to the branch' do + allow(event.project.repository).to receive(:branch_exists?).with(event.ref_name).and_return(true) + link = project_commits_path(event.project, event.ref_name) + + render partial: 'events/event/push', locals: { event: event } + + expect(rendered).to have_link(event.ref_name, href: link) + end + + context 'that has been deleted' do + it 'does not link to the branch' do + render partial: 'events/event/push', locals: { event: event } + + expect(rendered).not_to have_link(event.ref_name) + end + end + end + + context 'with a tag' do + let(:payload) { build_stubbed(:push_event_payload, event: event, ref_type: :tag, ref: 'v0.1.0') } + + before do + allow(event).to receive(:push_event_payload).and_return(payload) + end + + it 'links to the tag' do + allow(event.project.repository).to receive(:tag_exists?).with(event.ref_name).and_return(true) + link = project_commits_path(event.project, event.ref_name) + + render partial: 'events/event/push', locals: { event: event } + + expect(rendered).to have_link(event.ref_name, href: link) + end + + context 'that has been deleted' do + it 'does not link to the tag' do + render partial: 'events/event/push', locals: { event: event } + + expect(rendered).not_to have_link(event.ref_name) + end + end + end +end diff --git a/spec/workers/run_pipeline_schedule_worker_spec.rb b/spec/workers/run_pipeline_schedule_worker_spec.rb new file mode 100644 index 00000000000..481a84837f9 --- /dev/null +++ b/spec/workers/run_pipeline_schedule_worker_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe RunPipelineScheduleWorker do + describe '#perform' do + set(:project) { create(:project) } + set(:user) { create(:user) } + set(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly, project: project ) } + let(:worker) { described_class.new } + + context 'when a project not found' do + it 'does not call the Service' do + expect(Ci::CreatePipelineService).not_to receive(:new) + expect(worker).not_to receive(:run_pipeline_schedule) + + worker.perform(100000, user.id) + end + end + + context 'when a user not found' do + it 'does not call the Service' do + expect(Ci::CreatePipelineService).not_to receive(:new) + expect(worker).not_to receive(:run_pipeline_schedule) + + worker.perform(pipeline_schedule.id, 10000) + end + end + + context 'when everything is ok' do + let(:create_pipeline_service) { instance_double(Ci::CreatePipelineService) } + + it 'calls the Service' do + expect(Ci::CreatePipelineService).to receive(:new).with(project, user, ref: pipeline_schedule.ref).and_return(create_pipeline_service) + expect(create_pipeline_service).to receive(:execute).with(:schedule, ignore_skip_ci: true, save_on_errors: false, schedule: pipeline_schedule) + + worker.perform(pipeline_schedule.id, user.id) + end + end + end +end |