From b32178182d3f64d2e6d6b15b8cb2df48d61923dd Mon Sep 17 00:00:00 2001 From: James Allen Date: Fri, 29 Jan 2016 15:11:27 +0000 Subject: [PATCH] Explicitly create tags and get their id --- .../Features/Tags/TagsController.coffee | 17 ++- .../coffee/Features/Tags/TagsHandler.coffee | 105 ++++++++++-------- services/web/app/coffee/router.coffee | 7 +- .../web/app/views/project/list/modals.jade | 8 +- .../main/project-list/project-list.coffee | 12 +- .../main/project-list/tag-controllers.coffee | 21 +++- .../coffee/Tags/TagsControllerTests.coffee | 19 +++- .../coffee/Tags/TagsHandlerTests.coffee | 34 ++++-- 8 files changed, 147 insertions(+), 76 deletions(-) diff --git a/services/web/app/coffee/Features/Tags/TagsController.coffee b/services/web/app/coffee/Features/Tags/TagsController.coffee index 76f0976c4d..2e67be2fd4 100644 --- a/services/web/app/coffee/Features/Tags/TagsController.coffee +++ b/services/web/app/coffee/Features/Tags/TagsController.coffee @@ -2,9 +2,20 @@ TagsHandler = require("./TagsHandler") logger = require("logger-sharelatex") module.exports = - getAllTags: (req, res)-> - TagsHandler.getAllTags req.session.user._id, (err, allTags)-> - res.send(allTags) + getAllTags: (req, res, next)-> + user_id = req.session.user._id + logger.log {user_id}, "getting tags" + TagsHandler.getAllTags user_id, (error, allTags)-> + return next(error) if error? + res.json(allTags) + + createTag: (req, res, next) -> + user_id = req.session.user._id + name = req.body.name + logger.log {user_id, name}, "creating tag" + TagsHandler.createTag user_id, name, (error, tag) -> + return next(error) if error? + res.json(tag) addProjectToTag: (req, res, next) -> user_id = req.session.user._id diff --git a/services/web/app/coffee/Features/Tags/TagsHandler.coffee b/services/web/app/coffee/Features/Tags/TagsHandler.coffee index 61e335356a..2dc57608bd 100644 --- a/services/web/app/coffee/Features/Tags/TagsHandler.coffee +++ b/services/web/app/coffee/Features/Tags/TagsHandler.coffee @@ -5,7 +5,59 @@ logger = require("logger-sharelatex") TIMEOUT = 1000 module.exports = TagsHandler = - _handleResponse: (res, params, callback) -> + getAllTags: (user_id, callback)-> + @_requestTags user_id, (err, allTags)=> + if !allTags? + allTags = [] + @_groupTagsByProject allTags, (err, groupedByProject)-> + logger.log allTags:allTags, user_id:user_id, groupedByProject:groupedByProject, "got all tags from tags api" + callback err, allTags, groupedByProject + + createTag: (user_id, name, callback = (error, tag) ->) -> + opts = + url: "#{settings.apis.tags.url}/user/#{user_id}/tag" + json: + name: name + timeout: TIMEOUT + request.post opts, (err, res, body)-> + TagsHandler._handleResponse err, res, {user_id}, (error) -> + return callback(error) if error? + callback(null, body or {}) + + 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) -> + TagsHandler._handleResponse err, 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, timeout: TIMEOUT}, (err, res, body) -> + TagsHandler._handleResponse err, res, {url, user_id, tag_id}, 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 err, res, {url, user_id, tag_id, project_id}, callback + + addProjectToTag: (user_id, tag_id, project_id, callback)-> + url = "#{settings.apis.tags.url}/user/#{user_id}/tag/#{tag_id}/project/#{project_id}" + request.post {url, timeout: TIMEOUT}, (err, res, body) -> + TagsHandler._handleResponse err, res, {url, user_id, tag_id, project_id}, callback + + removeProjectFromAllTags: (user_id, project_id, callback)-> + url = "#{settings.apis.tags.url}/user/#{user_id}/project/#{project_id}" + opts = + url: url + timeout:TIMEOUT + request.del opts, (err, res, body) -> + TagsHandler._handleResponse err, res, {url, user_id, project_id}, callback + + _handleResponse: (err, res, params, callback) -> if err? params.err = err logger.err params, "error in tag api" @@ -18,58 +70,17 @@ module.exports = TagsHandler = 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) -> - 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, timeout: TIMEOUT}, (err, res, body) -> - TagsHandler._handleResponse res, {url, user_id, tag_id}, 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 - - addProjectToTag: (user_id, tag_id, project_id, callback)-> - url = "#{settings.apis.tags.url}/user/#{user_id}/tag/#{tag_id}/project/#{project_id}" - request.post {url, timeout: TIMEOUT}, (err, res, body) -> - TagsHandler._handleResponse res, {url, user_id, tag_id, project_id}, callback - - requestTags: (user_id, callback)-> + _requestTags: (user_id, callback)-> opts = url: "#{settings.apis.tags.url}/user/#{user_id}/tag" json: true timeout: TIMEOUT request.get opts, (err, res, body)-> - TagsHandler._handleResponse res, {user_id}, (error) -> + TagsHandler._handleResponse err, res, {user_id}, (error) -> return callback(error, []) if error? callback(null, body or []) - getAllTags: (user_id, callback)-> - @requestTags user_id, (err, allTags)=> - if !allTags? - allTags = [] - @groupTagsByProject allTags, (err, groupedByProject)-> - logger.log allTags:allTags, user_id:user_id, groupedByProject:groupedByProject, "getting all tags from tags api" - callback err, allTags, groupedByProject - - removeProjectFromAllTags: (user_id, project_id, callback)-> - uri = buildUri(user_id, project_id) - opts = - uri:"#{settings.apis.tags.url}/user/#{user_id}/project/#{project_id}" - timeout:TIMEOUT - logger.log user_id:user_id, project_id:project_id, "removing project_id from tags" - request.del opts, callback - - groupTagsByProject: (tags, callback)-> + _groupTagsByProject: (tags, callback)-> result = {} _.each tags, (tag)-> _.each tag.project_ids, (project_id)-> @@ -81,7 +92,3 @@ module.exports = TagsHandler = delete clonedTag.project_ids result[project_id].push(clonedTag) callback null, result - - -buildUri = (user_id, project_id)-> - uri = "#{settings.apis.tags.url}/user/#{user_id}/project/#{project_id}/tag" diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index d5510976a0..4fe9f2a247 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -131,11 +131,12 @@ module.exports = class Router webRouter.get '/Project/:Project_id/download/zip', SecurityManager.requestCanAccessProject, ProjectDownloadsController.downloadProject webRouter.get '/project/download/zip', SecurityManager.requestCanAccessMultipleProjects, ProjectDownloadsController.downloadMultipleProjects - webRouter.get '/tag', AuthenticationController.requireLogin(), TagsController.getAllTags + webRouter.get '/tag', AuthenticationController.requireLogin(), TagsController.getAllTags + webRouter.post '/tag', AuthenticationController.requireLogin(), TagsController.createTag + webRouter.post '/tag/:tag_id/rename', AuthenticationController.requireLogin(), TagsController.renameTag + webRouter.delete '/tag/:tag_id', AuthenticationController.requireLogin(), TagsController.deleteTag webRouter.post '/tag/:tag_id/project/:project_id', AuthenticationController.requireLogin(), TagsController.addProjectToTag 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 # Deprecated in favour of /internal/project/:project_id but still used by versioning apiRouter.get '/project/:project_id/details', AuthenticationController.httpAuth, ProjectApiController.getProjectDetails diff --git a/services/web/app/views/project/list/modals.jade b/services/web/app/views/project/list/modals.jade index 78f0aae8fe..8a09c0bec4 100644 --- a/services/web/app/views/project/list/modals.jade +++ b/services/web/app/views/project/list/modals.jade @@ -18,6 +18,8 @@ script(type='text/ng-template', id='newTagModalTemplate') stop-propagation="click" ) .modal-footer + .modal-footer-left + span.text-danger.error(ng-show="state.error") #{translate("generic_something_went_wrong")} //- We stop propagation to stop the clicks from closing the //- 'move to folder' menu. button.btn.btn-default( @@ -25,10 +27,12 @@ script(type='text/ng-template', id='newTagModalTemplate') stop-propagation="click" ) #{translate("cancel")} button.btn.btn-primary( - ng-disabled="newTagForm.$invalid" + ng-disabled="newTagForm.$invalid || state.inflight" ng-click="create()" stop-propagation="click" - ) #{translate("create")} + ) + span(ng-show="!state.inflight") #{translate("create")} + span(ng-show="state.inflight") #{translate("creating")}... script(type='text/ng-template', id='deleteTagModalTemplate') .modal-header diff --git a/services/web/public/coffee/main/project-list/project-list.coffee b/services/web/public/coffee/main/project-list/project-list.coffee index a1ab71555a..59d6ca2db8 100644 --- a/services/web/public/coffee/main/project-list/project-list.coffee +++ b/services/web/public/coffee/main/project-list/project-list.coffee @@ -203,12 +203,6 @@ define [ } $scope.createTag = (name) -> - event_tracking.send 'project-list-page-interaction', 'project action', 'createTag' - $scope.tags.push tag = { - name: name - project_ids: [] - showWhenEmpty: true - } return tag $scope.openNewTagModal = (e) -> @@ -218,8 +212,9 @@ define [ ) modalInstance.result.then( - (newTagName) -> - tag = $scope.createTag(newTagName) + (tag) -> + console.log "Created tag", tag + $scope.tags.push tag $scope.addSelectedProjectsToTag(tag) ) @@ -343,6 +338,7 @@ define [ $scope._removeProjectIdsFromTagArray(tag, selected_project_ids) for project in selected_projects + project.tags = [] if project.accessLevel == "owner" project.archived = true queuedHttp { diff --git a/services/web/public/coffee/main/project-list/tag-controllers.coffee b/services/web/public/coffee/main/project-list/tag-controllers.coffee index 1124449611..e21dd9533d 100644 --- a/services/web/public/coffee/main/project-list/tag-controllers.coffee +++ b/services/web/public/coffee/main/project-list/tag-controllers.coffee @@ -61,9 +61,13 @@ define [ $scope.recalculateProjectsInTag() $scope.recalculateProjectsInTag() - App.controller 'NewTagModalController', ($scope, $modalInstance, $timeout) -> + App.controller 'NewTagModalController', ($scope, $modalInstance, $timeout, $http) -> $scope.inputs = newTagName: "" + + $scope.state = + inflight: false + error: false $modalInstance.opened.then () -> $timeout () -> @@ -71,7 +75,20 @@ define [ , 200 $scope.create = () -> - $modalInstance.close($scope.inputs.newTagName) + name = $scope.inputs.newTagName + $scope.state.inflight = true + $scope.state.error = false + $http + .post "/tag", { + _csrf: window.csrfToken, + name: name + } + .success (data, status, headers, config) -> + $scope.state.inflight = false + $modalInstance.close(data) + .error () -> + $scope.state.inflight = false + $scope.state.error = true $scope.cancel = () -> $modalInstance.dismiss('cancel') diff --git a/services/web/test/UnitTests/coffee/Tags/TagsControllerTests.coffee b/services/web/test/UnitTests/coffee/Tags/TagsControllerTests.coffee index e63e619bdf..dbce6c094a 100644 --- a/services/web/test/UnitTests/coffee/Tags/TagsControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Tags/TagsControllerTests.coffee @@ -16,6 +16,7 @@ describe 'TagsController', -> removeProjectFromTag: sinon.stub().callsArgWith(3) deleteTag: sinon.stub().callsArg(2) renameTag: sinon.stub().callsArg(3) + createTag: sinon.stub() @controller = SandboxedModule.require modulePath, requires: "./TagsHandler":@handler 'logger-sharelatex': @@ -31,15 +32,31 @@ describe 'TagsController', -> @res = {} @res.status = sinon.stub().returns @res @res.end = sinon.stub() + @res.json = sinon.stub() 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)=> + @controller.getAllTags @req, json:(body)=> body.should.equal allTags @handler.getAllTags.calledWith(user_id).should.equal true done() + + describe "createTag", -> + beforeEach -> + @handler.createTag.callsArgWith(2, null, @tag = {"mock": "tag"}) + @req.session.user._id = @user_id = "user-id-123" + @req.body = name: @name = "tag-name" + @controller.createTag @req, @res + + it "should create the tag in the backend", -> + @handler.createTag + .calledWith(@user_id, @name) + .should.equal true + + it "should return the tag", -> + @res.json.calledWith(@tag).should.equal true describe "deleteTag", -> beforeEach -> diff --git a/services/web/test/UnitTests/coffee/Tags/TagsHandlerTests.coffee b/services/web/test/UnitTests/coffee/Tags/TagsHandlerTests.coffee index f0b57c813d..df75cf827f 100644 --- a/services/web/test/UnitTests/coffee/Tags/TagsHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Tags/TagsHandlerTests.coffee @@ -29,10 +29,10 @@ describe 'TagsHandler', -> 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 + @request.del.calledWith({url:"#{tagsUrl}/user/#{user_id}/project/#{project_id}", timeout:1000}).should.equal true done() - describe "groupTagsByProject", -> + describe "_groupTagsByProject", -> it 'should group the tags by project_id', (done)-> rawTags = [ {name:"class101", project_ids:["1234", "51db33e31a55afd212000007"]} @@ -41,35 +41,35 @@ describe 'TagsHandler', -> {name:"different", project_ids:["1234", "e2c39a2f09000100"]} ] - @handler.groupTagsByProject rawTags, (err, tags)-> + @handler._groupTagsByProject rawTags, (err, tags)-> _.size(tags).should.equal 7 done() - describe "requestTags", -> + describe "_requestTags", -> it 'should return an err and empty array on error', (done)-> @request.get.callsArgWith(1, {something:"wrong"}, {statusCode:200}, []) - @handler.requestTags user_id, (err, allTags)=> + @handler._requestTags user_id, (err, allTags)=> allTags.length.should.equal 0 assert.isDefined err done() it 'should return an err and empty array on no body', (done)-> @request.get.callsArgWith(1, {something:"wrong"}, {statusCode:200}, undefined) - @handler.requestTags user_id, (err, allTags)=> + @handler._requestTags user_id, (err, allTags)=> allTags.length.should.equal 0 assert.isDefined err done() it 'should return an err and empty array on non 200 response', (done)-> @request.get.callsArgWith(1, null, {statusCode:201}, []) - @handler.requestTags user_id, (err, allTags)=> + @handler._requestTags user_id, (err, allTags)=> allTags.length.should.equal 0 assert.isDefined err done() it 'should return an err and empty array on no body and no response', (done)-> @request.get.callsArgWith(1, {something:"wrong"}, undefined, undefined) - @handler.requestTags user_id, (err, allTags)=> + @handler._requestTags user_id, (err, allTags)=> allTags.length.should.equal 0 assert.isDefined err done() @@ -93,6 +93,24 @@ describe 'TagsHandler', -> allTags.length.should.equal 0 _.size(projectGroupedTags).should.equal 0 + describe "createTag", -> + beforeEach -> + @request.post = sinon.stub().callsArgWith(1, null, {statusCode: 204}, "") + @handler.createTag user_id, @name = "tag_name", @callback + + it "should send a request to the tag backend", -> + @request.post + .calledWith({ + url: "#{tagsUrl}/user/#{user_id}/tag" + json: + name: @name + timeout: 1000 + }) + .should.equal true + + it "should call the callback with no error", -> + @callback.calledWith(null).should.equal true + describe "deleteTag", -> describe "successfully", -> beforeEach ->