Allow tags to be deleted

services/web/app/coffee/Features/Tags/TagsController.coffee

@@ -8,7 +8,7 @@ module.exports =
 		project_id = req.params.project_id
 		if req.body.deletedTag?
 			tag = req.body.deletedTag
-			TagsHandler.deleteTag user_id, project_id, tag, ->
+			TagsHandler.removeProject user_id, project_id, tag, ->
 				res.send()
 		else
 			tag = req.body.tag
@@ -19,3 +19,11 @@ module.exports =
 	getAllTags: (req, res)->
 		TagsHandler.getAllTags req.session.user._id, (err, allTags)->
 			res.send(allTags)
+	
+	deleteTag: (req, res, next) ->
+		user_id = req.session.user._id
+		tag_id = req.params.tag_id
+		logger.log {user_id, tag_id}, "deleting tag"
+		TagsHandler.deleteTag user_id, tag_id, (error) ->
+			return next(error) if error?
+			res.status(204).end()

services/web/app/coffee/Features/Tags/TagsHandler.coffee

@@ -5,9 +5,20 @@ logger = require("logger-sharelatex")
 
 oneSecond = 1000
 module.exports = 
+	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)
 
-
-	deleteTag: (user_id, project_id, tag, callback)->
+	removeProject: (user_id, project_id, tag, callback)->
 		uri = buildUri(user_id, project_id)
 		opts =
 			uri:uri

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', AuthenticationController.requireLogin(), TagsController.deleteTag
 
 		# Deprecated in favour of /internal/project/:project_id but still used by versioning
 		apiRouter.get  '/project/:project_id/details', AuthenticationController.httpAuth, ProjectApiController.getProjectDetails

services/web/app/views/project/list/modals.jade

@@ -30,6 +30,32 @@ script(type='text/ng-template', id='newTagModalTemplate')
 			stop-propagation="click"
 		) #{translate("create")}
 
+script(type='text/ng-template', id='deleteTagModalTemplate')
+	.modal-header
+		button.close(
+			type="button"
+			data-dismiss="modal"
+			ng-click="cancel()"
+		) ×
+		h3 #{translate("delete_tag")}
+	.modal-body
+		p #{translate("about_to_delete_tag")}
+		ul
+			li
+				strong {{tag.name}}
+	.modal-footer
+		.modal-footer-left
+			span.text-danger.error(ng-show="state.error") #{translate("generic_something_went_wrong")}
+		button.btn.btn-default(
+			ng-click="cancel()"
+		) #{translate("cancel")}
+		button.btn.btn-danger(
+			ng-click="delete()",
+			ng-disabled="state.inflight"
+		)
+			span(ng-show="state.inflight") #{translate("deleting")}...
+			span(ng-show="!state.inflight") #{translate("delete")}
+
 script(type='text/ng-template', id='renameProjectModalTemplate')
 	.modal-header
 		button.close(

services/web/app/views/project/list/project-list.jade

@@ -58,7 +58,7 @@
 					)
 						li.dropdown-header #{translate("add_to_folder")}
 						li(
-							ng-repeat="tag in tags | filter:nonEmpty | orderBy:'name'",
+							ng-repeat="tag in tags | orderBy:'name'",
 							ng-controller="TagDropdownItemController"
 						)
 							a(href="#", ng-click="addOrRemoveProjectsFromTag()", stop-propagation="click")
@@ -79,7 +79,7 @@
 						href='#',
 						data-toggle="dropdown",
 						dropdown-toggle
-					) #{translate("more")}
+					) #{translate("more")} 
 						span.caret
 					ul.dropdown-menu.dropdown-menu-right(role="menu")
 						li(ng-show="getFirstSelectedProject().accessLevel == 'owner'")

services/web/app/views/project/list/side-bar.jade

@@ -37,22 +37,23 @@
 	ul.list-unstyled.folders-menu(
 		ng-controller="TagListController"
 	)
-		li(ng-class="{active: (filter == 'all')}")
-			a(href, ng-click="filterProjects('all')") #{translate("all_projects")}
-		li(ng-class="{active: (filter == 'owned')}")
-			a(href, ng-click="filterProjects('owned')") #{translate("your_projects")}
-		li(ng-class="{active: (filter == 'shared')}")
-			a(href, ng-click="filterProjects('shared')") #{translate("shared_with_you")}
-		li(ng-class="{active: (filter == 'archived')}")
-			a(href, ng-click="filterProjects('archived')") #{translate("deleted_projects")}
+		li(ng-class="{active: (filter == 'all')}", ng-click="filterProjects('all')")
+			a(href) #{translate("all_projects")}
+		li(ng-class="{active: (filter == 'owned')}", ng-click="filterProjects('owned')")
+			a(href) #{translate("your_projects")}
+		li(ng-class="{active: (filter == 'shared')}", ng-click="filterProjects('shared')")
+			a(href) #{translate("shared_with_you")}
+		li(ng-class="{active: (filter == 'archived')}", ng-click="filterProjects('archived')")
+			a(href) #{translate("deleted_projects")}
 		li
 			h2 #{translate("folders")}
-		li(
-			ng-repeat="tag in tags | filter:nonEmpty",
+		li.tag(
+			ng-repeat="tag in tags | orderBy:name",
 			ng-class="{active: tag.selected}",
-			ng-cloak
+			ng-cloak,
+			ng-click="selectTag(tag)"
 		)
-			a.tag(href, ng-click="selectTag(tag)")
+			a.tag-name(href)
 				i.icon.fa.fa-fw(
 					ng-class="{\
 						'fa-folder-open-o': tag.selected,\
@@ -61,6 +62,23 @@
 				)
 				span.name {{tag.name}} 
 					span.subdued  ({{tag.project_ids.length}})
+			span.dropdown.tag-menu(dropdown)
+				a.dropdown-toggle(
+					href="#",
+					data-toggle="dropdown",
+					dropdown-toggle,
+					stop-propagation="click"
+				)
+					span.caret
+				ul.dropdown-menu.dropdown-menu-right(
+					role="menu"
+				)
+					li
+						a(href)
+							| #{translate("rename")}
+					li
+						a(href, ng-click="deleteTag(tag)")
+							| #{translate("delete")}
 		li(ng-cloak)
 			a.tag(href, ng-click="openNewTagModal()")
 				i.fa.fa-fw.fa-plus

services/web/public/coffee/main/project-list/modal-controllers.coffee

@@ -101,4 +101,29 @@ define [
 
 		$scope.onComplete = (error, name, response) ->
 			if response.project_id?
-				window.location = '/project/' + response.project_id
+				window.location = '/project/' + response.project_id
+
+	App.controller 'DeleteTagModalController', ($scope, $modalInstance, $http, tag) ->
+		$scope.tag = tag
+		$scope.state =
+			inflight: false
+			error: false
+		
+		$scope.delete = () ->
+			$scope.state.inflight = true
+			$scope.state.error = false
+			$http({
+				method: "DELETE"
+				url: "/tag/#{tag._id}"
+				headers:
+					"X-CSRF-Token": window.csrfToken
+			})
+				.success () ->
+					$scope.state.inflight = false
+					$modalInstance.close()
+				.error () ->
+					$scope.state.inflight = false
+					$scope.state.error = true
+		
+		$scope.cancel = () ->
+			$modalInstance.dismiss('cancel')

services/web/public/coffee/main/project-list/tag-controllers.coffee

@@ -2,7 +2,7 @@ define [
 	"base"
 ], (App) ->
 
-	App.controller "TagListController", ($scope) ->
+	App.controller "TagListController", ($scope, $modal) ->
 		$scope.filterProjects = (filter = "all") ->
 			$scope._clearTags()
 			$scope.setFilter(filter)
@@ -10,17 +10,21 @@ define [
 		$scope._clearTags = () ->
 			for tag in $scope.tags
 				tag.selected = false
-
-		$scope.nonEmpty = (tag) ->
-			# The showWhenEmpty property will be set on any tag which we have
-			# modified during this session. Otherwise, tags which are empty
-			# when loading the page are not shown.
-			tag.project_ids.length > 0 or !!tag.showWhenEmpty
 			
 		$scope.selectTag = (tag) ->
 			$scope._clearTags()
 			tag.selected = true
 			$scope.setFilter("tag")
+		
+		$scope.deleteTag = (tag) ->
+			modalInstance = $modal.open(
+				templateUrl: "deleteTagModalTemplate"
+				controller: "DeleteTagModalController"
+				resolve:
+					tag: () -> tag
+			)
+			modalInstance.result.then () ->
+				$scope.tags = $scope.tags.filter (t) -> t != tag
 
 	App.controller "TagDropdownItemController", ($scope) ->
 		$scope.recalculateProjectsInTag = () ->

services/web/public/stylesheets/app/project-list.less

@@ -33,9 +33,11 @@ ul.folders-menu {
 	.subdued {
 		color: @gray-light;
 	}
-	li {
+	> li {
+		cursor: pointer;
 		line-height: 1.8;
-		a {
+		position: relative;
+		> a {
 			font-size: 0.9rem;
 			color: #333;
 			padding: (@line-height-computed / 4);
@@ -43,19 +45,19 @@ ul.folders-menu {
 			line-height: 1.2;
 		}
 	}
-	li.active {
+	> li.active {
 		//border-right: 4px solid @red;
-		a {
+		background-color: @link-color;
+		border-radius: @border-radius-small;
+		> a {
 			font-weight: 700;
 			color: white;
-			background-color: @link-color;
-			border-radius: @border-radius-small;
 			.subdued {
 				color: white;
 			}
 		}
 	}
-	li > a.small {
+	> li > a.small {
 		color: @gray;
 	}
 	h2 {
@@ -65,19 +67,67 @@ ul.folders-menu {
 		font-weight: 500;
 		font-family: @font-family-sans-serif;
 	}
-	a.tag {
-		padding: 2px (@line-height-computed / 4);
-		display: inline-block;
-		position: relative;
-		i {
-			position: absolute;
-			top: 5px;
-			left: 6px;
+	> li.tag {
+		&.active {
+			.tag-menu > a {
+				color: white;
+				border-color: white;
+				&:hover {
+					background-color: darken(@brand-primary, 10%);
+				}
+			}
 		}
-		span.name {
+		&:hover {
+			&:not(.active) {
+				background-color: darken(@gray-lightest, 2%);
+			}
+			.tag-menu {
+				display: block
+			}
+		}
+		&:not(.active) {
+			.tag-menu > a:hover {
+				background-color: @gray-light;
+			}
+		}
+		a.tag-name {
+			padding: 2px (@line-height-computed / 4);
+			margin-right: 18px;
 			display: inline-block;
-			padding-left: 22px;
-			line-height: 1.4;
+			position: relative;
+			i {
+				position: absolute;
+				top: 5px;
+				left: 6px;
+			}
+			span.name {
+				display: inline-block;
+				padding-left: 22px;
+				line-height: 1.4;
+			}
+		}
+		.tag-menu {
+			> a {
+				border: 1px solid @gray;
+				border-radius: @border-radius-small;
+				color: @text-color;
+				display: block;
+				width: 16px;
+				height: 16px;
+				position: relative;
+				.caret {
+					position: absolute;
+					top: 6px;
+					left: 1px;
+				}
+			}
+			display: none;
+			position: absolute;
+			top: 6px;
+			right: 4px;
+			&.open {
+				display: block;
+			}
 		}
 	}
 }

services/web/test/UnitTests/coffee/Tags/TagsControllerTests.coffee

@@ -13,7 +13,8 @@ describe 'Tags controller', ->
 	beforeEach ->
 		@handler = 
 			addTag: sinon.stub().callsArgWith(3)
-			deleteTag: sinon.stub().callsArgWith(3)
+			removeProject: sinon.stub().callsArgWith(3)
+			deleteTag: sinon.stub().callsArg(2)
 		@controller = SandboxedModule.require modulePath, requires:
 			"./TagsHandler":@handler
 			'logger-sharelatex':
@@ -25,26 +26,45 @@ describe 'Tags controller', ->
 			session:
 				user:
 					_id:user_id
+		
+		@res = {}
+		@res.status = sinon.stub().returns @res
+		@res.end = sinon.stub()
+	
+	describe "processTagsUpdate", ->
+		it 'Should post the request to the tags api with the user id in the url', (done)->
+			@req.body = {tag:tag}
+			@controller.processTagsUpdate @req, send:=>
+				@handler.addTag.calledWith(user_id, project_id, tag).should.equal true
+				done()
 
 
-	it 'Should post the request to the tags api with the user id in the url', (done)->
-		@req.body = {tag:tag}
-		@controller.processTagsUpdate @req, send:=>
-			@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()
 
-
-	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.deleteTag.calledWith(user_id, project_id, tag).should.equal true
-			done()
-
-
-	it 'should ask the handler for all tags', (done)->
-		allTags = [{name:"tag", projects:["123423","423423"]}]
-		@handler.getAllTags = sinon.stub().callsArgWith(1, null, allTags)
-		@controller.getAllTags @req, send:(body)=>
-			body.should.equal allTags
-			@handler.getAllTags.calledWith(user_id).should.equal true
-			done()
+	describe "getAllTags", ->
+		it 'should ask the handler for all tags', (done)->
+			allTags = [{name:"tag", projects:["123423","423423"]}]
+			@handler.getAllTags = sinon.stub().callsArgWith(1, null, allTags)
+			@controller.getAllTags @req, send:(body)=>
+				body.should.equal allTags
+				@handler.getAllTags.calledWith(user_id).should.equal true
+				done()
+	
+	describe "deleteTag", ->
+		beforeEach ->
+			@req.params.tag_id = @tag_id = "tag-id-123"
+			@req.session.user._id = @user_id = "user-id-123"
+			@controller.deleteTag @req, @res
+			
+		it "should delete the tag in the backend", ->
+			@handler.deleteTag
+				.calledWith(@user_id, @tag_id)
+				.should.equal true
+		
+		it "should return 204 status code", ->
+			@res.status.calledWith(204).should.equal true
+			@res.end.called.should.equal true

services/web/test/UnitTests/coffee/Tags/TagsHandlerTests.coffee

@@ -8,6 +8,7 @@ _ = require('underscore')
 
 describe 'TagsHandler', ->
 	user_id = "123nd3ijdks"
+	tag_id = "tag-id-123"
 	project_id = "123njdskj9jlk"
 	tagsUrl = "tags.sharelatex.testing"
 	tag = "class101"
@@ -17,6 +18,7 @@ describe 'TagsHandler', ->
 			post: sinon.stub().callsArgWith(1)
 			del: sinon.stub().callsArgWith(1)
 			get: sinon.stub()
+		@callback = sinon.stub()
 		@handler = SandboxedModule.require modulePath, requires:
 			"settings-sharelatex": apis:{tags:{url:tagsUrl}}
 			"request":@request
@@ -24,20 +26,23 @@ describe 'TagsHandler', ->
 				log:->
 				err:->
 
-	it 'Should post the request to the tags api with the user id in the url', (done)->
-		@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()
-
-	it 'should send a delete request when a delete has been recived with the body format standardised', (done)->
-		@handler.deleteTag 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 "addTag", ->
+		it 'Should post the request to the tags api with the user id in the url', (done)->
+			@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()
 
-	it 'should tell the tags api to remove the project_id from all the users tags', (done)->
-		@handler.removeProjectFromAllTags user_id, project_id, =>
-			@request.del.calledWith({uri:"#{tagsUrl}/user/#{user_id}/project/#{project_id}", timeout:1000}).should.equal true
-			done()
+	describe "removeProjectFromAllTags", ->
+		it 'should tell the tags api to remove the project_id from all the users tags', (done)->
+			@handler.removeProjectFromAllTags user_id, project_id, =>
+				@request.del.calledWith({uri:"#{tagsUrl}/user/#{user_id}/project/#{project_id}", timeout:1000}).should.equal true
+				done()
 
 	describe "groupTagsByProject", ->
 		it 'should 	group the tags by project_id', (done)->
@@ -99,4 +104,26 @@ describe 'TagsHandler', ->
 			@handler.getAllTags user_id, (err, allTags, projectGroupedTags)=>
 				allTags.length.should.equal 0
 				_.size(projectGroupedTags).should.equal 0
-
+	
+	describe "deleteTag", ->
+		describe "successfully", ->
+			beforeEach ->
+				@request.del = sinon.stub().callsArgWith(1, null, {statusCode: 204}, "")
+				@handler.deleteTag user_id, tag_id, @callback
+			
+			it "should send a request to the tag backend", ->
+				@request.del
+					.calledWith("#{tagsUrl}/user/#{user_id}/tag/#{tag_id}")
+					.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.deleteTag user_id, tag_id, @callback
+			
+			it "should call the callback with an Error", ->
+				@callback.calledWith(new Error()).should.equal true
+