From ca3d868c3731018e18c4b5931d5a37466a16fa6a Mon Sep 17 00:00:00 2001 From: Fatih Acet Date: Wed, 10 May 2017 23:05:25 +0300 Subject: [PATCH 1/5] MRWidget: Implement new design for nothing to commit state. --- .../states/mr_widget_nothing_to_merge.js | 51 +++++++++++++++---- .../stores/mr_widget_store.js | 1 + .../stylesheets/pages/merge_requests.scss | 14 +++++ app/serializers/merge_request_entity.rb | 6 +++ .../shared/icons/_mr_widget_empty_state.svg | 1 + 5 files changed, 62 insertions(+), 11 deletions(-) create mode 100644 app/views/shared/icons/_mr_widget_empty_state.svg diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_nothing_to_merge.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_nothing_to_merge.js index 8c4535f1337..1d9c30389e5 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_nothing_to_merge.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_nothing_to_merge.js @@ -1,17 +1,46 @@ +import emptyStateSVG from 'icons/_mr_widget_empty_state.svg'; + export default { name: 'MRWidgetNothingToMerge', + props: { + mr: { + type: Object, + default: false, + }, + }, + data() { + return { + canCreateNewFile: true, + emptyStateSVG, + }; + }, template: ` -
- - - There is nothing to merge from source branch into target branch. - Please push new commits or use a different branch. - +
+
+
+ +
+
+ + Merge requests are a place to propose changes you have made to a project + and discuss those changes with others. + +

+ Interested parties can even contribute by pushing commits if they want to. +

+

+ Currently there are no changes in this merge request's source branch. + Please push new commits or use a different branch. +

+ + Create file + +
+
`, }; diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js index 05e67706983..06661b930e3 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js @@ -58,6 +58,7 @@ export default class MergeRequestStore { this.statusPath = data.status_path; this.emailPatchesPath = data.email_patches_path; this.plainDiffPath = data.plain_diff_path; + this.newBlobPath = data.new_blob_path; this.createIssueToResolveDiscussionsPath = data.create_issue_to_resolve_discussions_path; this.mergeCheckPath = data.merge_check_path; this.mergeActionsContentPath = data.commit_change_content_path; diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index d208e54207e..b66b43c6b43 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -349,6 +349,20 @@ margin-top: 10px; margin-left: 12px; } + + &.empty-state { + .artwork { + margin-bottom: $gl-padding; + } + .text { + span { + font-weight: bold; + } + p { + margin-top: $gl-padding; + } + } + } } .mr-widget-footer { diff --git a/app/serializers/merge_request_entity.rb b/app/serializers/merge_request_entity.rb index a2542c54f7a..d31057c030c 100644 --- a/app/serializers/merge_request_entity.rb +++ b/app/serializers/merge_request_entity.rb @@ -97,6 +97,12 @@ class MergeRequestEntity < IssuableEntity presenter(merge_request).target_branch_commits_path end + expose :new_blob_path do |merge_request| + namespace_project_new_blob_path(merge_request.project.namespace, + merge_request.project, + merge_request.source_branch) + end + expose :conflict_resolution_path do |merge_request| presenter(merge_request).conflict_resolution_path end diff --git a/app/views/shared/icons/_mr_widget_empty_state.svg b/app/views/shared/icons/_mr_widget_empty_state.svg new file mode 100644 index 00000000000..6a811893b2d --- /dev/null +++ b/app/views/shared/icons/_mr_widget_empty_state.svg @@ -0,0 +1 @@ +illustration From 3f6121f2f57fab49326a2082a14dae249a91ac9d Mon Sep 17 00:00:00 2001 From: Oswaldo Ferreira Date: Wed, 10 May 2017 20:29:00 -0300 Subject: [PATCH 2/5] Returns new_blob_path only when user can push_code to project --- app/serializers/merge_request_entity.rb | 8 +++++--- spec/serializers/merge_request_entity_spec.rb | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/app/serializers/merge_request_entity.rb b/app/serializers/merge_request_entity.rb index d31057c030c..a49f4d834cd 100644 --- a/app/serializers/merge_request_entity.rb +++ b/app/serializers/merge_request_entity.rb @@ -98,9 +98,11 @@ class MergeRequestEntity < IssuableEntity end expose :new_blob_path do |merge_request| - namespace_project_new_blob_path(merge_request.project.namespace, - merge_request.project, - merge_request.source_branch) + if can?(current_user, :push_code, merge_request.project) + namespace_project_new_blob_path(merge_request.project.namespace, + merge_request.project, + merge_request.source_branch) + end end expose :conflict_resolution_path do |merge_request| diff --git a/spec/serializers/merge_request_entity_spec.rb b/spec/serializers/merge_request_entity_spec.rb index bb6e83ae4bd..b75c73e78c2 100644 --- a/spec/serializers/merge_request_entity_spec.rb +++ b/spec/serializers/merge_request_entity_spec.rb @@ -65,6 +65,23 @@ describe MergeRequestEntity do .to eq(resource.merge_commit_message(include_description: true)) end + describe 'new_blob_path' do + context 'when user can push to project' do + it 'returns path' do + project.add_developer(user) + + expect(subject[:new_blob_path]) + .to eq("/#{resource.project.full_path}/new/#{resource.source_branch}") + end + end + + context 'when user cannot push to project' do + it 'returns nil' do + expect(subject[:new_blob_path]).to be_nil + end + end + end + describe 'diff_head_sha' do before do allow(resource).to receive(:diff_head_sha) { 'sha' } From a2dbb69331ad13c77986bc2c4ed64298d5bc1388 Mon Sep 17 00:00:00 2001 From: Oswaldo Ferreira Date: Wed, 10 May 2017 20:35:19 -0300 Subject: [PATCH 3/5] Add new_blob_path to MR json schema --- spec/fixtures/api/schemas/entities/merge_request.json | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/fixtures/api/schemas/entities/merge_request.json b/spec/fixtures/api/schemas/entities/merge_request.json index 0a7e0e2d5f2..e5df3e7b6d1 100644 --- a/spec/fixtures/api/schemas/entities/merge_request.json +++ b/spec/fixtures/api/schemas/entities/merge_request.json @@ -86,6 +86,7 @@ "email_patches_path": { "type": "string" }, "plain_diff_path": { "type": "string" }, "status_path": { "type": "string" }, + "new_blob_path": { "type": "string" }, "merge_check_path": { "type": "string" }, "ci_environments_status_path": { "type": "string" }, "merge_commit_message_with_description": { "type": "string" }, From 9f8000790426e6c013dcb828c997a7dd72f5e3e1 Mon Sep 17 00:00:00 2001 From: Fatih Acet Date: Fri, 12 May 2017 14:47:58 +0300 Subject: [PATCH 4/5] MRWidget: Fix specs and address WIP comments. --- .../states/mr_widget_nothing_to_merge.js | 11 ++++------- app/assets/stylesheets/pages/merge_requests.scss | 2 ++ .../states/mr_widget_nothing_to_merge_spec.js | 16 ++++++++++++++-- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_nothing_to_merge.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_nothing_to_merge.js index 1d9c30389e5..c314776d9bb 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_nothing_to_merge.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_nothing_to_merge.js @@ -5,20 +5,17 @@ export default { props: { mr: { type: Object, - default: false, + required: true, }, }, data() { - return { - canCreateNewFile: true, - emptyStateSVG, - }; + return { emptyStateSVG }; }, template: `
- +
@@ -33,7 +30,7 @@ export default { Please push new commits or use a different branch.

diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index b66b43c6b43..16fa03efcb2 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -354,10 +354,12 @@ .artwork { margin-bottom: $gl-padding; } + .text { span { font-weight: bold; } + p { margin-top: $gl-padding; } diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_nothing_to_merge_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_nothing_to_merge_spec.js index d40c67b189d..0d632c7f4e5 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_nothing_to_merge_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_nothing_to_merge_spec.js @@ -4,14 +4,26 @@ import nothingToMergeComponent from '~/vue_merge_request_widget/components/state describe('MRWidgetNothingToMerge', () => { describe('template', () => { const Component = Vue.extend(nothingToMergeComponent); + const newBlobPath = '/foo'; const vm = new Component({ el: document.createElement('div'), + propsData: { + mr: { newBlobPath }, + }, }); + it('should have correct elements', () => { expect(vm.$el.classList.contains('mr-widget-body')).toBeTruthy(); - expect(vm.$el.querySelector('button').getAttribute('disabled')).toBeTruthy(); - expect(vm.$el.innerText).toContain('There is nothing to merge from source branch into target branch.'); + expect(vm.$el.querySelector('a').href).toContain(newBlobPath); + expect(vm.$el.innerText).toContain("Currently there are no changes in this merge request's source branch"); expect(vm.$el.innerText).toContain('Please push new commits or use a different branch.'); }); + + it('should not show new blob link if there is no link available', () => { + vm.mr.newBlobPath = null; + Vue.nextTick(() => { + expect(vm.$el.querySelector('a')).toEqual(null); + }); + }); }); }); From 48a4a934c47dbfd43e03aa61e177c936b0d8d016 Mon Sep 17 00:00:00 2001 From: Fatih Acet Date: Tue, 16 May 2017 15:11:27 +0300 Subject: [PATCH 5/5] Address MR comments. --- .../components/states/mr_widget_nothing_to_merge.js | 1 - .../components/states/mr_widget_nothing_to_merge_spec.js | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_nothing_to_merge.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_nothing_to_merge.js index c314776d9bb..375a382615a 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_nothing_to_merge.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_nothing_to_merge.js @@ -32,7 +32,6 @@ export default { Create file diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_nothing_to_merge_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_nothing_to_merge_spec.js index 0d632c7f4e5..a8a02fa6b66 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_nothing_to_merge_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_nothing_to_merge_spec.js @@ -15,7 +15,7 @@ describe('MRWidgetNothingToMerge', () => { it('should have correct elements', () => { expect(vm.$el.classList.contains('mr-widget-body')).toBeTruthy(); expect(vm.$el.querySelector('a').href).toContain(newBlobPath); - expect(vm.$el.innerText).toContain("Currently there are no changes in this merge request's source branch"); + expect(vm.$el.innerText).toContain('Currently there are no changes in this merge request\'s source branch'); expect(vm.$el.innerText).toContain('Please push new commits or use a different branch.'); });