Merge pull request #167 from unfoldingWord/new.2.1.2

Correct two important bugs plus some small improvements

package.json

@@ -1,7 +1,7 @@
 {
   "name": "uw-content-validation",
   "description": "Functions for Checking Door43.org Scriptural Content/Resources.",
-  "version": "2.1.1",
+  "version": "2.1.2",
   "private": false,
   "homepage": "https://unfoldingword.github.io/uw-content-validation/",
   "repository": {

src/__tests__/__snapshots__/tn-table-row-check.test.js.snap

@@ -500,6 +500,25 @@ Object {
 }
 `;
 
+exports[`checkTN_TSV9DataRow() -  Original Quote tests -  should fail with valid but high occurrence number 1`] = `
+Object {
+  "noticeList": Array [
+    Object {
+      "C": "2",
+      "V": "2",
+      "bookID": "MAT",
+      "details": "occurrence=2 but only 1 occurrence found, passage ►λέγοντες, ποῦ ἐστιν ὁ τεχθεὶς Βασιλεὺς τῶν Ἰουδαίων? εἴδομεν γὰρ αὐτοῦ τὸν ἀστέρα ἐν τῇ ἀνατολῇ καὶ ἤλθομεν προσκυνῆσαι αὐτῷ.◄",
+      "excerpt": "προσκυνῆσαροσκυνῆσαι",
+      "fieldName": "OrigQuote",
+      "location": " that was supplied",
+      "message": "Unable to find duplicate original language quote in verse text",
+      "priority": 917,
+      "rowID": "v248",
+    },
+  ],
+}
+`;
+
 exports[`checkTN_TSV9DataRow() -  Original Quote tests -  should find missing Original Quote 1`] = `
 Object {
   "noticeList": Array [

src/__tests__/tn-table-row-check.test.js

@@ -214,6 +214,20 @@ describe('checkTN_TSV9DataRow() - ', () => {
       expect(rawResults).toMatchSnapshot();
     });
 
+    it('should fail with valid but high occurrence number', async () => {
+      const chosenLine = "MAT\t2\t2\tv248\t\tπροσκυνῆσαι\t2\tto worship\tPossible meanings are (1) they intended to worship the baby as divine, or (2) they wanted to honor him as a human king. If your language has a word that includes both meanings, you should consider using it here.";
+      const rawResults = await checkTN_TSV9DataRow(languageCode, repoCode, chosenLine, 'MAT', '2', '2', 'that was supplied', optionalCheckingOptions);
+      expect(rawResults.noticeList.length).toEqual(1);
+      expect(rawResults).toMatchSnapshot();
+    });
+
+    it('should pass with correct quote', async () => {
+      const chosenLine = "MAT\t2\t2\tv248\t\tπροσκυνῆσαι\t1\tto worship\tPossible meanings are (1) they intended to worship the baby as divine, or (2) they wanted to honor him as a human king. If your language has a word that includes both meanings, you should consider using it here.";
+      const rawResults = await checkTN_TSV9DataRow(languageCode, repoCode, chosenLine, 'MAT', '2', '2', 'that was supplied', optionalCheckingOptions);
+      // console.log(`Got raw results: ${JSON.stringify(rawResults)}`);
+      expect(rawResults.noticeList.length).toEqual(0);
+    });
+
   });
 
   describe('Occurrence Note tests - ', () => {

src/core/getApi.js

@@ -5,10 +5,11 @@ import { setup } from 'axios-cache-adapter';
 import JSZip from 'jszip';
 import * as books from './books';
 import { clearCheckedArticleCache } from './notes-links-check';
-import { functionLog, userLog, parameterAssert } from './utilities';
+// eslint-disable-next-line no-unused-vars
+import { functionLog, debugLog, userLog, parameterAssert } from './utilities';
 
 
-// const GETAPI_VERSION_STRING = '0.6.11';
+// const GETAPI_VERSION_STRING = '0.7.0';
 
 const MAX_INDIVIDUAL_FILES_TO_DOWNLOAD = 5; // More than this and it downloads the zipfile for the entire repo
 
@@ -28,7 +29,7 @@ const zipStore = localforage.createInstance({
   name: 'CV-zip-store',
 });
 
-// caches http file fetches done by cachedFetchFileFromServer()
+// caches http file fetches done by cachedFetchFileFromServerBranch()
 const cacheStore = localforage.createInstance({
   driver: [localforage.INDEXEDDB],
   name: 'CV-web-cache',
@@ -133,7 +134,7 @@ async function getUnZippedFile(path) {
  * searches for files in this order:
  *   - cache of uncompressed files (unzipStore)
  *   - cache of zipped repos (zipStore)
- *   - and finally calls cachedFetchFileFromServer() which first checks in cacheStore to see if already fetched. * @param {string} username
+ *   - and finally calls cachedFetchFileFromServerBranch() which first checks in cacheStore to see if already fetched. * @param {string} username
  * @param {string} repository
  * @param {string} path
  * @param {string} branch
@@ -159,7 +160,7 @@ export async function cachedGetFile({ username, repository, path, branch }) {
   //   if (filePath.indexOf('_tq/') < 0) // Don’t log for TQ2 files coz too many
   //     userLog(`  cachedGetFile got ${filePath} from zipfile`);
   if (!contents) {
-    contents = await cachedFetchFileFromServer({ username, repository, path, branch });
+    contents = await cachedFetchFileFromServerBranch({ username, repository, path, branch });
   }
 
   if (contents) {
@@ -312,17 +313,56 @@ export async function preloadReposIfNecessary(username, languageCode, bookIDList
  * @param {string} branch
  * @return {Promise<null|any>} resolves to file content
  */
-async function cachedFetchFileFromServer({ username, repository, path, branch = 'master' }) {
-  // debugLog(`cachedFetchFileFromServer(${username}, ${repository}, ${path}, ${branch})…`);
+async function cachedFetchFileFromServerBranch({ username, repository, path, branch = 'master' }) {
+  // debugLog(`cachedFetchFileFromServerBranch(${username}, ${repository}, ${path}, ${branch})…`);
   // TODO: Check how slow this next call is -- can it be avoided or cached?
   // RJH removed this 2Oct2020 -- what’s the point -- it just slows things down --
   //      if it needs to be checked, should be checked before this point
   // const repositoryExistsOnDoor43 = await repositoryExistsOnDoor43({ username, repository });
   // let uri;
   const uri = Path.join(username, repository, 'raw/branch', branch, path);
+  return await cachedFetchFileFromServerWorker(uri, username, repository, path, branch);
+};
+
+
+/**
+ * does http file fetch from server  uses cacheStore to minimize repeated fetches of same file
+ * @param {string} username
+ * @param {string} repository
+ * @param {string} path
+ * @param {string} tag
+ * @return {Promise<null|any>} resolves to file content
+ */
+ export async function cachedFetchFileFromServerTag({ username, repository, path, tag }) {
+  // debugLog(`cachedFetchFileFromServerTag(${username}, ${repository}, ${path}, ${tag})…`);
+  // TODO: Check how slow this next call is -- can it be avoided or cached?
+  // RJH removed this 2Oct2020 -- what’s the point -- it just slows things down --
+  //      if it needs to be checked, should be checked before this point
+  // const repositoryExistsOnDoor43 = await repositoryExistsOnDoor43({ username, repository });
+  // let uri;
+  const uri = Path.join(username, repository, 'raw/tag', tag, path);
+  return await cachedFetchFileFromServerWorker(uri, username, repository, path, tag);
+};
+
+
+/**
+ * does http file fetch from server  uses cacheStore to minimize repeated fetches of same file
+ * @param {string} username
+ * @param {string} repository
+ * @param {string} path
+ * @param {string} branch
+ * @return {Promise<null|any>} resolves to file content
+ */
+ async function cachedFetchFileFromServerWorker(uri, username, repository, path, branchOrTag) {
+  // debugLog(`cachedFetchFileFromServerWorker(${uri}, ${username}, ${repository}, ${path}, ${branchOrTag})…`);
+  // TODO: Check how slow this next call is -- can it be avoided or cached?
+  // RJH removed this 2Oct2020 -- what’s the point -- it just slows things down --
+  //      if it needs to be checked, should be checked before this point
+  // const repositoryExistsOnDoor43 = await repositoryExistsOnDoor43({ username, repository });
+  // let uri;
   const failMessage = await failedStore.getItem(uri.toLowerCase());
   if (failMessage) {
-    // debugLog(`  cachedFetchFileFromServer failed previously for ${uri}: ${failMessage}`);
+    // debugLog(`  cachedFetchFileFromServerWorker failed previously for ${uri}: ${failMessage}`);
     return null;
   }
   try {
@@ -332,12 +372,12 @@ async function cachedFetchFileFromServer({ username, repository, path, branch =
     return data;
   }
   catch (fffsError) {
-    console.error(`cachedFetchFileFromServer could not fetch ${username} ${repository} ${branch} ${path}: ${fffsError}`)
+    console.error(`cachedFetchFileFromServerWorker could not fetch ${username} ${repository} ${branchOrTag} ${path}: ${fffsError}`)
       /* await */ failedStore.setItem(uri.toLowerCase(), fffsError.message);
     return null;
   }
   // } else { // ! repositoryExistsOnDoor43
-  //   console.error(`cachedFetchFileFromServer repo ${username} '${repository}' does not exist!`);
+  //   console.error(`cachedFetchFileFromServerWorker repo ${username} '${repository}' does not exist!`);
   //   /* await */ failedStore.setItem(uri.toLowerCase(), `Repo '${repository}' does not exist!`);
   //   return null;
   // }
@@ -358,7 +398,7 @@ async function cachedGetFileFromZipOrServer({ username, repository, path, branch
   let file;
   file = await getFileFromZip({ username, repository, path, branch });
   if (!file) {
-    file = await cachedFetchFileFromServer({ username, repository, path, branch });
+    file = await cachedFetchFileFromServerBranch({ username, repository, path, branch });
   }
   return file;
 }

src/core/notes-tsv7-row-check.js

@@ -5,7 +5,7 @@ import { checkTextField } from './field-text-check';
 import { checkMarkdownText } from './markdown-text-check';
 import { checkSupportReferenceInTA } from './ta-reference-check';
 // import { checkNotesLinksToOutside } from './notes-links-check';
-import { checkOriginalLanguageQuote } from './orig-quote-check';
+import { checkOriginalLanguageQuoteAndOccurrence } from './orig-quote-check';
 import { parameterAssert } from './utilities';
 
 
@@ -232,37 +232,37 @@ export async function checkNotesTSV7DataRow(languageCode, repoCode, line, bookID
     // end of ourCheckSupportReferenceInTA function
 
 
-    async function ourCheckTNOriginalLanguageQuote(rowID, fieldName, fieldText, occurrence, rowLocation, checkingOptions) {
+    async function ourCheckTNOriginalLanguageQuoteAndOccurrence(rowID, fieldName, fieldText, occurrence, rowLocation, checkingOptions) {
         // Checks that the Hebrew/Greek quote can be found in the original texts
 
         // Uses the bookID,C,V values from the main function call
 
         // Updates the global list of notices
 
-        // functionLog(`checkNotesTSV7DataRow ourCheckTNOriginalLanguageQuote(${fieldName}, (${fieldText.length}) '${fieldText}', ${rowLocation}, …)`);
-        parameterAssert(rowID !== undefined, "checkNotesTSV7DataRow ourCheckTNOriginalLanguageQuote: 'rowID' parameter should be defined");
-        parameterAssert(typeof rowID === 'string', `checkNotesTSV7DataRow ourCheckTNOriginalLanguageQuote: 'rowID' parameter should be a string not a '${typeof rowID}'`);
-        parameterAssert(fieldName !== undefined, "checkNotesTSV7DataRow ourCheckTNOriginalLanguageQuote: 'fieldName' parameter should be defined");
-        parameterAssert(typeof fieldName === 'string', `checkNotesTSV7DataRow ourCheckTNOriginalLanguageQuote: 'fieldName' parameter should be a string not a '${typeof fieldName}'`);
-        parameterAssert(fieldText !== undefined, "checkNotesTSV7DataRow ourCheckTNOriginalLanguageQuote: 'fieldText' parameter should be defined");
-        parameterAssert(typeof fieldText === 'string', `checkNotesTSV7DataRow ourCheckTNOriginalLanguageQuote: 'fieldText' parameter should be a string not a '${typeof fieldText}'`);
-        parameterAssert(occurrence !== undefined, "checkNotesTSV7DataRow ourCheckTNOriginalLanguageQuote: 'occurrence' parameter should be defined");
-        parameterAssert(typeof occurrence === 'string', `checkNotesTSV7DataRow ourCheckTNOriginalLanguageQuote: 'occurrence' parameter should be a string not a '${typeof occurrence}'`);
-        parameterAssert(rowLocation.indexOf(fieldName) < 0, `checkNotesTSV7DataRow ourCheckTNOriginalLanguageQuote: 'rowLocation' parameter should be not contain fieldName=${fieldName}`);
+        // functionLog(`checkNotesTSV7DataRow ourCheckTNOriginalLanguageQuoteAndOccurrence(${fieldName}, (${fieldText.length}) '${fieldText}', ${rowLocation}, …)`);
+        parameterAssert(rowID !== undefined, "checkNotesTSV7DataRow ourCheckTNOriginalLanguageQuoteAndOccurrence: 'rowID' parameter should be defined");
+        parameterAssert(typeof rowID === 'string', `checkNotesTSV7DataRow ourCheckTNOriginalLanguageQuoteAndOccurrence: 'rowID' parameter should be a string not a '${typeof rowID}'`);
+        parameterAssert(fieldName !== undefined, "checkNotesTSV7DataRow ourCheckTNOriginalLanguageQuoteAndOccurrence: 'fieldName' parameter should be defined");
+        parameterAssert(typeof fieldName === 'string', `checkNotesTSV7DataRow ourCheckTNOriginalLanguageQuoteAndOccurrence: 'fieldName' parameter should be a string not a '${typeof fieldName}'`);
+        parameterAssert(fieldText !== undefined, "checkNotesTSV7DataRow ourCheckTNOriginalLanguageQuoteAndOccurrence: 'fieldText' parameter should be defined");
+        parameterAssert(typeof fieldText === 'string', `checkNotesTSV7DataRow ourCheckTNOriginalLanguageQuoteAndOccurrence: 'fieldText' parameter should be a string not a '${typeof fieldText}'`);
+        parameterAssert(occurrence !== undefined, "checkNotesTSV7DataRow ourCheckTNOriginalLanguageQuoteAndOccurrence: 'occurrence' parameter should be defined");
+        parameterAssert(typeof occurrence === 'string', `checkNotesTSV7DataRow ourCheckTNOriginalLanguageQuoteAndOccurrence: 'occurrence' parameter should be a string not a '${typeof occurrence}'`);
+        parameterAssert(rowLocation.indexOf(fieldName) < 0, `checkNotesTSV7DataRow ourCheckTNOriginalLanguageQuoteAndOccurrence: 'rowLocation' parameter should be not contain fieldName=${fieldName}`);
 
-        const coqResultObject = await checkOriginalLanguageQuote(languageCode, repoCode, fieldName, fieldText, occurrence, bookID, givenC, givenV, rowLocation, checkingOptions);
+        const coqResultObject = await checkOriginalLanguageQuoteAndOccurrence(languageCode, repoCode, fieldName, fieldText, occurrence, bookID, givenC, givenV, rowLocation, checkingOptions);
 
         // Choose only ONE of the following
         // This is the fast way of append the results from this field
         // result.noticeList = result.noticeList.concat(coqResultObject.noticeList);
         // If we need to put everything through addNoticePartial, e.g., for debugging or filtering
         //  process results line by line
         for (const noticeEntry of coqResultObject.noticeList) {
-            // parameterAssert(Object.keys(noticeEntry).length === 5, `TL ourCheckTNOriginalLanguageQuote notice length=${Object.keys(noticeEntry).length}`);
+            // parameterAssert(Object.keys(noticeEntry).length === 5, `TL ourCheckTNOriginalLanguageQuoteAndOccurrence notice length=${Object.keys(noticeEntry).length}`);
             addNoticePartial({ ...noticeEntry, rowID, fieldName });
         }
     }
-    // end of ourCheckTNOriginalLanguageQuote function
+    // end of ourCheckTNOriginalLanguageQuoteAndOccurrence function
 
 
     /*
@@ -476,7 +476,7 @@ export async function checkNotesTSV7DataRow(languageCode, repoCode, line, bookID
         if (quote.length) { // need to check UTN against UHB and UGNT
             QSuggestion = ourCheckTextField(rowID, 'Quote', quote, false, ourRowLocation, checkingOptions);
             if (occurrence.length)
-                await ourCheckTNOriginalLanguageQuote(rowID, 'Quote', quote, occurrence, ourRowLocation, checkingOptions);
+                await ourCheckTNOriginalLanguageQuoteAndOccurrence(rowID, 'Quote', quote, occurrence, ourRowLocation, checkingOptions);
             else
                 addNoticePartial({ priority: 750, message: "Missing occurrence field when we have an original quote", fieldName: 'Occurrence', rowID, location: ourRowLocation });
         }

src/core/notes-tsv7-table-check.js

@@ -5,7 +5,7 @@ import { removeDisabledNotices } from './disabled-notices';
 import { parameterAssert } from './utilities';
 
 
-const NOTES_TABLE_VALIDATOR_VERSION_STRING = '0.3.3';
+const NOTES_TABLE_VALIDATOR_VERSION_STRING = '0.3.4';
 
 const NUM_EXPECTED_NOTES_TSV_FIELDS = 7; // so expects 6 tabs per line
 const EXPECTED_NOTES_HEADING_LINE = 'Reference\tID\tTags\tSupportReference\tQuote\tOccurrence\tNote';
@@ -106,7 +106,7 @@ export async function checkNotesTSV7Table(languageCode, repoCode, bookID, filena
     // debugLog(`  '${location}' has ${lines.length.toLocaleString()} total lines (expecting ${NUM_EXPECTED_TN_FIELDS} fields in each line)`);
 
     let lastC = '', lastV = '';
-    let rowIDList = [], uniqueRowList = [];
+    let rowIDListForVerse = [], uniqueRowListForVerse = [];
     let numVersesThisChapter = 0;
     for (let n = 0; n < lines.length; n++) {
         // functionLog(`checkNotesTSV7Table checking line ${n}: ${JSON.stringify(lines[n])}`);
@@ -155,18 +155,18 @@ export async function checkNotesTSV7Table(languageCode, repoCode, bookID, filena
 
                 // So here we only have to check against the previous and next fields for out-of-order problems and duplicate problems
                 if (C !== lastC || V !== lastV) {
-                    rowIDList = []; // ID's only need to be unique within each verse
-                    uniqueRowList = []; // Same for these
+                    rowIDListForVerse = []; // ID's only need to be unique within each verse
+                    uniqueRowListForVerse = []; // Same for these
                 }
 
                 // TODO: Check if we need this at all (even though tC 3.0 can’t display these "duplicate" notes)
                 // Check for duplicate notes
                 const uniqueID = C + V + supportReference + quote + occurrence; // This combination should not be repeated
-                // if (uniqueRowList.includes(uniqueID))
+                // if (uniqueRowListForVerse.includes(uniqueID))
                 //     addNoticePartial({ priority: 880, C, V, message: `Duplicate note`, rowID, lineNumber: n + 1, location: ourLocation });
-                // if (uniqueRowList.includes(uniqueID))
+                // if (uniqueRowListForVerse.includes(uniqueID))
                 //     addNoticePartial({ priority: 80, C, V, message: `Note: tC 3.0 won’t display duplicate note`, rowID, lineNumber: n + 1, location: ourLocation });
-                uniqueRowList.push(uniqueID);
+                uniqueRowListForVerse.push(uniqueID);
 
                 if (C) {
                     if (C === 'front') { }
@@ -219,8 +219,9 @@ export async function checkNotesTSV7Table(languageCode, repoCode, bookID, filena
                     addNoticePartial({ priority: 790, C, V, message: "Missing verse number", rowID, lineNumber: n + 1, location: ` after ${C}:${lastV}${ourLocation}` });
 
                 if (rowID) {
-                    if (rowIDList.includes(rowID))
+                    if (rowIDListForVerse.includes(rowID))
                         addNoticePartial({ priority: 729, C, V, message: `Duplicate '${rowID}' ID`, fieldName: 'ID', rowID, lineNumber: n + 1, location: ourLocation });
+                    rowIDListForVerse.push(rowID);
                 } else
                     addNoticePartial({ priority: 730, C, V, message: "Missing ID", fieldName: 'ID', lineNumber: n + 1, location: ourLocation });