From 03ea267f67ba749673037edf9d210eda4e918a99 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 29 Jul 2016 15:50:58 +0100 Subject: [PATCH] Added tests for hidden jump to button --- .../components/jump_to_discussion.js.es6 | 2 +- .../diff_notes/components/resolve_btn.js.es6 | 2 ++ .../diff_notes/services/resolve.js.es6 | 26 ++++++++++++------- .../merge_requests/diff_notes_resolve_spec.rb | 14 ++++++++++ 4 files changed, 34 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/diff_notes/components/jump_to_discussion.js.es6 b/app/assets/javascripts/diff_notes/components/jump_to_discussion.js.es6 index 004098a786a..82cdc5a7772 100644 --- a/app/assets/javascripts/diff_notes/components/jump_to_discussion.js.es6 +++ b/app/assets/javascripts/diff_notes/components/jump_to_discussion.js.es6 @@ -17,7 +17,7 @@ return Object.keys(this.discussions).length; }, showButton: function () { - return this.discussionsCount > 1 || !this.discussionId; + return this.discussionsCount > 0 && (this.discussionsCount > 1 || !this.discussionId); } }, methods: { diff --git a/app/assets/javascripts/diff_notes/components/resolve_btn.js.es6 b/app/assets/javascripts/diff_notes/components/resolve_btn.js.es6 index c8cf6b8ad3a..01f51d2c20c 100644 --- a/app/assets/javascripts/diff_notes/components/resolve_btn.js.es6 +++ b/app/assets/javascripts/diff_notes/components/resolve_btn.js.es6 @@ -71,6 +71,8 @@ CommentsStore.update(this.discussionId, this.noteId, !this.isResolved, user); ResolveService.updateUpdatedHtml(this.discussionId, data); + } else { + new Flash('An error occurred when trying to resolve a comment. Please try again.', 'alert'); } this.$nextTick(this.updateTooltip); diff --git a/app/assets/javascripts/diff_notes/services/resolve.js.es6 b/app/assets/javascripts/diff_notes/services/resolve.js.es6 index d929fba06b4..d15959c0909 100644 --- a/app/assets/javascripts/diff_notes/services/resolve.js.es6 +++ b/app/assets/javascripts/diff_notes/services/resolve.js.es6 @@ -45,12 +45,16 @@ mergeRequestId, discussionId }, {}).then((response) => { - const data = response.data; - const user = data ? data.resolved_by : null; - discussion.resolveAllNotes(user); - discussion.loading = false; + if (response.status === 200) { + const data = response.data; + const user = data ? data.resolved_by : null; + discussion.resolveAllNotes(user); + discussion.loading = false; - this.updateUpdatedHtml(discussionId, data); + this.updateUpdatedHtml(discussionId, data); + } else { + new Flash('An error occurred when trying to resolve a discussion. Please try again.', 'alert'); + } }); } @@ -66,11 +70,15 @@ mergeRequestId, discussionId }, {}).then((response) => { - const data = response.data; - discussion.unResolveAllNotes(); - discussion.loading = false; + if (response.status === 200) { + const data = response.data; + discussion.unResolveAllNotes(); + discussion.loading = false; - this.updateUpdatedHtml(discussionId, data); + this.updateUpdatedHtml(discussionId, data); + } else { + new Flash('An error occurred when trying to unresolve a discussion. Please try again.', 'alert'); + } }); } diff --git a/spec/features/merge_requests/diff_notes_resolve_spec.rb b/spec/features/merge_requests/diff_notes_resolve_spec.rb index a98f795b195..cdfaffb19f9 100644 --- a/spec/features/merge_requests/diff_notes_resolve_spec.rb +++ b/spec/features/merge_requests/diff_notes_resolve_spec.rb @@ -16,6 +16,20 @@ feature 'Diff notes resolve', feature: true, js: true do ) end + context 'no discussions' do + before do + project.team << [user, :master] + login_as user + note.destroy + visit_merge_request + end + + it 'displays no discussion resolved data' do + expect(page).not_to have_content('discussion resolved') + expect(page).not_to have_selector('.discussion-next-btn') + end + end + context 'as authorized user' do before do project.team << [user, :master]