refactored out helper; more specs

Rakefile

@@ -7,8 +7,8 @@ Bundler.require
 
 desc "Load the environment"
 task :environment do
-  env = ENV["SINATRA_ENV"] || "development"
-  Sinatra::Base.environment = env
+  environment = ENV["SINATRA_ENV"] || "development"
+  Sinatra::Base.environment = environment
   Mongoid.load!("config/mongoid.yml")
   Mongoid.logger.level = Logger::INFO
   module CommentService

app.rb

@@ -6,7 +6,8 @@ Bundler.require
 
 env_index = ARGV.index("-e")
 env_arg = ARGV[env_index + 1] if env_index
-env = env_arg || ENV["SINATRA_ENV"] || "development"
+environment = env_arg || ENV["SINATRA_ENV"] || "development"
+RACK_ENV = environment
 
 module CommentService
   class << self; attr_accessor :config; end
@@ -20,47 +21,6 @@ Mongoid.logger.level = Logger::INFO
 Dir[File.dirname(__FILE__) + '/models/*.rb'].each {|file| require file}
 Dir[File.dirname(__FILE__) + '/lib/**/*.rb'].each {|file| require file}
 
-
-helpers do
-  def commentable
-    @commentable ||= Commentable.find(params[:commentable_id])
-  end
-
-  def user # TODO handle 404 if integrated user service
-    @user ||= (User.find_or_create_by(external_id: params[:user_id]) if params[:user_id])
-  end
-
-  def thread
-    @thread ||= CommentThread.find(params[:thread_id])
-  end
-
-  def comment
-    @comment ||= Comment.find(params[:comment_id])
-  end
-
-  def source
-    @source ||= case params["source_type"]
-      when "user"
-        User.find_or_create_by(external_id: params["source_id"])
-      when "thread"
-        CommentThread.find(params["source_id"])
-      when "other"
-        Commentable.find(params["source_id"])
-    end
-  end
-
-  def vote_for(obj)
-    user.vote(obj, params["value"].to_sym)
-    obj.reload.to_hash.to_json
-  end
-
-  def undo_vote_for(obj)
-    user.unvote(obj)
-    obj.reload.to_hash.to_json
-  end
-
-end
-
 delete '/api/v1/:commentable_id/threads' do |commentable_id|
   commentable.comment_threads.destroy_all
   {}.to_json
@@ -176,7 +136,7 @@ get '/api/v1/search' do
   end
 end
 
-if env.to_s == "development"
+if environment.to_s == "development"
   get '/api/v1/clean' do
     Comment.delete_all
     CommentThread.delete_all
@@ -188,5 +148,9 @@ if env.to_s == "development"
 end
 
 error BSON::InvalidObjectId do
-  error 404
+  error 400, "requested object not found"
+end
+
+error ValueError do
+  error 400, env['sinatra.error'].message
 end

lib/exceptions.rb

+class ValueError < Exception
+
+end

lib/helpers.rb

+helpers do
+  def commentable
+    @commentable ||= Commentable.find(params[:commentable_id])
+  end
+
+  def user # TODO handle 404 if integrated user service
+    @user ||= (User.find_or_create_by(external_id: params[:user_id]) if params[:user_id])
+  end
+
+  def thread
+    @thread ||= CommentThread.find(params[:thread_id])
+  end
+
+  def comment
+    @comment ||= Comment.find(params[:comment_id])
+  end
+
+  def source
+    @source ||= case params["source_type"]
+      when "user"
+        User.find_or_create_by(external_id: params["source_id"])
+      when "thread"
+        CommentThread.find(params["source_id"])
+      when "other"
+        Commentable.find(params["source_id"])
+    end
+  end
+
+  def vote_for(obj)
+    raise ValueError, "must provide user id" unless user
+    raise ValueError, "must provide value" unless params["value"]
+    raise ValueError, "must provide valid value" unless %w[up down].include? params["value"]
+    user.vote(obj, params["value"].to_sym)
+    obj.reload.to_hash.to_json
+  end
+
+  def undo_vote_for(obj)
+    raise ValueError, "must provide user id" unless user
+    user.unvote(obj)
+    obj.reload.to_hash.to_json
+  end
+
+end

spec/app_spec.rb

@@ -218,9 +218,9 @@ describe "app" do
         response_thread["children"].length.should == thread.root_comments.length
         response_thread["children"].index{|c| c["body"] == thread.root_comments.first.body}.should_not be_nil
       end
-      it "returns error message when the thread does not exist" do
+      it "returns 400 when the thread does not exist" do
         get "/api/v1/threads/does_not_exist"
-        last_response.status.should == 404
+        last_response.status.should == 400
       end
     end
     describe "PUT /api/v1/threads/:thread_id" do
@@ -232,9 +232,9 @@ describe "app" do
         changed_thread.body.should == "new body"
         changed_thread.title.should == "new title"
       end
-      it "returns error message when the thread does not exist" do
+      it "returns 400 when the thread does not exist" do
         put "/api/v1/threads/does_not_exist", body: "new body", title: "new title"
-        last_response.status.should == 404
+        last_response.status.should == 400
       end
     end
     describe "POST /api/v1/threads/:thread_id/comments" do
@@ -258,9 +258,9 @@ describe "app" do
         comment = changed_thread["children"].select{|c| c["body"] == "new comment"}.first
         comment.should_not be_nil
       end
-      it "returns error message when the thread does not exist" do
+      it "returns 400 when the thread does not exist" do
         post "/api/v1/threads/does_not_exist/comments", body: "new comment", course_id: "1", user_id: User.first.id
-        last_response.status.should == 404
+        last_response.status.should == 400
       end
     end
     describe "DELETE /api/v1/threads/:thread_id" do
@@ -270,6 +270,10 @@ describe "app" do
         last_response.should be_ok
         CommentThread.where(title: thread["title"]).first.should be_nil
       end
+      it "returns 400 when the thread does not exist" do
+        delete "/api/v1/threads/does_not_exist"
+        last_response.status.should == 400
+      end
     end
   end
 
@@ -299,6 +303,10 @@ describe "app" do
         retrieved["children"].length.should == comment.children.length
         retrieved["children"].select{|c| c["body"] == comment.children.first.body}.first.should_not be_nil
       end
+      it "returns 400 when the comment does not exist" do
+        get "/api/v1/comments/does_not_exist"
+        last_response.status.should == 400
+      end
     end
     describe "PUT /api/v1/comments/:comment_id" do
       it "update information of the comment" do
@@ -309,6 +317,10 @@ describe "app" do
         new_comment.body.should == "new body"
         new_comment.endorsed.should == true
       end
+      it "returns 400 when the comment does not exist" do
+        put "/api/v1/comments/does_not_exist", body: "new body", endorsed: true
+        last_response.status.should == 400
+      end
     end
     describe "POST /api/v1/comments/:comment_id" do
       it "create a sub comment to the comment" do
@@ -322,6 +334,10 @@ describe "app" do
         subcomment.should_not be_nil
         subcomment["user_id"].should == user.id
       end
+      it "returns 400 when the comment does not exist" do
+        post "/api/v1/comments/does_not_exist", body: "new comment", course_id: "1", user_id: User.first.id
+        last_response.status.should == 400
+      end
     end
     describe "DELETE /api/v1/comments/:comment_id" do
       it "delete the comment and its sub comments" do
@@ -332,6 +348,10 @@ describe "app" do
         Comment.count.should == prev_count - cnt_comments
         Comment.all.select{|c| c.id == comment.id}.first.should be_nil
       end
+      it "returns 400 when the comment does not exist" do
+        delete "/api/v1/comments/does_not_exist"
+        last_response.status.should == 400
+      end
     end
   end
   describe "votes" do
@@ -347,6 +367,20 @@ describe "app" do
         comment.up_votes_count.should == prev_up_votes - 1
         comment.down_votes_count.should == prev_down_votes + 1
       end
+      it "returns 400 when the comment does not exist" do
+        put "/api/v1/comments/does_not_exist/votes", user_id: User.first.id, value: "down"
+        last_response.status.should == 400
+      end
+      it "returns 400 when user_id is not provided" do
+        put "/api/v1/comments/#{Comment.first.id}/votes", value: "down"
+        last_response.status.should == 400
+      end
+      it "returns 400 when value is not provided or invalid" do
+        put "/api/v1/comments/#{Comment.first.id}/votes", user_id: User.first.id
+        last_response.status.should == 400
+        put "/api/v1/comments/#{Comment.first.id}/votes", user_id: User.first.id, value: "superdown"
+        last_response.status.should == 400
+      end
     end
     describe "DELETE /api/v1/comments/:comment_id/votes" do
       it "unvote on the comment" do
@@ -359,6 +393,14 @@ describe "app" do
         comment.up_votes_count.should == prev_up_votes - 1
         comment.down_votes_count.should == prev_down_votes
       end
+      it "returns 400 when the comment does not exist" do
+        delete "/api/v1/comments/does_not_exist/votes", user_id: User.first.id
+        last_response.status.should == 400
+      end
+      it "returns 400 when user_id is not provided" do
+        delete "/api/v1/comments/#{Comment.first.id}/votes"
+        last_response.status.should == 400
+      end
     end
     describe "PUT /api/v1/threads/:thread_id/votes" do
       it "create or update the vote on the thread" do