spec for users & updated @ extract method

app.rb

@@ -11,6 +11,8 @@ RACK_ENV = environment
 
 module CommentService
   class << self; attr_accessor :config; end
+  API_VERSION = 'v1'
+  API_PREFIX = "/api/#{API_VERSION}"
 end
 
 CommentService.config = YAML.load_file("config/application.yml")
@@ -21,7 +23,9 @@ Mongoid.logger.level = Logger::INFO
 Dir[File.dirname(__FILE__) + '/lib/**/*.rb'].each {|file| require file}
 Dir[File.dirname(__FILE__) + '/models/*.rb'].each {|file| require file}
 
-get '/api/v1/search/threads' do 
+api_prefix = CommentService::API_PREFIX
+
+get "#{api_prefix}/search/threads" do 
 
   sort_key_mapper = {
     "date" => :created_at,
@@ -60,12 +64,12 @@ get '/api/v1/search/threads' do
   end
 end
 
-delete '/api/v1/:commentable_id/threads' do |commentable_id|
+delete "#{api_prefix}/:commentable_id/threads" do |commentable_id|
   commentable.comment_threads.destroy_all
   {}.to_json
 end
 
-get '/api/v1/:commentable_id/threads' do |commentable_id|
+get "#{api_prefix}/:commentable_id/threads" do |commentable_id|
 
   sort_key_mapper = {
     "date" => :created_at,
@@ -99,7 +103,7 @@ get '/api/v1/:commentable_id/threads' do |commentable_id|
   end
 end
 
-post '/api/v1/:commentable_id/threads' do |commentable_id|
+post "#{api_prefix}/:commentable_id/threads" do |commentable_id|
   thread = CommentThread.new(params.slice(*%w[title body course_id]).merge(commentable_id: commentable_id))
   thread.anonymous = value_to_boolean(params["anonymous"]) || false
   thread.tags = params["tags"] || ""
@@ -113,19 +117,19 @@ post '/api/v1/:commentable_id/threads' do |commentable_id|
   end
 end
 
-get '/api/v1/threads/tags' do
+get "#{api_prefix}/threads/tags" do
   CommentThread.tags.to_json
 end
 
-get '/api/v1/threads/tags/autocomplete' do
+get "#{api_prefix}/threads/tags/autocomplete" do
   CommentThread.tags_autocomplete(params["value"].strip, max: 5, sort_by_count: true).map(&:first).to_json
 end
 
-get '/api/v1/threads/:thread_id' do |thread_id|
+get "#{api_prefix}/threads/:thread_id" do |thread_id|
   CommentThread.find(thread_id).to_hash(recursive: value_to_boolean(params["recursive"])).to_json
 end
 
-put '/api/v1/threads/:thread_id' do |thread_id|
+put "#{api_prefix}/threads/:thread_id" do |thread_id|
   thread.update_attributes(params.slice(*%w[title body]))
   if params["tags"]
     thread.tags = params["tags"]
@@ -138,7 +142,7 @@ put '/api/v1/threads/:thread_id' do |thread_id|
   end
 end
 
-post '/api/v1/threads/:thread_id/comments' do |thread_id|
+post "#{api_prefix}/threads/:thread_id/comments" do |thread_id|
   comment = thread.comments.new(params.slice(*%w[body course_id]))
   comment.anonymous = value_to_boolean(params["anonymous"]) || false
   comment.author = user 
@@ -151,16 +155,16 @@ post '/api/v1/threads/:thread_id/comments' do |thread_id|
   end
 end
 
-delete '/api/v1/threads/:thread_id' do |thread_id|
+delete "#{api_prefix}/threads/:thread_id" do |thread_id|
   thread.destroy
   thread.to_hash.to_json
 end
 
-get '/api/v1/comments/:comment_id' do |comment_id|
+get "#{api_prefix}/comments/:comment_id" do |comment_id|
   comment.to_hash(recursive: value_to_boolean(params["recursive"])).to_json
 end
 
-put '/api/v1/comments/:comment_id' do |comment_id|
+put "#{api_prefix}/comments/:comment_id" do |comment_id|
   comment.update_attributes(params.slice(*%w[body endorsed]))
   if comment.errors.any?
     error 400, comment.errors.full_messages.to_json
@@ -169,7 +173,7 @@ put '/api/v1/comments/:comment_id' do |comment_id|
   end
 end
 
-post '/api/v1/comments/:comment_id' do |comment_id|
+post "#{api_prefix}/comments/:comment_id" do |comment_id|
   sub_comment = comment.children.new(params.slice(*%w[body course_id]))
   sub_comment.anonymous = value_to_boolean(params["anonymous"]) || false
   sub_comment.author = user
@@ -183,28 +187,28 @@ post '/api/v1/comments/:comment_id' do |comment_id|
   end
 end
 
-delete '/api/v1/comments/:comment_id' do |comment_id|
+delete "#{api_prefix}/comments/:comment_id" do |comment_id|
   comment.destroy
   comment.to_hash.to_json
 end
 
-put '/api/v1/comments/:comment_id/votes' do |comment_id|
+put "#{api_prefix}/comments/:comment_id/votes" do |comment_id|
   vote_for comment
 end
 
-delete '/api/v1/comments/:comment_id/votes' do |comment_id|
+delete "#{api_prefix}/comments/:comment_id/votes" do |comment_id|
   undo_vote_for comment
 end
 
-put '/api/v1/threads/:thread_id/votes' do |thread_id|
+put "#{api_prefix}/threads/:thread_id/votes" do |thread_id|
   vote_for thread
 end
 
-delete '/api/v1/threads/:thread_id/votes' do |thread_id|
+delete "#{api_prefix}/threads/:thread_id/votes" do |thread_id|
   undo_vote_for thread
 end
 
-post '/api/v1/users' do
+post "#{api_prefix}/users" do
   user = User.new
   user.external_id = params["id"]
   user.username = params["username"]
@@ -217,12 +221,12 @@ post '/api/v1/users' do
   end
 end
 
-get '/api/v1/users/:user_id' do |user_id|
+get "#{api_prefix}/users/:user_id" do |user_id|
   user.to_hash(complete: value_to_boolean(params["complete"])).to_json
 end
 
-put '/api/v1/users/:user_id' do |user_id|
-  user.update_attributes(params.slice(*%w[username email])
+put "#{api_prefix}/users/:user_id" do |user_id|
+  user.update_attributes(params.slice(*%w[username email]))
   user.save
   if user.errors.any?
     error 400, user.errors.full_messages.to_json
@@ -231,20 +235,20 @@ put '/api/v1/users/:user_id' do |user_id|
   end
 end
 
-get '/api/v1/users/:user_id/notifications' do |user_id|
+get "#{api_prefix}/users/:user_id/notifications" do |user_id|
   user.notifications.map(&:to_hash).to_json
 end
 
-post '/api/v1/users/:user_id/subscriptions' do |user_id|
+post "#{api_prefix}/users/:user_id/subscriptions" do |user_id|
   user.subscribe(source).to_hash.to_json
 end
 
-delete '/api/v1/users/:user_id/subscriptions' do |user_id|
+delete "#{api_prefix}/users/:user_id/subscriptions" do |user_id|
   user.unsubscribe(source).to_hash.to_json
 end
 
 if environment.to_s == "development"
-  get '/api/v1/clean' do
+  get "#{api_prefix}/clean" do
     Comment.delete_all
     CommentThread.delete_all
     User.delete_all

lib/helpers.rb

@@ -5,7 +5,7 @@ helpers do
 
   def user # TODO handle 404 if integrated user service
     raise ArgumentError, "User id is required" unless @user || params[:user_id]
-    @user ||= User.find_or_create_by(external_id: params[:user_id])
+    @user ||= User.find_by(external_id: params[:user_id])
   end
 
   def thread
@@ -19,7 +19,7 @@ helpers do
   def source
     @source ||= case params["source_type"]
     when "user"
-      User.find_or_create_by(external_id: params["source_id"])
+      User.find_by(external_id: params["source_id"])
     when "thread"
       CommentThread.find(params["source_id"])
     when "other"

models/content.rb

@@ -3,6 +3,8 @@ class Content
   
   AT_NOTIFICATION_REGEX = /(?<=^|\s)(@[A-Za-z0-9_]+)(?!\w)/
   
+
+private
   def self.get_marked_text(text)
     counter = -1
     text.gsub AT_NOTIFICATION_REGEX do
@@ -15,7 +17,11 @@ class Content
     list = []
     text.gsub AT_NOTIFICATION_REGEX do
       parts = $1.rpartition('_')
-      list << [parts.last.to_i, parts.first[1..-1]]
+      username = parts.first[1..-1]
+      user = User.where(username: username).first
+      if user
+        list << [parts.last.to_i, parts.first[1..-1], user.id]
+      end
     end
     list
   end
@@ -28,4 +34,5 @@ class Content
     self.get_at_position_list html.to_s
   end
 
+
 end

models/user.rb

@@ -4,6 +4,8 @@ class User
 
   field :_id, type: String, default: -> { external_id }
   field :external_id, type: String
+  field :username, type: String
+  field :email, type: String
   
   has_many :comments, inverse_of: :author
   has_many :comment_threads, inverse_of: :author
@@ -11,7 +13,11 @@ class User
   has_and_belongs_to_many :notifications, inverse_of: :receivers
 
   validates_presence_of :external_id
+  validates_presence_of :username
+  validates_presence_of :email
   validates_uniqueness_of :external_id
+  validates_uniqueness_of :username
+  validates_uniqueness_of :email
 
   index external_id: 1
 
@@ -36,7 +42,7 @@ class User
   end
 
   def to_hash(params={})
-    hash = as_document.slice(*%w[_id external_id])
+    hash = as_document.slice(*%w[_id username external_id])
     if params[:complete]
       hash = hash.merge("subscribed_thread_ids" => subscribed_thread_ids,
                         "subscribed_commentable_ids" => subscribed_commentable_ids,

spec/api/comment_thread_spec.rb

@@ -85,7 +85,9 @@ describe "app" do
       end
     end
     describe "POST /api/v1/threads/:thread_id/comments" do
-      default_params = {body: "new comment", course_id: "1", user_id: User.first.id}
+      let :default_params  do
+        {body: "new comment", course_id: "1", user_id: User.first.id}
+      end
       it "create a comment to the comment thread" do
         thread = CommentThread.first.to_hash(recursive: true)
         user = User.first

spec/api/subscription_and_notification_spec.rb

@@ -16,9 +16,9 @@ describe "app" do
         notification_not_for_me_neither = notifications.select{|f| f["notification_type"] == "post_reply" and f["info"]["comment_id"] == not_for_me_neither.id.to_s}.first
         notification_not_for_me_neither.should_not be_nil
       end
-      it "returns empty array if user does not exist" do #TODO may change later if have user service
+      it "returns error if user does not exist" do #TODO may change later if have user service
         get "/api/v1/users/does_not_exist/notifications"
-        parse(last_response.body).length.should == 0
+        last_response.status.should == 400
       end
       it "get all notifications on the subscribed commentable for the user" do
         user = User.find("1")
@@ -55,7 +55,7 @@ describe "app" do
         User.find("2").followers.length.should == 1
       end
       it "does not follow oneself" do
-        user = User.find_or_create_by(external_id: "3")
+        user = create_test_user(3)
         post "/api/v1/users/#{user.external_id}/subscriptions", source_type: "user", source_id: user.external_id
         last_response.status.should == 400
         user.reload.followers.length.should == 0
@@ -76,21 +76,21 @@ describe "app" do
         User.find("1").followers.length.should == 0
       end
       it "subscribe a commentable" do
-        user3 = User.find_or_create_by(external_id: "3")
+        user3 = create_test_user(3)
         post "/api/v1/users/#{user3.external_id}/subscriptions", source_type: "other", source_id: "question_1"
         last_response.should be_ok
         Commentable.find("question_1").subscribers.length.should == 3
         Commentable.find("question_1").subscribers.should include user3
       end
       it "unsubscribe a commentable" do
-        user2 = User.find_or_create_by(external_id: "2")
+        user2 = User.find_by(external_id: "2")
         delete "/api/v1/users/#{user2.external_id}/subscriptions", source_type: "other", source_id: "question_1"
         last_response.should be_ok
         Commentable.find("question_1").subscribers.length.should == 1
         Commentable.find("question_1").subscribers.should_not include user2
       end
       it "subscribe a comment thread" do
-        user1 = User.find_or_create_by(external_id: "1")
+        user1 = User.find_by(external_id: "1")
         thread = CommentThread.where(body: "it is unsolvable").first
         post "/api/v1/users/#{user1.external_id}/subscriptions", source_type: "thread", source_id: thread.id
         last_response.should be_ok
@@ -99,7 +99,7 @@ describe "app" do
         thread.subscribers.should include user1
       end
       it "unsubscribe a comment thread" do
-        user2 = User.find_or_create_by(external_id: "2")
+        user2 = User.find_by(external_id: "2")
         thread = CommentThread.where(body: "it is unsolvable").first
         delete "/api/v1/users/#{user2.external_id}/subscriptions", source_type: "thread", source_id: thread.id
         last_response.should be_ok

spec/api/user_spec.rb

+require 'spec_helper'
+
+describe "app" do
+  describe "users" do
+    before :each do
+      User.delete_all
+      create_test_user 1
+      create_test_user 2
+    end
+    describe "POST /api/v1/users" do
+      it "creates a user" do
+        post "/api/v1/users", id: "100", username: "user100", email: "user100@test.com"
+        last_response.should be_ok
+        user = User.find_by(external_id: "100")
+        user.username.should == "user100"
+        user.email.should == "user100@test.com"
+      end
+      it "returns error when id / username / email already exists" do
+        post "/api/v1/users", id: "1", username: "user100", email: "user100@test.com"
+        last_response.status.should == 400
+        post "/api/v1/users", id: "100", username: "user1", email: "user100@test.com"
+        last_response.status.should == 400
+        post "/api/v1/users", id: "100", username: "user100", email: "user1@test.com"
+        last_response.status.should == 400
+      end
+    end
+    describe "PUT /api/v1/users/:user_id" do
+      it "updates user information" do
+        put "/api/v1/users/1", username: "new_user_1"
+        last_response.should be_ok
+        user = User.find_by("1")
+        user.username.should == "new_user_1"
+      end
+      it "does not update id" do
+        put "/api/v1/users/1", id: "100"
+        last_response.should be_ok
+        user = User.find_by("1")
+        user.should_not be_nil
+      end
+      it "returns error if user does not exist" do
+        put "/api/v1/users/100", id: "100"
+        last_response.status.should == 400
+      end
+      it "returns error if new information has conflict with other users" do
+        put "/api/v1/users/1", username: "user2"
+        last_response.status.should == 400 
+      end
+    end
+  end
+end

spec/models/content_spec.rb

@@ -20,6 +20,9 @@ and also the following code
     end
 what is the 'at' symbol doing there? @dementrock
 """
+    User.delete_all
+    User.create!(external_id: "1", username: "tom", email: "tom@test.com")
+    User.create!(external_id: "2", username: "pi314", email: "pi314@test.com")
   end
 
   describe "#get_marked_text(text)" do
@@ -36,10 +39,9 @@ what is the 'at' symbol doing there? @dementrock
   describe "#get_valid_at_position_list(text)" do
     it "returns the list of positions for the valid @ notifications, filtering out the ones in code blocks" do
       list = Content.get_valid_at_position_list(@text)
-      list.should include [0, "tom"]
-      list.should include [1, "pi314"]
-      list.should include [4, "dementrock"]
-      list.length.should == 3
+      list.should include [0, "tom", "1"]
+      list.should include [1, "pi314", "2"]
+      list.length.should == 2
     end
   end
 end

spec/spec_helper.rb

@@ -24,6 +24,10 @@ def parse(text)
   Yajl::Parser.parse text
 end
 
+def create_test_user(id)
+  User.create!(external_id: id.to_s, username: "user#{id}", email: "user#{id}@test.com")
+end
+
 def init_without_subscriptions
   Comment.delete_all
   CommentThread.delete_all
@@ -33,7 +37,7 @@ def init_without_subscriptions
   
   commentable = Commentable.new("question_1")
 
-  user = User.create!(external_id: "1")
+  user = create_test_user(1)
 
   thread = CommentThread.new(title: "I can't solve this problem", body: "can anyone help me?", course_id: "1", commentable_id: commentable.id)
   thread.author = user
@@ -80,7 +84,7 @@ def init_without_subscriptions
   comment1.comment_thread = thread
   comment1.save!
 
-  users = (2..10).map{|id| User.find_or_create_by(external_id: id.to_s)}
+  users = (2..10).map{|id| create_test_user(id)}
 
   Comment.all.each do |c|
     user.vote(c, :up) # make the first user always vote up for convenience
@@ -101,8 +105,8 @@ def init_with_subscriptions
   Notification.delete_all
   Subscription.delete_all
 
-  user1 = User.create!(external_id: "1")
-  user2 = User.create!(external_id: "2")
+  user1 = create_test_user(1)
+  user2 = create_test_user(2)
 
   user2.subscribe(user1)