Merge branch 'improve-discussions-feature-attempt-1' into 'master'

Improve discussion_comments_spec perf

See merge request !10648

5 files changed, 78 insertions, 94 deletions

diff --git a/spec/features/discussion_comments/commit_spec.rb b/spec/features/discussion_comments/commit_spec.rb
new file mode 100644
index 00000000000..96e0b78f6b9
--- /dev/null
+++ b/spec/features/discussion_comments/commit_spec.rb
@@ -0,0 +1,18 @@
+require 'spec_helper'
+
+describe 'Discussion Comments Merge Request', :feature, :js do
+  include RepoHelpers
+
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+  let(:merge_request) { create(:merge_request, source_project: project) }
+
+  before do
+    project.add_master(user)
+    login_as(user)
+
+    visit namespace_project_commit_path(project.namespace, project, sample_commit.id)
+  end
+
+  it_behaves_like 'discussion comments', 'commit'
+end
diff --git a/spec/features/discussion_comments/issue_spec.rb b/spec/features/discussion_comments/issue_spec.rb
new file mode 100644
index 00000000000..ccc9efccd18
--- /dev/null
+++ b/spec/features/discussion_comments/issue_spec.rb
@@ -0,0 +1,16 @@
+require 'spec_helper'
+
+describe 'Discussion Comments Issue', :feature, :js do
+  let(:user) { create(:user) }
+  let(:project) { create(:empty_project) }
+  let(:issue) { create(:issue, project: project) }
+
+  before do
+    project.add_master(user)
+    login_as(user)
+
+    visit namespace_project_issue_path(project.namespace, project, issue)
+  end
+
+  it_behaves_like 'discussion comments', 'issue'
+end
diff --git a/spec/features/discussion_comments/merge_request_spec.rb b/spec/features/discussion_comments/merge_request_spec.rb
new file mode 100644
index 00000000000..f99ebeb9cd9
--- /dev/null
+++ b/spec/features/discussion_comments/merge_request_spec.rb
@@ -0,0 +1,16 @@
+require 'spec_helper'
+
+describe 'Discussion Comments Merge Request', :feature, :js do
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+  let(:merge_request) { create(:merge_request, source_project: project) }
+
+  before do
+    project.add_master(user)
+    login_as(user)
+
+    visit namespace_project_merge_request_path(project.namespace, project, merge_request)
+  end
+
+  it_behaves_like 'discussion comments', 'merge request'
+end
diff --git a/spec/features/discussion_comments/snippets_spec.rb b/spec/features/discussion_comments/snippets_spec.rb
new file mode 100644
index 00000000000..19a306511b2
--- /dev/null
+++ b/spec/features/discussion_comments/snippets_spec.rb
@@ -0,0 +1,16 @@
+require 'spec_helper'
+
+describe 'Discussion Comments Issue', :feature, :js do
+  let(:user) { create(:user) }
+  let(:project) { create(:empty_project) }
+  let(:snippet) { create(:project_snippet, :private, project: project, author: user) }
+
+  before do
+    project.add_master(user)
+    login_as(user)
+
+    visit namespace_project_snippet_path(project.namespace, project, snippet)
+  end
+
+  it_behaves_like 'discussion comments', 'snippet'
+end
diff --git a/spec/features/discussion_comments_spec.rb b/spec/support/features/discussion_comments_shared_example.rb
index ae778118c5c..1a061ef069e 100644
--- a/spec/features/discussion_comments_spec.rb
+++ b/spec/support/features/discussion_comments_shared_example.rb
@@ -1,5 +1,3 @@
-require 'spec_helper'
-
 shared_examples 'discussion comments' do |resource_name|
   let(:form_selector) { '.js-main-target-form' }
   let(:dropdown_selector) { "#{form_selector} .comment-type-dropdown" }
@@ -9,11 +7,9 @@ shared_examples 'discussion comments' do |resource_name|
   let(:close_selector) { "#{form_selector} .btn-comment-and-close" }
   let(:comments_selector) { '.timeline > .note.timeline-entry' }
 
-  it 'should show a comment type toggle' do
+  it 'clicking "Comment" will post a comment' do
     expect(page).to have_selector toggle_selector
-  end
 
-  it 'clicking "Comment" will post a comment' do
     find("#{form_selector} .note-textarea").send_keys('a')
 
     find(submit_selector).click
@@ -49,44 +45,29 @@ shared_examples 'discussion comments' do |resource_name|
       find(toggle_selector).click
     end
 
-    it 'opens a comment type dropdown with "Comment" and "Start discussion"' do
+    it 'has a "Comment" item (selected by default) and "Start discussion" item' do
       expect(page).to have_selector menu_selector
-    end
-
-    it 'has a "Comment" item' do
-      menu = find(menu_selector)
-
-      expect(menu).to have_content 'Comment'
-      expect(menu).to have_content "Add a general comment to this #{resource_name}."
-    end
 
-    it 'has a "Start discussion" item' do
-      menu = find(menu_selector)
-
-      expect(menu).to have_content 'Start discussion'
-      expect(menu).to have_content "Discuss a specific suggestion or question#{' that needs to be resolved' if resource_name == 'merge request'}."
-    end
-
-    it 'has the "Comment" item selected by default' do
       find("#{menu_selector} li", match: :first)
       items = all("#{menu_selector} li")
 
       expect(items.first).to have_content 'Comment'
+      expect(items.first).to have_content "Add a general comment to this #{resource_name}."
       expect(items.first).to have_selector '.fa-check'
       expect(items.first['class']).to match 'droplab-item-selected'
 
       expect(items.last).to have_content 'Start discussion'
+      expect(items.last).to have_content "Discuss a specific suggestion or question#{' that needs to be resolved' if resource_name == 'merge request'}."
       expect(items.last).not_to have_selector '.fa-check'
       expect(items.last['class']).not_to match 'droplab-item-selected'
     end
 
-    it 'closes the menu when clicking the toggle' do
+    it 'closes the menu when clicking the toggle or body' do
       find(toggle_selector).click
 
       expect(page).not_to have_selector menu_selector
-    end
 
-    it 'closes the menu when clicking the body' do
+      find(toggle_selector).click
       find('body').click
 
       expect(page).not_to have_selector menu_selector
@@ -104,12 +85,10 @@ shared_examples 'discussion comments' do |resource_name|
         all("#{menu_selector} li").last.click
       end
 
-      it 'updates the note_type input to "DiscussionNote"' do
-        expect(find("#{form_selector} #note_type", visible: false).value).to eq('DiscussionNote')
-      end
-
-      it 'updates the submit button text' do
+      it 'updates the submit button text, note_type input and closes the dropdown' do
         expect(find(dropdown_selector)).to have_content 'Start discussion'
+        expect(find("#{form_selector} #note_type", visible: false).value).to eq('DiscussionNote')
+        expect(page).not_to have_selector menu_selector
       end
 
       if resource_name =~ /(issue|merge request)/
@@ -124,10 +103,6 @@ shared_examples 'discussion comments' do |resource_name|
         end
       end
 
-      it 'closes the dropdown' do
-        expect(page).not_to have_selector menu_selector
-      end
-
       it 'clicking "Start discussion" will post a discussion' do
         find(submit_selector).click
 
@@ -176,12 +151,10 @@ shared_examples 'discussion comments' do |resource_name|
             find("#{menu_selector} li", match: :first).click
           end
 
-          it 'clears the note_type input"' do
-            expect(find("#{form_selector} #note_type", visible: false).value).to eq('')
-          end
-
-          it 'updates the submit button text' do
+          it 'updates the submit button text, clears the note_type input and closes the dropdown' do
             expect(find(dropdown_selector)).to have_content 'Comment'
+            expect(find("#{form_selector} #note_type", visible: false).value).to eq('')
+            expect(page).not_to have_selector menu_selector
           end
 
           if resource_name =~ /(issue|merge request)/
@@ -196,10 +169,6 @@ shared_examples 'discussion comments' do |resource_name|
             end
           end
 
-          it 'closes the dropdown' do
-            expect(page).not_to have_selector menu_selector
-          end
-
           it 'should have "Comment" selected when opening the menu' do
             find(toggle_selector).click
 
@@ -242,54 +211,3 @@ shared_examples 'discussion comments' do |resource_name|
     end
   end
 end
-
-describe 'Discussion Comments', :feature, :js do
-  include RepoHelpers
-
-  let(:user) { create(:user) }
-  let(:project) { create(:project) }
-
-  before do
-    project.team << [user, :developer]
-
-    login_as(user)
-  end
-
-  describe 'on a merge request' do
-    let(:merge_request) { create(:merge_request, source_project: project) }
-
-    before do
-      visit namespace_project_merge_request_path(project.namespace, project, merge_request)
-    end
-
-    it_behaves_like 'discussion comments', 'merge request'
-  end
-
-  describe 'on an issue' do
-    let(:issue) { create(:issue, project: project) }
-
-    before do
-      visit namespace_project_issue_path(project.namespace, project, issue)
-    end
-
-    it_behaves_like 'discussion comments', 'issue'
-  end
-
-  describe 'on an snippet' do
-    let(:snippet) { create(:project_snippet, :private, project: project, author: user) }
-
-    before do
-      visit namespace_project_snippet_path(project.namespace, project, snippet)
-    end
-
-    it_behaves_like 'discussion comments', 'snippet'
-  end
-
-  describe 'on a commit' do
-    before do
-      visit namespace_project_commit_path(project.namespace, project, sample_commit.id)
-    end
-
-    it_behaves_like 'discussion comments', 'commit'
-  end
-end