diff options
72 files changed, 829 insertions, 421 deletions
diff --git a/GITLAB_PAGES_VERSION b/GITLAB_PAGES_VERSION index faef31a4357..39e898a4f95 100644 --- a/GITLAB_PAGES_VERSION +++ b/GITLAB_PAGES_VERSION @@ -1 +1 @@ -0.7.0 +0.7.1 diff --git a/app/assets/javascripts/gfm_auto_complete.js b/app/assets/javascripts/gfm_auto_complete.js index 43a5325cf71..8259133c95b 100644 --- a/app/assets/javascripts/gfm_auto_complete.js +++ b/app/assets/javascripts/gfm_auto_complete.js @@ -132,9 +132,8 @@ class GfmAutoComplete { callbacks: { ...this.getDefaultCallbacks(), matcher(flag, subtext) { - const relevantText = subtext.trim().split(/\s/).pop(); const regexp = new RegExp(`(?:[^${glRegexp.unicodeLetters}0-9:]|\n|^):([^:]*)$`, 'gi'); - const match = regexp.exec(relevantText); + const match = regexp.exec(subtext); return match && match.length ? match[1] : null; }, diff --git a/app/assets/javascripts/monitoring/components/graph.vue b/app/assets/javascripts/monitoring/components/graph.vue index 9e67a6f2146..42615d2bb8e 100644 --- a/app/assets/javascripts/monitoring/components/graph.vue +++ b/app/assets/javascripts/monitoring/components/graph.vue @@ -209,6 +209,7 @@ const xAxis = d3.axisBottom() .scale(axisXScale) + .ticks(this.graphWidth / 120) .tickFormat(timeScaleFormat); const yAxis = d3.axisLeft() diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index c640003d958..6d1b2f452c0 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -16,6 +16,10 @@ import Autosize from 'autosize'; import 'vendor/jquery.caret'; // required by jquery.atwho import 'vendor/jquery.atwho'; import AjaxCache from '~/lib/utils/ajax_cache'; +import Vue from 'vue'; +import syntaxHighlight from '~/syntax_highlight'; +import SkeletonLoadingContainer from '~/vue_shared/components/skeleton_loading_container.vue'; +import { __ } from '~/locale'; import axios from './lib/utils/axios_utils'; import { getLocationHash } from './lib/utils/url_utility'; import Flash from './flash'; @@ -99,6 +103,13 @@ export default class Notes { $('.note-edit-form').clone() .addClass('mr-note-edit-form').insertAfter('.note-edit-form'); } + + const hash = getLocationHash(); + const $anchor = hash && document.getElementById(hash); + + if ($anchor) { + this.loadLazyDiff({ currentTarget: $anchor }); + } } setViewType(view) { @@ -135,6 +146,8 @@ export default class Notes { this.$wrapperEl.on('click', '.js-close-discussion-note-form', this.cancelDiscussionForm); // toggle commit list this.$wrapperEl.on('click', '.system-note-commit-list-toggler', this.toggleCommitList); + + this.$wrapperEl.on('click', '.js-toggle-lazy-diff', this.loadLazyDiff); // fetch notes when tab becomes visible this.$wrapperEl.on('visibilitychange', this.visibilityChange); // when issue status changes, we need to refresh data @@ -173,6 +186,7 @@ export default class Notes { this.$wrapperEl.off('keydown', '.js-note-text'); this.$wrapperEl.off('click', '.js-comment-resolve-button'); this.$wrapperEl.off('click', '.system-note-commit-list-toggler'); + this.$wrapperEl.off('click', '.js-toggle-lazy-diff'); this.$wrapperEl.off('ajax:success', '.js-main-target-form'); this.$wrapperEl.off('ajax:success', '.js-discussion-note-form'); this.$wrapperEl.off('ajax:complete', '.js-main-target-form'); @@ -1207,6 +1221,60 @@ export default class Notes { return this.notesCountBadge.text(parseInt(this.notesCountBadge.text(), 10) + updateCount); } + static renderPlaceholderComponent($container) { + const el = $container.find('.js-code-placeholder').get(0); + new Vue({ // eslint-disable-line no-new + el, + components: { + SkeletonLoadingContainer, + }, + render(createElement) { + return createElement('skeleton-loading-container'); + }, + }); + } + + static renderDiffContent($container, data) { + const { discussion_html } = data; + const lines = $(discussion_html).find('.line_holder'); + lines.addClass('fade-in'); + $container.find('tbody').prepend(lines); + const fileHolder = $container.find('.file-holder'); + $container.find('.line-holder-placeholder').remove(); + syntaxHighlight(fileHolder); + } + + static renderDiffError($container) { + $container.find('.line_content').html( + $(` + <div class="nothing-here-block"> + ${__('Unable to load the diff.')} <a class="js-toggle-lazy-diff" href="javascript:void(0)">Try again</a>? + </div> + `), + ); + } + + loadLazyDiff(e) { + const $container = $(e.currentTarget).closest('.js-toggle-container'); + Notes.renderPlaceholderComponent($container); + + $container.find('.js-toggle-lazy-diff').removeClass('js-toggle-lazy-diff'); + + const tableEl = $container.find('tbody'); + if (tableEl.length === 0) return; + + const fileHolder = $container.find('.file-holder'); + const url = fileHolder.data('linesPath'); + + axios.get(url) + .then(({ data }) => { + Notes.renderDiffContent($container, data); + }) + .catch(() => { + Notes.renderDiffError($container); + }); + } + toggleCommitList(e) { const $element = $(e.currentTarget); const $closestSystemCommitList = $element.siblings('.system-note-commit-list'); diff --git a/app/assets/javascripts/notes/components/comment_form.vue b/app/assets/javascripts/notes/components/comment_form.vue index 1785be01a0d..42bc383f4d2 100644 --- a/app/assets/javascripts/notes/components/comment_form.vue +++ b/app/assets/javascripts/notes/components/comment_form.vue @@ -1,6 +1,6 @@ <script> import $ from 'jquery'; - import { mapActions, mapGetters } from 'vuex'; + import { mapActions, mapGetters, mapState } from 'vuex'; import _ from 'underscore'; import Autosize from 'autosize'; import { __, sprintf } from '~/locale'; @@ -53,6 +53,9 @@ 'getNotesData', 'openState', ]), + ...mapState([ + 'isToggleStateButtonLoading', + ]), noteableDisplayName() { return this.noteableType.replace(/_/g, ' '); }, @@ -143,6 +146,7 @@ 'closeIssue', 'reopenIssue', 'toggleIssueLocalState', + 'toggleStateButtonLoading', ]), setIsSubmitButtonDisabled(note, isSubmitting) { if (!_.isEmpty(note) && !isSubmitting) { @@ -170,13 +174,14 @@ if (this.noteType === constants.DISCUSSION) { noteData.data.note.type = constants.DISCUSSION_NOTE; } + this.note = ''; // Empty textarea while being requested. Repopulate in catch this.resizeTextarea(); this.stopPolling(); this.saveNote(noteData) .then((res) => { - this.isSubmitting = false; + this.enableButton(); this.restartPolling(); if (res.errors) { @@ -198,7 +203,7 @@ } }) .catch(() => { - this.isSubmitting = false; + this.enableButton(); this.discard(false); const msg = `Your comment could not be submitted! @@ -220,6 +225,7 @@ Please check your network connection and try again.`; .then(() => this.enableButton()) .catch(() => { this.enableButton(); + this.toggleStateButtonLoading(false); Flash( sprintf( __('Something went wrong while closing the %{issuable}. Please try again later'), @@ -232,6 +238,7 @@ Please check your network connection and try again.`; .then(() => this.enableButton()) .catch(() => { this.enableButton(); + this.toggleStateButtonLoading(false); Flash( sprintf( __('Something went wrong while reopening the %{issuable}. Please try again later'), @@ -419,13 +426,13 @@ append-right-10 comment-type-dropdown js-comment-type-dropdown droplab-dropdown" <loading-button v-if="canUpdateIssue" - :loading="isSubmitting" + :loading="isToggleStateButtonLoading" @click="handleSave(true)" :container-class="[ actionButtonClassNames, 'btn btn-comment btn-comment-and-close js-action-button' ]" - :disabled="isSubmitting" + :disabled="isToggleStateButtonLoading || isSubmitting" :label="issueActionButtonTitle" /> diff --git a/app/assets/javascripts/notes/stores/actions.js b/app/assets/javascripts/notes/stores/actions.js index dc0e3c39775..ebbacb576d6 100644 --- a/app/assets/javascripts/notes/stores/actions.js +++ b/app/assets/javascripts/notes/stores/actions.js @@ -71,21 +71,32 @@ export const toggleResolveNote = ({ commit }, { endpoint, isResolved, discussion commit(mutationType, res); }); -export const closeIssue = ({ commit, dispatch, state }) => service +export const closeIssue = ({ commit, dispatch, state }) => { + dispatch('toggleStateButtonLoading', true); + return service .toggleIssueState(state.notesData.closePath) .then(res => res.json()) .then((data) => { commit(types.CLOSE_ISSUE); dispatch('emitStateChangedEvent', data); + dispatch('toggleStateButtonLoading', false); }); +}; -export const reopenIssue = ({ commit, dispatch, state }) => service +export const reopenIssue = ({ commit, dispatch, state }) => { + dispatch('toggleStateButtonLoading', true); + return service .toggleIssueState(state.notesData.reopenPath) .then(res => res.json()) .then((data) => { commit(types.REOPEN_ISSUE); dispatch('emitStateChangedEvent', data); + dispatch('toggleStateButtonLoading', false); }); +}; + +export const toggleStateButtonLoading = ({ commit }, value) => + commit(types.TOGGLE_STATE_BUTTON_LOADING, value); export const emitStateChangedEvent = ({ commit, getters }, data) => { const event = new CustomEvent('issuable_vue_app:change', { detail: { diff --git a/app/assets/javascripts/notes/stores/index.js b/app/assets/javascripts/notes/stores/index.js index 488a9ca38d3..9ed19bf171e 100644 --- a/app/assets/javascripts/notes/stores/index.js +++ b/app/assets/javascripts/notes/stores/index.js @@ -12,6 +12,9 @@ export default new Vuex.Store({ targetNoteHash: null, lastFetchedAt: null, + // View layer + isToggleStateButtonLoading: false, + // holds endpoints and permissions provided through haml notesData: {}, userData: {}, diff --git a/app/assets/javascripts/notes/stores/mutation_types.js b/app/assets/javascripts/notes/stores/mutation_types.js index da1b5a9e51a..b455e23ecde 100644 --- a/app/assets/javascripts/notes/stores/mutation_types.js +++ b/app/assets/javascripts/notes/stores/mutation_types.js @@ -17,3 +17,4 @@ export const UPDATE_DISCUSSION = 'UPDATE_DISCUSSION'; // Issue export const CLOSE_ISSUE = 'CLOSE_ISSUE'; export const REOPEN_ISSUE = 'REOPEN_ISSUE'; +export const TOGGLE_STATE_BUTTON_LOADING = 'TOGGLE_STATE_BUTTON_LOADING'; diff --git a/app/assets/javascripts/notes/stores/mutations.js b/app/assets/javascripts/notes/stores/mutations.js index 949628a65c0..9308daa36f1 100644 --- a/app/assets/javascripts/notes/stores/mutations.js +++ b/app/assets/javascripts/notes/stores/mutations.js @@ -199,4 +199,8 @@ export default { [types.REOPEN_ISSUE](state) { Object.assign(state.noteableData, { state: constants.REOPENED }); }, + + [types.TOGGLE_STATE_BUTTON_LOADING](state, value) { + Object.assign(state, { isToggleStateButtonLoading: value }); + }, }; diff --git a/app/assets/javascripts/performance_bar.js b/app/assets/javascripts/performance_bar.js index ef44e2323ef..c22598ee665 100644 --- a/app/assets/javascripts/performance_bar.js +++ b/app/assets/javascripts/performance_bar.js @@ -14,8 +14,6 @@ export default class PerformanceBar { init(opts) { const $container = $(opts.container); - this.$sqlProfileLink = $container.find('.js-toggle-modal-peek-sql'); - this.$sqlProfileModal = $container.find('#modal-peek-pg-queries'); this.$lineProfileLink = $container.find('.js-toggle-modal-peek-line-profile'); this.$lineProfileModal = $('#modal-peek-line-profile'); this.initEventListeners(); @@ -23,7 +21,6 @@ export default class PerformanceBar { } initEventListeners() { - this.$sqlProfileLink.on('click', () => this.handleSQLProfileLink()); this.$lineProfileLink.on('click', e => this.handleLineProfileLink(e)); $(document).on('click', '.js-lineprof-file', PerformanceBar.toggleLineProfileFile); } @@ -36,10 +33,6 @@ export default class PerformanceBar { } } - handleSQLProfileLink() { - PerformanceBar.toggleModal(this.$sqlProfileModal); - } - handleLineProfileLink(e) { const lineProfilerParameter = getParameterValues('lineprofiler'); const lineProfilerParameterRegex = new RegExp(`lineprofiler=${lineProfilerParameter[0]}`); diff --git a/app/assets/stylesheets/framework/mixins.scss b/app/assets/stylesheets/framework/mixins.scss index ddd9dbb2be4..e12b5aab381 100644 --- a/app/assets/stylesheets/framework/mixins.scss +++ b/app/assets/stylesheets/framework/mixins.scss @@ -17,8 +17,6 @@ */ @mixin markdown-table { width: auto; - display: block; - overflow-x: auto; } /* diff --git a/app/assets/stylesheets/pages/commits.scss b/app/assets/stylesheets/pages/commits.scss index 8b680c2dc52..b487f6278c2 100644 --- a/app/assets/stylesheets/pages/commits.scss +++ b/app/assets/stylesheets/pages/commits.scss @@ -194,8 +194,6 @@ .commit-actions { @media (min-width: $screen-sm-min) { - font-size: 0; - .fa-spinner { font-size: 12px; } @@ -204,7 +202,7 @@ .ci-status-link { display: inline-block; position: relative; - top: 1px; + top: 2px; } .btn-clipboard, @@ -226,7 +224,7 @@ .ci-status-icon { position: relative; - top: 1px; + top: 2px; } } diff --git a/app/assets/stylesheets/pages/wiki.scss b/app/assets/stylesheets/pages/wiki.scss index e70a57c2a67..9a0ec936979 100644 --- a/app/assets/stylesheets/pages/wiki.scss +++ b/app/assets/stylesheets/pages/wiki.scss @@ -180,6 +180,11 @@ ul.wiki-pages-list.content-list { } } +.wiki-holder { + overflow-x: auto; + overflow-y: hidden; +} + .wiki { table { @include markdown-table; diff --git a/app/controllers/projects/discussions_controller.rb b/app/controllers/projects/discussions_controller.rb index ee507009e50..cba9a53dc4b 100644 --- a/app/controllers/projects/discussions_controller.rb +++ b/app/controllers/projects/discussions_controller.rb @@ -19,6 +19,12 @@ class Projects::DiscussionsController < Projects::ApplicationController render_discussion end + def show + render json: { + discussion_html: view_to_html_string('discussions/_diff_with_notes', discussion: discussion, expanded: true) + } + end + private def render_discussion diff --git a/app/helpers/javascript_helper.rb b/app/helpers/javascript_helper.rb index d5e77c7e271..cd4075b340d 100644 --- a/app/helpers/javascript_helper.rb +++ b/app/helpers/javascript_helper.rb @@ -2,9 +2,4 @@ module JavascriptHelper def page_specific_javascript_tag(js) javascript_include_tag asset_path(js) end - - # deprecated; use webpack_bundle_tag directly instead - def page_specific_javascript_bundle_tag(bundle) - webpack_bundle_tag(bundle) - end end diff --git a/app/models/project.rb b/app/models/project.rb index 5f9d9785d64..0183e3d0a38 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1083,7 +1083,7 @@ class Project < ActiveRecord::Base # Forked import is handled asynchronously return if forked? && !force - if gitlab_shell.add_repository(repository_storage, disk_path) + if gitlab_shell.create_repository(repository_storage, disk_path) repository.after_create true else diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb index f6041da986c..52e067cb44c 100644 --- a/app/models/project_wiki.rb +++ b/app/models/project_wiki.rb @@ -169,7 +169,7 @@ class ProjectWiki private def create_repo!(raw_repository) - gitlab_shell.add_repository(project.repository_storage, disk_path) + gitlab_shell.create_repository(project.repository_storage, disk_path) raise CouldNotCreateWikiError unless raw_repository.exists? diff --git a/app/views/discussions/_diff_with_notes.html.haml b/app/views/discussions/_diff_with_notes.html.haml index f9bfc01f213..8680ec2e298 100644 --- a/app/views/discussions/_diff_with_notes.html.haml +++ b/app/views/discussions/_diff_with_notes.html.haml @@ -2,8 +2,12 @@ - blob = discussion.blob - discussions = { discussion.original_line_code => [discussion] } - diff_file_class = diff_file.text? ? 'text-file' : 'js-image-file' +- diff_data = {} +- expanded = discussion.expanded? || local_assigns.fetch(:expanded, nil) +- unless expanded + - diff_data = { lines_path: project_merge_request_discussion_path(discussion.project, discussion.noteable, discussion) } -.diff-file.file-holder{ class: diff_file_class } +.diff-file.file-holder{ class: diff_file_class, data: diff_data } .js-file-title.file-title.file-title-flex-parent .file-header-content = render "projects/diffs/file_header", diff_file: diff_file, url: discussion_path(discussion), show_toggle: false @@ -11,17 +15,24 @@ - if diff_file.text? .diff-content.code.js-syntax-highlight %table - = render partial: "projects/diffs/line", - collection: discussion.truncated_diff_lines, - as: :line, - locals: { diff_file: diff_file, - discussions: discussions, - discussion_expanded: true, - plain: true } + - if expanded + - discussions = { discussion.original_line_code => [discussion] } + = render partial: "projects/diffs/line", + collection: discussion.truncated_diff_lines, + as: :line, + locals: { diff_file: diff_file, + discussions: discussions, + discussion_expanded: true, + plain: true } + - else + %tr.line_holder.line-holder-placeholder + %td.old_line.diff-line-num + %td.new_line.diff-line-num + %td.line_content + .js-code-placeholder + = render "discussions/diff_discussion", discussions: [discussion], expanded: true - else - partial = (diff_file.new_file? || diff_file.deleted_file?) ? 'single_image_diff' : 'replaced_image_diff' - = render partial: "projects/diffs/#{partial}", locals: { diff_file: diff_file, position: discussion.position.to_json, click_to_comment: false } - .note-container = render partial: "discussions/notes", locals: { discussion: discussion, show_toggle: false, show_image_comment_badge: true, disable_collapse_class: true } diff --git a/app/views/discussions/_discussion.html.haml b/app/views/discussions/_discussion.html.haml index 8b9fa3d6b05..e9589213f80 100644 --- a/app/views/discussions/_discussion.html.haml +++ b/app/views/discussions/_discussion.html.haml @@ -8,7 +8,7 @@ .discussion.js-toggle-container{ data: { discussion_id: discussion.id } } .discussion-header .discussion-actions - %button.note-action-button.discussion-toggle-button.js-toggle-button{ type: "button" } + %button.note-action-button.discussion-toggle-button.js-toggle-button{ type: "button", class: ("js-toggle-lazy-diff" unless expanded) } - if expanded = icon("chevron-up") - else diff --git a/app/views/peek/views/_gc.html.haml b/app/views/peek/views/_gc.html.haml new file mode 100644 index 00000000000..9fc83e56ee7 --- /dev/null +++ b/app/views/peek/views/_gc.html.haml @@ -0,0 +1,7 @@ +- local_assigns.fetch(:view) + +%span.bold + %span{ title: 'Invoke Time', data: { defer_to: "#{view.defer_key}-gc_time" } }... + \/ + %span{ title: 'Invoke Count', data: { defer_to: "#{view.defer_key}-invokes" } }... +gc diff --git a/app/views/peek/views/_gitaly.html.haml b/app/views/peek/views/_gitaly.html.haml index a7d040d6821..945bb287429 100644 --- a/app/views/peek/views/_gitaly.html.haml +++ b/app/views/peek/views/_gitaly.html.haml @@ -1,7 +1,17 @@ - local_assigns.fetch(:view) -%strong - %span{ data: { defer_to: "#{view.defer_key}-duration" } } ... +%button.btn-blank.btn-link.bold{ type: 'button', data: { toggle: 'modal', target: '#modal-peek-gitaly-details' } } + %span{ data: { defer_to: "#{view.defer_key}-duration" } }... \/ - %span{ data: { defer_to: "#{view.defer_key}-calls" } } ... - Gitaly + %span{ data: { defer_to: "#{view.defer_key}-calls" } }... +#modal-peek-gitaly-details.modal{ tabindex: -1, role: 'dialog' } + .modal-dialog.modal-full + .modal-content + .modal-header + %button.close{ type: 'button', data: { dismiss: 'modal' }, 'aria-label' => 'Close' } + %span{ 'aria-hidden' => 'true' } + × + %h4 + Gitaly requests + .modal-body{ data: { defer_to: "#{view.defer_key}-details" } }... +gitaly diff --git a/app/views/peek/views/_redis.html.haml b/app/views/peek/views/_redis.html.haml new file mode 100644 index 00000000000..f7fba6c95fc --- /dev/null +++ b/app/views/peek/views/_redis.html.haml @@ -0,0 +1,7 @@ +- local_assigns.fetch(:view) + +%span.bold + %span{ data: { defer_to: "#{view.defer_key}-duration" } }... + \/ + %span{ data: { defer_to: "#{view.defer_key}-calls" } }... +redis diff --git a/app/views/peek/views/_sidekiq.html.haml b/app/views/peek/views/_sidekiq.html.haml new file mode 100644 index 00000000000..7efbc05890d --- /dev/null +++ b/app/views/peek/views/_sidekiq.html.haml @@ -0,0 +1,7 @@ +- local_assigns.fetch(:view) + +%span.bold + %span{ data: { defer_to: "#{view.defer_key}-duration" } }... + \/ + %span{ data: { defer_to: "#{view.defer_key}-calls" } }... +sidekiq diff --git a/app/views/peek/views/_sql.html.haml b/app/views/peek/views/_sql.html.haml index dd8b524064f..36583df898a 100644 --- a/app/views/peek/views/_sql.html.haml +++ b/app/views/peek/views/_sql.html.haml @@ -1,13 +1,14 @@ -%strong - %a.js-toggle-modal-peek-sql - %span{ data: { defer_to: "#{view.defer_key}-duration" } }... - \/ - %span{ data: { defer_to: "#{view.defer_key}-calls" } }... +%button.btn-blank.btn-link.bold{ type: 'button', data: { toggle: 'modal', target: '#modal-peek-pg-queries' } } + %span{ data: { defer_to: "#{view.defer_key}-duration" } }... + \/ + %span{ data: { defer_to: "#{view.defer_key}-calls" } }... #modal-peek-pg-queries.modal{ tabindex: -1 } .modal-dialog.modal-full .modal-content .modal-header - %button.close.btn.btn-link.btn-sm{ type: 'button', data: { dismiss: 'modal' } } X + %button.close{ type: 'button', data: { dismiss: 'modal' }, 'aria-label' => 'Close' } + %span{ 'aria-hidden' => 'true' } + × %h4 SQL queries .modal-body{ data: { defer_to: "#{view.defer_key}-queries" } }... diff --git a/changelogs/unreleased/35475-lazy-diff.yml b/changelogs/unreleased/35475-lazy-diff.yml new file mode 100644 index 00000000000..bafa66ebe39 --- /dev/null +++ b/changelogs/unreleased/35475-lazy-diff.yml @@ -0,0 +1,5 @@ +--- +title: lazy load diffs on merge request discussions +merge_request: +author: +type: performance diff --git a/changelogs/unreleased/43720-update-fe-webpack-docs.yml b/changelogs/unreleased/43720-update-fe-webpack-docs.yml new file mode 100644 index 00000000000..9e461eaaec8 --- /dev/null +++ b/changelogs/unreleased/43720-update-fe-webpack-docs.yml @@ -0,0 +1,6 @@ +--- +title: Update documentation to reflect current minimum required versions of node and + yarn +merge_request: 17706 +author: +type: other diff --git a/changelogs/unreleased/43805-list-gitaly-calls-and-arguments-in-the-performance-bar.yml b/changelogs/unreleased/43805-list-gitaly-calls-and-arguments-in-the-performance-bar.yml new file mode 100644 index 00000000000..4c63e69f0bb --- /dev/null +++ b/changelogs/unreleased/43805-list-gitaly-calls-and-arguments-in-the-performance-bar.yml @@ -0,0 +1,5 @@ +--- +title: Add Gitaly call details to performance bar +merge_request: +author: +type: added diff --git a/changelogs/unreleased/44024-fix-table-extra-column.yml b/changelogs/unreleased/44024-fix-table-extra-column.yml new file mode 100644 index 00000000000..92c354a0844 --- /dev/null +++ b/changelogs/unreleased/44024-fix-table-extra-column.yml @@ -0,0 +1,5 @@ +--- +title: Fix markdown table showing extra column +merge_request: 17669 +author: +type: fixed diff --git a/changelogs/unreleased/44149-issue-comment-buttons.yml b/changelogs/unreleased/44149-issue-comment-buttons.yml new file mode 100644 index 00000000000..c874c0d3d66 --- /dev/null +++ b/changelogs/unreleased/44149-issue-comment-buttons.yml @@ -0,0 +1,5 @@ +--- +title: Fix broken loading state for close issue button +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/fix-emoji-popup.yml b/changelogs/unreleased/fix-emoji-popup.yml new file mode 100644 index 00000000000..c81d061a5d7 --- /dev/null +++ b/changelogs/unreleased/fix-emoji-popup.yml @@ -0,0 +1,5 @@ +--- +title: Hide emoji popup after multiple spaces +merge_request: +author: Jan Beckmann +type: fixed diff --git a/config/application.rb b/config/application.rb index 422b16a7719..0ff95e33a9c 100644 --- a/config/application.rb +++ b/config/application.rb @@ -70,7 +70,6 @@ module Gitlab # - Webhook URLs (:hook) # - Sentry DSN (:sentry_dsn) # - Deploy keys (:key) - # - Secret variable values (:value) config.filter_parameters += [/token$/, /password/, /secret/] config.filter_parameters += %i( certificate @@ -82,7 +81,6 @@ module Gitlab sentry_dsn trace variables - value ) # Enable escaping HTML in JSON. diff --git a/config/initializers/peek.rb b/config/initializers/peek.rb index 11759801112..ba04a2bf5fa 100644 --- a/config/initializers/peek.rb +++ b/config/initializers/peek.rb @@ -16,11 +16,11 @@ else end Peek.into PEEK_DB_VIEW +Peek.into Peek::Views::Gitaly +Peek.into Peek::Views::Rblineprof Peek.into Peek::Views::Redis Peek.into Peek::Views::Sidekiq -Peek.into Peek::Views::Rblineprof Peek.into Peek::Views::GC -Peek.into Peek::Views::Gitaly # rubocop:disable Naming/ClassAndModuleCamelCase class PEEK_DB_CLIENT diff --git a/config/routes/project.rb b/config/routes/project.rb index b82ed27664c..c803737d40b 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -130,7 +130,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do post :bulk_update end - resources :discussions, only: [], constraints: { id: /\h{40}/ } do + resources :discussions, only: [:show], constraints: { id: /\h{40}/ } do member do post :resolve delete :resolve, action: :unresolve diff --git a/doc/administration/monitoring/performance/img/performance_bar.png b/doc/administration/monitoring/performance/img/performance_bar.png Binary files differindex b3c6bc474e3..48212f6276a 100644 --- a/doc/administration/monitoring/performance/img/performance_bar.png +++ b/doc/administration/monitoring/performance/img/performance_bar.png diff --git a/doc/administration/monitoring/performance/img/performance_bar_gitaly_calls.png b/doc/administration/monitoring/performance/img/performance_bar_gitaly_calls.png Binary files differnew file mode 100644 index 00000000000..52176df9ecd --- /dev/null +++ b/doc/administration/monitoring/performance/img/performance_bar_gitaly_calls.png diff --git a/doc/administration/monitoring/performance/performance_bar.md b/doc/administration/monitoring/performance/performance_bar.md index b9464945cea..ec1cbce1bad 100644 --- a/doc/administration/monitoring/performance/performance_bar.md +++ b/doc/administration/monitoring/performance/performance_bar.md @@ -11,10 +11,12 @@ It allows you to see (from left to right): - the timing of the page (backend, frontend) - time taken and number of DB queries, click through for details of these queries  -- time taken and number of calls to Redis -- time taken and number of background jobs created by Sidekiq +- time taken and number of [Gitaly] calls, click through for details of these calls + - profile of the code used to generate the page, line by line for either _all_, _app & lib_ , or _views_. In the profile view, the numbers in the left panel represent wall time, cpu time, and number of calls (based on [rblineprof](https://github.com/tmm1/rblineprof)).  +- time taken and number of calls to Redis +- time taken and number of background jobs created by Sidekiq - time taken and number of Ruby GC calls ## Enable the Performance Bar via the Admin panel @@ -39,3 +41,5 @@ You can toggle the Bar using the same shortcut.  --- + +[Gitaly]: ../../gitaly/index.md diff --git a/doc/development/ee_features.md b/doc/development/ee_features.md index 1eb90c30ebd..fea92e740cb 100644 --- a/doc/development/ee_features.md +++ b/doc/development/ee_features.md @@ -360,27 +360,15 @@ Instead place EE specs in the `ee/spec` folder. ## JavaScript code in `assets/javascripts/` -To separate EE-specific JS-files we can also move the files into an `ee` folder. +To separate EE-specific JS-files we should also move the files into an `ee` folder. For example there can be an `app/assets/javascripts/protected_branches/protected_branches_bundle.js` and an EE counterpart `ee/app/assets/javascripts/protected_branches/protected_branches_bundle.js`. -That way we can create a separate webpack bundle in `webpack.config.js`: - -```javascript - protected_branches: '~/protected_branches', - ee_protected_branches: 'ee/protected_branches/protected_branches_bundle.js', -``` - -With the separate bundle in place, we can decide which bundle to load inside the -view, using the `page_specific_javascript_bundle_tag` helper. - -```haml -- content_for :page_specific_javascripts do - = page_specific_javascript_bundle_tag('protected_branches') -``` +See the frontend guide [performance section](./fe_guide/performance.md) for +information on managing page-specific javascript within EE. ## SCSS code in `assets/stylesheets` diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md index 12dfc10812b..2280cf79f86 100644 --- a/doc/development/fe_guide/index.md +++ b/doc/development/fe_guide/index.md @@ -14,8 +14,8 @@ support through [webpack][webpack]. We also utilize [webpack][webpack] to handle the bundling, minification, and compression of our assets. -Working with our frontend assets requires Node (v4.3 or greater) and Yarn -(v0.17 or greater). You can find information on how to install these on our +Working with our frontend assets requires Node (v6.0 or greater) and Yarn +(v1.2 or greater). You can find information on how to install these on our [installation guide][install]. [jQuery][jquery] is used throughout the application's JavaScript, with diff --git a/doc/development/fe_guide/performance.md b/doc/development/fe_guide/performance.md index 98e43931a02..1320efaf767 100644 --- a/doc/development/fe_guide/performance.md +++ b/doc/development/fe_guide/performance.md @@ -23,7 +23,7 @@ controlled by the server. 1. The backend code will most likely be using etags. You do not and should not check for status `304 Not Modified`. The browser will transform it for you. -### Lazy Loading +### Lazy Loading Images To improve the time to first render we are using lazy loading for images. This works by setting the actual image source on the `data-src` attribute. After the HTML is rendered and JavaScript is loaded, @@ -47,41 +47,103 @@ properties once, and handle the actual animation with transforms. ## Reducing Asset Footprint -### Page-specific JavaScript +### Universal code -Certain pages may require the use of a third party library, such as [d3][d3] for -the User Activity Calendar and [Chart.js][chartjs] for the Graphs pages. These -libraries increase the page size significantly, and impact load times due to -bandwidth bottlenecks and the browser needing to parse more JavaScript. - -In cases where libraries are only used on a few specific pages, we use -"page-specific JavaScript" to prevent the main `main.js` file from -becoming unnecessarily large. - -Steps to split page-specific JavaScript from the main `main.js`: - -1. Create a directory for the specific page(s), e.g. `graphs/`. -1. In that directory, create a `namespace_bundle.js` file, e.g. `graphs_bundle.js`. -1. Add the new "bundle" file to the list of entry files in `config/webpack.config.js`. - - For example: `graphs: './graphs/graphs_bundle.js',`. -1. Move code reliant on these libraries into the `graphs` directory. -1. In `graphs_bundle.js` add CommonJS `require('./path_to_some_component.js');` statements to load any other files in this directory. Make sure to use relative urls. -1. In the relevant views, add the scripts to the page with the following: - -```haml -- content_for :page_specific_javascripts do - = webpack_bundle_tag 'lib_chart' - = webpack_bundle_tag 'graphs' -``` +Code that is contained within `main.js` and `commons/index.js` are loaded and +run on _all_ pages. **DO NOT ADD** anything to these files unless it is truly +needed _everywhere_. These bundles include ubiquitous libraries like `vue`, +`axios`, and `jQuery`, as well as code for the main navigation and sidebar. +Where possible we should aim to remove modules from these bundles to reduce our +code footprint. + +### Page-specific JavaScript -The above loads `chart.js` and `graphs_bundle.js` for this page only. `chart.js` -is separated from the bundle file so it can be cached separately from the bundle -and reused for other pages that also rely on the library. For an example, see -[this Haml file][page-specific-js-example]. +Webpack has been configured to automatically generate entry point bundles based +on the file structure within `app/assets/javascripts/pages/*`. The directories +within the `pages` directory correspond to Rails controllers and actions. These +auto-generated bundles will be automatically included on the corresponding +pages. + +For example, if you were to visit [gitlab.com/gitlab-org/gitlab-ce/issues](https://gitlab.com/gitlab-org/gitlab-ce/issues), +you would be accessing the `app/controllers/projects/issues_controller.rb` +controller with the `index` action. If a corresponding file exists at +`pages/projects/issues/index/index.js`, it will be compiled into a webpack +bundle and included on the page. + +> **Note:** Previously we had encouraged the use of +> `content_for :page_specific_javascripts` within haml files, along with +> manually generated webpack bundles. However under this new system you should +> not ever need to manually add an entry point to the `webpack.config.js` file. + +> **Tip:** +> If you are unsure what controller and action corresponds to a given page, you +> can find this out by inspecting `document.body.dataset.page` within your +> browser's developer console while on any page within gitlab. + +#### Important Considerations: + +- **Keep Entry Points Lite:** + Page-specific javascript entry points should be as lite as possible. These + files are exempt from unit tests, and should be used primarily for + instantiation and dependency injection of classes and methods that live in + modules outside of the entry point script. Just import, read the DOM, + instantiate, and nothing else. + +- **Entry Points May Be Asynchronous:** + _DO NOT ASSUME_ that the DOM has been fully loaded and available when an + entry point script is run. If you require that some code be run after the + DOM has loaded, you should attach an event handler to the `DOMContentLoaded` + event with: + + ```javascript + import initMyWidget from './my_widget'; + + document.addEventListener('DOMContentLoaded', () => { + initMyWidget(); + }); + ``` + +- **Supporting Module Placement:** + - If a class or a module is _specific to a particular route_, try to locate + it close to the entry point it will be used. For instance, if + `my_widget.js` is only imported within `pages/widget/show/index.js`, you + should place the module at `pages/widget/show/my_widget.js` and import it + with a relative path (e.g. `import initMyWidget from './my_widget';`). + + - If a class or module is _used by multiple routes_, place it within a + shared directory at the closest common parent directory for the entry + points that import it. For example, if `my_widget.js` is imported within + both `pages/widget/show/index.js` and `pages/widget/run/index.js`, then + place the module at `pages/widget/shared/my_widget.js` and import it with + a relative path if possible (e.g. `../shared/my_widget`). + +- **Enterprise Edition Caveats:** + For GitLab Enterprise Edition, page-specific entry points will override their + Community Edition counterparts with the same name, so if + `ee/app/assets/javascripts/pages/foo/bar/index.js` exists, it will take + precedence over `app/assets/javascripts/pages/foo/bar/index.js`. If you want + to minimize duplicate code, you can import one entry point from the other. + This is not done automatically to allow for flexibility in overriding + functionality. ### Code Splitting -> *TODO* flesh out this section once webpack is ready for code-splitting +For any code that does not need to be run immediately upon page load, (e.g. +modals, dropdowns, and other behaviors that can be lazy-loaded), you can split +your module into asynchronous chunks with dynamic import statements. These +imports return a Promise which will be resolved once the script has loaded: + +```javascript +import(/* webpackChunkName: 'emoji' */ '~/emoji') + .then(/* do something */) + .catch(/* report error */) +``` + +Please try to use `webpackChunkName` when generating these dynamic imports as +it will provide a deterministic filename for the chunk which can then be cached +the browser across GitLab versions. + +More information is available in [webpack's code splitting documentation](https://webpack.js.org/guides/code-splitting/#dynamic-imports). ### Minimizing page size @@ -95,7 +157,8 @@ General tips: - Prefer font formats with better compression, e.g. WOFF2 is better than WOFF, which is better than TTF. - Compress and minify assets wherever possible (For CSS/JS, Sprockets and webpack do this for us). - If some functionality can reasonably be achieved without adding extra libraries, avoid them. -- Use page-specific JavaScript as described above to dynamically load libraries that are only needed on certain pages. +- Use page-specific JavaScript as described above to load libraries that are only needed on certain pages. +- Use code-splitting dynamic imports wherever possible to lazy-load code that is not needed initially. - [High Performance Animations][high-perf-animations] ------- @@ -112,8 +175,5 @@ General tips: [pagespeed-insights]: https://developers.google.com/speed/pagespeed/insights/ [google-devtools-profiling]: https://developers.google.com/web/tools/chrome-devtools/profile/?hl=en [browser-diet]: https://browserdiet.com/ -[d3]: https://d3js.org/ -[chartjs]: http://www.chartjs.org/ -[page-specific-js-example]: https://gitlab.com/gitlab-org/gitlab-ce/blob/13bb9ed77f405c5f6ee4fdbc964ecf635c9a223f/app/views/projects/graphs/_head.html.haml#L6-8 [high-perf-animations]: https://www.html5rocks.com/en/tutorials/speed/high-performance-animations/ [flip]: https://aerotwist.com/blog/flip-your-animations/ diff --git a/doc/development/new_fe_guide/development/security.md b/doc/development/new_fe_guide/development/security.md index debda7de0c6..5bb38f17988 100644 --- a/doc/development/new_fe_guide/development/security.md +++ b/doc/development/new_fe_guide/development/security.md @@ -1,3 +1,14 @@ # Security -> TODO: Add content +## Avoid inline scripts and styles + +Inline scripts and styles should be avoided in almost all cases. In an effort to protect users from [XSS vulnerabilities](https://en.wikipedia.org/wiki/Cross-site_scripting), we will be disabling inline scripts using Content Security Policy. + +## Including external resources + +External fonts, CSS, and JavaScript should never be used with the exception of Google Analytics and Piwik - and only when the instance has enabled it. Assets should always be hosted and served locally from the GitLab instance. Embedded resources via `iframes` should never be used except in certain circumstances such as with ReCaptcha, which cannot be used without an `iframe`. + +## Resources for security testing + +- [Mozilla's HTTP Observatory CLI](https://github.com/mozilla/http-observatory-cli) +- [Qualys SSL Labs Server Test](https://www.ssllabs.com/ssltest/analyze.html) diff --git a/doc/downgrade_ee_to_ce/README.md b/doc/downgrade_ee_to_ce/README.md index 75bae324585..ff1ac94ac58 100644 --- a/doc/downgrade_ee_to_ce/README.md +++ b/doc/downgrade_ee_to_ce/README.md @@ -70,7 +70,7 @@ To downgrade an Omnibus installation, it is sufficient to install the Community Edition package on top of the currently installed one. You can do this manually, by directly [downloading the package](https://packages.gitlab.com/gitlab/gitlab-ce) you need, or by adding our CE package repository and following the -[CE installation instructions](https://about.gitlab.com/downloads/). +[CE installation instructions](https://about.gitlab.com/downloads/?version=ce). **Source Installation** diff --git a/doc/install/installation.md b/doc/install/installation.md index 6eb767b00b3..1abbfd78738 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -162,13 +162,14 @@ page](https://golang.org/dl). ## 4. Node -Since GitLab 8.17, GitLab requires the use of node >= v4.3.0 to compile -javascript assets, and yarn >= v0.17.0 to manage javascript dependencies. -In many distros the versions provided by the official package repositories -are out of date, so we'll need to install through the following commands: - - # install node v7.x - curl --location https://deb.nodesource.com/setup_7.x | sudo bash - +Since GitLab 8.17, GitLab requires the use of Node to compile javascript +assets, and Yarn to manage javascript dependencies. The current minimum +requirements for these are node >= v6.0.0 and yarn >= v1.2.0. In many distros +the versions provided by the official package repositories are out of date, so +we'll need to install through the following commands: + + # install node v8.x + curl --location https://deb.nodesource.com/setup_8.x | sudo bash - sudo apt-get install -y nodejs curl --silent --show-error https://dl.yarnpkg.com/debian/pubkey.gpg | sudo apt-key add - diff --git a/doc/update/10.5-to-10.6.md b/doc/update/10.5-to-10.6.md index af8343b5958..f5c5c305726 100644 --- a/doc/update/10.5-to-10.6.md +++ b/doc/update/10.5-to-10.6.md @@ -56,8 +56,8 @@ sudo gem install bundler --no-ri --no-rdoc ### 4. Update Node -GitLab now runs [webpack](http://webpack.js.org) to compile frontend assets. -We require a minimum version of node v6.0.0. +GitLab utilizes [webpack](http://webpack.js.org) to compile frontend assets. +This requires a minimum version of node v6.0.0. You can check which version you are running with `node -v`. If you are running a version older than `v6.0.0` you will need to update to a newer version. You @@ -66,8 +66,8 @@ from source at the nodejs.org website. <https://nodejs.org/en/download/> -Since 8.17, GitLab requires the use of yarn `>= v0.17.0` to manage -JavaScript dependencies. +GitLab also requires the use of yarn `>= v1.2.0` to manage JavaScript +dependencies. ```bash curl --silent --show-error https://dl.yarnpkg.com/debian/pubkey.gpg | sudo apt-key add - diff --git a/features/project/graph.feature b/features/project/graph.feature deleted file mode 100644 index b25c73ad870..00000000000 --- a/features/project/graph.feature +++ /dev/null @@ -1,33 +0,0 @@ -Feature: Project Graph - Background: - Given I sign in as a user - And I own project "Shop" - - @javascript - Scenario: I should see project graphs - When I visit project "Shop" graph page - Then page should have graphs - - @javascript - Scenario: I should see project languages & commits graphs on commits graph url - When I visit project "Shop" commits graph page - Then page should have commits graphs - Then page should have languages graphs - - @javascript - Scenario: I should see project ci graphs - Given project "Shop" has CI enabled - When I visit project "Shop" CI graph page - Then page should have CI graphs - - @javascript - Scenario: I should see project languages & commits graphs on language graph url - When I visit project "Shop" languages graph page - Then page should have languages graphs - Then page should have commits graphs - - @javascript - Scenario: I should see project languages & commits graphs on charts url - When I visit project "Shop" chart page - Then page should have languages graphs - Then page should have commits graphs diff --git a/features/project/redirects.feature b/features/project/redirects.feature deleted file mode 100644 index a2e77e7bf30..00000000000 --- a/features/project/redirects.feature +++ /dev/null @@ -1,38 +0,0 @@ -Feature: Project Redirects - Background: - Given public project "Community" - And private project "Enterprise" - - Scenario: I visit public project page - When I visit project "Community" page - Then I should see project "Community" home page - - Scenario: I visit private project page - When I visit project "Enterprise" page - Then I should be redirected to sign in page - - Scenario: I visit a non-existent project page - When I visit project "CommunityDoesNotExist" page - Then I should be redirected to sign in page - - Scenario: I visit a non-existent project page as user - Given I sign in as a user - When I visit project "CommunityDoesNotExist" page - Then page status code should be 404 - - Scenario: I visit unauthorized project page as user - Given I sign in as a user - When I visit project "Enterprise" page - Then page status code should be 404 - - Scenario: I visit a public project without signing in - When I visit project "Community" page - And I should see project "Community" home page - And I click on "Sign In" - And Authenticate - Then I should be redirected to "Community" page - - Scenario: I visit private project page without signing in - When I visit project "Enterprise" page - And I get redirected to signin page where I sign in - Then I should be redirected to "Enterprise" page diff --git a/features/steps/project/graph.rb b/features/steps/project/graph.rb deleted file mode 100644 index b9cddf4041d..00000000000 --- a/features/steps/project/graph.rb +++ /dev/null @@ -1,50 +0,0 @@ -class Spinach::Features::ProjectGraph < Spinach::FeatureSteps - include SharedAuthentication - include SharedProject - - step 'page should have graphs' do - expect(page).to have_selector ".stat-graph" - end - - When 'I visit project "Shop" graph page' do - visit project_graph_path(project, "master") - end - - step 'I visit project "Shop" commits graph page' do - visit commits_project_graph_path(project, "master") - end - - step 'I visit project "Shop" languages graph page' do - visit languages_project_graph_path(project, "master") - end - - step 'I visit project "Shop" chart page' do - visit charts_project_graph_path(project, "master") - end - - step 'page should have languages graphs' do - expect(page).to have_content /Ruby 66.* %/ - expect(page).to have_content /JavaScript 22.* %/ - end - - step 'page should have commits graphs' do - expect(page).to have_content "Commit statistics for master" - expect(page).to have_content "Commits per day of month" - end - - step 'I visit project "Shop" CI graph page' do - visit ci_project_graph_path(project, 'master') - end - - step 'page should have CI graphs' do - expect(page).to have_content 'Overall' - expect(page).to have_content 'Pipelines for last week' - expect(page).to have_content 'Pipelines for last month' - expect(page).to have_content 'Pipelines for last year' - expect(page).to have_content 'Commit duration in minutes for last 30 commits' - end - - def project - @project ||= Project.find_by(name: "Shop") - end -end diff --git a/features/steps/project/redirects.rb b/features/steps/project/redirects.rb deleted file mode 100644 index 9ce86ca45d0..00000000000 --- a/features/steps/project/redirects.rb +++ /dev/null @@ -1,67 +0,0 @@ -class Spinach::Features::ProjectRedirects < Spinach::FeatureSteps - include SharedAuthentication - include SharedPaths - include SharedProject - - step 'public project "Community"' do - create(:project, :public, name: 'Community') - end - - step 'private project "Enterprise"' do - create(:project, :private, name: 'Enterprise') - end - - step 'I visit project "Community" page' do - project = Project.find_by(name: 'Community') - visit project_path(project) - end - - step 'I should see project "Community" home page' do - Gitlab.config.gitlab.should_receive(:host).and_return("www.example.com") - page.within '.breadcrumbs .breadcrumb-item-text' do - expect(page).to have_content 'Community' - end - end - - step 'I visit project "Enterprise" page' do - project = Project.find_by(name: 'Enterprise') - visit project_path(project) - end - - step 'I visit project "CommunityDoesNotExist" page' do - project = Project.find_by(name: 'Community') - visit project_path(project) + 'DoesNotExist' - end - - step 'I click on "Sign In"' do - first(:link, "Sign in").click - end - - step 'Authenticate' do - admin = create(:admin) - fill_in "user_login", with: admin.email - fill_in "user_password", with: admin.password - click_button "Sign in" - Thread.current[:current_user] = admin - end - - step 'I should be redirected to "Community" page' do - project = Project.find_by(name: 'Community') - expect(current_path).to eq "/#{project.full_path}" - expect(status_code).to eq 200 - end - - step 'I get redirected to signin page where I sign in' do - admin = create(:admin) - fill_in "user_login", with: admin.email - fill_in "user_password", with: admin.password - click_button "Sign in" - Thread.current[:current_user] = admin - end - - step 'I should be redirected to "Enterprise" page' do - project = Project.find_by(name: 'Enterprise') - expect(current_path).to eq "/#{project.full_path}" - expect(status_code).to eq 200 - end -end diff --git a/lib/gitlab/git/gitlab_projects.rb b/lib/gitlab/git/gitlab_projects.rb index 5e1e22ae65c..a142ed6b2ef 100644 --- a/lib/gitlab/git/gitlab_projects.rb +++ b/lib/gitlab/git/gitlab_projects.rb @@ -67,7 +67,7 @@ module Gitlab tags_option = tags ? '--tags' : '--no-tags' logger.info "Fetching remote #{name} for repository #{repository_absolute_path}." - cmd = %W(git fetch #{name} --quiet) + cmd = %W(#{Gitlab.config.git.bin_path} fetch #{name} --quiet) cmd << '--prune' if prune cmd << '--force' if force cmd << tags_option @@ -85,7 +85,7 @@ module Gitlab def push_branches(remote_name, timeout, force, branch_names) logger.info "Pushing branches from #{repository_absolute_path} to remote #{remote_name}: #{branch_names}" - cmd = %w(git push) + cmd = %W(#{Gitlab.config.git.bin_path} push) cmd << '--force' if force cmd += %W(-- #{remote_name}).concat(branch_names) @@ -102,7 +102,7 @@ module Gitlab branches = branch_names.map { |branch_name| ":#{branch_name}" } logger.info "Pushing deleted branches from #{repository_absolute_path} to remote #{remote_name}: #{branch_names}" - cmd = %W(git push -- #{remote_name}).concat(branches) + cmd = %W(#{Gitlab.config.git.bin_path} push -- #{remote_name}).concat(branches) success = run(cmd, repository_absolute_path) @@ -143,7 +143,7 @@ module Gitlab end def remove_origin_in_repo - cmd = %w(git remote rm origin) + cmd = %W(#{Gitlab.config.git.bin_path} remote rm origin) run(cmd, repository_absolute_path) end @@ -223,7 +223,7 @@ module Gitlab masked_source = mask_password_in_url(source) logger.info "Importing project from <#{masked_source}> to <#{repository_absolute_path}>." - cmd = %W(git clone --bare -- #{source} #{repository_absolute_path}) + cmd = %W(#{Gitlab.config.git.bin_path} clone --bare -- #{source} #{repository_absolute_path}) success = run_with_timeout(cmd, timeout, nil) @@ -266,7 +266,7 @@ module Gitlab FileUtils.mkdir_p(File.dirname(to_path), mode: 0770) logger.info "Forking repository from <#{from_path}> to <#{to_path}>." - cmd = %W(git clone --bare --no-local -- #{from_path} #{to_path}) + cmd = %W(#{Gitlab.config.git.bin_path} clone --bare --no-local -- #{from_path} #{to_path}) run(cmd, nil) && Gitlab::Git::Repository.create_hooks(to_path, global_hooks_path) end diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index 9cd76630484..8ca30ffc232 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -119,6 +119,9 @@ module Gitlab # def self.call(storage, service, rpc, request, remote_storage: nil, timeout: nil) start = Gitlab::Metrics::System.monotonic_time + request_hash = request.is_a?(Google::Protobuf::MessageExts) ? request.to_h : {} + @current_call_id ||= SecureRandom.uuid + enforce_gitaly_request_limits(:call) kwargs = request_kwargs(storage, timeout, remote_storage: remote_storage) @@ -135,6 +138,10 @@ module Gitlab gitaly_controller_action_duration_seconds.observe( current_transaction_labels.merge(gitaly_service: service.to_s, rpc: rpc.to_s), duration) + + add_call_details(id: @current_call_id, feature: service, duration: duration, request: request_hash) + + @current_call_id = nil end def self.handle_grpc_unavailable!(ex) @@ -252,12 +259,16 @@ module Gitlab feature_stack.unshift(feature) begin start = Gitlab::Metrics::System.monotonic_time + @current_call_id = SecureRandom.uuid + call_details = { id: @current_call_id } yield is_enabled ensure total_time = Gitlab::Metrics::System.monotonic_time - start gitaly_migrate_call_duration_seconds.observe({ gitaly_enabled: is_enabled, feature: feature }, total_time) feature_stack.shift Thread.current[:gitaly_feature_stack] = nil if feature_stack.empty? + + add_call_details(call_details.merge(feature: feature, duration: total_time)) end end end @@ -344,6 +355,22 @@ module Gitlab end end + def self.add_call_details(details) + id = details.delete(:id) + + return unless id && RequestStore.active? && RequestStore.store[:peek_enabled] + + RequestStore.store['gitaly_call_details'] ||= {} + RequestStore.store['gitaly_call_details'][id] ||= {} + RequestStore.store['gitaly_call_details'][id].merge!(details) + end + + def self.list_call_details + return {} unless RequestStore.active? && RequestStore.store[:peek_enabled] + + RequestStore.store['gitaly_call_details'] || {} + end + def self.expected_server_version path = Rails.root.join(SERVER_VERSION_FILE) path.read.chomp diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb index dda7afc0999..3a8f5826818 100644 --- a/lib/gitlab/shell.rb +++ b/lib/gitlab/shell.rb @@ -69,13 +69,14 @@ module Gitlab # name - project disk path # # Ex. - # add_repository("/path/to/storage", "gitlab/gitlab-ci") + # create_repository("/path/to/storage", "gitlab/gitlab-ci") # - def add_repository(storage, name) + def create_repository(storage, name) relative_path = name.dup relative_path << '.git' unless relative_path.end_with?('.git') - gitaly_migrate(:create_repository) do |is_enabled| + gitaly_migrate(:create_repository, + status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| if is_enabled repository = Gitlab::Git::Repository.new(storage, relative_path, '') repository.gitaly_repository_client.create_repository @@ -85,7 +86,7 @@ module Gitlab Gitlab::Git::Repository.create(repo_path, bare: true, symlink_hooks_to: gitlab_shell_hooks_path) end end - rescue => err + rescue => err # Once the Rugged codes gets removes this can be improved Rails.logger.error("Failed to add repository #{storage}/#{name}: #{err}") false end @@ -487,8 +488,8 @@ module Gitlab Gitlab.config.gitlab_shell.git_timeout end - def gitaly_migrate(method, &block) - Gitlab::GitalyClient.migrate(method, &block) + def gitaly_migrate(method, status: Gitlab::GitalyClient::MigrationStatus::OPT_IN, &block) + Gitlab::GitalyClient.migrate(method, status: status, &block) rescue GRPC::NotFound, GRPC::BadStatus => e # Old Popen code returns [Error, output] to the caller, so we # need to do the same here... diff --git a/lib/peek/views/gitaly.rb b/lib/peek/views/gitaly.rb index d519d8e86fa..ab35f7a2258 100644 --- a/lib/peek/views/gitaly.rb +++ b/lib/peek/views/gitaly.rb @@ -10,11 +10,29 @@ module Peek end def results - { duration: formatted_duration, calls: calls } + { + duration: formatted_duration, + calls: calls, + details: details + } end private + def details + ::Gitlab::GitalyClient.list_call_details + .values + .sort { |a, b| b[:duration] <=> a[:duration] } + .map(&method(:format_call_details)) + end + + def format_call_details(call) + pretty_request = call[:request]&.reject { |k, v| v.blank? }.to_h.pretty_inspect + + call.merge(duration: (call[:duration] * 1000).round(3), + request: pretty_request || {}) + end + def formatted_duration ms = duration * 1000 if ms >= 1000 diff --git a/lib/tasks/gitlab/shell.rake b/lib/tasks/gitlab/shell.rake index 844664b12d4..4fcbbbf8c9d 100644 --- a/lib/tasks/gitlab/shell.rake +++ b/lib/tasks/gitlab/shell.rake @@ -69,7 +69,7 @@ namespace :gitlab do if File.exist?(path_to_repo) print '-' else - if Gitlab::Shell.new.add_repository(project.repository_storage, + if Gitlab::Shell.new.create_repository(project.repository_storage, project.disk_path) print '.' else diff --git a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb index 3e83a549682..b4ad4b64d8e 100644 --- a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb +++ b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb @@ -108,6 +108,7 @@ describe 'Merge request > User resolves diff notes and discussions', :js do it 'shows resolved discussion when toggled' do find(".timeline-content .discussion[data-discussion-id='#{note.discussion_id}'] .discussion-toggle-button").click + expect(page.find(".line-holder-placeholder")).to be_visible expect(page.find(".timeline-content #note_#{note.id}")).to be_visible end end diff --git a/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb b/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb index 8a834adbf17..565e375600b 100644 --- a/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb +++ b/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb @@ -5,15 +5,18 @@ describe 'Merge request > User scrolls to note on load', :js do let(:user) { project.creator } let(:merge_request) { create(:merge_request, source_project: project, author: user) } let(:note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project) } + let(:resolved_note) { create(:diff_note_on_merge_request, :resolved, noteable: merge_request, project: project) } let(:fragment_id) { "#note_#{note.id}" } + let(:collapsed_fragment_id) { "#note_#{resolved_note.id}" } before do sign_in(user) page.current_window.resize_to(1000, 300) - visit "#{project_merge_request_path(project, merge_request)}#{fragment_id}" end - it 'scrolls down to fragment' do + it 'scrolls note into view' do + visit "#{project_merge_request_path(project, merge_request)}#{fragment_id}" + page_height = page.current_window.size[1] page_scroll_y = page.evaluate_script("window.scrollY") fragment_position_top = page.evaluate_script("Math.round($('#{fragment_id}').offset().top)") @@ -23,4 +26,13 @@ describe 'Merge request > User scrolls to note on load', :js do expect(fragment_position_top).to be >= page_scroll_y expect(fragment_position_top).to be < (page_scroll_y + page_height) end + + it 'expands collapsed notes' do + visit "#{project_merge_request_path(project, merge_request)}#{collapsed_fragment_id}" + note_element = find(collapsed_fragment_id) + note_container = note_element.ancestor('.js-toggle-container') + + expect(note_element.visible?).to eq true + expect(note_container.find('.line_content.noteable_line.old', match: :first).visible?).to eq true + end end diff --git a/spec/features/projects/graph_spec.rb b/spec/features/projects/graph_spec.rb new file mode 100644 index 00000000000..57172610aed --- /dev/null +++ b/spec/features/projects/graph_spec.rb @@ -0,0 +1,75 @@ +require 'spec_helper' + +describe 'Project Graph', :js do + let(:user) { create :user } + let(:project) { create(:project, :repository, namespace: user.namespace) } + + before do + project.add_master(user) + + sign_in(user) + end + + shared_examples 'page should have commits graphs' do + it 'renders commits' do + expect(page).to have_content('Commit statistics for master') + expect(page).to have_content('Commits per day of month') + end + end + + shared_examples 'page should have languages graphs' do + it 'renders languages' do + expect(page).to have_content(/Ruby 66.* %/) + expect(page).to have_content(/JavaScript 22.* %/) + end + end + + it 'renders graphs' do + visit project_graph_path(project, 'master') + + expect(page).to have_selector('.stat-graph', visible: false) + end + + context 'commits graph' do + before do + visit commits_project_graph_path(project, 'master') + end + + it_behaves_like 'page should have commits graphs' + it_behaves_like 'page should have languages graphs' + end + + context 'languages graph' do + before do + visit languages_project_graph_path(project, 'master') + end + + it_behaves_like 'page should have commits graphs' + it_behaves_like 'page should have languages graphs' + end + + context 'charts graph' do + before do + visit charts_project_graph_path(project, 'master') + end + + it_behaves_like 'page should have commits graphs' + it_behaves_like 'page should have languages graphs' + end + + context 'when CI enabled' do + before do + project.enable_ci + + visit ci_project_graph_path(project, 'master') + end + + it 'renders CI graphs' do + expect(page).to have_content 'Overall' + expect(page).to have_content 'Pipelines for last week' + expect(page).to have_content 'Pipelines for last month' + expect(page).to have_content 'Pipelines for last year' + expect(page).to have_content 'Commit duration in minutes for last 30 commits' + end + end +end diff --git a/spec/features/projects/redirects_spec.rb b/spec/features/projects/redirects_spec.rb new file mode 100644 index 00000000000..d1d8ca07035 --- /dev/null +++ b/spec/features/projects/redirects_spec.rb @@ -0,0 +1,74 @@ +require 'spec_helper' + +describe 'Project redirects' do + let(:user) { create :user } + let(:public_project) { create :project, :public } + let(:private_project) { create :project, :private } + + before do + allow(Gitlab.config.gitlab).to receive(:host).and_return('www.example.com') + end + + it 'shows public project page' do + visit project_path(public_project) + + page.within '.breadcrumbs .breadcrumb-item-text' do + expect(page).to have_content(public_project.name) + end + end + + it 'redirects to sign in page when project is private' do + visit project_path(private_project) + + expect(current_path).to eq(new_user_session_path) + end + + it 'redirects to sign in page when project does not exist' do + visit project_path(build(:project, :public)) + + expect(current_path).to eq(new_user_session_path) + end + + it 'redirects to public project page after signing in' do + visit project_path(public_project) + + first(:link, 'Sign in').click + + fill_in 'user_login', with: user.email + fill_in 'user_password', with: user.password + click_button 'Sign in' + + expect(status_code).to eq(200) + expect(current_path).to eq("/#{public_project.full_path}") + end + + it 'redirects to private project page after sign in' do + visit project_path(private_project) + + owner = private_project.owner + fill_in 'user_login', with: owner.email + fill_in 'user_password', with: owner.password + click_button 'Sign in' + + expect(status_code).to eq(200) + expect(current_path).to eq("/#{private_project.full_path}") + end + + context 'when signed in' do + before do + sign_in(user) + end + + it 'returns 404 status when project does not exist' do + visit project_path(build(:project, :public)) + + expect(status_code).to eq(404) + end + + it 'returns 404 when project is private' do + visit project_path(private_project) + + expect(status_code).to eq(404) + end + end +end diff --git a/spec/javascripts/notes/components/comment_form_spec.js b/spec/javascripts/notes/components/comment_form_spec.js index 90016436cb7..224debbeff6 100644 --- a/spec/javascripts/notes/components/comment_form_spec.js +++ b/spec/javascripts/notes/components/comment_form_spec.js @@ -200,6 +200,20 @@ describe('issue_comment_form component', () => { done(); }); }); + + describe('when clicking close/reopen button', () => { + it('should disable button and show a loading spinner', (done) => { + const toggleStateButton = vm.$el.querySelector('.js-action-button'); + + toggleStateButton.click(); + Vue.nextTick(() => { + expect(toggleStateButton.disabled).toEqual(true); + expect(toggleStateButton.querySelector('.js-loading-button-icon')).not.toBeNull(); + + done(); + }); + }); + }); }); describe('issue is confidential', () => { diff --git a/spec/javascripts/notes/stores/actions_spec.js b/spec/javascripts/notes/stores/actions_spec.js index b838cc36fb3..91249b2c79e 100644 --- a/spec/javascripts/notes/stores/actions_spec.js +++ b/spec/javascripts/notes/stores/actions_spec.js @@ -88,6 +88,7 @@ describe('Actions Notes Store', () => { store.dispatch('closeIssue', { notesData: { closeIssuePath: '' } }) .then(() => { expect(store.state.noteableData.state).toEqual('closed'); + expect(store.state.isToggleStateButtonLoading).toEqual(false); done(); }) .catch(done.fail); @@ -99,6 +100,7 @@ describe('Actions Notes Store', () => { store.dispatch('reopenIssue', { notesData: { reopenIssuePath: '' } }) .then(() => { expect(store.state.noteableData.state).toEqual('reopened'); + expect(store.state.isToggleStateButtonLoading).toEqual(false); done(); }) .catch(done.fail); @@ -117,6 +119,20 @@ describe('Actions Notes Store', () => { }); }); + describe('toggleStateButtonLoading', () => { + it('should set loading as true', (done) => { + testAction(actions.toggleStateButtonLoading, true, {}, [ + { type: 'TOGGLE_STATE_BUTTON_LOADING', payload: true }, + ], done); + }); + + it('should set loading as false', (done) => { + testAction(actions.toggleStateButtonLoading, false, {}, [ + { type: 'TOGGLE_STATE_BUTTON_LOADING', payload: false }, + ], done); + }); + }); + describe('toggleIssueLocalState', () => { it('sets issue state as closed', (done) => { testAction(actions.toggleIssueLocalState, 'closed', {}, [ diff --git a/spec/javascripts/notes/stores/mutation_spec.js b/spec/javascripts/notes/stores/mutation_spec.js index 34884f8968f..98f101d6bc5 100644 --- a/spec/javascripts/notes/stores/mutation_spec.js +++ b/spec/javascripts/notes/stores/mutation_spec.js @@ -228,4 +228,70 @@ describe('Notes Store mutations', () => { expect(state.notes[0].notes[0].note).toEqual('Foo'); }); }); + + describe('CLOSE_ISSUE', () => { + it('should set issue as closed', () => { + const state = { + notes: [], + targetNoteHash: null, + lastFetchedAt: null, + isToggleStateButtonLoading: false, + notesData: {}, + userData: {}, + noteableData: {}, + }; + + mutations.CLOSE_ISSUE(state); + expect(state.noteableData.state).toEqual('closed'); + }); + }); + + describe('REOPEN_ISSUE', () => { + it('should set issue as closed', () => { + const state = { + notes: [], + targetNoteHash: null, + lastFetchedAt: null, + isToggleStateButtonLoading: false, + notesData: {}, + userData: {}, + noteableData: {}, + }; + + mutations.REOPEN_ISSUE(state); + expect(state.noteableData.state).toEqual('reopened'); + }); + }); + + describe('TOGGLE_STATE_BUTTON_LOADING', () => { + it('should set isToggleStateButtonLoading as true', () => { + const state = { + notes: [], + targetNoteHash: null, + lastFetchedAt: null, + isToggleStateButtonLoading: false, + notesData: {}, + userData: {}, + noteableData: {}, + }; + + mutations.TOGGLE_STATE_BUTTON_LOADING(state, true); + expect(state.isToggleStateButtonLoading).toEqual(true); + }); + + it('should set isToggleStateButtonLoading as false', () => { + const state = { + notes: [], + targetNoteHash: null, + lastFetchedAt: null, + isToggleStateButtonLoading: true, + notesData: {}, + userData: {}, + noteableData: {}, + }; + + mutations.TOGGLE_STATE_BUTTON_LOADING(state, false); + expect(state.isToggleStateButtonLoading).toEqual(false); + }); + }); }); diff --git a/spec/lib/gitlab/bare_repository_import/repository_spec.rb b/spec/lib/gitlab/bare_repository_import/repository_spec.rb index 9f42cf1dfca..5cb1f4deb5f 100644 --- a/spec/lib/gitlab/bare_repository_import/repository_spec.rb +++ b/spec/lib/gitlab/bare_repository_import/repository_spec.rb @@ -61,7 +61,7 @@ describe ::Gitlab::BareRepositoryImport::Repository do let(:wiki_path) { File.join(root_path, "#{hashed_path}.wiki.git") } before do - gitlab_shell.add_repository(repository_storage, hashed_path) + gitlab_shell.create_repository(repository_storage, hashed_path) repository = Rugged::Repository.new(repo_path) repository.config['gitlab.fullpath'] = 'to/repo' end diff --git a/spec/lib/gitlab/git/gitlab_projects_spec.rb b/spec/lib/gitlab/git/gitlab_projects_spec.rb index 45bcd730332..dfccc15a4f3 100644 --- a/spec/lib/gitlab/git/gitlab_projects_spec.rb +++ b/spec/lib/gitlab/git/gitlab_projects_spec.rb @@ -28,7 +28,7 @@ describe Gitlab::Git::GitlabProjects do describe '#push_branches' do let(:remote_name) { 'remote-name' } let(:branch_name) { 'master' } - let(:cmd) { %W(git push -- #{remote_name} #{branch_name}) } + let(:cmd) { %W(#{Gitlab.config.git.bin_path} push -- #{remote_name} #{branch_name}) } let(:force) { false } subject { gl_projects.push_branches(remote_name, 600, force, [branch_name]) } @@ -46,7 +46,7 @@ describe Gitlab::Git::GitlabProjects do end context 'with --force' do - let(:cmd) { %W(git push --force -- #{remote_name} #{branch_name}) } + let(:cmd) { %W(#{Gitlab.config.git.bin_path} push --force -- #{remote_name} #{branch_name}) } let(:force) { true } it 'executes the command' do @@ -65,7 +65,7 @@ describe Gitlab::Git::GitlabProjects do let(:tags) { true } let(:args) { { force: force, tags: tags, prune: prune }.merge(extra_args) } let(:extra_args) { {} } - let(:cmd) { %W(git fetch #{remote_name} --quiet --prune --tags) } + let(:cmd) { %W(#{Gitlab.config.git.bin_path} fetch #{remote_name} --quiet --prune --tags) } subject { gl_projects.fetch_remote(remote_name, 600, args) } @@ -98,7 +98,7 @@ describe Gitlab::Git::GitlabProjects do context 'with --force' do let(:force) { true } - let(:cmd) { %W(git fetch #{remote_name} --quiet --prune --force --tags) } + let(:cmd) { %W(#{Gitlab.config.git.bin_path} fetch #{remote_name} --quiet --prune --force --tags) } it 'executes the command with forced option' do stub_spawn(cmd, 600, tmp_repo_path, {}, success: true) @@ -109,7 +109,7 @@ describe Gitlab::Git::GitlabProjects do context 'with --no-tags' do let(:tags) { false } - let(:cmd) { %W(git fetch #{remote_name} --quiet --prune --no-tags) } + let(:cmd) { %W(#{Gitlab.config.git.bin_path} fetch #{remote_name} --quiet --prune --no-tags) } it 'executes the command' do stub_spawn(cmd, 600, tmp_repo_path, {}, success: true) @@ -120,7 +120,7 @@ describe Gitlab::Git::GitlabProjects do context 'with no prune' do let(:prune) { false } - let(:cmd) { %W(git fetch #{remote_name} --quiet --tags) } + let(:cmd) { %W(#{Gitlab.config.git.bin_path} fetch #{remote_name} --quiet --tags) } it 'executes the command' do stub_spawn(cmd, 600, tmp_repo_path, {}, success: true) @@ -165,7 +165,7 @@ describe Gitlab::Git::GitlabProjects do describe '#import_project' do let(:project) { create(:project) } let(:import_url) { TestEnv.factory_repo_path_bare } - let(:cmd) { %W(git clone --bare -- #{import_url} #{tmp_repo_path}) } + let(:cmd) { %W(#{Gitlab.config.git.bin_path} clone --bare -- #{import_url} #{tmp_repo_path}) } let(:timeout) { 600 } subject { gl_projects.import_project(import_url, timeout) } diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 52c9876cbb6..54ada3e423f 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -681,7 +681,7 @@ describe Gitlab::Git::Repository, seed_helper: true do subject { new_repository.fetch_repository_as_mirror(repository) } before do - Gitlab::Shell.new.add_repository('default', 'my_project') + Gitlab::Shell.new.create_repository('default', 'my_project') end after do diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index 56b45d8da3c..14b59c5e945 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -20,7 +20,7 @@ describe Gitlab::Shell do it { is_expected.to respond_to :add_key } it { is_expected.to respond_to :remove_key } - it { is_expected.to respond_to :add_repository } + it { is_expected.to respond_to :create_repository } it { is_expected.to respond_to :remove_repository } it { is_expected.to respond_to :fork_repository } @@ -402,8 +402,8 @@ describe Gitlab::Shell do allow(Gitlab.config.gitlab_shell).to receive(:git_timeout).and_return(800) end - describe '#add_repository' do - shared_examples '#add_repository' do + describe '#create_repository' do + shared_examples '#create_repository' do let(:repository_storage) { 'default' } let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] } let(:repo_name) { 'project/path' } @@ -414,7 +414,7 @@ describe Gitlab::Shell do end it 'creates a repository' do - expect(gitlab_shell.add_repository(repository_storage, repo_name)).to be_truthy + expect(gitlab_shell.create_repository(repository_storage, repo_name)).to be_truthy expect(File.stat(created_path).mode & 0o777).to eq(0o770) @@ -426,19 +426,19 @@ describe Gitlab::Shell do it 'returns false when the command fails' do FileUtils.mkdir_p(File.dirname(created_path)) # This file will block the creation of the repo's .git directory. That - # should cause #add_repository to fail. + # should cause #create_repository to fail. FileUtils.touch(created_path) - expect(gitlab_shell.add_repository(repository_storage, repo_name)).to be_falsy + expect(gitlab_shell.create_repository(repository_storage, repo_name)).to be_falsy end end context 'with gitaly' do - it_behaves_like '#add_repository' + it_behaves_like '#create_repository' end context 'without gitaly', :skip_gitaly_mock do - it_behaves_like '#add_repository' + it_behaves_like '#create_repository' end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index c27313ed88b..6e202de0db9 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1424,6 +1424,17 @@ describe Ci::Build do { key: 'CI_COMMIT_SHA', value: build.sha, public: true }, { key: 'CI_COMMIT_REF_NAME', value: build.ref, public: true }, { key: 'CI_COMMIT_REF_SLUG', value: build.ref_slug, public: true }, + { key: 'CI_REGISTRY_USER', value: 'gitlab-ci-token', public: true }, + { key: 'CI_REGISTRY_PASSWORD', value: build.token, public: false }, + { key: 'CI_REPOSITORY_URL', value: build.repo_url, public: false }, + { key: 'CI_BUILD_ID', value: build.id.to_s, public: true }, + { key: 'CI_BUILD_TOKEN', value: build.token, public: false }, + { key: 'CI_BUILD_REF', value: build.sha, public: true }, + { key: 'CI_BUILD_BEFORE_SHA', value: build.before_sha, public: true }, + { key: 'CI_BUILD_REF_NAME', value: build.ref, public: true }, + { key: 'CI_BUILD_REF_SLUG', value: build.ref_slug, public: true }, + { key: 'CI_BUILD_NAME', value: 'test', public: true }, + { key: 'CI_BUILD_STAGE', value: 'test', public: true }, { key: 'CI_PROJECT_ID', value: project.id.to_s, public: true }, { key: 'CI_PROJECT_NAME', value: project.path, public: true }, { key: 'CI_PROJECT_PATH', value: project.full_path, public: true }, @@ -1433,9 +1444,7 @@ describe Ci::Build do { key: 'CI_PROJECT_VISIBILITY', value: 'private', public: true }, { key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true }, { key: 'CI_CONFIG_PATH', value: pipeline.ci_yaml_file_path, public: true }, - { key: 'CI_REGISTRY_USER', value: 'gitlab-ci-token', public: true }, - { key: 'CI_REGISTRY_PASSWORD', value: build.token, public: false }, - { key: 'CI_REPOSITORY_URL', value: build.repo_url, public: false } + { key: 'CI_PIPELINE_SOURCE', value: pipeline.source, public: true } ] end @@ -1834,39 +1843,6 @@ describe Ci::Build do it { is_expected.to include(ci_config_path) } end - context 'returns variables in valid order' do - let(:build_pre_var) { { key: 'build', value: 'value' } } - let(:project_pre_var) { { key: 'project', value: 'value' } } - let(:pipeline_pre_var) { { key: 'pipeline', value: 'value' } } - let(:build_yaml_var) { { key: 'yaml', value: 'value' } } - - before do - allow(build).to receive(:predefined_variables) { [build_pre_var] } - allow(build).to receive(:yaml_variables) { [build_yaml_var] } - - allow_any_instance_of(Project) - .to receive(:predefined_variables) { [project_pre_var] } - - allow_any_instance_of(Project) - .to receive(:secret_variables_for) - .with(ref: 'master', environment: nil) do - [create(:ci_variable, key: 'secret', value: 'value')] - end - - allow_any_instance_of(Ci::Pipeline) - .to receive(:predefined_variables) { [pipeline_pre_var] } - end - - it do - is_expected.to eq( - [build_pre_var, - project_pre_var, - pipeline_pre_var, - build_yaml_var, - { key: 'secret', value: 'value', public: false }]) - end - end - context 'when using auto devops' do context 'and is enabled' do before do @@ -1890,6 +1866,81 @@ describe Ci::Build do end end end + + context 'when pipeline variable overrides build variable' do + before do + build.yaml_variables = [{ key: 'MYVAR', value: 'myvar', public: true }] + pipeline.variables.build(key: 'MYVAR', value: 'pipeline value') + end + + it 'overrides YAML variable using a pipeline variable' do + variables = subject.reverse.uniq { |variable| variable[:key] }.reverse + + expect(variables) + .not_to include(key: 'MYVAR', value: 'myvar', public: true) + expect(variables) + .to include(key: 'MYVAR', value: 'pipeline value', public: false) + end + end + + describe 'variables ordering' do + context 'when variables hierarchy is stubbed' do + let(:build_pre_var) { { key: 'build', value: 'value' } } + let(:project_pre_var) { { key: 'project', value: 'value' } } + let(:pipeline_pre_var) { { key: 'pipeline', value: 'value' } } + let(:build_yaml_var) { { key: 'yaml', value: 'value' } } + + before do + allow(build).to receive(:predefined_variables) { [build_pre_var] } + allow(build).to receive(:yaml_variables) { [build_yaml_var] } + + allow_any_instance_of(Project) + .to receive(:predefined_variables) { [project_pre_var] } + + allow_any_instance_of(Project) + .to receive(:secret_variables_for) + .with(ref: 'master', environment: nil) do + [create(:ci_variable, key: 'secret', value: 'value')] + end + + allow_any_instance_of(Ci::Pipeline) + .to receive(:predefined_variables) { [pipeline_pre_var] } + end + + it 'returns variables in order depending on resource hierarchy' do + is_expected.to eq( + [build_pre_var, + project_pre_var, + pipeline_pre_var, + build_yaml_var, + { key: 'secret', value: 'value', public: false }]) + end + end + + context 'when build has environment and user-provided variables' do + let(:expected_variables) do + predefined_variables.map { |variable| variable.fetch(:key) } + + %w[YAML_VARIABLE CI_ENVIRONMENT_NAME CI_ENVIRONMENT_SLUG + CI_ENVIRONMENT_URL] + end + + before do + create(:environment, project: build.project, + name: 'staging') + + build.yaml_variables = [{ key: 'YAML_VARIABLE', + value: 'var', + public: true }] + build.environment = 'staging' + end + + it 'matches explicit variables ordering' do + received_variables = subject.map { |variable| variable.fetch(:key) } + + expect(received_variables).to eq expected_variables + end + end + end end describe 'state transition: any => [:pending]' do diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 14d234f6aab..86bb2fefae1 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -172,10 +172,10 @@ describe Ci::Pipeline, :mailer do it { is_expected.to be_an(Array) } - it 'includes the defined keys' do - keys = subject.map { |v| v[:key] } + it 'includes all predefined variables in a valid order' do + keys = subject.map { |variable| variable.fetch(:key) } - expect(keys).to include('CI_PIPELINE_ID', 'CI_CONFIG_PATH', 'CI_PIPELINE_SOURCE') + expect(keys).to eq %w[CI_PIPELINE_ID CI_CONFIG_PATH CI_PIPELINE_SOURCE] end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e970cd7dfdb..4cf8d861595 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1378,7 +1378,7 @@ describe Project do context 'using a regular repository' do it 'creates the repository' do - expect(shell).to receive(:add_repository) + expect(shell).to receive(:create_repository) .with(project.repository_storage, project.disk_path) .and_return(true) @@ -1388,7 +1388,7 @@ describe Project do end it 'adds an error if the repository could not be created' do - expect(shell).to receive(:add_repository) + expect(shell).to receive(:create_repository) .with(project.repository_storage, project.disk_path) .and_return(false) @@ -1402,7 +1402,7 @@ describe Project do context 'using a forked repository' do it 'does nothing' do expect(project).to receive(:forked?).and_return(true) - expect(shell).not_to receive(:add_repository) + expect(shell).not_to receive(:create_repository) project.create_repository end @@ -1421,7 +1421,7 @@ describe Project do allow(project).to receive(:repository_exists?) .and_return(false) - allow(shell).to receive(:add_repository) + allow(shell).to receive(:create_repository) .with(project.repository_storage_path, project.disk_path) .and_return(true) @@ -1445,7 +1445,7 @@ describe Project do allow(project).to receive(:repository_exists?) .and_return(false) - expect(shell).to receive(:add_repository) + expect(shell).to receive(:create_repository) .with(project.repository_storage, project.disk_path) .and_return(true) diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index 8b4b5873704..d87c1ca14f0 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -74,7 +74,7 @@ describe ProjectWiki do # Create a fresh project which will not have a wiki project_wiki = described_class.new(create(:project), user) gitlab_shell = double(:gitlab_shell) - allow(gitlab_shell).to receive(:add_repository) + allow(gitlab_shell).to receive(:create_repository) allow(project_wiki).to receive(:gitlab_shell).and_return(gitlab_shell) expect { project_wiki.send(:wiki) }.to raise_exception(ProjectWiki::CouldNotCreateWikiError) diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 9a44dfde41b..8471467d2fa 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -164,7 +164,7 @@ describe Projects::CreateService, '#execute' do context 'with legacy storage' do before do - gitlab_shell.add_repository(repository_storage, "#{user.namespace.full_path}/existing") + gitlab_shell.create_repository(repository_storage, "#{user.namespace.full_path}/existing") end after do @@ -200,7 +200,7 @@ describe Projects::CreateService, '#execute' do end before do - gitlab_shell.add_repository(repository_storage, hashed_path) + gitlab_shell.create_repository(repository_storage, hashed_path) end after do diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index 409d5de8d43..d1011b07db6 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -108,7 +108,7 @@ describe Projects::ForkService do let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] } before do - gitlab_shell.add_repository(repository_storage, "#{@to_user.namespace.full_path}/#{@from_project.path}") + gitlab_shell.create_repository(repository_storage, "#{@to_user.namespace.full_path}/#{@from_project.path}") end after do diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index ae0e22e3dc0..ce567fe3879 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -151,7 +151,7 @@ describe Projects::TransferService do before do group.add_owner(user) - unless gitlab_shell.add_repository(repository_storage, "#{group.full_path}/#{project.path}") + unless gitlab_shell.create_repository(repository_storage, "#{group.full_path}/#{project.path}") raise 'failed to add repository' end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index d454ac0bda5..f3f97b6b921 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -196,7 +196,7 @@ describe Projects::UpdateService do let(:project) { create(:project, :legacy_storage, :repository, creator: user, namespace: user.namespace) } before do - gitlab_shell.add_repository(repository_storage, "#{user.namespace.full_path}/existing") + gitlab_shell.create_repository(repository_storage, "#{user.namespace.full_path}/existing") end after do diff --git a/vendor/assets/javascripts/peek.js b/vendor/assets/javascripts/peek.js index 695eeb27c17..7c6d226fa6a 100644 --- a/vendor/assets/javascripts/peek.js +++ b/vendor/assets/javascripts/peek.js @@ -1,14 +1,14 @@ /* - * This is a modified version of https://github.com/peek/peek/blob/master/app/assets/javascripts/peek.js + * this is a modified version of https://github.com/peek/peek/blob/master/app/assets/javascripts/peek.js * * - Removed the dependency on jquery.tipsy * - Removed the initializeTipsy and toggleBar functions - * - Customized updatePerformanceBar to handle SQL queries report specificities + * - Customized updatePerformanceBar to handle SQL query and Gitaly call lists * - Changed /peek/results to /-/peek/results * - Removed the keypress, pjax:end, page:change, and turbolinks:load handlers */ (function($) { - var fetchRequestResults, getRequestId, peekEnabled, updatePerformanceBar; + var fetchRequestResults, getRequestId, peekEnabled, updatePerformanceBar, createTable, createTableRow; getRequestId = function() { return $('#peek').data('requestId'); }; @@ -16,39 +16,55 @@ return $('#peek').length; }; updatePerformanceBar = function(results) { - var key, label, data, table, html, tr, duration_td, sql_td, strong; - Object.keys(results.data).forEach(function(key) { Object.keys(results.data[key]).forEach(function(label) { - data = results.data[key][label]; + var data = results.data[key][label]; + var table = createTable(key, label, data); + var target = $('[data-defer-to="' + key + '-' + label + '"]'); + + if (table) { + target.html(table); + } else { + target.text(data); + } + }); + }); + return $(document).trigger('peek:render', [getRequestId(), results]); + }; + createTable = function(key, label, data) { + if (label !== 'queries' && label !== 'details') { + return; + } - if (label == 'queries') { - table = document.createElement('table'); + var table = document.createElement('table'); - for (var i = 0; i < data.length; i += 1) { - tr = document.createElement('tr'); - duration_td = document.createElement('td'); - sql_td = document.createElement('td'); - strong = document.createElement('strong'); + for (var i = 0; i < data.length; i += 1) { + table.appendChild(createTableRow(data[i])); + } - strong.append(data[i]['duration'] + 'ms'); - duration_td.appendChild(strong); - tr.appendChild(duration_td); + table.className = 'table'; - sql_td.appendChild(document.createTextNode(data[i]['sql'])); - tr.appendChild(sql_td); + return table; + }; + createTableRow = function(row) { + var tr = document.createElement('tr'); + var durationTd = document.createElement('td'); + var strong = document.createElement('strong'); - table.appendChild(tr); - } + strong.append(row['duration'] + 'ms'); + durationTd.appendChild(strong); + tr.appendChild(durationTd); - table.className = 'table'; - $("[data-defer-to=" + key + "-" + label + "]").html(table); - } else { - $("[data-defer-to=" + key + "-" + label + "]").text(results.data[key][label]); - } - }); + ['sql', 'feature', 'enabled', 'request'].forEach(function(key) { + if (!row[key]) { return; } + + var td = document.createElement('td'); + + td.appendChild(document.createTextNode(row[key])); + tr.appendChild(td); }); - return $(document).trigger('peek:render', [getRequestId(), results]); + + return tr; }; fetchRequestResults = function() { return $.ajax('/-/peek/results', { |
