From 1d405914fdcc906119f4b64619415c20ad6d0cf4 Mon Sep 17 00:00:00 2001 From: Tim Alby Date: Tue, 18 Feb 2020 09:03:16 -0500 Subject: [PATCH 1/2] don't learn the same word twice --- .../spelling/app/js/LearnedWordsManager.js | 2 +- .../spelling/test/acceptance/js/LearnTest.js | 29 +++++++++++++++---- .../test/unit/js/LearnedWordsManagerTests.js | 2 +- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/services/spelling/app/js/LearnedWordsManager.js b/services/spelling/app/js/LearnedWordsManager.js index bbf7651d87..bfb9bcc91f 100644 --- a/services/spelling/app/js/LearnedWordsManager.js +++ b/services/spelling/app/js/LearnedWordsManager.js @@ -15,7 +15,7 @@ const LearnedWordsManager = { token: userToken }, { - $push: { learnedWords: word } + $addToSet: { learnedWords: word } }, { upsert: true diff --git a/services/spelling/test/acceptance/js/LearnTest.js b/services/spelling/test/acceptance/js/LearnTest.js index c516de3162..a3e8bed07a 100644 --- a/services/spelling/test/acceptance/js/LearnTest.js +++ b/services/spelling/test/acceptance/js/LearnTest.js @@ -27,18 +27,37 @@ const unlearnWord = word => }) }) +const getDict = () => + request.get({ + url: `/user/${USER_ID}` + }) + const deleteDict = () => request.del({ url: `/user/${USER_ID}` }) describe('learning words', function() { - it('should return status 204 when posting a word successfully', async function () { + afterEach(async function() { + await deleteDict() + }) + + it('should return status 204 when posting a word successfully', async function() { const response = await learnWord('abcd') expect(response.statusCode).to.equal(204) }) - it('should return no misspellings after a word is learnt', async function () { + it('should not learn the same word twice', async function() { + await learnWord('foobar') + const learnResponse = await learnWord('foobar') + expect(learnResponse.statusCode).to.equal(204) + + const dictResponse = await getDict() + const responseBody = JSON.parse(dictResponse.body) + expect(responseBody.length).to.equals(1) + }) + + it('should return no misspellings after a word is learnt', async function() { const response = await checkWord(['abv']) const responseBody = JSON.parse(response.body) expect(responseBody.misspellings.length).to.equals(1) @@ -50,7 +69,7 @@ describe('learning words', function() { expect(responseBody2.misspellings.length).to.equals(0) }) - it('should return misspellings again after a personal dictionary is deleted', async function () { + it('should return misspellings again after a personal dictionary is deleted', async function() { await learnWord('bvc') await deleteDict() @@ -61,12 +80,12 @@ describe('learning words', function() { }) describe('unlearning words', function() { - it('should return status 204 when posting a word successfully', async function () { + it('should return status 204 when posting a word successfully', async function() { const response = await unlearnWord('anything') expect(response.statusCode).to.equal(204) }) - it('should return misspellings after a word is unlearnt', async function () { + it('should return misspellings after a word is unlearnt', async function() { await learnWord('abv') const response = await checkWord(['abv']) diff --git a/services/spelling/test/unit/js/LearnedWordsManagerTests.js b/services/spelling/test/unit/js/LearnedWordsManagerTests.js index 32ea14a18a..c617b1fb81 100644 --- a/services/spelling/test/unit/js/LearnedWordsManagerTests.js +++ b/services/spelling/test/unit/js/LearnedWordsManagerTests.js @@ -51,7 +51,7 @@ describe('LearnedWordsManager', function() { token: this.token }, { - $push: { learnedWords: this.word } + $addToSet: { learnedWords: this.word } }, { upsert: true From 72d16c7c1fac693d44ed9f30f1df8c08452ab945 Mon Sep 17 00:00:00 2001 From: Tim Alby Date: Tue, 18 Feb 2020 10:58:03 -0500 Subject: [PATCH 2/2] filter out duplicates learned words --- services/spelling/app/js/LearnedWordsManager.js | 8 +++++++- services/spelling/test/acceptance/js/LearnTest.js | 2 ++ .../spelling/test/unit/js/LearnedWordsManagerTests.js | 9 +++++++-- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/services/spelling/app/js/LearnedWordsManager.js b/services/spelling/app/js/LearnedWordsManager.js index bfb9bcc91f..2b57697014 100644 --- a/services/spelling/app/js/LearnedWordsManager.js +++ b/services/spelling/app/js/LearnedWordsManager.js @@ -60,8 +60,14 @@ const LearnedWordsManager = { if (error != null) { return callback(error) } - const words = + let words = (preferences != null ? preferences.learnedWords : undefined) || [] + if (words) { + // remove duplicates + words = words.filter( + (value, index, self) => self.indexOf(value) === index + ) + } mongoCache.set(userToken, words) callback(null, words) }) diff --git a/services/spelling/test/acceptance/js/LearnTest.js b/services/spelling/test/acceptance/js/LearnTest.js index a3e8bed07a..7d50b22798 100644 --- a/services/spelling/test/acceptance/js/LearnTest.js +++ b/services/spelling/test/acceptance/js/LearnTest.js @@ -54,6 +54,8 @@ describe('learning words', function() { const dictResponse = await getDict() const responseBody = JSON.parse(dictResponse.body) + // the response from getlearnedwords filters out duplicates, so this test + // can succeed even if the word is stored twice in the database expect(responseBody.length).to.equals(1) }) diff --git a/services/spelling/test/unit/js/LearnedWordsManagerTests.js b/services/spelling/test/unit/js/LearnedWordsManagerTests.js index c617b1fb81..3fa0ec22b6 100644 --- a/services/spelling/test/unit/js/LearnedWordsManagerTests.js +++ b/services/spelling/test/unit/js/LearnedWordsManagerTests.js @@ -22,6 +22,9 @@ describe('LearnedWordsManager', function() { del: sinon.stub() } this.LearnedWordsManager = SandboxedModule.require(modulePath, { + globals: { + console: console + }, requires: { './DB': this.db, './MongoCache': this.cache, @@ -92,8 +95,10 @@ describe('LearnedWordsManager', function() { describe('getLearnedWords', function() { beforeEach(function() { this.wordList = ['apples', 'bananas', 'pears'] + this.wordListWithDuplicates = this.wordList.slice() + this.wordListWithDuplicates.push('bananas') this.db.spellingPreferences.findOne = (conditions, callback) => { - callback(null, { learnedWords: this.wordList }) + callback(null, { learnedWords: this.wordListWithDuplicates }) } sinon.spy(this.db.spellingPreferences, 'findOne') this.LearnedWordsManager.getLearnedWords(this.token, this.callback) @@ -105,7 +110,7 @@ describe('LearnedWordsManager', function() { ).to.equal(true) }) - it('should return the word list in the callback', function() { + it('should return the word list in the callback without duplicates', function() { expect(this.callback.calledWith(null, this.wordList)).to.equal(true) }) })