Merge pull request #34 from overleaf/spd-unlearn

Add endpoint to remove learned words

services/spelling/app.js

@@ -30,6 +30,7 @@ server.del('/user/:user_id', SpellingAPIController.deleteDic)
 server.get('/user/:user_id', SpellingAPIController.getDic)
 server.post('/user/:user_id/check', SpellingAPIController.check)
 server.post('/user/:user_id/learn', SpellingAPIController.learn)
+server.post('/user/:user_id/unlearn', SpellingAPIController.unlearn)
 server.get('/status', (req, res) => res.send({ status: 'spelling api is up' }))
 
 server.get('/health_check', HealthCheckController.healthCheck)

services/spelling/app/js/LearnedWordsManager.js

@@ -24,6 +24,22 @@ const LearnedWordsManager = {
     )
   },
 
+  unlearnWord(userToken, word, callback) {
+    if (callback == null) {
+      callback = () => {}
+    }
+    mongoCache.del(userToken)
+    return db.spellingPreferences.update(
+      {
+        token: userToken
+      },
+      {
+        $pull: { learnedWords: word }
+      },
+      callback
+    )
+  },
+
   getLearnedWords(userToken, callback) {
     if (callback == null) {
       callback = () => {}
@@ -61,6 +77,7 @@ const LearnedWordsManager = {
 
 const promises = {
   learnWord: promisify(LearnedWordsManager.learnWord),
+  unlearnWord: promisify(LearnedWordsManager.unlearnWord),
   getLearnedWords: promisify(LearnedWordsManager.getLearnedWords),
   deleteUsersLearnedWords: promisify(
     LearnedWordsManager.deleteUsersLearnedWords
@@ -70,7 +87,7 @@ const promises = {
 LearnedWordsManager.promises = promises
 
 module.exports = LearnedWordsManager
-;['learnWord', 'getLearnedWords'].map(method =>
+;['learnWord', 'unlearnWord', 'getLearnedWords'].map(method =>
   metrics.timeAsyncMethod(
     LearnedWordsManager,
     method,

services/spelling/app/js/SpellingAPIController.js

@@ -44,8 +44,19 @@ module.exports = {
       if (error != null) {
         return next(error)
       }
-      res.sendStatus(200)
-      next()
+      res.sendStatus(204)
+    })
+  },
+
+  unlearn(req, res, next) {
+    metrics.inc('spelling-unlearn', 0.1)
+    const { token, word } = extractLearnRequestData(req)
+    logger.info({ token, word }, 'unlearning word')
+    SpellingAPIManager.unlearnWord(token, req.body, function(error) {
+      if (error != null) {
+        return next(error)
+      }
+      res.sendStatus(204)
     })
   },
 

services/spelling/app/js/SpellingAPIManager.js

@@ -30,6 +30,20 @@ const SpellingAPIManager = {
     return LearnedWordsManager.learnWord(token, request.word, callback)
   },
 
+  unlearnWord(token, request, callback) {
+    if (callback == null) {
+      callback = () => {}
+    }
+    if (request.word == null) {
+      return callback(new Error('malformed JSON'))
+    }
+    if (token == null) {
+      return callback(new Error('no token provided'))
+    }
+
+    return LearnedWordsManager.unlearnWord(token, request.word, callback)
+  },
+
   deleteDic(token, callback) {
     return LearnedWordsManager.deleteUsersLearnedWords(token, callback)
   },

services/spelling/test/acceptance/js/LearnTest.js

@@ -19,15 +19,23 @@ const learnWord = word =>
     })
   })
 
+const unlearnWord = word =>
+  request.post({
+    url: `/user/${USER_ID}/unlearn`,
+    body: JSON.stringify({
+      word
+    })
+  })
+
 const deleteDict = () =>
   request.del({
     url: `/user/${USER_ID}`
   })
 
 describe('learning words', () => {
-  it('should return status 200 when posting a word successfully', async () => {
+  it('should return status 204 when posting a word successfully', async () => {
     const response = await learnWord('abcd')
-    expect(response.statusCode).to.equal(200)
+    expect(response.statusCode).to.equal(204)
   })
 
   it('should return no misspellings after a word is learnt', async () => {
@@ -51,3 +59,24 @@ describe('learning words', () => {
     expect(responseBody.misspellings.length).to.equals(1)
   })
 })
+
+describe('unlearning words', () => {
+  it('should return status 204 when posting a word successfully', async () => {
+    const response = await unlearnWord('anything')
+    expect(response.statusCode).to.equal(204)
+  })
+
+  it('should return misspellings after a word is unlearnt', async () => {
+    await learnWord('abv')
+
+    const response = await checkWord(['abv'])
+    const responseBody = JSON.parse(response.body)
+    expect(responseBody.misspellings.length).to.equals(0)
+
+    await unlearnWord('abv')
+
+    const response2 = await checkWord(['abv'])
+    const responseBody2 = JSON.parse(response2.body)
+    expect(responseBody2.misspellings.length).to.equals(1)
+  })
+})

services/spelling/test/unit/js/LearnedWordsManagerTests.js

@@ -1,14 +1,3 @@
-/* eslint-disable
-    handle-callback-err,
-    no-undef
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Sanity-check the conversion and remove this comment.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 const sinon = require('sinon')
 const chai = require('chai')
 const { expect } = chai
@@ -24,7 +13,7 @@ describe('LearnedWordsManager', function() {
     this.callback = sinon.stub()
     this.db = {
       spellingPreferences: {
-        update: sinon.stub().callsArg(3)
+        update: sinon.stub().yields()
       }
     }
     this.cache = {
@@ -32,7 +21,7 @@ describe('LearnedWordsManager', function() {
       set: sinon.stub(),
       del: sinon.stub()
     }
-    return (this.LearnedWordsManager = SandboxedModule.require(modulePath, {
+    this.LearnedWordsManager = SandboxedModule.require(modulePath, {
       requires: {
         './DB': this.db,
         './MongoCache': this.cache,
@@ -46,21 +35,17 @@ describe('LearnedWordsManager', function() {
           inc: sinon.stub()
         }
       }
-    }))
+    })
   })
 
   describe('learnWord', function() {
     beforeEach(function() {
       this.word = 'instanton'
-      return this.LearnedWordsManager.learnWord(
-        this.token,
-        this.word,
-        this.callback
-      )
+      this.LearnedWordsManager.learnWord(this.token, this.word, this.callback)
     })
 
     it('should insert the word in the word list in the database', function() {
-      return expect(
+      expect(
         this.db.spellingPreferences.update.calledWith(
           {
             token: this.token
@@ -75,8 +60,32 @@ describe('LearnedWordsManager', function() {
       ).to.equal(true)
     })
 
-    return it('should call the callback', function() {
-      return expect(this.callback.called).to.equal(true)
+    it('should call the callback', function() {
+      expect(this.callback.called).to.equal(true)
+    })
+  })
+
+  describe('unlearnWord', function() {
+    beforeEach(function() {
+      this.word = 'instanton'
+      this.LearnedWordsManager.unlearnWord(this.token, this.word, this.callback)
+    })
+
+    it('should remove the word from the word list in the database', function() {
+      expect(
+        this.db.spellingPreferences.update.calledWith(
+          {
+            token: this.token
+          },
+          {
+            $pull: { learnedWords: this.word }
+          }
+        )
+      ).to.equal(true)
+    })
+
+    it('should call the callback', function() {
+      expect(this.callback.called).to.equal(true)
     })
   })
 
@@ -84,22 +93,20 @@ describe('LearnedWordsManager', function() {
     beforeEach(function() {
       this.wordList = ['apples', 'bananas', 'pears']
       this.db.spellingPreferences.findOne = (conditions, callback) => {
-        return callback(null, { learnedWords: this.wordList })
+        callback(null, { learnedWords: this.wordList })
       }
       sinon.spy(this.db.spellingPreferences, 'findOne')
-      return this.LearnedWordsManager.getLearnedWords(this.token, this.callback)
+      this.LearnedWordsManager.getLearnedWords(this.token, this.callback)
     })
 
     it('should get the word list for the given user', function() {
-      return expect(
+      expect(
         this.db.spellingPreferences.findOne.calledWith({ token: this.token })
       ).to.equal(true)
     })
 
-    return it('should return the word list in the callback', function() {
-      return expect(this.callback.calledWith(null, this.wordList)).to.equal(
-        true
-      )
+    it('should return the word list in the callback', function() {
+      expect(this.callback.calledWith(null, this.wordList)).to.equal(true)
     })
   })
 
@@ -108,14 +115,12 @@ describe('LearnedWordsManager', function() {
       this.wordList = ['apples', 'bananas', 'pears']
       this.cache.get.returns(this.wordList)
       this.db.spellingPreferences.findOne = sinon.stub()
-      return this.LearnedWordsManager.getLearnedWords(
-        this.token,
-        (err, spellings) => {
-          this.db.spellingPreferences.findOne.called.should.equal(false)
-          assert.deepEqual(this.wordList, spellings)
-          return done()
-        }
-      )
+      this.LearnedWordsManager.getLearnedWords(this.token, (err, spellings) => {
+        expect(err).not.to.exist
+        this.db.spellingPreferences.findOne.called.should.equal(false)
+        assert.deepEqual(this.wordList, spellings)
+        done()
+      })
     })
 
     it('should set the cache after hitting the db', function(done) {
@@ -123,41 +128,33 @@ describe('LearnedWordsManager', function() {
       this.db.spellingPreferences.findOne = sinon
         .stub()
         .callsArgWith(1, null, { learnedWords: this.wordList })
-      return this.LearnedWordsManager.getLearnedWords(
-        this.token,
-        (err, spellings) => {
-          this.cache.set
-            .calledWith(this.token, this.wordList)
-            .should.equal(true)
-          return done()
-        }
-      )
+      this.LearnedWordsManager.getLearnedWords(this.token, () => {
+        this.cache.set.calledWith(this.token, this.wordList).should.equal(true)
+        done()
+      })
     })
 
-    return it('should break cache when update is called', function(done) {
+    it('should break cache when update is called', function(done) {
       this.word = 'instanton'
-      return this.LearnedWordsManager.learnWord(this.token, this.word, () => {
+      this.LearnedWordsManager.learnWord(this.token, this.word, () => {
         this.cache.del.calledWith(this.token).should.equal(true)
-        return done()
+        done()
       })
     })
   })
 
-  return describe('deleteUsersLearnedWords', function() {
+  describe('deleteUsersLearnedWords', function() {
     beforeEach(function() {
-      return (this.db.spellingPreferences.remove = sinon.stub().callsArgWith(1))
+      this.db.spellingPreferences.remove = sinon.stub().callsArgWith(1)
     })
 
-    return it('should get the word list for the given user', function(done) {
-      return this.LearnedWordsManager.deleteUsersLearnedWords(
-        this.token,
-        () => {
-          this.db.spellingPreferences.remove
-            .calledWith({ token: this.token })
-            .should.equal(true)
-          return done()
-        }
-      )
+    it('should get the word list for the given user', function(done) {
+      this.LearnedWordsManager.deleteUsersLearnedWords(this.token, () => {
+        this.db.spellingPreferences.remove
+          .calledWith({ token: this.token })
+          .should.equal(true)
+        done()
+      })
     })
   })
 })

services/spelling/test/unit/js/SpellingAPIManagerTests.js

@@ -21,6 +21,7 @@ describe('SpellingAPIManager', function() {
     this.LearnedWordsManager = {
       getLearnedWords: sinon.stub().callsArgWith(1, null, this.learnedWords),
       learnWord: sinon.stub().callsArg(2),
+      unlearnWord: sinon.stub().callsArg(2),
       promises: {
         getLearnedWords: sinon.stub().returns(promiseStub(this.learnedWords))
       }
@@ -229,4 +230,56 @@ describe('SpellingAPIManager', function() {
       })
     })
   })
+
+  describe('unlearnWord', function() {
+    describe('without a token', function() {
+      beforeEach(function(done) {
+        this.SpellingAPIManager.unlearnWord(null, { word: 'banana' }, error => {
+          this.error = error
+          done()
+        })
+      })
+
+      it('should return an error', function() {
+        expect(this.error).to.exist
+        expect(this.error).to.be.instanceof(Error)
+        expect(this.error.message).to.equal('no token provided')
+      })
+    })
+
+    describe('without a word', function() {
+      beforeEach(function(done) {
+        this.SpellingAPIManager.unlearnWord(this.token, {}, error => {
+          this.error = error
+          done()
+        })
+      })
+
+      it('should return an error', function() {
+        expect(this.error).to.exist
+        expect(this.error).to.be.instanceof(Error)
+        expect(this.error.message).to.equal('malformed JSON')
+      })
+    })
+
+    describe('with a word and a token', function() {
+      beforeEach(function(done) {
+        this.word = 'banana'
+        this.SpellingAPIManager.unlearnWord(
+          this.token,
+          { word: this.word },
+          error => {
+            this.error = error
+            done()
+          }
+        )
+      })
+
+      it('should call LearnedWordsManager.unlearnWord', function() {
+        this.LearnedWordsManager.unlearnWord
+          .calledWith(this.token, this.word)
+          .should.equal(true)
+      })
+    })
+  })
 })