فهرست منبع

Use tag_id for deleting tags from project

James Allen 10 سال پیش
والد
کامیت
f1e65cc776

+ 11 - 8
services/web/app/coffee/Features/Tags/TagsController.coffee

@@ -6,20 +6,23 @@ module.exports =
 	processTagsUpdate: (req, res)->
 		user_id = req.session.user._id
 		project_id = req.params.project_id
-		if req.body.deletedTag?
-			tag = req.body.deletedTag
-			TagsHandler.removeProject user_id, project_id, tag, ->
-				res.send()
-		else
-			tag = req.body.tag
-			TagsHandler.addTag user_id, project_id, tag, ->
-				res.send()
+		tag = req.body.tag
+		TagsHandler.addTag user_id, project_id, tag, ->
+			res.send()
 		logger.log user_id:user_id, project_id:project_id, body:req.body, "processing tag update"
 
 	getAllTags: (req, res)->
 		TagsHandler.getAllTags req.session.user._id, (err, allTags)->
 			res.send(allTags)
 	
+	removeProjectFromTag: (req, res, next) ->
+		user_id = req.session.user._id
+		{tag_id, project_id} = req.params
+		logger.log {user_id, tag_id, project_id}, "removing tag from project"
+		TagsHandler.removeProjectFromTag user_id, tag_id, project_id, (error) ->
+			return next(error) if error?
+			res.status(204).end()
+	
 	deleteTag: (req, res, next) ->
 		user_id = req.session.user._id
 		tag_id = req.params.tag_id

+ 30 - 41
services/web/app/coffee/Features/Tags/TagsHandler.coffee

@@ -3,47 +3,40 @@ settings = require("settings-sharelatex")
 request = require("request")
 logger = require("logger-sharelatex")
 
-oneSecond = 1000
-module.exports = 
+TIMEOUT = 1000
+module.exports = TagsHandler =
+	_handleResponse: (res, params, callback) ->
+		if err?
+			params.err = err
+			logger.err params, "error in tag api"
+			return callback(err)
+		else if res? and res.statusCode >= 200 and res.statusCode < 300
+			return callback(null)
+		else
+			err = new Error("tags api returned a failure status code: #{res?.statusCode}")
+			params.err = err
+			logger.err params, "tags api returned failure status code: #{res?.statusCode}"
+			return callback(err)
+
 	renameTag: (user_id, tag_id, name, callback = (error) ->) ->
 		url = "#{settings.apis.tags.url}/user/#{user_id}/tag/#{tag_id}/rename"
 		request.post {
 			url: url
 			json:
 				name: name
+			timeout: TIMEOUT
 		}, (err, res, body) ->
-			if err?
-				logger.err {err, user_id, tag_id, name}, "error renaming tag in tag api"
-				return callback(err)
-			else if res.statusCode >= 200 and res.statusCode < 300
-				return callback(null)
-			else
-				err = new Error("tags api returned a failure status code: #{res.statusCode}")
-				logger.err {err, user_id, tag_id, name}, "tags api returned failure status code: #{res.statusCode}"
-				return callback(err)
+			TagsHandler._handleResponse res, {url, user_id, tag_id, name}, callback
 
 	deleteTag: (user_id, tag_id, callback = (error) ->) ->
 		url = "#{settings.apis.tags.url}/user/#{user_id}/tag/#{tag_id}"
-		request.del url, (err, res, body) ->
-			if err?
-				logger.err {err, user_id, tag_id}, "error deleting tag from tag api"
-				return callback(err)
-			else if res.statusCode >= 200 and res.statusCode < 300
-				return callback(null)
-			else
-				err = new Error("tags api returned a failure status code: #{res.statusCode}")
-				logger.err {err, user_id, tag_id}, "tags api returned failure status code: #{res.statusCode}"
-				return callback(err)
+		request.del {url, timeout: TIMEOUT}, (err, res, body) ->
+			TagsHandler._handleResponse res, {url, user_id, tag_id}, callback
 
-	removeProject: (user_id, project_id, tag, callback)->
-		uri = buildUri(user_id, project_id)
-		opts =
-			uri:uri
-			json:
-				name:tag
-			timeout:oneSecond
-		logger.log user_id:user_id, project_id:project_id, tag:tag, "send delete tag to tags api"
-		request.del opts, callback
+	removeProjectFromTag: (user_id, tag_id, project_id, callback)->
+		url = "#{settings.apis.tags.url}/user/#{user_id}/tag/#{tag_id}/project/#{project_id}"
+		request.del {url, timeout: TIMEOUT}, (err, res, body) ->
+			TagsHandler._handleResponse res, {url, user_id, tag_id, project_id}, callback
 
 	addTag: (user_id, project_id, tag, callback)->
 		uri = buildUri(user_id, project_id)
@@ -51,23 +44,19 @@ module.exports =
 			uri:uri
 			json:
 				name:tag
-			timeout:oneSecond
+			timeout: TIMEOUT
 		logger.log user_id:user_id, project_id:project_id, tag:tag, "send add tag to tags api"
 		request.post opts, callback
 
 	requestTags: (user_id, callback)->
 		opts = 
-			uri: "#{settings.apis.tags.url}/user/#{user_id}/tag"
+			url: "#{settings.apis.tags.url}/user/#{user_id}/tag"
 			json: true
-			timeout: 2000
+			timeout: TIMEOUT
 		request.get opts, (err, res, body)->
-			statusCode =  if res? then res.statusCode else 500
-			if err? or statusCode != 200
-				e = new Error("something went wrong getting tags, #{err}, #{statusCode}")
-				logger.err err:err
-				callback(e, [])
-			else
-				callback(null, body)
+			TagsHandler._handleResponse res, {user_id}, (error) ->
+				return callback(error, []) if error?
+				callback(null, body or [])
 
 	getAllTags: (user_id, callback)->
 		@requestTags user_id, (err, allTags)=>
@@ -81,7 +70,7 @@ module.exports =
 		uri = buildUri(user_id, project_id)
 		opts =
 			uri:"#{settings.apis.tags.url}/user/#{user_id}/project/#{project_id}"
-			timeout:oneSecond
+			timeout:TIMEOUT
 		logger.log user_id:user_id, project_id:project_id, "removing project_id from tags"
 		request.del opts, callback
 

+ 1 - 0
services/web/app/coffee/router.coffee

@@ -133,6 +133,7 @@ module.exports = class Router
 
 		webRouter.get '/tag', AuthenticationController.requireLogin(), TagsController.getAllTags
 		webRouter.post '/project/:project_id/tag', AuthenticationController.requireLogin(), TagsController.processTagsUpdate
+		webRouter.delete '/tag/:tag_id/project/:project_id', AuthenticationController.requireLogin(), TagsController.removeProjectFromTag
 		webRouter.delete '/tag/:tag_id', AuthenticationController.requireLogin(), TagsController.deleteTag
 		webRouter.post '/tag/:tag_id/rename', AuthenticationController.requireLogin(), TagsController.renameTag
 

+ 6 - 4
services/web/public/coffee/main/project-list/project-list.coffee

@@ -169,10 +169,12 @@ define [
 					project.tags.splice(index, 1)
 
 			for project_id in removed_project_ids
-				queuedHttp.post "/project/#{project_id}/tag", {
-					deletedTag: tag.name
-					_csrf: window.csrfToken
-				}
+				queuedHttp({
+					method: "DELETE"
+					url: "/tag/#{tag._id}/project/#{project_id}"
+					headers:
+						"X-CSRF-Token": window.csrfToken
+				})
 
 			# If we're filtering by this tag then we need to remove
 			# the projects from view

+ 17 - 8
services/web/test/UnitTests/coffee/Tags/TagsControllerTests.coffee

@@ -13,7 +13,7 @@ describe 'Tags controller', ->
 	beforeEach ->
 		@handler = 
 			addTag: sinon.stub().callsArgWith(3)
-			removeProject: sinon.stub().callsArgWith(3)
+			removeProjectFromTag: sinon.stub().callsArgWith(3)
 			deleteTag: sinon.stub().callsArg(2)
 			renameTag: sinon.stub().callsArg(3)
 		@controller = SandboxedModule.require modulePath, requires:
@@ -39,13 +39,6 @@ describe 'Tags controller', ->
 				@handler.addTag.calledWith(user_id, project_id, tag).should.equal true
 				done()
 
-
-		it 'should send a delete request when a delete has been recived with the body format standardised', (done)->
-			@req.body = {deletedTag:tag}
-			@controller.processTagsUpdate @req, send:=>
-				@handler.removeProject.calledWith(user_id, project_id, tag).should.equal true
-				done()
-
 	describe "getAllTags", ->
 		it 'should ask the handler for all tags', (done)->
 			allTags = [{name:"tag", projects:["123423","423423"]}]
@@ -99,4 +92,20 @@ describe 'Tags controller', ->
 			it "should return 400 (bad request) status code", ->
 				@res.status.calledWith(400).should.equal true
 				@res.end.called.should.equal true
+	
+	describe "removeProjectFromTag", ->
+		beforeEach ->
+			@req.params.tag_id = @tag_id = "tag-id-123"
+			@req.params.project_id = @project_id = "project-id-123"
+			@req.session.user._id = @user_id = "user-id-123"
+			@controller.removeProjectFromTag @req, @res
+			
+		it "should remove the tag from the project in the backend", ->
+			@handler.removeProjectFromTag
+				.calledWith(@user_id, @tag_id, @project_id)
+				.should.equal true
+		
+		it "should return 204 status code", ->
+			@res.status.calledWith(204).should.equal true
+			@res.end.called.should.equal true
 	

+ 35 - 12
services/web/test/UnitTests/coffee/Tags/TagsHandlerTests.coffee

@@ -7,11 +7,11 @@ _ = require('underscore')
 
 
 describe 'TagsHandler', ->
-	user_id = "123nd3ijdks"
+	user_id = "user-id-123"
 	tag_id = "tag-id-123"
-	project_id = "123njdskj9jlk"
+	project_id = "project-id-123"
 	tagsUrl = "tags.sharelatex.testing"
-	tag = "class101"
+	tag = "tag_name"
 
 	beforeEach ->
 		@request = 
@@ -31,12 +31,6 @@ describe 'TagsHandler', ->
 			@handler.addTag user_id, project_id, tag, =>
 				@request.post.calledWith({uri:"#{tagsUrl}/user/#{user_id}/project/#{project_id}/tag", timeout:1000, json:{name:tag}}).should.equal true
 				done()
-	
-	describe "removeProject", ->
-		it 'should send a delete request when a delete has been recived with the body format standardised', (done)->
-			@handler.removeProject user_id, project_id, tag, =>
-				@request.del.calledWith({uri:"#{tagsUrl}/user/#{user_id}/project/#{project_id}/tag", timeout:1000,  json:{name:tag}}).should.equal true
-				done()
 
 	describe "removeProjectFromAllTags", ->
 		it 'should tell the tags api to remove the project_id from all the users tags', (done)->
@@ -93,9 +87,9 @@ describe 'TagsHandler', ->
 			@handler.getAllTags user_id, (err, allTags)=>
 				stubbedAllTags.should.deep.equal allTags
 				getOpts =
-					uri: "#{tagsUrl}/user/#{user_id}/tag"
+					url: "#{tagsUrl}/user/#{user_id}/tag"
 					json:true
-					timeout:2000
+					timeout:1000
 				@request.get.calledWith(getOpts).should.equal true
 				done()
 
@@ -113,7 +107,10 @@ describe 'TagsHandler', ->
 			
 			it "should send a request to the tag backend", ->
 				@request.del
-					.calledWith("#{tagsUrl}/user/#{user_id}/tag/#{tag_id}")
+					.calledWith({
+						url: "#{tagsUrl}/user/#{user_id}/tag/#{tag_id}"
+						timeout: 1000
+					})
 					.should.equal true
 			
 			it "should call the callback with no error", ->
@@ -139,6 +136,7 @@ describe 'TagsHandler', ->
 						url: "#{tagsUrl}/user/#{user_id}/tag/#{tag_id}/rename"
 						json:
 							name: @name
+						timeout: 1000
 					})
 					.should.equal true
 			
@@ -152,3 +150,28 @@ describe 'TagsHandler', ->
 			
 			it "should call the callback with an Error", ->
 				@callback.calledWith(new Error()).should.equal true
+	
+	describe "removeProjectFromTag", ->
+		describe "successfully", ->
+			beforeEach ->
+				@request.del = sinon.stub().callsArgWith(1, null, {statusCode: 204}, "")
+				@handler.removeProjectFromTag user_id, tag_id, project_id, @callback
+			
+			it "should send a request to the tag backend", ->
+				@request.del
+					.calledWith({
+						url: "#{tagsUrl}/user/#{user_id}/tag/#{tag_id}/project/#{project_id}"
+						timeout: 1000
+					})
+					.should.equal true
+			
+			it "should call the callback with no error", ->
+				@callback.calledWith(null).should.equal true
+			
+		describe "with error", ->
+			beforeEach ->
+				@request.del = sinon.stub().callsArgWith(1, null, {statusCode: 500}, "")
+				@handler.removeProjectFromTag user_id, tag_id, project_id, @callback
+			
+			it "should call the callback with an Error", ->
+				@callback.calledWith(new Error()).should.equal true