Merge pull request #179 from edx/clintonb/spec-cleanup

Updated specs

spec/api/comment_thread_spec.rb

@@ -103,7 +103,7 @@ describe "app" do
           @threads["t1"].group_id = 123
           @threads["t1"].save!
           rs = thread_result course_id: DFLT_COURSE_ID, group_id: 321
-          rs.each.map {|res| res["group_id"].should be_nil }
+          rs.each.map { |res| res["group_id"].should be_nil }
         end
         context "when filtering flagged posts" do
           it "returns threads that are flagged" do
@@ -231,19 +231,19 @@ describe "app" do
           def thread_result_order (sort_key, sort_order)
             results = thread_result course_id: DFLT_COURSE_ID, sort_key: sort_key, sort_order: sort_order
             results.length.should == 10
-            results.map {|t| t["title"]}
+            results.map { |t| t["title"] }
           end
 
           def move_to_end(ary, *vals)
             vals.each do |val|
-              ary = ary.select {|v| v!=val } << val
+              ary = ary.select { |v| v!=val } << val
             end
             ary
           end
 
           def move_to_front(ary, *vals)
             vals.reverse.each do |val|
-              ary = ary.select {|v| v!=val }.insert(0, val)
+              ary = ary.select { |v| v!=val }.insert(0, val)
             end
             ary
           end
@@ -383,7 +383,7 @@ describe "app" do
                   result["num_pages"].should == num_pages
                 end
                 result["page"].should == page
-                actual_order += result["collection"].map {|v| v["title"]}
+                actual_order += result["collection"].map { |v| v["title"] }
               end
               actual_order.should == expected_order
             end
@@ -393,7 +393,7 @@ describe "app" do
               @threads["t7"].pinned = true
               @threads["t7"].save!
               expected_order = move_to_front(move_to_end(@default_order, "t5"), "t7")
-              test_paged_order({'sort_key'=>'comments', 'sort_dir'=>'asc', 'per_page'=>3}, expected_order)
+              test_paged_order({'sort_key' => 'comments', 'sort_dir' => 'asc', 'per_page' => 3}, expected_order)
             end
 
             it "orders correctly acrosss pages with unread filter" do
@@ -405,7 +405,7 @@ describe "app" do
               @threads["t7"].save!
               expected_order = move_to_front(move_to_end(@default_order[1..8], "t5"), "t7")
               test_paged_order(
-                {'sort_key'=>'comments', 'sort_dir'=>'asc', 'per_page'=>3},
+                  {'sort_key' => 'comments', 'sort_dir' => 'asc', 'per_page' => 3},
                   expected_order,
                   ["unread"],
                   user.id
@@ -417,9 +417,9 @@ describe "app" do
       end
 
       def test_unicode_data(text)
-        course_id = "unicode_course"
-        thread = make_thread(User.first, text, course_id, "unicode_commentable")
-        make_comment(User.first, thread, text)
+        course_id = 'unicode_course'
+        thread = create(:comment_thread, body: text, course_id: course_id)
+        create(:comment, comment_thread: thread, body: text)
         result = thread_result(course_id: course_id).first
         check_thread_result_json(nil, thread, result)
       end
@@ -427,39 +427,45 @@ describe "app" do
       include_examples "unicode data"
     end
 
-    describe "GET /api/v1/threads/:thread_id" do
-
-      before(:each) { init_without_subscriptions }
+    describe 'GET /api/v1/threads/:thread_id' do
+      let(:thread) do
+        comment = create(:comment)
+        comment.comment_thread
+      end
 
-      it "returns JSON" do
-        thread = CommentThread.first
+      subject do
         get "/api/v1/threads/#{thread.id}"
-        last_response.should be_ok
-        last_response.content_type.should == "application/json;charset=utf-8"
       end
 
-      it "get information of a single comment thread" do
-        thread = CommentThread.first
-        get "/api/v1/threads/#{thread.id}"
-        last_response.should be_ok
-        response_thread = parse last_response.body
-        check_thread_result_json(nil, thread, response_thread)
+      it { should be_ok }
+
+      it 'returns JSON' do
+        expect(subject.content_type).to eq 'application/json;charset=utf-8'
       end
 
-      it "computes endorsed correctly" do
-        thread = CommentThread.first
-        comment = thread.root_comments[1]
+      it 'get information of a single comment thread' do
+        check_thread_result_json(nil, thread, parse(subject.body))
+      end
+
+      it 'computes endorsed correctly' do
+        comment = thread.root_comments[0]
         comment.endorsed = true
         comment.save!
-        get "/api/v1/threads/#{thread.id}"
-        last_response.should be_ok
-        response_thread = parse last_response.body
-        response_thread["endorsed"].should == true
-        # re-request the thread from the database before checking it.
-        thread = CommentThread.find(thread.id)
-        check_thread_result_json(nil, thread, response_thread)
+
+        expect(subject).to be_ok
+        parsed = parse(subject.body)
+        expect(parsed).to include('endorsed' => true)
+        thread.reload
+        check_thread_result_json(nil, thread, parsed)
+      end
+
+      context 'when marking as read' do
+        subject do
+          get "/api/v1/threads/#{thread.id}", {:user_id => thread.author.id, :mark_as_read => true}
         end
 
+        it { should be_ok }
+
         # This is a test to ensure that the username is included even if the
         # thread's author is the one looking at the comment. This is because of a
         # regression in which we used User.only(:id, :read_states). This worked
@@ -467,67 +473,72 @@ describe "app" do
         # missing the username and was not refetched.
         # BBEGGS - Note 8/4/2015: Identify map has been removed during the mongoid 4.x upgrade.
         # Should no longer be an issue.
-      it "includes the username even if the thread is being marked as read for the thread author" do
-        thread = CommentThread.first
-        expected_username = thread.author.username
-        get "/api/v1/threads/#{thread.id}", {:user_id => thread.author_id, :mark_as_read => true}
-        last_response.should be_ok
-        response_thread = parse last_response.body
-        response_thread["username"].should == expected_username
+        it 'includes the username even if the thread is being marked as read for the thread author' do
+          expect(parse(subject.body)).to include('username' => thread.author.username)
+        end
       end
 
-      it "get information of a single comment thread with its comments" do
-        thread = CommentThread.first
+      context 'with comments' do
+        subject do
           get "/api/v1/threads/#{thread.id}", recursive: true
-        last_response.should be_ok
-        check_thread_result_json(nil, thread, parse(last_response.body))
-        check_thread_response_paging_json(thread, parse(last_response.body))
         end
 
-      it "returns 404 when the thread does not exist" do
-        thread = CommentThread.first
-        path = "/api/v1/threads/#{thread.id}"
-        get path
-        last_response.should be_ok
+        it { should be_ok }
+
+        it 'get information of a single comment thread with its comments' do
+          parsed = parse(subject.body)
+          check_thread_result_json(nil, thread, parsed)
+          check_thread_response_paging_json(thread, parsed)
+        end
+      end
+
+      it 'returns 404 when the thread does not exist' do
         thread.destroy
-        get path
-        last_response.status.should == 404
-        parse(last_response.body).first.should == I18n.t(:requested_object_not_found)
+        expect(subject.status).to eq 404
+        expect(parse(last_response.body).first).to eq I18n.t(:requested_object_not_found)
       end
 
-      it "marks thread as read and confirms its value on returned response" do
-        user = create_test_user(123)
-        thread = CommentThread.first
+      context 'with user specified' do
+        let(:user) { create(:user) }
+
+        subject do
           user.mark_as_read(thread)
           get "/api/v1/threads/#{thread.id}", user_id: user.id
-        last_response.should be_ok
-        json_response = parse(last_response.body)
-        changed_thread = CommentThread.find(thread.id)
-        check_thread_result_json(user, changed_thread, json_response)
-        json_response["read"].should == true
+          last_response
+        end
+
+        it { should be_ok }
+
+        it 'marks thread as read and confirms its value on returned response' do
+          parsed = parse(subject.body)
+          thread.reload
+          check_thread_result_json(user, thread, parsed)
+          expect(parsed).to include('read' => true)
+        end
       end
 
       def test_unicode_data(text)
-        thread = make_thread(User.first, text, "unicode_course", "unicode_commentable")
-        make_comment(User.first, thread, text)
+        thread = create(:comment_thread, body: text)
+        create(:comment, comment_thread: thread, body: text)
+
         get "/api/v1/threads/#{thread.id}", recursive: true
-        last_response.should be_ok
-        result = parse last_response.body
-        check_thread_result_json(nil, thread, result)
-        check_thread_response_paging_json(thread, result)
+        expect(last_response).to be_ok
+
+        parsed = parse(last_response.body)
+        check_thread_result_json(nil, thread, parsed)
+        check_thread_response_paging_json(thread, parsed)
       end
 
-      include_examples "unicode data"
+      include_examples 'unicode data'
 
       context "response pagination" do
-
         before(:each) do
           User.all.delete
           Content.all.delete
           @user = create_test_user(999)
           @threads = {}
           @comments = {}
-          [20,10,3,2,1,0].each do |n|
+          [20, 10, 3, 2, 1, 0].each do |n|
             thread_key = "t#{n}"
             thread = make_thread(@user, thread_key, DFLT_COURSE_ID, "pdq")
             @threads[n] = thread
@@ -676,7 +687,7 @@ describe "app" do
         last_response.should be_ok
         changed_thread = CommentThread.find(thread.id)
         changed_thread.comment_count.should == orig_count + 1
-        comment = changed_thread.comments.select{|c| c["body"] == "new comment"}.first
+        comment = changed_thread.comments.select { |c| c["body"] == "new comment" }.first
         comment.should_not be_nil
         comment.author_id.should == user.id
       end
@@ -688,7 +699,7 @@ describe "app" do
         last_response.should be_ok
         changed_thread = CommentThread.find(thread.id)
         changed_thread.comment_count.should == orig_count + 1
-        comment = changed_thread.comments.select{|c| c["body"] == "new comment"}.first
+        comment = changed_thread.comments.select { |c| c["body"] == "new comment" }.first
         comment.should_not be_nil
         comment.anonymous.should be_true
       end
@@ -720,18 +731,29 @@ describe "app" do
 
       include_examples "unicode data"
     end
-    describe "DELETE /api/v1/threads/:thread_id" do
-      before(:each) { init_without_subscriptions }
-      it "delete the comment thread and its comments" do
-        thread = CommentThread.first.to_hash
-        delete "/api/v1/threads/#{thread['id']}"
-        last_response.should be_ok
-        CommentThread.where(title: thread["title"]).first.should be_nil
+
+    describe 'DELETE /api/v1/threads/:thread_id' do
+      let(:thread) { create_comment_thread_and_comments }
+
+      subject { delete "/api/v1/threads/#{thread.id}" }
+
+      it { should be_ok }
+
+      it 'deletes the comment thread and its comments' do
+        expect(CommentThread.where(id: thread.id).count).to eq 1
+        expect(Comment.where(comment_thread: thread).count).to eq 2
+        subject
+        expect(CommentThread.where(id: thread.id).count).to eq 0
+        expect(Comment.where(comment_thread: thread).count).to eq 0
+      end
+
+      context 'when thread does not exist' do
+        subject { delete '/api/v1/threads/does_not_exist' }
+
+        it 'returns 400 when the thread does not exist' do
+          expect(subject.status).to eq 400
+          expect(parse(subject.body).first).to eq I18n.t(:requested_object_not_found)
         end
-      it "returns 400 when the thread does not exist" do
-        delete "/api/v1/threads/does_not_exist"
-        last_response.status.should == 400
-        parse(last_response.body).first.should == I18n.t(:requested_object_not_found)
       end
     end
   end

spec/api/commentable_spec.rb

 require 'spec_helper'
 require 'unicode_shared_examples'
 
-describe "app" do
-  describe "commentables" do
+describe 'app' do
+  describe 'commentables' do
+    before(:each) { set_api_key_header }
+    let(:commentable_id) { Faker::Lorem.word }
 
-    before(:each) do
-      init_without_subscriptions
-      set_api_key_header
+    describe 'DELETE /api/v1/:commentable_id/threads' do
+      it 'delete all associated threads and comments of a commentable' do
+        thread_count = 2
+        create_list(:comment_thread, thread_count, commentable_id: commentable_id)
+        expect(Commentable.find(commentable_id).comment_threads.count).to eq thread_count
+
+        delete "/api/v1/#{commentable_id}/threads"
+        expect(last_response).to be_ok
+        expect(Commentable.find(commentable_id).comment_threads.count).to eq 0
       end
 
-    describe "DELETE /api/v1/:commentable_id/threads" do
-      it "delete all associated threads and comments of a commentable" do
-        delete '/api/v1/question_1/threads'
-        last_response.should be_ok
-        Commentable.find("question_1").comment_threads.count.should == 0
+      context 'if the commentable does not exist' do
+        subject { delete '/api/v1/does_not_exist/threads' }
+
+        it { should be_ok }
       end
-      it "handle normally when commentable does not exist" do
-        delete '/api/v1/does_not_exist/threads'
-        last_response.should be_ok
+    end
+
+    describe 'GET /api/v1/:commentable_id/threads' do
+      let(:returned_threads) { parse(subject.body)['collection'] }
+      subject { get "/api/v1/#{commentable_id}/threads" }
+
+      shared_examples_for 'a filterable API endpoint' do
+        let!(:ignored_threads) { create_list(:comment_thread, 3, commentable_id: commentable_id) }
+        subject { get "/api/v1/#{commentable_id}/threads", parameters }
+
+        it { should be_ok }
+
+        it 'returns the correct CommentThreads' do
+          expect(returned_threads.length).to eq threads.length
+          threads.sort_by!(&:_id).reverse!
+          threads.each_with_index do |thread, index|
+            expect(returned_threads[index]).to include('id' => thread.id.to_s, 'body' => thread.body)
           end
         end
-    describe "GET /api/v1/:commentable_id/threads" do
-      def thread_result(commentable_id, params={})
-        get "/api/v1/#{commentable_id}/threads", params
-        last_response.should be_ok
-        parse(last_response.body)["collection"]
-      end
-      it "get all comment threads associated with a commentable object" do
-        threads = thread_result "question_1"
-        threads.length.should == 2
-        threads.index{|c| c["body"] == "can anyone help me?"}.should_not be_nil
-        threads.index{|c| c["body"] == "it is unsolvable"}.should_not be_nil
-      end
-      it "returns standalone threads if explicitly requested" do
-        threads = thread_result "question_1", context: "standalone"
-        threads.length.should == 1
-        threads[0]["body"].should == "no one can see us"
-      end
-      it "filters by course_id" do
-        course1_threads = thread_result "question_1", course_id: "1"
-        course1_threads.length.should == 1
-        course2_threads = thread_result "question_1", course_id: "2"
-        course2_threads.length.should == 1
-        course1_threads.should_not == course2_threads
-      end
-      it "filters by group_id" do
-        group_thread = Commentable.find("question_1").comment_threads.sort(_id: 1).first
-        threads = thread_result "question_1", group_id: 42
-        threads.length.should == 2
-        group_thread.group_id = 43
-        group_thread.save!
-        threads = thread_result "question_1", group_id: 42
-        threads.length.should == 1
-        group_thread.group_id = 42
-        group_thread.save!
-        threads = thread_result "question_1", group_id: 42
-        threads.length.should == 2
-      end
-      it "filters by group_ids" do
-        group_thread = Commentable.find("question_1").comment_threads.sort(_id: 1).first
-        group_thread.group_id = 42
-        group_thread.save!
-        threads = thread_result "question_1", group_ids: "42,43"
-        threads.length.should == 2
-        group_thread.group_id = 43
-        group_thread.save!
-        threads = thread_result "question_1", group_ids: "42,43"
-        threads.length.should == 2
-        group_thread.group_id = 44
-        group_thread.save
-        threads = thread_result "question_1", group_ids: "42,43"
-        threads.length.should == 1
-      end
-      it "returns an empty array when the commentable object does not exist (no threads)" do
-        threads = thread_result "does_not_exist"
-        threads.length.should == 0
+      end
+
+      context 'without filtering' do
+        let(:parameters) { {} }
+        let!(:threads) { ignored_threads + create_list(:comment_thread, 3, :with_group_id, commentable_id: commentable_id) }
+
+        it_behaves_like 'a filterable API endpoint'
+      end
+
+      context 'when filtering by the standalone context' do
+        let(:parameters) { {context: :standalone} }
+        let!(:threads) { create_list(:comment_thread, 3, commentable_id: commentable_id, context: :standalone) }
+
+        it_behaves_like 'a filterable API endpoint'
+      end
+
+      context 'when filtering by course_id' do
+        let(:course_id) { Faker::Lorem.word }
+        let(:parameters) { {course_id: course_id} }
+        let!(:threads) { create_list(:comment_thread, 3, commentable_id: commentable_id, course_id: course_id) }
+
+
+        it_behaves_like 'a filterable API endpoint'
+      end
+
+      context 'when filtering by group_id' do
+        let(:group_id) { Faker::Number.number(4) }
+        let(:parameters) { {group_id: group_id} }
+        let!(:threads) { create_list(:comment_thread, 3, commentable_id: commentable_id, group_id: group_id) }
+
+
+        it_behaves_like 'a filterable API endpoint'
+      end
+
+      context 'when filtering by multiple group_id values' do
+        let(:group_ids) { [Faker::Number.number(4), Faker::Number.number(4)] }
+        let(:parameters) { {group_ids: group_ids.join(',')} }
+
+
+        it_behaves_like 'a filterable API endpoint' do
+          let!(:threads) do
+            threads = []
+
+            group_ids.each do |group_id|
+              threads += create_list(:comment_thread, 3, commentable_id: commentable_id, group_id: group_id)
+            end
+
+            threads
+          end
+        end
+      end
+
+      context 'when the commentable does not exist' do
+        subject { get '/api/v1/does_not_exist/threads' }
+
+        it { should be_ok }
+
+        it 'should not return any results' do
+          expect(returned_threads.length).to eq 0
+        end
       end
 
       def test_unicode_data(text)
-        commentable_id = "unicode_commentable"
-        thread = make_thread(User.first, text, "unicode_course", commentable_id)
-        make_comment(User.first, thread, text)
+        commentable_id = 'unicode_commentable'
+        thread = create(:comment_thread, commentable_id: commentable_id, body: text)
+        create(:comment, comment_thread: thread, body: text)
+
         get "/api/v1/#{commentable_id}/threads"
         last_response.should be_ok
-        result = parse(last_response.body)["collection"]
+        result = parse(last_response.body)['collection']
         result.should_not be_empty
         check_thread_result_json(nil, thread, result.first)
       end
 
-      include_examples "unicode data"
+      include_examples 'unicode data'
     end
-    describe "POST /api/v1/:commentable_id/threads" do
-      let(:default_params) do
-        {title: "Interesting question", body: "cool", course_id: "1", user_id: "1"}
+
+    describe 'POST /api/v1/:commentable_id/threads' do
+      let(:commentable_id) { Faker::Lorem.word }
+      let(:user) { create(:user) }
+      let(:parameters) { attributes_for(:comment_thread, user_id: user.id) }
+      subject { post "/api/v1/#{commentable_id}/threads", parameters }
+
+      shared_examples_for 'CommentThread creation API' do |context='course'|
+        it 'creates a new CommentThread' do
+          expect(CommentThread.count).to eq 0
+
+          body = parse(subject.body)
+          expect(body).to include('read' => false,
+                                  'unread_comments_count' => 0,
+                                  'endorsed' => false,
+                                  'resp_total' => 0)
+
+          expect(CommentThread.count).to eq 1
+
+          thread = CommentThread.find(body['id'])
+          expect(thread).to_not be_nil
+          expect(thread.context).to eq context
         end
-      it "create a new comment thread for the commentable object" do
-        old_count = CommentThread.count
-        post '/api/v1/question_1/threads', default_params
-        last_response.should be_ok
-        result = parse(last_response.body)
-        result["read"].should == false
-        result["unread_comments_count"].should == 0
-        result["endorsed"].should == false
-        result["resp_total"].should == 0
-        CommentThread.count.should == old_count + 1
-        thread = CommentThread.where(title: "Interesting question").first
-        thread.should_not be_nil
-        thread.context.should == "course"
-      end
-      it "can create a standalone thread" do
-        old_count = CommentThread.count
-        post '/api/v1/question_1/threads', default_params.merge(:context => "standalone")
-        CommentThread.count.should == old_count + 1
-        thread = CommentThread.where(title: "Interesting question").first
-        thread.should_not be_nil
-        thread.context.should == "standalone"
       end
+
+      it { should be_ok }
+
+      it_behaves_like 'CommentThread creation API'
+      it_behaves_like 'CommentThread creation API', 'standalone' do
+        let(:parameters) { attributes_for(:comment_thread, user_id: user.id, context: 'standalone') }
+      end
+
       CommentThread.thread_type.values.each do |thread_type|
         it "can create a #{thread_type} thread" do
           old_count = CommentThread.where(thread_type: thread_type).count
-          post "/api/v1/question_1/threads", default_params.merge(thread_type: thread_type.to_s)
+          post '/api/v1/question_1/threads', parameters.merge(thread_type: thread_type.to_s)
           last_response.should be_ok
-          parse(last_response.body)["thread_type"].should == thread_type.to_s
+          parse(last_response.body)['thread_type'].should == thread_type.to_s
           CommentThread.where(thread_type: thread_type).count.should == old_count + 1
         end
       end
-      it "allows anonymous thread" do
-        old_count = CommentThread.count
-        post '/api/v1/question_1/threads', default_params.merge(anonymous: true)
-        last_response.should be_ok
-        CommentThread.count.should == old_count + 1
-        c = CommentThread.where(title: "Interesting question").first
-        c.should_not be_nil
-        c["anonymous"].should be_true
-      end
-      it "create a new comment thread for a new commentable object" do
-        post '/api/v1/does_not_exist/threads', default_params
+
+      it 'allows anonymous thread' do
+        post '/api/v1/question_1/threads', parameters.merge!(anonymous: true)
         last_response.should be_ok
-        Commentable.find("does_not_exist").comment_threads.length.should == 1
-        Commentable.find("does_not_exist").comment_threads.first.body.should == "cool"
+        body = parse(subject.body)
+
+        thread = CommentThread.find(body['id'])
+        expect(thread).to_not be_nil
+        expect(thread['anonymous']).to be_true
       end
-      it "returns error when title, body or course id does not exist" do
-        params = default_params.dup
-        params.delete(:title)
-        post '/api/v1/question_1/threads', params
-        last_response.status.should == 400
-        params = default_params.dup
-        params.delete(:body)
-        post '/api/v1/question_1/threads', params
-        last_response.status.should == 400
-        params = default_params.dup
-        params.delete(:course_id)
+
+      it 'returns error when title, body or course id does not exist' do
+        [:title, :body, :course_id].each do |parameter|
+          params = parameters.dup
+          params.delete(parameter)
           post '/api/v1/question_1/threads', params
           last_response.status.should == 400
         end
+      end
+
       it "returns error when title or body is blank (only consists of spaces and new lines)" do
-        post '/api/v1/question_1/threads', default_params.merge(title: "     ")
+        post '/api/v1/question_1/threads', parameters.merge(title: "     ")
         last_response.status.should == 400
-        post '/api/v1/question_1/threads', default_params.merge(body: "     \n    \n")
+        post '/api/v1/question_1/threads', parameters.merge(body: "     \n    \n")
         last_response.status.should == 400
       end
-      it "returns 503 and does not create when the post content is blocked" do
-        post '/api/v1/question_1/threads', default_params.merge(body: "BLOCKED POST")
+
+      it 'returns 503 and does not create when the post content is blocked' do
+        body = 'BLOCKED POST'
+        hash = block_post_body
+        post '/api/v1/question_1/threads', parameters.merge!(body: body)
         last_response.status.should == 503
-        parse(last_response.body).first.should == I18n.t(:blocked_content_with_body_hash, :hash => Digest::MD5.hexdigest("blocked post"))
-        CommentThread.where(body: "BLOCKED POST").to_a.should be_empty
+        parse(last_response.body).first.should == I18n.t(:blocked_content_with_body_hash, :hash => hash)
+        expect(CommentThread.where(body: body).length).to eq 0
       end
 
       def test_unicode_data(text)
-        commentable_id = "unicode_commentable"
-        post "/api/v1/#{commentable_id}/threads", default_params.merge(body: text, title: text)
+        commentable_id = 'unicode_commentable'
+        post "/api/v1/#{commentable_id}/threads", parameters.merge!(body: text, title: text)
         last_response.should be_ok
-        CommentThread.where(commentable_id: commentable_id, body: text, title: text).should_not be_empty
+        expect(CommentThread.where(commentable_id: commentable_id, body: text, title: text)).to_not be_empty
       end
 
-      include_examples "unicode data"
+      include_examples 'unicode data'
     end
   end
 end