Revert "chore: instantly return when submitting sentences"

This reverts commit 05450e4.
common-voice · May 29, 2020 · e7ff5b5 · e7ff5b5
1 parent 05450e4
commit e7ff5b5
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 10 deletions.
diff --git a/server/lib/sentences.js b/server/lib/sentences.js
@@ -164,7 +164,7 @@ async function getUserAddedSentencesPerLocale(user) {
   return sentencesStats;
 }
 
-function addSentences(data) {
+async function addSentences(data) {
   const {
     sentences,
     user,
@@ -177,6 +177,7 @@ function addSentences(data) {
   const { valid, validValidated, filtered } = validateSentences(locale, sentences);
 
   debug('Creating database entries');
+  let duplicateCounter = 0;
 
   const addSentenceToDatabase = (sentence, isValidated) => {
     const params = {
@@ -187,7 +188,7 @@ function addSentences(data) {
       localeId: locale,
     };
 
-    Sentence.create(params)
+    return Sentence.create(params)
       .then((sentence) => {
         if (!isValidated) {
           return sentence;
@@ -203,14 +204,20 @@ function addSentences(data) {
       .catch((error) => {
         if (error.parent && error.parent.errno === DUPLICATE_ERROR) {
           debug('Ignoring duplicate sentence', sentence);
+          duplicateCounter++;
         }
       });
   };
 
-  valid.map((sentence) => addSentenceToDatabase(sentence, false));
-  validValidated.map((sentence) => addSentenceToDatabase(sentence, true));
+  const validPromises = valid.map((sentence) => addSentenceToDatabase(sentence, false));
+  const validatedPromises = validValidated.map((sentence) => addSentenceToDatabase(sentence, true));
 
-  return { errors: filtered };
+  await Promise.all([
+    ...validPromises,
+    ...validatedPromises,
+  ]);
+
+  return { errors: filtered, duplicates: duplicateCounter };
 }
 
 function getReviewQuery({ locale, user }) {

diff --git a/server/routes/sentences.js b/server/routes/sentences.js
@@ -38,9 +38,16 @@ router.get('/rejected', async (req, res) => {
 router.put('/', async (req, res) => {
   debug('CREATE_SENTENCES', req.body);
 
-  const result = sentences.addSentences(req.body);
-  res.status(STATUS_CREATED);
-  res.json(result);
+  sentences.addSentences(req.body)
+    .then((result) => {
+        res.status(STATUS_CREATED);
+        res.json(result);
+      })
+      .catch((error) => {
+        debug('CREATE_SENTENCES_ERROR', error);
+        res.status(STATUS_ERROR);
+        res.json({ message: error.message });
+      });
 });
 
 router.get('/:locale', async (req, res) => {

diff --git a/server/tests/lib/sentences.test.js b/server/tests/lib/sentences.test.js
@@ -81,6 +81,7 @@ test.serial('addedSentences: should add all unreviewed sentences', async (t) =>
 
   t.false(votes.addVoteForSentence.called);
   t.is(result.errors.length, 0);
+  t.is(result.duplicates, 0);
 });
 
 test.serial('addedSentences: should fall back to en if no locale provided', async (t) => {
@@ -120,6 +121,31 @@ test.serial('addedSentences: should add all sentences - mixed validated and unre
 
   t.is(Sentence.create.callCount, 3);
   t.is(result.errors.length, 0);
+  t.is(result.duplicates, 0);
+});
+
+test.serial('addedSentences: should return duplicate counter', async (t) => {
+  const error = new Error('duplicate');
+  error.parent = {
+    errno: 1062,
+  };
+  Sentence.create.onCall(2).rejects(error);
+
+  const sentenceParams = {
+    sentences: {
+      unreviewed: ['Hi!'],
+      validated: ['I am a test', 'I am a test'],
+    },
+    user: 'foo',
+    source: 'source',
+    locale: 'en',
+  };
+
+  const result = await sentences.addSentences(sentenceParams);
+
+  t.is(Sentence.create.callCount, 3);
+  t.is(result.errors.length, 0);
+  t.is(result.duplicates, 1);
 });
 
 test.serial('getStats: should fetch all stats correctly', async (t) => {

diff --git a/server/tests/routes/sentences.test.js b/server/tests/routes/sentences.test.js
@@ -132,3 +132,15 @@ test.serial('should add sentences', async (t) => {
   t.is(response.status, 201);
   t.true(sentences.addSentences.calledWith(sentenceParams));
 });
+
+test.serial('adding sentences should pass on error message', async (t) => {
+  sentences.addSentences.rejects(new Error('nope'));
+
+  const response = await request(app)
+    .put('/sentences');
+
+  t.is(response.status, 500);
+  t.deepEqual(response.body, {
+    message: 'nope',
+  });
+});
diff --git a/web/src/components/pages/add.jsx b/web/src/components/pages/add.jsx
@@ -156,7 +156,7 @@ class Add extends React.Component {
       const sentences = this.getReadySentences();
       const locale = this.state.language;
       const source = this.state.source;
-      const { errors } = await this.props.uploadSentences({
+      const { errors, duplicates } = await this.props.uploadSentences({
         locale,
         sentences,
         source,
@@ -170,7 +170,7 @@ class Add extends React.Component {
       this.historyUnblock();
       this.resetState();
       this.setState({
-        message: `Submitted sentences.`,
+        message: `Submitted sentences. ${duplicates} sentences were rejected as duplicates.`,
         error: errors && errors.length > 0 ? `${errors.length} sentences failed` : '',
       });
     } catch (err) {