Merge pull request #6 from sharelatex/ja-dont-put-chat-messages-in-global-thread

Don't read from comment thread if no global message thread exists

services/chat/app/coffee/Features/Threads/ThreadManager.coffee

@@ -6,21 +6,33 @@ module.exports = ThreadManager =
 	GLOBAL_THREAD: "GLOBAL"
 
 	findOrCreateThread: (project_id, thread_id, callback = (error, thread) ->) ->
-		query =
-			project_id: ObjectId(project_id.toString())
+		project_id = ObjectId(project_id.toString())
+		if thread_id != ThreadManager.GLOBAL_THREAD
+			thread_id = ObjectId(thread_id.toString())
 
-		if thread_id? and thread_id != ThreadManager.GLOBAL_THREAD
-			query.thread_id = ObjectId(thread_id.toString())
-
-		# Threads used to be called rooms, and still are in the DB
-		db.rooms.findOne query, (error, thread) ->
+		if thread_id == ThreadManager.GLOBAL_THREAD
+			query = {
+				project_id: project_id
+				thread_id: { $exists: false }
+			}
+			update = {
+				project_id: project_id
+			}
+		else
+			query = {
+				project_id: project_id
+				thread_id: thread_id
+			}
+			update = {
+				project_id: project_id
+				thread_id: thread_id
+			}
+		
+		db.rooms.update query, update, { upsert: true }, (error) ->
 			return callback(error) if error?
-			if thread?
-				callback null, thread
-			else
-				db.rooms.save query, (error, thread) ->
-					return callback(error) if error?
-					callback null, thread
+			db.rooms.find query, (error, rooms = []) ->
+				return callback(error) if error?
+				return callback null, rooms[0]
 	
 	findAllThreadRooms: (project_id, callback = (error, rooms) ->) ->
 		db.rooms.find {

services/chat/test/acceptance/coffee/SendingAMessageTests.coffee

@@ -5,12 +5,11 @@ ChatClient = require "./helpers/ChatClient"
 
 describe "Sending a message", ->
 	before ->
-		@project_id = ObjectId().toString()
-		@user_id = ObjectId().toString()
-		@thread_id = ObjectId().toString()
 
 	describe "globally", ->
 		before (done) ->
+			@project_id = ObjectId().toString()
+			@user_id = ObjectId().toString()
 			@content = "global message"
 			ChatClient.sendGlobalMessage @project_id, @user_id, @content, (error, response, body) =>
 				expect(error).to.be.null
@@ -30,6 +29,9 @@ describe "Sending a message", ->
 
 	describe "to a thread", ->
 		before (done) ->
+			@project_id = ObjectId().toString()
+			@user_id = ObjectId().toString()
+			@thread_id = ObjectId().toString()
 			@content = "thread message"
 			ChatClient.sendMessage @project_id, @thread_id, @user_id, @content, (error, response, body) =>
 				expect(error).to.be.null
@@ -46,39 +48,52 @@ describe "Sending a message", ->
 				expect(threads[@thread_id].messages.length).to.equal 1
 				expect(threads[@thread_id].messages[0].content).to.equal @content
 				done()
-	
-	describe "with a malformed user_id", ->
-		it "should return a graceful error", (done) ->
-			ChatClient.sendMessage @project_id, @thread_id, "malformed-user", "content", (error, response, body) =>
-				expect(response.statusCode).to.equal 400
-				expect(body).to.equal "Invalid user_id"
+		
+		it "should not appear in the global messages", (done) ->
+			ChatClient.getGlobalMessages @project_id, (error, response, messages) =>
+				expect(error).to.be.null
+				expect(response.statusCode).to.equal 200
+				expect(messages.length).to.equal 0
 				done()
 	
-	describe "with a malformed project_id", ->
-		it "should return a graceful error", (done) ->
-			ChatClient.sendMessage "malformed-project", @thread_id, @user_id, "content", (error, response, body) =>
-				expect(response.statusCode).to.equal 400
-				expect(body).to.equal "Invalid project_id"
-				done()
-	
-	describe "with a malformed thread_id", ->
-		it "should return a graceful error", (done) ->
-			ChatClient.sendMessage @project_id, "malformed-thread-id", @user_id, "content", (error, response, body) =>
-				expect(response.statusCode).to.equal 400
-				expect(body).to.equal "Invalid thread_id"
-				done()
-	
-	describe "with no content", ->
-		it "should return a graceful error", (done) ->
-			ChatClient.sendMessage @project_id, @thread_id, @user_id, null, (error, response, body) =>
-				expect(response.statusCode).to.equal 400
-				expect(body).to.equal "No content provided"
-				done()
-	
-	describe "with very long content", ->
-		it "should return a graceful error", (done) ->
-			content = new Buffer(10240).toString("hex")
-			ChatClient.sendMessage @project_id, @thread_id, @user_id, content, (error, response, body) =>
-				expect(response.statusCode).to.equal 400
-				expect(body).to.equal "Content too long (> 10240 bytes)"
-				done()
+	describe "failure cases", ->
+		before () ->
+			@project_id = ObjectId().toString()
+			@user_id = ObjectId().toString()
+			@thread_id = ObjectId().toString()
+		
+		describe "with a malformed user_id", ->
+			it "should return a graceful error", (done) ->
+				ChatClient.sendMessage @project_id, @thread_id, "malformed-user", "content", (error, response, body) =>
+					expect(response.statusCode).to.equal 400
+					expect(body).to.equal "Invalid user_id"
+					done()
+		
+		describe "with a malformed project_id", ->
+			it "should return a graceful error", (done) ->
+				ChatClient.sendMessage "malformed-project", @thread_id, @user_id, "content", (error, response, body) =>
+					expect(response.statusCode).to.equal 400
+					expect(body).to.equal "Invalid project_id"
+					done()
+		
+		describe "with a malformed thread_id", ->
+			it "should return a graceful error", (done) ->
+				ChatClient.sendMessage @project_id, "malformed-thread-id", @user_id, "content", (error, response, body) =>
+					expect(response.statusCode).to.equal 400
+					expect(body).to.equal "Invalid thread_id"
+					done()
+		
+		describe "with no content", ->
+			it "should return a graceful error", (done) ->
+				ChatClient.sendMessage @project_id, @thread_id, @user_id, null, (error, response, body) =>
+					expect(response.statusCode).to.equal 400
+					expect(body).to.equal "No content provided"
+					done()
+		
+		describe "with very long content", ->
+			it "should return a graceful error", (done) ->
+				content = new Buffer(10240).toString("hex")
+				ChatClient.sendMessage @project_id, @thread_id, @user_id, content, (error, response, body) =>
+					expect(response.statusCode).to.equal 400
+					expect(body).to.equal "Content too long (> 10240 bytes)"
+					done()