Merge pull request #164 from edx/clintonb/factories

Added factories and updated specs

Gemfile

@@ -14,7 +14,7 @@ gem 'sinatra'
 gem 'yajl-ruby'
 
 
-gem 'mongoid', "~>5.0"
+gem 'mongoid', '~> 5.0.0'
 gem 'bson', '~>3.1'
 gem 'bson_ext'
 gem 'protected_attributes'
@@ -27,7 +27,6 @@ gem 'mongoid-tree', :git => 'https://github.com/macdiesel/mongoid-tree'
 gem 'rs_voteable_mongo', :git => 'https://github.com/navneet35371/voteable_mongo.git'
 gem 'mongoid_magic_counter_cache'
 
-gem 'faker'
 gem 'will_paginate_mongoid', "~>2.0"
 gem 'rdiscount'
 gem 'nokogiri', "~>1.6.7.1"
@@ -40,12 +39,14 @@ gem 'dalli'
 gem 'rest-client'
 
 group :test do
-  gem 'rspec'
-  gem 'rack-test', :require => "rack/test"
+  gem 'codecov', :require => false
+  gem 'database_cleaner', '~> 1.5.1'
+  gem 'factory_girl', '~> 4.0'
+  gem 'faker', '~> 1.6'
   gem 'guard'
   gem 'guard-unicorn'
-  gem 'codecov', :require => false
-  gem 'database_cleaner', "~>1.5.1" 
+  gem 'rack-test', :require => 'rack/test'
+  gem 'rspec', '~> 2.11.0'
 end
 
 gem 'newrelic_rpm'

Gemfile.lock

@@ -55,8 +55,10 @@ GEM
       unf (>= 0.0.5, < 1.0.0)
     enumerize (0.11.0)
       activesupport (>= 3.2)
-    faker (1.0.1)
-      i18n (~> 0.4)
+    factory_girl (4.5.0)
+      activesupport (>= 3.0.0)
+    faker (1.6.1)
+      i18n (~> 0.5)
     guard (1.3.2)
       listen (>= 0.4.2)
       thor (>= 0.14.6)
@@ -173,11 +175,12 @@ DEPENDENCIES
   delayed_job
   delayed_job_mongoid
   enumerize
-  faker
+  factory_girl (~> 4.0)
+  faker (~> 1.6)
   guard
   guard-unicorn
   i18n
-  mongoid (~> 5.0)
+  mongoid (~> 5.0.0)
   mongoid-tree!
   mongoid_magic_counter_cache
   newrelic_rpm
@@ -192,10 +195,13 @@ DEPENDENCIES
   rdiscount
   rest-client
   rs_voteable_mongo!
-  rspec
+  rspec (~> 2.11.0)
   sinatra
   tire (= 0.6.2)
   tire-contrib
   unicorn
   will_paginate_mongoid (~> 2.0)
   yajl-ruby
+
+BUNDLED WITH
+   1.11.2

spec/api/abuse_spec.rb

 require 'spec_helper'
 
-def create_comment_flag(comment_id, user_id)
-  create_flag("/api/v1/comments/" + comment_id + "/abuse_flag", user_id)
-end
+describe 'Abuse API' do
+  before(:each) { set_api_key_header }
 
-def create_thread_flag(thread_id, user_id)
-  create_flag("/api/v1/threads/" + thread_id + "/abuse_flag", user_id)
-end
+  shared_examples 'an abuse endpoint' do
+    let(:affected_entity_id) { affected_entity.id }
+    let(:user_id) { create(:user).id }
 
-def remove_thread_flag(thread_id, user_id)
-  remove_flag("/api/v1/threads/" + thread_id + "/abuse_unflag", user_id)
-end
+    it { should be_ok }
 
-def remove_comment_flag(comment_id, user_id)
-  remove_flag("/api/v1/comments/" + comment_id + "/abuse_unflag", user_id)
-end
+    it 'updates the abuse flaggers' do
+      subject
 
-def create_flag(put_command, user_id)
-  if user_id.nil?
-    put put_command
-  else
-    put put_command, user_id: user_id
-  end
-end
+      affected_entity.reload
+      expect(affected_entity.abuse_flaggers).to eq expected_abuse_flaggers
+      expect(non_affected_entity.abuse_flaggers).to have(0).items
+    end
 
-def remove_flag(put_command, user_id)
-  if user_id.nil?
-    put put_command
-  else
-    put put_command, user_id: user_id
+    context 'if the comment does not exist' do
+      let(:affected_entity_id) { 'does_not_exist' }
+      it { should be_bad_request }
+      its(:body) { should eq "[\"#{I18n.t(:requested_object_not_found)}\"]" }
+    end
+
+    context 'if no user_id is provided' do
+      let(:user_id) { nil }
+      it { should be_bad_request }
+      its(:body) { should eq "[\"#{I18n.t(:user_id_is_required)}\"]" }
+    end
   end
-end
 
-describe "app" do
-  describe "abuse" do
+  describe 'comment actions' do
+    let(:affected_entity) { create(:comment, abuse_flaggers: []) }
+    let(:non_affected_entity) { affected_entity.comment_thread }
 
-    before(:each) do
-      init_without_subscriptions
-      set_api_key_header
-    end
+    context 'when flagging a comment for abuse' do
+      let(:expected_abuse_flaggers) { [user_id] }
+      subject { put "/api/v1/comments/#{affected_entity_id}/abuse_flag", user_id: user_id }
 
-    describe "flag a comment as abusive" do
-      it "create or update the abuse_flags on the comment" do
-        comment = Comment.first
-        prev_abuse_flaggers_count = comment.abuse_flaggers.length
-        create_comment_flag("#{comment.id}", User.first.id)
-
-        comment = Comment.find(comment.id)
-        comment.abuse_flaggers.count.should == prev_abuse_flaggers_count + 1
-        # verify that the thread doesn't automatically get flagged
-        comment.comment_thread.abuse_flaggers.length.should == 0
-      end
-      it "returns 400 when the comment does not exist" do
-        create_comment_flag("does_not_exist", User.first.id)
-        last_response.status.should == 400
-        parse(last_response.body).first.should == I18n.t(:requested_object_not_found)
-      end
-      it "returns 400 when user_id is not provided" do
-        create_comment_flag("#{Comment.first.id}", nil)
-        last_response.status.should == 400
-        parse(last_response.body).first.should == I18n.t(:user_id_is_required)
-      end
-      #Would like to test the output of to_hash, but not sure how to deal with a Moped::BSON::Document object
-      #it "has a correct hash" do
-      #  create_flag("#{Comment.first.id}", User.first.id)
-      #  Comment.first.to_hash
-      #end
+      it_behaves_like 'an abuse endpoint'
     end
-    describe "flag a thread as abusive" do
-      it "create or update the abuse_flags on the comment" do
-        comment = Comment.first
-        thread = comment.comment_thread
-        prev_abuse_flaggers_count = thread.abuse_flaggers.count
-        create_thread_flag("#{thread.id}", User.first.id)
-
-        comment = Comment.find(comment.id)
-        comment.comment_thread.abuse_flaggers.count.should == prev_abuse_flaggers_count + 1
-        # verify that the comment doesn't automatically get flagged
-        comment.abuse_flaggers.length.should == 0
-      end
-      it "returns 400 when the thread does not exist" do
-        create_thread_flag("does_not_exist", User.first.id)
-        last_response.status.should == 400
-        parse(last_response.body).first.should == I18n.t(:requested_object_not_found)
-      end
-      it "returns 400 when user_id is not provided" do
-        create_thread_flag("#{Comment.first.comment_thread.id}", nil)
-        last_response.status.should == 400
-        parse(last_response.body).first.should == I18n.t(:user_id_is_required)
-      end
-      #Would like to test the output of to_hash, but not sure how to deal with a Moped::BSON::Document object
-      #it "has a correct hash" do
-      #  create_thread_flag("#{Comment.first.comment_thread.id}", User.first.id)
-      #  Comment.first.comment_thread.to_hash
-      #end
+
+    context 'when un-flagging a comment for abuse' do
+      let(:affected_entity) { create(:comment, abuse_flaggers: [user_id]) }
+      let(:expected_abuse_flaggers) { [] }
+      subject { put "/api/v1/comments/#{affected_entity_id}/abuse_unflag", user_id: user_id }
+
+      it_behaves_like 'an abuse endpoint'
     end
-    describe "unflag a comment as abusive" do
-      it "removes the user from the existing abuse_flaggers" do
-        comment = Comment.first
-        create_comment_flag("#{comment.id}", User.first.id)
-        
-        comment = Comment.first
-        prev_abuse_flaggers       = comment.abuse_flaggers
-        prev_abuse_flaggers_count = prev_abuse_flaggers.count
-        
-        prev_abuse_flaggers.should include User.first.id 
-        
-        remove_comment_flag("#{comment.id}", User.first.id)
-
-        comment = Comment.find(comment.id)
-        comment.abuse_flaggers.count.should == prev_abuse_flaggers_count - 1 
-        comment.abuse_flaggers.to_a.should_not include User.first.id
-      end
-      it "returns 400 when the comment does not exist" do
-        remove_comment_flag("does_not_exist", User.first.id)
-        last_response.status.should == 400
-        parse(last_response.body).first.should == I18n.t(:requested_object_not_found)
-      end
-      it "returns 400 when the thread does not exist" do
-        remove_thread_flag("does_not_exist", User.first.id)
-        last_response.status.should == 400
-        parse(last_response.body).first.should == I18n.t(:requested_object_not_found)
-      end
-      it "returns 400 when user_id is not provided" do
-        remove_thread_flag("#{Comment.first.comment_thread.id}", nil)
-        last_response.status.should == 400
-        parse(last_response.body).first.should == I18n.t(:user_id_is_required)
-      end
-      #Would like to test the output of to_hash, but not sure how to deal with a Moped::BSON::Document object
-      #it "has a correct hash" do
-      #  create_thread_flag("#{Comment.first.comment_thread.id}", User.first.id)
-      #  Comment.first.comment_thread.to_hash
-      #end
+  end
+
+  describe 'comment thread actions' do
+    let(:affected_entity) { create(:comment_thread, abuse_flaggers: []) }
+    let(:non_affected_entity) { create(:comment, comment_thread: affected_entity) }
+
+    context 'when flagging a comment thread for abuse' do
+      let(:expected_abuse_flaggers) { [user_id] }
+      subject { put "/api/v1/threads/#{affected_entity_id}/abuse_flag", user_id: user_id }
+
+      it_behaves_like 'an abuse endpoint'
     end
-    describe "unflag a thread as abusive" do
-      it "removes the user from the existing abuse_flaggers" do
-        thread = CommentThread.first
-        create_thread_flag("#{thread.id}", User.first.id)
-        
-        thread = CommentThread.first
-        prev_abuse_flaggers = thread.abuse_flaggers
-        prev_abuse_flaggers_count = prev_abuse_flaggers.count
-        
-        prev_abuse_flaggers.should include User.first.id 
-        
-        remove_thread_flag("#{thread.id}", User.first.id)
-
-        thread = CommentThread.find(thread.id)
-        thread.abuse_flaggers.count.should == prev_abuse_flaggers_count - 1 
-        thread.abuse_flaggers.to_a.should_not include User.first.id
-      end
-      it "returns 400 when the thread does not exist" do
-        remove_thread_flag("does_not_exist", User.first.id)
-        last_response.status.should == 400
-        parse(last_response.body).first.should == I18n.t(:requested_object_not_found)
-      end
-      it "returns 400 when user_id is not provided" do
-        remove_thread_flag("#{Comment.first.comment_thread.id}", nil)
-        last_response.status.should == 400
-        parse(last_response.body).first.should == I18n.t(:user_id_is_required)
-      end
-      #Would like to test the output of to_hash, but not sure how to deal with a Moped::BSON::Document object
-      #it "has a correct hash" do
-      #  create_thread_flag("#{Comment.first.comment_thread.id}", User.first.id)
-      #  Comment.first.comment_thread.to_hash
-      #end
+
+    context 'when un-flagging a comment thread for abuse' do
+      let(:affected_entity) { create(:comment_thread, abuse_flaggers: [user_id]) }
+      let(:expected_abuse_flaggers) { [] }
+      subject { put "/api/v1/threads/#{affected_entity_id}/abuse_unflag", user_id: user_id }
+
+      it_behaves_like 'an abuse endpoint'
     end
   end
 end

spec/api/i18n_spec.rb

-require "spec_helper"
+require 'spec_helper'
 
-describe "i18n" do
+describe 'i18n' do
 
   before(:each) { set_api_key_header }
 
-  it "should respect the Accept-Language header" do
-    put "/api/v1/comments/does_not_exist/votes", {}, {"HTTP_ACCEPT_LANGUAGE" => "x-test"}
+  it 'should respect the Accept-Language header' do
+    put '/api/v1/comments/does_not_exist/votes', {}, {'HTTP_ACCEPT_LANGUAGE' => 'x-test'}
     last_response.status.should == 400
-    parse(last_response.body).first.should == "##x-test## requested object not found"
+    parse(last_response.body).first.should == '##x-test## requested object not found'
   end
 end

spec/api/query_spec.rb

 require 'spec_helper'
+require 'faker'
 
-describe "app" do
 
-  before (:each) { set_api_key_header }
+describe 'app' do
+  before(:each) { set_api_key_header }
+  let(:body) { Faker::Lorem.word }
 
-  let(:author) { create_test_user(1) }
-  describe "thread search" do
-    describe "GET /api/v1/search/threads" do
-      it "returns thread with query match" do
-        commentable = Commentable.new("question_1")
+  describe 'GET /api/v1/search/threads' do
 
-        random_string = (0...8).map{ ('a'..'z').to_a[rand(26)] }.join
+    shared_examples_for 'a search endpoint' do
+      subject do
+        refresh_es_index
+        get '/api/v1/search/threads', text: body
+      end
 
-        thread = CommentThread.new(title: "Test title", body: random_string, course_id: "1", commentable_id: commentable.id)
-        thread.thread_type = :discussion
-        thread.author = author
-        thread.save!
+      let(:matched_thread) { parse(subject.body)['collection'].select { |t| t['id'] == thread.id.to_s }.first }
 
-        sleep 3
+      it { should be_ok }
 
-        get "/api/v1/search/threads", text: random_string
-        last_response.should be_ok
-        threads = parse(last_response.body)['collection']
-        check_thread_result_json(nil, thread, threads.select{|t| t["id"] == thread.id.to_s}.first)
+      it 'returns thread with query match' do
+        expect(matched_thread).to_not be_nil
+        check_thread_result_json(nil, thread, matched_thread)
       end
-
     end
-  end
-
-  describe "comment search" do
-    describe "GET /api/v1/search/threads" do
-      it "returns thread with comment query match" do
-        commentable = Commentable.new("question_1")
-
-        random_string = (0...8).map{ ('a'..'z').to_a[rand(26)] }.join
 
-        thread = CommentThread.new(title: "Test title", body: "elephant otter", course_id: "1", commentable_id: commentable.id)
-        thread.thread_type = :discussion
-        thread.author = author
-        thread.save!
+    context 'when searching on thread content' do
+      let!(:thread) { create(:comment_thread, body: body) }
 
-        sleep 3
-
-        comment = Comment.new(body: random_string, course_id: "1")
-        comment.commentable_id = commentable.id
-        comment.author = author
-        comment.comment_thread = thread
-        comment.save!
-
-        sleep 1
+      it_behaves_like 'a search endpoint'
+    end
 
-        get "/api/v1/search/threads", text: random_string
-        last_response.should be_ok
-        threads = parse(last_response.body)['collection']
-        check_thread_result_json(nil, thread, threads.select{|t| t["id"] == thread.id.to_s}.first)
+    context 'when searching on comment content' do
+      let!(:thread) do
+        comment = create(:comment, body: body)
+        thread = comment.comment_thread
       end
+
+      it_behaves_like 'a search endpoint'
     end
   end
 end

spec/factories.rb

+require 'faker'
+
+# Reload i18n data for faker
+I18n.reload!
+
+FactoryGirl.define do
+  factory :user do
+    # Initialize the model with all attributes since we are using a custom _id field.
+    # See https://github.com/thoughtbot/factory_girl/issues/544.
+    initialize_with { new(attributes) }
+
+    sequence(:username) { |n| "#{Faker::Internet.user_name}_#{n}" }
+    sequence(:external_id) { username }
+  end
+
+  factory :comment_thread do
+    title { Faker::Lorem.sentence }
+    body { Faker::Lorem.paragraph }
+    course_id { Faker::Lorem.word }
+    thread_type :discussion
+    commentable_id { Faker::Lorem.word }
+    association :author, factory: :user
+    group_id nil
+    pinned false
+
+    trait :subscribe_author do
+      after(:create) do |thread|
+        thread.author.subscribe(thread)
+      end
+    end
+
+    trait :with_group_id do
+      group_id { Faker::Number.number(4) }
+    end
+  end
+
+  factory :comment do
+    association :author, factory: :user
+    comment_thread { parent ? parent.comment_thread : create(:comment_thread) }
+    body { Faker::Lorem.paragraph }
+    course_id { comment_thread.course_id }
+    commentable_id { comment_thread.commentable_id }
+  end
+end

spec/spec_helper.rb

@@ -9,13 +9,13 @@ end
 
 require File.join(File.dirname(__FILE__), '..', 'app')
 
-require 'sinatra'
 require 'rack/test'
+require 'sinatra'
 require 'yajl'
-require 'database_cleaner'
 
 require 'support/database_cleaner'
 require 'support/elasticsearch'
+require 'support/factory_girl'
 
 # setup test environment
 set :environment, :test
@@ -59,6 +59,20 @@ def create_test_user(id)
   User.create!(external_id: id.to_s, username: "user#{id}")
 end
 
+# Add the given body of text to the list of blocked texts/hashes.
+def block_post_body(body='blocked post')
+  body = body.strip.downcase.gsub(/[^a-z ]/, '').gsub(/\s+/, ' ')
+  blocked_hash = Digest::MD5.hexdigest(body)
+  Content.mongo_client[:blocked_hash].insert_one(hash: blocked_hash)
+
+  # reload the global holding the blocked hashes
+  CommentService.blocked_hashes = Content.mongo_client[:blocked_hash].find(nil, projection: {hash: 1}).map do |d|
+    d['hash']
+  end
+
+  blocked_hash
+end
+
 def init_without_subscriptions
   commentable = Commentable.new("question_1")
 
@@ -139,11 +153,7 @@ def init_without_subscriptions
     users[2, 9].each { |user| user.vote(c, [:up, :down].sample) }
   end
 
-  Content.mongo_client[:blocked_hash].insert_one(hash: Digest::MD5.hexdigest("blocked post"))
-  # reload the global holding the blocked hashes
-  CommentService.blocked_hashes = Content.mongo_client[:blocked_hash].find(nil, projection: {hash: 1}).map do |d|
-    d["hash"]
-  end
+  block_post_body
 end
 
 # this method is used to test results produced using the helper function handle_threads_query
@@ -370,3 +380,16 @@ def setup_10_threads
   end
   @default_order = 10.times.map { |i| "t#{i}" }.reverse
 end
+
+# Creates a CommentThread with a Comment, and nested child Comment.
+# The author of the thread is subscribed to the thread.
+def create_comment_thread_and_comments
+  # Create a new comment thread, and subscribe the author to the thread
+  thread = create(:comment_thread, :subscribe_author)
+
+  # Create a comment along with a nested child comment
+  comment = create(:comment, comment_thread: thread)
+  create(:comment, parent: comment)
+
+  thread
+end

spec/support/factory_girl.rb

+require 'factory_girl'
+
+RSpec.configure do |config|
+  config.include FactoryGirl::Syntax::Methods
+
+  FactoryGirl.find_definitions
+
+  config.before(:suite) do
+    begin
+      DatabaseCleaner.start
+      FactoryGirl.lint
+    ensure
+      DatabaseCleaner.clean
+    end
+  end
+end