diff --git a/services/spelling/app.js b/services/spelling/app.js index 30024dc230..cf2065f902 100644 --- a/services/spelling/app.js +++ b/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) diff --git a/services/spelling/app/js/LearnedWordsManager.js b/services/spelling/app/js/LearnedWordsManager.js index adeb9b7dd4..bbf7651d87 100644 --- a/services/spelling/app/js/LearnedWordsManager.js +++ b/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, diff --git a/services/spelling/app/js/SpellingAPIController.js b/services/spelling/app/js/SpellingAPIController.js index 2239d997e0..f771a7a567 100644 --- a/services/spelling/app/js/SpellingAPIController.js +++ b/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) }) }, diff --git a/services/spelling/app/js/SpellingAPIManager.js b/services/spelling/app/js/SpellingAPIManager.js index 7c330eb312..3c437d929f 100644 --- a/services/spelling/app/js/SpellingAPIManager.js +++ b/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) }, diff --git a/services/spelling/test/acceptance/js/LearnTest.js b/services/spelling/test/acceptance/js/LearnTest.js index 6f6bc21a2e..b37efc5e80 100644 --- a/services/spelling/test/acceptance/js/LearnTest.js +++ b/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) + }) +}) diff --git a/services/spelling/test/unit/js/LearnedWordsManagerTests.js b/services/spelling/test/unit/js/LearnedWordsManagerTests.js index 76122b1d5c..32ea14a18a 100644 --- a/services/spelling/test/unit/js/LearnedWordsManagerTests.js +++ b/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() + }) }) }) }) diff --git a/services/spelling/test/unit/js/SpellingAPIManagerTests.js b/services/spelling/test/unit/js/SpellingAPIManagerTests.js index 5baecbc8b0..f2e1bc55cc 100644 --- a/services/spelling/test/unit/js/SpellingAPIManagerTests.js +++ b/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) + }) + }) + }) })