Merge pull request #1907 from overleaf/as-fix-tag-project-count

Fix tag project count

GitOrigin-RevId: 60236571403f8cc02f70bbe49e652ef18bbeab4d

services/web/app/src/Features/Project/ProjectController.js

@@ -365,7 +365,7 @@ module.exports = ProjectController = {
         const v1Tags =
           (results.v1Projects != null ? results.v1Projects.tags : undefined) ||
           []
-        const tags = results.tags[0].concat(v1Tags)
+        const tags = results.tags.concat(v1Tags)
         const notifications = require('underscore').map(
           results.notifications,
           function(notification) {

services/web/app/src/Features/Tags/TagsHandler.js

@@ -13,7 +13,6 @@
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
 let TagsHandler
-const _ = require('underscore')
 const settings = require('settings-sharelatex')
 const request = require('request')
 const logger = require('logger-sharelatex')
@@ -21,17 +20,11 @@ const logger = require('logger-sharelatex')
 const TIMEOUT = 1000
 module.exports = TagsHandler = {
   getAllTags(user_id, callback) {
-    return this._requestTags(user_id, (err, allTags) => {
+    this._requestTags(user_id, (err, allTags) => {
       if (allTags == null) {
         allTags = []
       }
-      return this._groupTagsByProject(allTags, function(err, groupedByProject) {
-        logger.log(
-          { allTags, user_id, groupedByProject },
-          'got all tags from tags api'
-        )
-        return callback(err, allTags, groupedByProject)
-      })
+      callback(err, allTags)
     })
   },
 
@@ -210,21 +203,5 @@ module.exports = TagsHandler = {
         return callback(null, body || [])
       })
     )
-  },
-
-  _groupTagsByProject(tags, callback) {
-    const result = {}
-    _.each(tags, tag =>
-      _.each(tag.project_ids, project_id => (result[project_id] = []))
-    )
-
-    _.each(tags, tag =>
-      _.each(tag.project_ids, function(project_id) {
-        const clonedTag = _.clone(tag)
-        delete clonedTag.project_ids
-        return result[project_id].push(clonedTag)
-      })
-    )
-    return callback(null, result)
   }
 }

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

@@ -85,7 +85,7 @@
 					aria-hidden="true"
 				)
 				span.name {{tag.name}} 
-					span.subdued  ({{tag.project_ids.length}})
+					span.subdued  ({{countProjectsForTag(tag)}})
 					span.v1-badge(
 						ng-if="tag.isV1",
 						ng-cloak,

services/web/public/src/main/project-list/project-list.js

@@ -125,7 +125,9 @@ define(['base', 'main/project-list/services/project-list'], function(App) {
       projectsById[project.id] = project
     }
 
-    for (var tag of $scope.tags) {
+    $scope.getProjectById = id => projectsById[id]
+
+    for (let tag of $scope.tags) {
       for (let projectId of tag.project_ids || []) {
         let project = projectsById[projectId]
         if (project) {
@@ -273,7 +275,7 @@ define(['base', 'main/project-list/services/project-list'], function(App) {
     }
 
     $scope.getSelectedTag = function() {
-      for (tag of $scope.tags) {
+      for (let tag of $scope.tags) {
         if (tag.selected) {
           return tag
         }
@@ -396,8 +398,6 @@ define(['base', 'main/project-list/services/project-list'], function(App) {
       }
     }
 
-    $scope.createTag = name => tag
-
     $scope.openNewTagModal = function(e) {
       const modalInstance = $modal.open({
         templateUrl: 'newTagModalTemplate',
@@ -631,7 +631,7 @@ define(['base', 'main/project-list/services/project-list'], function(App) {
       }
 
       // Remove project from any tags
-      for (tag of $scope.tags) {
+      for (let tag of $scope.tags) {
         $scope._removeProjectIdsFromTagArray(tag, selectedProjectIds)
       }
 
@@ -709,7 +709,7 @@ define(['base', 'main/project-list/services/project-list'], function(App) {
       })
 
     const markTagAsSelected = id => {
-      for (tag of $scope.tags) {
+      for (let tag of $scope.tags) {
         if (tag._id === id) {
           tag.selected = true
         } else {

services/web/public/src/main/project-list/tag-controllers.js

@@ -21,6 +21,24 @@ define(['base', 'ide/colors/ColorManager'], function(App, ColorManager) {
       $scope.setFilter('untagged')
     }
 
+    $scope.countProjectsForTag = function(tag) {
+      return tag.project_ids.reduce((acc, projectId) => {
+        const project = $scope.getProjectById(projectId)
+
+        // There is a bug where the tag is not cleaned up when you leave a
+        // project, so tag.project_ids can contain a project that the user can
+        // no longer access. If the project cannot be found, ignore it
+        if (!project) return acc
+
+        // Ignore archived projects as they are not shown in the filter
+        if (!project.archived) {
+          return acc + 1
+        } else {
+          return acc
+        }
+      }, 0)
+    }
+
     $scope.getHueForTagId = tagId => ColorManager.getHueForTagId(tagId)
 
     $scope.deleteTag = function(tag) {

services/web/test/acceptance/src/SessionTests.js

@@ -18,12 +18,14 @@ const request = require('./helpers/request')
 const settings = require('settings-sharelatex')
 const redis = require('./helpers/redis')
 const MockV1Api = require('./helpers/MockV1Api')
+const MockTagsApi = require('./helpers/MockTagsApi')
 
 describe('Sessions', function() {
   before(function(done) {
     this.timeout(20000)
     this.user1 = new User()
     this.site_admin = new User({ email: 'admin@example.com' })
+    MockTagsApi.tags[this.user1] = []
     return async.series(
       [cb => this.user1.login(cb), cb => this.user1.logout(cb)],
       done

services/web/test/acceptance/src/helpers/MockTagsApi.js

@@ -0,0 +1,29 @@
+const express = require('express')
+const app = express()
+
+const MockTagsApi = {
+  tags: {},
+
+  run() {
+    app.get('/user/:userId/tag', (req, res) => {
+      const { userId } = req.params
+      const tags = this.tags[userId]
+      res.json(tags)
+    })
+
+    app
+      .listen(3012, function(error) {
+        if (error) {
+          throw error
+        }
+      })
+      .on('error', function(error) {
+        console.error('error starting MockTagsApi:', error.message)
+        process.exit(1)
+      })
+  }
+}
+
+MockTagsApi.run()
+
+module.exports = MockTagsApi

services/web/test/unit/src/Project/ProjectControllerTests.js

@@ -404,7 +404,7 @@ describe('ProjectController', function() {
       }
 
       this.LimitationsManager.hasPaidSubscription.callsArgWith(1, null, false)
-      this.TagsHandler.getAllTags.callsArgWith(1, null, this.tags, {})
+      this.TagsHandler.getAllTags.callsArgWith(1, null, this.tags)
       this.NotificationsHandler.getUserNotifications = sinon
         .stub()
         .callsArgWith(1, null, this.notifications, {})
@@ -656,7 +656,7 @@ describe('ProjectController', function() {
       }
 
       this.LimitationsManager.hasPaidSubscription.callsArgWith(1, null, false)
-      this.TagsHandler.getAllTags.callsArgWith(1, null, this.tags, {})
+      this.TagsHandler.getAllTags.callsArgWith(1, null, this.tags)
       this.NotificationsHandler.getUserNotifications = sinon
         .stub()
         .callsArgWith(1, null, this.notifications, {})

services/web/test/unit/src/Tags/TagsHandlerTests.js

@@ -62,24 +62,6 @@ describe('TagsHandler', function() {
       })
     }))
 
-  describe('_groupTagsByProject', () =>
-    it('should 	group the tags by project_id', function(done) {
-      const rawTags = [
-        { name: 'class101', project_ids: ['1234', '51db33e31a55afd212000007'] },
-        { name: 'class201', project_ids: ['1234', '51db33e31a55afd212000007'] },
-        {
-          name: 'research group',
-          project_ids: ['12', '51da65f2e2c39a2f09000100', 'odjaskdas', 'dasdsa']
-        },
-        { name: 'different', project_ids: ['1234', 'e2c39a2f09000100'] }
-      ]
-
-      return this.handler._groupTagsByProject(rawTags, function(err, tags) {
-        _.size(tags).should.equal(7)
-        return done()
-      })
-    }))
-
   describe('_requestTags', function() {
     it('should return an err and empty array on error', function(done) {
       this.request.get.callsArgWith(