diff options
25 files changed, 221 insertions, 72 deletions
diff --git a/.gitlab/issue_templates/Feature proposal.md b/.gitlab/issue_templates/Feature proposal.md index ad517f0457d..639a236631d 100644 --- a/.gitlab/issue_templates/Feature proposal.md +++ b/.gitlab/issue_templates/Feature proposal.md @@ -4,7 +4,7 @@ ### Target audience -<!--- For whom are we doing this? Include either a persona from https://design.gitlab.com/#/getting-started/personas or define a specific company role. e.a. "Release Manager" or "Security Analyst" --> +<!--- For whom are we doing this? Include either a persona from https://design.gitlab.com/getting-started/personas or define a specific company role. e.a. "Release Manager" or "Security Analyst" --> ### Further details diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue index d9dd08a7a6b..be451401e97 100644 --- a/app/assets/javascripts/notes/components/noteable_discussion.vue +++ b/app/assets/javascripts/notes/components/noteable_discussion.vue @@ -178,31 +178,32 @@ export default { commitId = `<span class="commit-sha">${truncateSha(commitId)}</span>`; } - let text = s__('MergeRequests|started a discussion'); + const { + for_commit: isForCommit, + diff_discussion: isDiffDiscussion, + active: isActive, + } = this.discussion; - if (this.discussion.for_commit) { + let text = s__('MergeRequests|started a discussion'); + if (isForCommit) { text = s__( 'MergeRequests|started a discussion on commit %{linkStart}%{commitId}%{linkEnd}', ); - } else if (this.discussion.diff_discussion) { - if (this.discussion.active) { - text = s__('MergeRequests|started a discussion on %{linkStart}the diff%{linkEnd}'); - } else { - text = s__( - 'MergeRequests|started a discussion on %{linkStart}an old version of the diff%{linkEnd}', - ); - } + } else if (isDiffDiscussion && commitId) { + text = isActive + ? s__('MergeRequests|started a discussion on commit %{linkStart}%{commitId}%{linkEnd}') + : s__( + 'MergeRequests|started a discussion on an outdated change in commit %{linkStart}%{commitId}%{linkEnd}', + ); + } else if (isDiffDiscussion) { + text = isActive + ? s__('MergeRequests|started a discussion on %{linkStart}the diff%{linkEnd}') + : s__( + 'MergeRequests|started a discussion on %{linkStart}an old version of the diff%{linkEnd}', + ); } - return sprintf( - text, - { - commitId, - linkStart, - linkEnd, - }, - false, - ); + return sprintf(text, { commitId, linkStart, linkEnd }, false); }, diffLine() { if (this.discussion.diff_discussion && this.discussion.truncated_diff_lines) { @@ -211,6 +212,16 @@ export default { return this.line; }, + commit() { + if (!this.discussion.for_commit) { + return null; + } + + return { + id: this.discussion.commit_id, + url: this.discussion.discussion_path, + }; + }, }, watch: { isReplying() { @@ -376,6 +387,7 @@ Please check your network connection and try again.`; :note="componentData(initialDiscussion)" :line="line" :help-page-path="helpPagePath" + :commit="commit" @handleDeleteNote="deleteNoteHandler" > <note-edited-text diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue index 4c02588127e..6edc65c856e 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/app/assets/javascripts/notes/components/noteable_note.vue @@ -2,6 +2,8 @@ import $ from 'jquery'; import { mapGetters, mapActions } from 'vuex'; import { escape } from 'underscore'; +import { truncateSha } from '~/lib/utils/text_utility'; +import { s__, sprintf } from '~/locale'; import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item.vue'; import Flash from '../../flash'; import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue'; @@ -37,6 +39,11 @@ export default { required: false, default: '', }, + commit: { + type: Object, + required: false, + default: () => null, + }, }, data() { return { @@ -73,6 +80,24 @@ export default { isTarget() { return this.targetNoteHash === this.noteAnchorId; }, + actionText() { + if (this.commit) { + const { id, url } = this.commit; + const linkStart = `<a class="commit-sha monospace" href="${escape(url)}">`; + const linkEnd = '</a>'; + return sprintf( + s__('MergeRequests|commented on commit %{linkStart}%{commitId}%{linkEnd}'), + { + commitId: truncateSha(id), + linkStart, + linkEnd, + }, + false, + ); + } + + return '<span class="d-none d-sm-inline">·</span>'; + }, }, created() { @@ -200,13 +225,9 @@ export default { </div> <div class="timeline-content"> <div class="note-header"> - <note-header - v-once - :author="author" - :created-at="note.created_at" - :note-id="note.id" - action-text="commented" - /> + <note-header v-once :author="author" :created-at="note.created_at" :note-id="note.id"> + <span v-html="actionText"></span> + </note-header> <note-actions :author-id="author.id" :note-id="note.id" diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index 3ac7b6b704b..037a5adfb7e 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -24,7 +24,7 @@ } } - &:not(.use-csslab) table { + table { @extend .table; } diff --git a/app/assets/stylesheets/framework/highlight.scss b/app/assets/stylesheets/framework/highlight.scss index 73533571a2f..946f575ac13 100644 --- a/app/assets/stylesheets/framework/highlight.scss +++ b/app/assets/stylesheets/framework/highlight.scss @@ -42,7 +42,6 @@ padding: 10px; text-align: right; float: left; - line-height: 1; a { font-family: $monospace-font; @@ -69,3 +68,9 @@ } } } + +// Vertically aligns <table> line numbers (eg. blame view) +// see https://gitlab.com/gitlab-org/gitlab-ce/issues/54048 +td.line-numbers { + line-height: 1; +} diff --git a/app/assets/stylesheets/framework/modal.scss b/app/assets/stylesheets/framework/modal.scss index 7e30747963a..95291b4a9ad 100644 --- a/app/assets/stylesheets/framework/modal.scss +++ b/app/assets/stylesheets/framework/modal.scss @@ -25,8 +25,8 @@ &.w-100 { // after upgrading to Bootstrap 4.2 we can use $modal-header-padding-x here // https://github.com/twbs/bootstrap/pull/26976 - margin-right: -2rem; - padding-right: 2rem; + margin-right: -28px; + padding-right: 28px; } } diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index ce5aaa8963c..343c09b4a3e 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -198,7 +198,7 @@ $well-light-text-color: #5b6169; $gl-font-size: 14px; $gl-font-size-xs: 11px; $gl-font-size-small: 12px; -$gl-font-size-medium: 1.43rem; +$gl-font-size-medium: 20px; $gl-font-size-large: 16px; $gl-font-weight-normal: 400; $gl-font-weight-bold: 600; diff --git a/app/assets/stylesheets/framework/variables_overrides.scss b/app/assets/stylesheets/framework/variables_overrides.scss index 5ca76bb6c5a..069f45bff49 100644 --- a/app/assets/stylesheets/framework/variables_overrides.scss +++ b/app/assets/stylesheets/framework/variables_overrides.scss @@ -28,3 +28,9 @@ $popover-border-width: 1px; $popover-border-color: $border-color; $popover-box-shadow: 0 $border-radius-small $border-radius-default 0 $shadow-color; $popover-arrow-outer-color: $shadow-color; +$h1-font-size: 14px * 2.5; +$h2-font-size: 14px * 2; +$h3-font-size: 14px * 1.75; +$h4-font-size: 14px * 1.5; +$h5-font-size: 14px * 1.25; +$h6-font-size: 14px; diff --git a/app/assets/stylesheets/pages/builds.scss b/app/assets/stylesheets/pages/builds.scss index c7dde2f0f2a..09235661cea 100644 --- a/app/assets/stylesheets/pages/builds.scss +++ b/app/assets/stylesheets/pages/builds.scss @@ -135,6 +135,7 @@ .build-loader-animation { @include build-loader-animation; float: left; + padding-left: $gl-padding-8; } } diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index 033686823a2..293dd20ad49 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -85,7 +85,7 @@ module NotesHelper diffs_project_merge_request_path(discussion.project, discussion.noteable, path_params) elsif discussion.for_commit? - anchor = discussion.line_code if discussion.diff_discussion? + anchor = discussion.diff_discussion? ? discussion.line_code : "note_#{discussion.first_note.id}" project_commit_path(discussion.project, discussion.noteable, anchor: anchor) end diff --git a/changelogs/unreleased/51668-fix-line-numbers.yml b/changelogs/unreleased/51668-fix-line-numbers.yml new file mode 100644 index 00000000000..ac6e45e3cc7 --- /dev/null +++ b/changelogs/unreleased/51668-fix-line-numbers.yml @@ -0,0 +1,5 @@ +--- +title: Adjust line-height of blame view line numbers +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/52620-fix-loader-animation-alignment.yml b/changelogs/unreleased/52620-fix-loader-animation-alignment.yml new file mode 100644 index 00000000000..5cfb7fc019f --- /dev/null +++ b/changelogs/unreleased/52620-fix-loader-animation-alignment.yml @@ -0,0 +1,5 @@ +--- +title: Aligns build loader animation with the job log +merge_request: 23959 +author: +type: fixed diff --git a/changelogs/unreleased/fix-55448.yml b/changelogs/unreleased/fix-55448.yml new file mode 100644 index 00000000000..e0bdbb6eda4 --- /dev/null +++ b/changelogs/unreleased/fix-55448.yml @@ -0,0 +1,5 @@ +--- +title: Remove deprecated xhr from specs +merge_request: 23949 +author: Jasper Maes +type: other diff --git a/changelogs/unreleased/winh-discussion-header-commented.yml b/changelogs/unreleased/winh-discussion-header-commented.yml new file mode 100644 index 00000000000..8d08409b504 --- /dev/null +++ b/changelogs/unreleased/winh-discussion-header-commented.yml @@ -0,0 +1,5 @@ +--- +title: Display "commented" only for commit discussions on merge requests +merge_request: 23622 +author: +type: fixed diff --git a/changelogs/unreleased/winh-merge-request-commit-context.yml b/changelogs/unreleased/winh-merge-request-commit-context.yml new file mode 100644 index 00000000000..9e12a926af4 --- /dev/null +++ b/changelogs/unreleased/winh-merge-request-commit-context.yml @@ -0,0 +1,5 @@ +--- +title: Display commit ID for discussions made on merge request commits +merge_request: 23837 +author: +type: fixed diff --git a/doc/user/project/clusters/serverless/index.md b/doc/user/project/clusters/serverless/index.md index c1f2e844362..a1e5735670a 100644 --- a/doc/user/project/clusters/serverless/index.md +++ b/doc/user/project/clusters/serverless/index.md @@ -31,7 +31,7 @@ To run Knative on Gitlab, you will need: external IP address for all the applications served by Knative. You will be prompted to enter a wildcard domain where your applications will be served. Configure your DNS server to use the external IP address for that domain. -1. **`gitlab-ci.yml`:** GitLab uses [Kaniko](https://github.com/GoogleContainerTools/kaniko) +1. **`.gitlab-ci.yml`:** GitLab uses [Kaniko](https://github.com/GoogleContainerTools/kaniko) to build the application and the [TriggerMesh CLI](https://github.com/triggermesh/tm) to simplify the deployment of knative services and functions. 1. **`serverless.yml`** (for [functions only](#deploying-functions)): When using serverless to deploy functions, the `serverless.yml` file @@ -81,7 +81,7 @@ you to focus on a single task that can be executed/scaled automatically and inde In order to deploy functions to your Knative instance, the following templates must be present: -1. `gitlab-ci.yml`: This template allows to define the stage, environment, and +1. `.gitlab-ci.yml`: This template allows to define the stage, environment, and image to be used for your functions. It must be included at the root of your repository: ```yaml diff --git a/locale/gitlab.pot b/locale/gitlab.pot index fed490309cc..776cd9d03d4 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4126,6 +4126,9 @@ msgstr "" msgid "MergeRequests|View replaced file @ %{commitId}" msgstr "" +msgid "MergeRequests|commented on commit %{linkStart}%{commitId}%{linkEnd}" +msgstr "" + msgid "MergeRequests|started a discussion" msgstr "" @@ -4135,6 +4138,9 @@ msgstr "" msgid "MergeRequests|started a discussion on %{linkStart}the diff%{linkEnd}" msgstr "" +msgid "MergeRequests|started a discussion on an outdated change in commit %{linkStart}%{commitId}%{linkEnd}" +msgstr "" + msgid "MergeRequests|started a discussion on commit %{linkStart}%{commitId}%{linkEnd}" msgstr "" diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index 00a00306ff9..ed38dadfd6b 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -78,9 +78,11 @@ describe Groups::GroupMembersController do Gitlab::Access.options.each do |label, value| it "can change the access level to #{label}" do - xhr :put, :update, group_member: { access_level: value }, - group_id: group, - id: requester + put :update, params: { + group_member: { access_level: value }, + group_id: group, + id: requester + }, xhr: true expect(requester.reload.human_access).to eq(label) end @@ -130,7 +132,7 @@ describe Groups::GroupMembersController do end it '[JS] removes user from members' do - xhr :delete, :destroy, group_id: group, id: member + delete :destroy, params: { group_id: group, id: member }, xhr: true expect(response).to be_success expect(group.members).not_to include member diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index f9193513a6a..a239ac16c0d 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -206,12 +206,12 @@ describe Projects::IssuesController do describe 'Redirect after sign in' do context 'with an AJAX request' do it 'does not store the visited URL' do - xhr :get, - :show, + get :show, params: { format: :json, namespace_id: project.namespace, project_id: project, id: issue.iid + }, xhr: true expect(session['user_return_to']).to be_blank end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 962dfb91c9f..1ab227bde39 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -333,7 +333,7 @@ describe Projects::MergeRequestsController do before do project.add_reporter(user) - xhr :post, :merge, params: base_params + post :merge, params: base_params, xhr: true end it 'returns 404' do @@ -681,13 +681,14 @@ describe Projects::MergeRequestsController do merge_request.title = merge_request.wip_title merge_request.save - xhr :post, :remove_wip, - format: :json, + post :remove_wip, params: { + format: :json, namespace_id: merge_request.project.namespace.to_param, project_id: merge_request.project, id: merge_request.iid - } + }, + xhr: true end it 'removes the wip status' do @@ -701,13 +702,14 @@ describe Projects::MergeRequestsController do describe 'POST cancel_merge_when_pipeline_succeeds' do subject do - xhr :post, :cancel_merge_when_pipeline_succeeds, - format: :json, + post :cancel_merge_when_pipeline_succeeds, params: { + format: :json, namespace_id: merge_request.project.namespace.to_param, project_id: merge_request.project, id: merge_request.iid - } + }, + xhr: true end it 'calls MergeRequests::MergeWhenPipelineSucceedsService' do diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 009f85903b5..3cc3fe69fba 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -82,10 +82,12 @@ describe Projects::ProjectMembersController do Gitlab::Access.options.each do |label, value| it "can change the access level to #{label}" do - xhr :put, :update, project_member: { access_level: value }, - namespace_id: project.namespace, - project_id: project, - id: requester + put :update, params: { + project_member: { access_level: value }, + namespace_id: project.namespace, + project_id: project, + id: requester + }, xhr: true expect(requester.reload.human_access).to eq(label) end @@ -148,9 +150,11 @@ describe Projects::ProjectMembersController do end it '[JS] removes user from members' do - xhr :delete, :destroy, namespace_id: project.namespace, - project_id: project, - id: member + delete :destroy, params: { + namespace_id: project.namespace, + project_id: project, + id: member + }, xhr: true expect(response).to be_success expect(project.members).not_to include member diff --git a/spec/controllers/projects/refs_controller_spec.rb b/spec/controllers/projects/refs_controller_spec.rb index ea299e1e8c5..62f2af947e4 100644 --- a/spec/controllers/projects/refs_controller_spec.rb +++ b/spec/controllers/projects/refs_controller_spec.rb @@ -22,13 +22,13 @@ describe Projects::RefsController do end def xhr_get(format = :html) - xhr :get, - :logs_tree, - namespace_id: project.namespace.to_param, - project_id: project, - id: 'master', - path: 'foo/bar/baz.html', - format: format + get :logs_tree, params: { + namespace_id: project.namespace.to_param, + project_id: project, + id: 'master', + path: 'foo/bar/baz.html', + format: format + }, xhr: true end it 'never throws MissingTemplate' do diff --git a/spec/features/merge_request/user_sees_discussions_spec.rb b/spec/features/merge_request/user_sees_discussions_spec.rb index 4ab9a87ad4b..d130ea05654 100644 --- a/spec/features/merge_request/user_sees_discussions_spec.rb +++ b/spec/features/merge_request/user_sees_discussions_spec.rb @@ -88,5 +88,13 @@ describe 'Merge request > User sees discussions', :js do expect(page).to have_content "started a discussion on commit #{note.commit_id[0...7]}" end end + + context 'a commit non-diff discussion' do + let(:note) { create(:discussion_note_on_commit, project: project) } + + it 'displays correct header' do + expect(page).to have_content "commented on commit #{note.commit_id[0...7]}" + end + end end end diff --git a/spec/helpers/notes_helper_spec.rb b/spec/helpers/notes_helper_spec.rb index 21461e46cf4..0715f34dafe 100644 --- a/spec/helpers/notes_helper_spec.rb +++ b/spec/helpers/notes_helper_spec.rb @@ -185,8 +185,8 @@ describe NotesHelper do context 'for a non-diff discussion' do let(:discussion) { create(:discussion_note_on_commit, project: project).to_discussion } - it 'returns the commit path' do - expect(helper.discussion_path(discussion)).to eq(project_commit_path(project, commit)) + it 'returns the commit path with the note anchor' do + expect(helper.discussion_path(discussion)).to eq(project_commit_path(project, commit, anchor: "note_#{discussion.first_note.id}")) end end end diff --git a/spec/javascripts/notes/components/noteable_discussion_spec.js b/spec/javascripts/notes/components/noteable_discussion_spec.js index 106a4ac2546..3aff2dd0641 100644 --- a/spec/javascripts/notes/components/noteable_discussion_spec.js +++ b/spec/javascripts/notes/components/noteable_discussion_spec.js @@ -133,8 +133,10 @@ describe('noteable_discussion component', () => { }); }); - describe('commit discussion', () => { + describe('action text', () => { const commitId = 'razupaltuff'; + const truncatedCommitId = commitId.substr(0, 8); + let commitElement; beforeEach(() => { vm.$destroy(); @@ -143,7 +145,6 @@ describe('noteable_discussion component', () => { projectPath: 'something', }; - vm.$destroy(); vm = new Component({ propsData: { discussion: { @@ -159,17 +160,73 @@ describe('noteable_discussion component', () => { }, store, }).$mount(); + + commitElement = vm.$el.querySelector('.commit-sha'); + }); + + describe('for commit discussions', () => { + it('should display a monospace started a discussion on commit', () => { + expect(vm.$el).toContainText(`started a discussion on commit ${truncatedCommitId}`); + expect(commitElement).not.toBe(null); + expect(commitElement).toHaveText(truncatedCommitId); + }); }); - it('displays a monospace started a discussion on commit', () => { - const truncatedCommitId = commitId.substr(0, 8); + describe('for diff discussion with a commit id', () => { + it('should display started discussion on commit header', done => { + vm.discussion.for_commit = false; - expect(vm.$el).toContainText(`started a discussion on commit ${truncatedCommitId}`); + vm.$nextTick(() => { + expect(vm.$el).toContainText(`started a discussion on commit ${truncatedCommitId}`); + expect(commitElement).not.toBe(null); - const commitElement = vm.$el.querySelector('.commit-sha'); + done(); + }); + }); - expect(commitElement).not.toBe(null); - expect(commitElement).toHaveText(truncatedCommitId); + it('should display outdated change on commit header', done => { + vm.discussion.for_commit = false; + vm.discussion.active = false; + + vm.$nextTick(() => { + expect(vm.$el).toContainText( + `started a discussion on an outdated change in commit ${truncatedCommitId}`, + ); + + expect(commitElement).not.toBe(null); + + done(); + }); + }); + }); + + describe('for diff discussions without a commit id', () => { + it('should show started a discussion on the diff text', done => { + Object.assign(vm.discussion, { + for_commit: false, + commit_id: null, + }); + + vm.$nextTick(() => { + expect(vm.$el).toContainText('started a discussion on the diff'); + + done(); + }); + }); + + it('should show discussion on older version text', done => { + Object.assign(vm.discussion, { + for_commit: false, + commit_id: null, + active: false, + }); + + vm.$nextTick(() => { + expect(vm.$el).toContainText('started a discussion on an old version of the diff'); + + done(); + }); + }); }); }); }); |